From 2bc6a8f3fa3ce206b0cc17d7dbc8d99953a66c24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Jun 2025 16:23:23 +0000 Subject: [PATCH 1/3] Initial plan for issue From 66d34b6cb45a7f9be293be89d2a02494c0ebd734 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Jun 2025 16:34:08 +0000 Subject: [PATCH 2/3] Add configuration and core logic for expired role membership removal Co-authored-by: davidjrh <489976+davidjrh@users.noreply.github.com> --- .../components/advanced/sync/syncSettings.jsx | 16 +- .../Components/AzureConfig.cs | 4 + .../ScheduledTasks/SyncSchedule.cs | 176 ++++++++++++++++++ .../Services/AzureADProviderSettings.cs | 8 +- .../App_LocalResources/AzureAD.resx | 6 + 5 files changed, 204 insertions(+), 6 deletions(-) diff --git a/DotNetNuke.Authentication.Azure/AzureAD.Web/src/components/advanced/sync/syncSettings.jsx b/DotNetNuke.Authentication.Azure/AzureAD.Web/src/components/advanced/sync/syncSettings.jsx index c0d17ef..829ae48 100644 --- a/DotNetNuke.Authentication.Azure/AzureAD.Web/src/components/advanced/sync/syncSettings.jsx +++ b/DotNetNuke.Authentication.Azure/AzureAD.Web/src/components/advanced/sync/syncSettings.jsx @@ -50,7 +50,8 @@ class SyncSettings extends Component { userSyncEnabled: (key === "userSyncEnabled") ? !props.userSyncEnabled : props.userSyncEnabled, profileSyncEnabled: (key === "profileSyncEnabled") ? !props.profileSyncEnabled : props.profileSyncEnabled, usernamePrefixEnabled: (key === "usernamePrefixEnabled") ? !props.usernamePrefixEnabled : props.usernamePrefixEnabled, - groupNamePrefixEnabled: (key === "groupNamePrefixEnabled") ? !props.groupNamePrefixEnabled : props.groupNamePrefixEnabled + groupNamePrefixEnabled: (key === "groupNamePrefixEnabled") ? !props.groupNamePrefixEnabled : props.groupNamePrefixEnabled, + removeExpiredRoleMembershipsEnabled: (key === "removeExpiredRoleMembershipsEnabled") ? !props.removeExpiredRoleMembershipsEnabled : props.removeExpiredRoleMembershipsEnabled })); } @@ -71,7 +72,8 @@ class SyncSettings extends Component { userSyncEnabled: props.userSyncEnabled, profileSyncEnabled: props.profileSyncEnabled, usernamePrefixEnabled: props.usernamePrefixEnabled, - groupNamePrefixEnabled: props.groupNamePrefixEnabled + groupNamePrefixEnabled: props.groupNamePrefixEnabled, + removeExpiredRoleMembershipsEnabled: props.removeExpiredRoleMembershipsEnabled }, () => { utils.utilities.notify(resx.get("SettingsUpdateSuccess")); this.setState({ @@ -103,6 +105,10 @@ class SyncSettings extends Component { tooltipMessage={resx.get("lblProfileSyncEnabled.Help")} value={this.props.profileSyncEnabled} onChange={this.onSettingChange.bind(this, "profileSyncEnabled")} /> +

{resx.get("lblAADSettings")}

@@ -199,7 +205,8 @@ SyncSettings.propTypes = { userSyncEnabled: PropTypes.bool, profileSyncEnabled: PropTypes.bool, usernamePrefixEnabled: PropTypes.bool, - groupNamePrefixEnabled: PropTypes.bool + groupNamePrefixEnabled: PropTypes.bool, + removeExpiredRoleMembershipsEnabled: PropTypes.bool }; @@ -213,7 +220,8 @@ function mapStateToProps(state) { userSyncEnabled: state.settings.userSyncEnabled, profileSyncEnabled: state.settings.profileSyncEnabled, usernamePrefixEnabled: state.settings.usernamePrefixEnabled, - groupNamePrefixEnabled: state.settings.groupNamePrefixEnabled + groupNamePrefixEnabled: state.settings.groupNamePrefixEnabled, + removeExpiredRoleMembershipsEnabled: state.settings.removeExpiredRoleMembershipsEnabled }; } export default connect(mapStateToProps)(SyncSettings); \ No newline at end of file diff --git a/DotNetNuke.Authentication.Azure/Components/AzureConfig.cs b/DotNetNuke.Authentication.Azure/Components/AzureConfig.cs index 7d0a66f..1a16b54 100644 --- a/DotNetNuke.Authentication.Azure/Components/AzureConfig.cs +++ b/DotNetNuke.Authentication.Azure/Components/AzureConfig.cs @@ -68,6 +68,7 @@ protected internal AzureConfig(string service, int portalId) : base(service, por AutoMatchExistingUsers = bool.Parse(GetScopedSetting(Service + "_AutoMatchExistingUsers", portalId, "false")); AuthorizationCodePrompt = GetScopedSetting(Service + "_AuthorizationCodePrompt", portalId, ""); DomainHint = GetScopedSetting(Service + "_DomainHint", portalId, ""); + RemoveExpiredRoleMembershipsEnabled = bool.Parse(GetScopedSetting(Service + "_RemoveExpiredRoleMembershipsEnabled", portalId, "false")); } public static string GetSetting(string service, string key, int portalId, string defaultValue) @@ -130,6 +131,8 @@ internal string GetScopedSetting(string key, int portalId, string defaultValue) public bool UserSyncEnabled { get; set; } [SortOrder(24)] public bool AutoMatchExistingUsers { get; set; } + [SortOrder(25)] + public bool RemoveExpiredRoleMembershipsEnabled { get; set; } @@ -177,6 +180,7 @@ public static void UpdateConfig(AzureConfig config) UpdateScopedSetting(config.UseGlobalSettings, config.PortalID, config.Service + "_AutoMatchExistingUsers", config.AutoMatchExistingUsers.ToString()); UpdateScopedSetting(config.UseGlobalSettings, config.PortalID, config.Service + "_AuthorizationCodePrompt", config.AuthorizationCodePrompt); UpdateScopedSetting(config.UseGlobalSettings, config.PortalID, config.Service + "_DomainHint", config.DomainHint); + UpdateScopedSetting(config.UseGlobalSettings, config.PortalID, config.Service + "_RemoveExpiredRoleMembershipsEnabled", config.RemoveExpiredRoleMembershipsEnabled.ToString()); UpdateConfig((OAuthConfigBase)config); diff --git a/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs b/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs index ebfca12..e25eb46 100644 --- a/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs +++ b/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs @@ -8,6 +8,7 @@ using DotNetNuke.Entities.Users; using DotNetNuke.Instrumentation; using DotNetNuke.Security.Roles; +using DotNetNuke.Security.Membership; using DotNetNuke.Services.Authentication; using DotNetNuke.Services.FileSystem; using DotNetNuke.Services.Scheduling; @@ -72,6 +73,13 @@ public override void DoWork() { var message = SyncRoles(portal.PortalID, settings); ScheduleHistoryItem.AddLogNote(message); + + // Remove expired role memberships from Entra ID if enabled + if (settings.RemoveExpiredRoleMembershipsEnabled) + { + var expiredMessage = RemoveExpiredRoleMemberships(portal.PortalID, settings); + ScheduleHistoryItem.AddLogNote(expiredMessage); + } } if (settings.UserSyncEnabled) { @@ -240,6 +248,174 @@ internal string SyncRoles(int portalId, AzureConfig settings) } } + internal string RemoveExpiredRoleMemberships(int portalId, AzureConfig settings) + { + try + { + if (!settings.RemoveExpiredRoleMembershipsEnabled || !settings.RoleSyncEnabled) + { + return $"Expired role membership removal is disabled for portal {portalId}.\n"; + } + + var syncErrorsDesc = ""; + var syncErrors = 0; + var membershipsRemoved = 0; + var customRoleMappings = GetRoleMappingsForPortal(portalId, settings); + + Utils.ValidateAadParameters(portalId, settings); + var graphClient = settings.GraphUseCustomParams + ? new GraphClient(settings.AADApplicationId, settings.AADApplicationKey, settings.AADTenantId) + : new GraphClient(settings.APIKey, settings.APISecret, settings.TenantId); + + // Get all DNN roles imported from AAD + var dnnAadRoles = GetDnnAadRoles(portalId); + + foreach (var dnnRole in dnnAadRoles) + { + try + { + // Get expired role memberships for this role + var expiredMemberships = RoleController.Instance.GetUserRoles(portalId, null, dnnRole.RoleName) + .Where(ur => ur.ExpiryDate != Null.NullDate && ur.ExpiryDate < DateTime.Now && ur.Status == RoleStatus.Approved) + .ToList(); + + foreach (var expiredMembership in expiredMemberships) + { + try + { + // Get the user + var userInfo = UserController.GetUserById(portalId, expiredMembership.UserID); + if (userInfo == null) continue; + + // Find the AAD user ID from the user authentication records + var aadUserId = GetAadUserIdFromDnnUser(userInfo, graphClient, settings); + if (string.IsNullOrEmpty(aadUserId)) continue; + + // Determine the AAD group name + var aadGroupName = dnnRole.RoleName; + var mapping = customRoleMappings?.FirstOrDefault(x => x.DnnRoleName == dnnRole.RoleName); + if (mapping != null) + { + aadGroupName = mapping.AadRoleName; + } + else if (settings.GroupNamePrefixEnabled && dnnRole.RoleName.StartsWith($"{AzureConfig.ServiceName}-")) + { + aadGroupName = dnnRole.RoleName.Substring($"{AzureConfig.ServiceName}-".Length); + } + + // Get the AAD group ID by name + var aadGroups = graphClient.GetAllGroups(aadGroupName); + var aadGroup = aadGroups?.CurrentPage?.FirstOrDefault(g => g.DisplayName == aadGroupName); + if (aadGroup != null) + { + // Remove the user from the AAD group + graphClient.RemoveGroupMember(aadGroup.Id, aadUserId); + membershipsRemoved++; + } + } + catch (Exception ex) + { + syncErrors++; + syncErrorsDesc += $"\nError removing expired role membership for user {expiredMembership.UserID} from role {dnnRole.RoleName}: {ex.Message}"; + } + } + } + catch (Exception ex) + { + syncErrors++; + syncErrorsDesc += $"\nError processing expired memberships for role {dnnRole.RoleName}: {ex.Message}"; + } + } + + var syncResultDesc = ""; + var syncResultStats = $"sync errors: {syncErrors}; expired memberships removed: {membershipsRemoved}"; + if (!string.IsNullOrEmpty(syncErrorsDesc)) + { + Logger.Error($"AAD Expired Role Membership Removal errors detected: {syncErrorsDesc}"); + syncResultDesc = $"Portal {portalId} expired role membership removal completed with errors, check logs for more information ({syncResultStats}).\n"; + } + else + { + syncResultDesc = $"Successfully removed expired role memberships for portal {portalId} ({syncResultStats}).\n"; + } + return syncResultDesc; + } + catch (Exception e) + { + string message = $"Error while removing expired role memberships from portal {portalId}: {e}.\n"; + Logger.Error(message); + return message; + } + } + + private string GetAadUserIdFromDnnUser(UserInfo userInfo, GraphClient graphClient, AzureConfig settings) + { + try + { + // Get the authentication record for this user + var userAuthentications = DotNetNuke.Security.Membership.MembershipProvider.Instance().GetUserAuthentication(userInfo.UserID); + var aadAuth = userAuthentications?.FirstOrDefault(ua => ua.AuthenticationType == AzureConfig.ServiceName); + if (aadAuth == null) + { + return null; // User is not an AAD user + } + + // The AuthenticationToken contains the username used during authentication + var authToken = aadAuth.AuthenticationToken; + if (string.IsNullOrEmpty(authToken)) + { + return null; + } + + // If the username has a prefix, remove it to get the original AAD identifier + var aadIdentifier = authToken; + if (settings.UsernamePrefixEnabled && authToken.StartsWith($"{AzureConfig.ServiceName}-")) + { + aadIdentifier = authToken.Substring($"{AzureConfig.ServiceName}-".Length); + } + + // Try to get the user from AAD using the identifier + // This could be a UPN, email, or object ID depending on the user mapping configuration + try + { + // First, try to get by object ID (if it looks like a GUID) + if (Guid.TryParse(aadIdentifier, out _)) + { + var user = graphClient.GetUser(aadIdentifier); + return user?.Id; + } + + // If not a GUID, search for the user by UPN/email + var users = graphClient.GetAllUsers(); + while (users != null && users.Count > 0) + { + var foundUser = users.CurrentPage?.FirstOrDefault(u => + u.UserPrincipalName?.Equals(aadIdentifier, StringComparison.OrdinalIgnoreCase) == true || + u.Mail?.Equals(aadIdentifier, StringComparison.OrdinalIgnoreCase) == true || + u.Id?.Equals(aadIdentifier, StringComparison.OrdinalIgnoreCase) == true); + + if (foundUser != null) + { + return foundUser.Id; + } + + users = users.NextPageRequest?.GetSync(); + } + } + catch (Exception ex) + { + Logger.Warn($"Error looking up AAD user for DNN user {userInfo.UserID}: {ex.Message}", ex); + } + + return null; + } + catch (Exception ex) + { + Logger.Error($"Error getting AAD user ID for DNN user {userInfo.UserID}: {ex.Message}", ex); + return null; + } + } + private UserInfo AddUser(int portalId, string userName, string displayName, string firstName, string lastName, string eMail) { var user = new UserInfo() diff --git a/DotNetNuke.Authentication.Azure/Services/AzureADProviderSettings.cs b/DotNetNuke.Authentication.Azure/Services/AzureADProviderSettings.cs index 4df475c..20221d5 100644 --- a/DotNetNuke.Authentication.Azure/Services/AzureADProviderSettings.cs +++ b/DotNetNuke.Authentication.Azure/Services/AzureADProviderSettings.cs @@ -77,6 +77,8 @@ public class AzureADProviderSettings public string DomainHint { get; set; } [DataMember(Name = "autoMatchExistingUsers")] public bool AutoMatchExistingUsers { get; set; } + [DataMember(Name = "removeExpiredRoleMembershipsEnabled")] + public bool RemoveExpiredRoleMembershipsEnabled { get; set; } @@ -108,7 +110,8 @@ public static AzureADProviderSettings LoadSettings(string service, int portalId) UsernamePrefixEnabled = config.UsernamePrefixEnabled, GroupNamePrefixEnabled = config.GroupNamePrefixEnabled, AuthorizationCodePrompt = config.AuthorizationCodePrompt, - DomainHint = config.DomainHint + DomainHint = config.DomainHint, + RemoveExpiredRoleMembershipsEnabled = config.RemoveExpiredRoleMembershipsEnabled }; } @@ -143,7 +146,8 @@ public static void SaveAdvancedSyncSettings(string service, int portalId, AzureA UserSyncEnabled = settings.UserSyncEnabled, ProfileSyncEnabled = settings.ProfileSyncEnabled, UsernamePrefixEnabled = settings.UsernamePrefixEnabled, - GroupNamePrefixEnabled = settings.GroupNamePrefixEnabled + GroupNamePrefixEnabled = settings.GroupNamePrefixEnabled, + RemoveExpiredRoleMembershipsEnabled = settings.RemoveExpiredRoleMembershipsEnabled }; AzureConfig.UpdateConfig(config); diff --git a/DotNetNuke.Authentication.Azure/admin/personaBar/App_LocalResources/AzureAD.resx b/DotNetNuke.Authentication.Azure/admin/personaBar/App_LocalResources/AzureAD.resx index d0137e5..1e3dbee 100644 --- a/DotNetNuke.Authentication.Azure/admin/personaBar/App_LocalResources/AzureAD.resx +++ b/DotNetNuke.Authentication.Azure/admin/personaBar/App_LocalResources/AzureAD.resx @@ -351,6 +351,12 @@ Profile Sync + + When enabled, users with expired role memberships in DNN will be removed from corresponding groups in Entra ID during synchronization + + + Remove Expired Role Memberships from Entra ID + If specified, the redirect uri after a successful login. By default (blank), the user will be redirected to the page originating the login redirection. From 8db6898d3f8a7ee4b922e55a747fd697129fd783 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Jun 2025 16:38:18 +0000 Subject: [PATCH 3/3] Fix authentication API usage and finalize implementation Co-authored-by: davidjrh <489976+davidjrh@users.noreply.github.com> --- .../ScheduledTasks/SyncSchedule.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs b/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs index e25eb46..0e19927 100644 --- a/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs +++ b/DotNetNuke.Authentication.Azure/ScheduledTasks/SyncSchedule.cs @@ -8,7 +8,6 @@ using DotNetNuke.Entities.Users; using DotNetNuke.Instrumentation; using DotNetNuke.Security.Roles; -using DotNetNuke.Security.Membership; using DotNetNuke.Services.Authentication; using DotNetNuke.Services.FileSystem; using DotNetNuke.Services.Scheduling; @@ -352,8 +351,8 @@ private string GetAadUserIdFromDnnUser(UserInfo userInfo, GraphClient graphClien { try { - // Get the authentication record for this user - var userAuthentications = DotNetNuke.Security.Membership.MembershipProvider.Instance().GetUserAuthentication(userInfo.UserID); + // Get the authentication record for this user using AuthenticationController + var userAuthentications = AuthenticationController.GetUserAuthentications(userInfo.UserID); var aadAuth = userAuthentications?.FirstOrDefault(ua => ua.AuthenticationType == AzureConfig.ServiceName); if (aadAuth == null) {