From a01dda44a47f9430dbe1fea1fda48b7c13ab1c50 Mon Sep 17 00:00:00 2001 From: Yvan Duhamel Date: Mon, 20 Nov 2023 17:48:17 +0100 Subject: [PATCH 1/3] Update EntraIDEntityProvider.cs --- .../EntraIDEntityProvider.cs | 183 +++++++++--------- 1 file changed, 95 insertions(+), 88 deletions(-) diff --git a/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs b/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs index c6b01fb..466a694 100644 --- a/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs +++ b/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs @@ -28,113 +28,120 @@ public EntraIDEntityProvider(string claimsProviderName) : base(claimsProviderNam public async override Task> GetEntityGroupsAsync(OperationContext currentContext, DirectoryObjectProperty groupProperty) { List azureTenants = currentContext.AzureTenants; + // Create a task for each tenant to query + IEnumerable>> tenantTasks = azureTenants.Select(async tenant => + { + // Wrap the call to GetEntityGroupsFromTenantAsync() in a task to avoid a hang on check permissions feature + List groupsInTenant = await Task.Run(() => GetEntityGroupsFromTenantAsync(currentContext, groupProperty, tenant)).ConfigureAwait(false); + return groupsInTenant; + }); + + List groups = new List(); + // Wait for all tenantTasks to complete + List[] groupsInAllTenants = await Task.WhenAll(tenantTasks).ConfigureAwait(false); + for (int i = 0; i < groupsInAllTenants.Length; i++) + { + groups.AddRange(groupsInAllTenants[i]); + } + return groups; + } + + public async Task> GetEntityGroupsFromTenantAsync(OperationContext currentContext, DirectoryObjectProperty groupProperty, EntraIDTenant tenant) + { // URL encode the filter to prevent that it gets truncated like this: "UserPrincipalName eq 'guest_contoso.com" instead of "UserPrincipalName eq 'guest_contoso.com#EXT#@TENANT.onmicrosoft.com'" string getMemberUserFilter = $"{currentContext.IncomingEntityClaimTypeConfig.EntityProperty} eq '{currentContext.IncomingEntity.Value}'"; string getGuestUserFilter = $"userType eq 'Guest' and {currentContext.Settings.IdentityClaimTypeConfig.DirectoryObjectPropertyForGuestUsers} eq '{currentContext.IncomingEntity.Value}'"; - // Create a task for each tenant to query - IEnumerable>> tenantTasks = azureTenants.Select(async tenant => + List groupsInTenant = new List(); + Stopwatch timer = new Stopwatch(); + timer.Start(); + try { - List groupsInTenant = new List(); - Stopwatch timer = new Stopwatch(); - timer.Start(); - try + // Search the user as a member + var userCollectionResult = await tenant.GraphService.Users.GetAsync((config) => { - // Search the user as a member - var userCollectionResult = await tenant.GraphService.Users.GetAsync((config) => + config.QueryParameters.Filter = getMemberUserFilter; + config.QueryParameters.Select = new[] { "Id" }; + config.QueryParameters.Top = 1; + }).ConfigureAwait(false); + + User user = userCollectionResult?.Value?.FirstOrDefault(); + if (user == null) + { + // If user was not found, he might be a Guest user. Query to check this: /users?$filter=userType eq 'Guest' and mail eq 'guest@live.com'&$select=userPrincipalName, Id + //string guestFilter = HttpUtility.UrlEncode($"userType eq 'Guest' and {IdentityClaimTypeConfig.DirectoryObjectPropertyForGuestUsers} eq '{currentContext.IncomingEntity.Value}'"); + //userResult = await tenant.GraphService.Users.Request().Filter(guestFilter).Select(HttpUtility.UrlEncode("userPrincipalName, Id")).GetAsync().ConfigureAwait(false); + //userResult = await Task.Run(() => tenant.GraphService.Users.Request().Filter(guestFilter).Select(HttpUtility.UrlEncode("userPrincipalName, Id")).GetAsync()).ConfigureAwait(false); + userCollectionResult = await Task.Run(() => tenant.GraphService.Users.GetAsync((config) => { - config.QueryParameters.Filter = getMemberUserFilter; + config.QueryParameters.Filter = getGuestUserFilter; config.QueryParameters.Select = new[] { "Id" }; config.QueryParameters.Top = 1; - }).ConfigureAwait(false); - - User user = userCollectionResult?.Value?.FirstOrDefault(); - if (user == null) - { - // If user was not found, he might be a Guest user. Query to check this: /users?$filter=userType eq 'Guest' and mail eq 'guest@live.com'&$select=userPrincipalName, Id - //string guestFilter = HttpUtility.UrlEncode($"userType eq 'Guest' and {IdentityClaimTypeConfig.DirectoryObjectPropertyForGuestUsers} eq '{currentContext.IncomingEntity.Value}'"); - //userResult = await tenant.GraphService.Users.Request().Filter(guestFilter).Select(HttpUtility.UrlEncode("userPrincipalName, Id")).GetAsync().ConfigureAwait(false); - //userResult = await Task.Run(() => tenant.GraphService.Users.Request().Filter(guestFilter).Select(HttpUtility.UrlEncode("userPrincipalName, Id")).GetAsync()).ConfigureAwait(false); - userCollectionResult = await Task.Run(() => tenant.GraphService.Users.GetAsync((config) => - { - config.QueryParameters.Filter = getGuestUserFilter; - config.QueryParameters.Select = new[] { "Id" }; - config.QueryParameters.Top = 1; - })).ConfigureAwait(false); - user = userCollectionResult?.Value?.FirstOrDefault(); - } - if (user == null) { return groupsInTenant; } + })).ConfigureAwait(false); + user = userCollectionResult?.Value?.FirstOrDefault(); + } + if (user == null) { return groupsInTenant; } - if (groupProperty == DirectoryObjectProperty.Id) - { - // POST to /v1.0/users/user@TENANT.onmicrosoft.com/microsoft.graph.getMemberGroups is the preferred way to return security groups as it includes nested groups - // But it returns only the group IDs so it can be used only if groupClaimTypeConfig.DirectoryObjectProperty == AzureADObjectProperty.Id - // For Guest users, it must be the id: POST to /v1.0/users/18ff6ae9-dd01-4008-a786-aabf71f1492a/microsoft.graph.getMemberGroups - GetMemberGroupsPostRequestBody getGroupsOptions = new GetMemberGroupsPostRequestBody { SecurityEnabledOnly = currentContext.Settings.FilterSecurityEnabledGroupsOnly }; - GetMemberGroupsPostResponse memberGroupsResponse = await Task.Run(() => tenant.GraphService.Users[user.Id].GetMemberGroups.PostAsGetMemberGroupsPostResponseAsync(getGroupsOptions)).ConfigureAwait(false); - if (memberGroupsResponse?.Value != null) - { - PageIterator memberGroupsPageIterator = PageIterator.CreatePageIterator( - tenant.GraphService, - memberGroupsResponse, - (groupId) => - { - groupsInTenant.Add(groupId); - return true; // return true to continue the iteration - }); - await memberGroupsPageIterator.IterateAsync().ConfigureAwait(false); - } - } - else + if (groupProperty == DirectoryObjectProperty.Id) + { + // POST to /v1.0/users/user@TENANT.onmicrosoft.com/microsoft.graph.getMemberGroups is the preferred way to return security groups as it includes nested groups + // But it returns only the group IDs so it can be used only if groupClaimTypeConfig.DirectoryObjectProperty == AzureADObjectProperty.Id + // For Guest users, it must be the id: POST to /v1.0/users/18ff6ae9-dd01-4008-a786-aabf71f1492a/microsoft.graph.getMemberGroups + GetMemberGroupsPostRequestBody getGroupsOptions = new GetMemberGroupsPostRequestBody { SecurityEnabledOnly = currentContext.Settings.FilterSecurityEnabledGroupsOnly }; + GetMemberGroupsPostResponse memberGroupsResponse = await Task.Run(() => tenant.GraphService.Users[user.Id].GetMemberGroups.PostAsGetMemberGroupsPostResponseAsync(getGroupsOptions)).ConfigureAwait(false); + if (memberGroupsResponse?.Value != null) { - // Fallback to GET to /v1.0/users/user@TENANT.onmicrosoft.com/memberOf, which returns all group properties but does not return nested groups - DirectoryObjectCollectionResponse memberOfResponse = await Task.Run(() => tenant.GraphService.Users[user.Id].MemberOf.GetAsync()).ConfigureAwait(false); - if (memberOfResponse?.Value != null) + PageIterator memberGroupsPageIterator = PageIterator.CreatePageIterator( + tenant.GraphService, + memberGroupsResponse, + (groupId) => { - PageIterator memberGroupsPageIterator = PageIterator.CreatePageIterator( - tenant.GraphService, - memberOfResponse, - (group) => - { - string groupClaimValue = GetPropertyValue(group, groupProperty.ToString()); - groupsInTenant.Add(groupClaimValue); - return true; // return true to continue the iteration - }); - await memberGroupsPageIterator.IterateAsync().ConfigureAwait(false); - } + groupsInTenant.Add(groupId); + return true; // return true to continue the iteration + }); + await memberGroupsPageIterator.IterateAsync().ConfigureAwait(false); } } - catch (TaskCanceledException ex) - { - Logger.LogException(ClaimsProviderName, $"while getting groups for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}': The task likely exceeded the timeout of {currentContext.Settings.Timeout} ms and was canceled", TraceCategory.Augmentation, ex); - } - catch (Exception ex) - { - Logger.LogException(ClaimsProviderName, $"while getting groups for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}'", TraceCategory.Augmentation, ex); - } - finally - { - timer.Stop(); - } - if (groupsInTenant != null) - { - Logger.Log($"[{ClaimsProviderName}] Got {groupsInTenant.Count} groups in {timer.ElapsedMilliseconds} ms for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}'", TraceSeverity.Verbose, EventSeverity.Information, TraceCategory.Augmentation); - } else { - Logger.Log($"[{ClaimsProviderName}] Got no group in {timer.ElapsedMilliseconds} ms for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}'", TraceSeverity.Verbose, EventSeverity.Information, TraceCategory.Augmentation); + // Fallback to GET to /v1.0/users/user@TENANT.onmicrosoft.com/memberOf, which returns all group properties but does not return nested groups + DirectoryObjectCollectionResponse memberOfResponse = await Task.Run(() => tenant.GraphService.Users[user.Id].MemberOf.GetAsync()).ConfigureAwait(false); + if (memberOfResponse?.Value != null) + { + PageIterator memberGroupsPageIterator = PageIterator.CreatePageIterator( + tenant.GraphService, + memberOfResponse, + (group) => + { + string groupClaimValue = GetPropertyValue(group, groupProperty.ToString()); + groupsInTenant.Add(groupClaimValue); + return true; // return true to continue the iteration + }); + await memberGroupsPageIterator.IterateAsync().ConfigureAwait(false); + } } - return groupsInTenant; - }); - - List groups = new List(); - // Wait for all tasks to complete - List[] groupsInAllTenants = await Task.WhenAll(tenantTasks).ConfigureAwait(false); - for (int i = 0; i < groupsInAllTenants.Length; i++) + } + catch (TaskCanceledException ex) { - groups.AddRange(groupsInAllTenants[i]); + Logger.LogException(ClaimsProviderName, $"while getting groups for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}': The task likely exceeded the timeout of {currentContext.Settings.Timeout} ms and was canceled", TraceCategory.Augmentation, ex); } - return groups; + catch (Exception ex) + { + Logger.LogException(ClaimsProviderName, $"while getting groups for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}'", TraceCategory.Augmentation, ex); + } + finally + { + timer.Stop(); + } + if (groupsInTenant != null) + { + Logger.Log($"[{ClaimsProviderName}] Got {groupsInTenant.Count} groups in {timer.ElapsedMilliseconds} ms for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}'", TraceSeverity.Verbose, EventSeverity.Information, TraceCategory.Augmentation); + } + else + { + Logger.Log($"[{ClaimsProviderName}] Got no group in {timer.ElapsedMilliseconds} ms for user '{currentContext.IncomingEntity.Value}' from tenant '{tenant.Name}'", TraceSeverity.Verbose, EventSeverity.Information, TraceCategory.Augmentation); + } + return groupsInTenant; } public async override Task> SearchOrValidateEntitiesAsync(OperationContext currentContext) From cea79f4dea74a2c187772620d5423a5d6671e98a Mon Sep 17 00:00:00 2001 From: Yvan Duhamel Date: Tue, 21 Nov 2023 09:29:45 +0100 Subject: [PATCH 2/3] work --- CHANGELOG.md | 1 + .../TEMPLATE/ADMIN/EntraCP/TroubleshootEntraCP.aspx | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ec4ed7..e9cf38e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +* Fix a bug causing a hang on the web application, when using the check permissions feature to verify the permissions of a trusted user * Improve the management of tenant credentials, including new helpers to renew the client secret of certificate using PowerShell * Improve page TroubleshootEntraCP.aspx * Add a mechanism to use custom settings instead of settings from the persisted objects diff --git a/Yvand.EntraCP/TEMPLATE/ADMIN/EntraCP/TroubleshootEntraCP.aspx b/Yvand.EntraCP/TEMPLATE/ADMIN/EntraCP/TroubleshootEntraCP.aspx index ee58eba..dca1199 100644 --- a/Yvand.EntraCP/TEMPLATE/ADMIN/EntraCP/TroubleshootEntraCP.aspx +++ b/Yvand.EntraCP/TEMPLATE/ADMIN/EntraCP/TroubleshootEntraCP.aspx @@ -83,7 +83,7 @@ // Calling constructor of EntraIDTenant may throw FileNotFoundException on Azure.Identity tenant = new EntraIDTenant(tenantName); tenant.SetCredentials(tenantClientId, tenantClientSecret); - + // EntraIDTenant.InitializeAuthentication() will throw an exception if .NET cannot load one of the following assemblies: // Azure.Core.dll, System.Diagnostics.DiagnosticSource.dll, Microsoft.IdentityModel.Abstractions.dll, System.Memory.dll, System.Runtime.CompilerServices.Unsafe.dll tenant.InitializeAuthentication(ClaimsProviderConstants.DEFAULT_TIMEOUT, proxy); @@ -159,9 +159,8 @@ IdentityClaimTypeConfig idClaim = claimsProvider.Settings.ClaimTypes.IdentityClaim; string originalIssuer = SPOriginalIssuers.Format(SPOriginalIssuerType.TrustedProvider, Utils.GetSPTrustAssociatedWithClaimsProvider("EntraCP").Name); SPClaim claim = new SPClaim(idClaim.ClaimType, input, idClaim.ClaimValueType, originalIssuer); - // TODO: Somehow, from this page claimsProvider.GetClaimsForEntity() causes a hang - //SPClaim[] groups = claimsProvider.GetClaimsForEntity(new Uri(context), claim); - //LblResult.Text += String.Format("
Test augmentation for user '{0}' on '{1}': OK with {2} groups returned.", input, context, groups == null ? 0 : groups.Length); + SPClaim[] groups = claimsProvider.GetClaimsForEntity(new Uri(context), claim); + LblResult.Text += String.Format("
Test augmentation for user '{0}' on '{1}': OK with {2} groups returned.", input, context, groups == null ? 0 : groups.Length); return true; } catch (Exception ex) From 47af67a710f27ec02eb689917dc92bfa181f4736 Mon Sep 17 00:00:00 2001 From: Yvan Duhamel Date: Tue, 21 Nov 2023 09:35:41 +0100 Subject: [PATCH 3/3] Update EntraIDEntityProvider.cs --- .../EntraIDEntityProvider.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs b/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs index 466a694..6aadbf9 100644 --- a/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs +++ b/Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs @@ -27,23 +27,22 @@ public EntraIDEntityProvider(string claimsProviderName) : base(claimsProviderNam public async override Task> GetEntityGroupsAsync(OperationContext currentContext, DirectoryObjectProperty groupProperty) { - List azureTenants = currentContext.AzureTenants; - // Create a task for each tenant to query - IEnumerable>> tenantTasks = azureTenants.Select(async tenant => + // Create a Task for each tenant to query + IEnumerable>> tenantTasks = currentContext.AzureTenants.Select(async tenant => { - // Wrap the call to GetEntityGroupsFromTenantAsync() in a task to avoid a hang on check permissions feature + // Wrap the call to GetEntityGroupsFromTenantAsync() in a Task to avoid a hang when using the "check permissions" dialog List groupsInTenant = await Task.Run(() => GetEntityGroupsFromTenantAsync(currentContext, groupProperty, tenant)).ConfigureAwait(false); return groupsInTenant; }); - List groups = new List(); // Wait for all tenantTasks to complete - List[] groupsInAllTenants = await Task.WhenAll(tenantTasks).ConfigureAwait(false); - for (int i = 0; i < groupsInAllTenants.Length; i++) + List[] listsFromAllTenants = await Task.WhenAll(tenantTasks).ConfigureAwait(false); + List allGroups = new List(); + for (int i = 0; i < listsFromAllTenants.Length; i++) { - groups.AddRange(groupsInAllTenants[i]); + allGroups.AddRange(listsFromAllTenants[i]); } - return groups; + return allGroups; } public async Task> GetEntityGroupsFromTenantAsync(OperationContext currentContext, DirectoryObjectProperty groupProperty, EntraIDTenant tenant)