Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hang triggered when using check permissions page #209

Merged
merged 3 commits into from
Nov 21, 2023
Merged

Fix hang triggered when using check permissions page #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a01dda4
Update EntraIDEntityProvider.cs
Yvand Nov 20, 2023
cea79f4
work
Yvand Nov 21, 2023
47af67a
Update EntraIDEntityProvider.cs
Yvand Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions Yvand.EntraCP/TEMPLATE/ADMIN/EntraCP/TroubleshootEntraCP.aspx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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("<br/>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("<br/>Test augmentation for user '{0}' on '{1}': OK with {2} groups returned.", input, context, groups == null ? 0 : groups.Length);
return true;
}
catch (Exception ex)
Expand Down
184 changes: 95 additions & 89 deletions Yvand.EntraCP/Yvand.EntraClaimsProvider/EntraIDEntityProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,114 +27,120 @@ public EntraIDEntityProvider(string claimsProviderName) : base(claimsProviderNam

public async override Task<List<string>> GetEntityGroupsAsync(OperationContext currentContext, DirectoryObjectProperty groupProperty)
{
List<EntraIDTenant> azureTenants = currentContext.AzureTenants;
// Create a Task for each tenant to query
IEnumerable<Task<List<string>>> tenantTasks = currentContext.AzureTenants.Select(async tenant =>
{
// Wrap the call to GetEntityGroupsFromTenantAsync() in a Task to avoid a hang when using the "check permissions" dialog
List<string> groupsInTenant = await Task.Run(() => GetEntityGroupsFromTenantAsync(currentContext, groupProperty, tenant)).ConfigureAwait(false);
return groupsInTenant;
});

// Wait for all tenantTasks to complete
List<string>[] listsFromAllTenants = await Task.WhenAll(tenantTasks).ConfigureAwait(false);
List<string> allGroups = new List<string>();
for (int i = 0; i < listsFromAllTenants.Length; i++)
{
allGroups.AddRange(listsFromAllTenants[i]);
}
return allGroups;
}

public async Task<List<string>> 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<Task<List<string>>> tenantTasks = azureTenants.Select(async tenant =>
List<string> groupsInTenant = new List<string>();
Stopwatch timer = new Stopwatch();
timer.Start();
try
{
List<string> groupsInTenant = new List<string>();
Stopwatch timer = new Stopwatch();
timer.Start();
try
// 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)
{
// Search the user as a member
var userCollectionResult = await tenant.GraphService.Users.GetAsync((config) =>
// If user was not found, he might be a Guest user. Query to check this: /users?$filter=userType eq 'Guest' and mail eq '[email protected]'&$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 '[email protected]'&$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/[email protected]/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<string, GetMemberGroupsPostResponse> memberGroupsPageIterator = PageIterator<string, GetMemberGroupsPostResponse>.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/[email protected]/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/[email protected]/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<string, GetMemberGroupsPostResponse> memberGroupsPageIterator = PageIterator<string, GetMemberGroupsPostResponse>.CreatePageIterator(
tenant.GraphService,
memberGroupsResponse,
(groupId) =>
{
PageIterator<Group, DirectoryObjectCollectionResponse> memberGroupsPageIterator = PageIterator<Group, DirectoryObjectCollectionResponse>.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/[email protected]/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<Group, DirectoryObjectCollectionResponse> memberGroupsPageIterator = PageIterator<Group, DirectoryObjectCollectionResponse>.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<string> groups = new List<string>();
// Wait for all tasks to complete
List<string>[] groupsInAllTenants = await Task.WhenAll(tenantTasks).ConfigureAwait(false);
for (int i = 0; i < groupsInAllTenants.Length; i++)
}
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
{
groups.AddRange(groupsInAllTenants[i]);
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 groups;
return groupsInTenant;
}

public async override Task<List<DirectoryObject>> SearchOrValidateEntitiesAsync(OperationContext currentContext)
Expand Down