Yvand/EntraCP

Request to add support for custom claim identifiers in AzureCP other than default

Closed this issue · 13 comments

My company is trying to integrate Azure AD federated authentication in its SharePoint 2019 farm. Current we use custom user identifier in on-prem AD like "EmpID". The old ADFS server will be decommissioned soon and we will transition to Azure AD federated authentication.

We have this custom property synced with the AzureAD too and currently can federate authentication using Azure AD Enterprise app without AzureCP. We plan to use AzureCP fo resolve user for better experience. But resolving user using EmpID does not work and thus setting permission via same fails.

We also have restrictions getting Application permission GroupMember.Read.All and User.Read.All in our tenant. So, is there a way we can change the application to use Delegate permissions?

Summarizing the changes we look forward to are listed below. We can also contribute if needed to bring below enhancements.

  1. Add support for non-standard user claim identifier like EmpID.
  2. Add support to resolve user with their additional properties like EmpID.
  3. Add support to use delegated permission for resolving user/groups(GroupMember.Read.All and User.Read.All)
Yvand commented

@sujeetsharma thank you for sharing your feedback.

About 3.: Dlegated permission means that a user needs to sign-in whenever AzureCP needs an access token. Since AzureCP runs in many SharePoint processes, this is impossible by design

About 1. and 2.: It is a fair request and technically possible to implement, but challenging and risky because of the risk of breaking the AzureCP configuration when updating AzureCP.wsp solution in the SharePoint farm:
The problem is that current design uses a fixed enum type as a source of user attributes list, and that would need to be changed to a list of string (that can be modified later by SharePoint admins), and it will be difficult to do this change without risking problems when updating AzureCP.

Hi @Yvand, Thanks for the quick response.

About 3- We plan to use a functional account/password to logon and not end-users account. This could help solve this.

About 1 & 2 - Can we provide the option in AzureCP config page to select default user type or custom user identifier textbox(With default selected to the fixed enum type of user attributes)?
Even if the solution is updated in future, the existing implementations will continue to have the same settings. If the SP Admins want to customize they can by coming to the AzureCP config page.

Yvand commented

Hi @sujeetsharma
About 3. It is by design totally impossible to use delegated permissions, AzureCP can only use application-only permissions.
About 1 & 2, it is an original design mistake which I made to use an enum type for the user attributes. The good solution is to change it completely to use a list of string. Your idea might also work, but at the cost of even more complexity and hard to maintain/understand. I keep in mind this update but I cannot promise if I will actually do it, as again it is a risky change

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Today I did some investigation on this issue as we are seeing the same requirement to add custom claims to AAD.

Initial test to confirm the complexity of the solution:

  1. I started of with extending the enumeration with the various claims (e.g. EmployeeType). I did this to see, if there are any further hurdles to take.
  2. Solution deployment went fine and no errors were visible from central admin.

Outcome so far:

  • The newly added values (e.g. EmployeeType) are not visible in the dropdown
  • There is a blocking check within the code. The code checks, if the properties are available within the MSGraph User/Group object.
  • This check causes the newly added values not being present in the configuration

https://github.com/Yvand/AzureCP/blob/243ba6d94975dba9afe3d10e537a04e1f201b3d8/AzureCP/TEMPLATE/ADMIN/AzureCP/ClaimTypesConfig.ascx.cs#L309-L317

The new value 'EmployeeType' would map to user.onPremisesExtensionAttributes.extensionattribute10 within AAD.

MSGraph user objects do have the collection of extension attributes:

//
// Summary:
//     Gets or sets on premises extension attributes. Contains extensionAttributes 1-15
//     for the user. Note that the individual extension attributes are neither selectable
//     nor filterable. For an onPremisesSyncEnabled user, the source of authority for
//     this set of properties is the on-premises and is read-only and is read-only.
//     For a cloud-only user (where onPremisesSyncEnabled is false), these properties
//     may be set during creation or update. These extension attributes are also known
//     as Exchange custom attributes 1-15. Supports $filter (eq, NOT, ge, le, in).
[JsonProperty(NullValueHandling = NullValueHandling.Ignore, PropertyName = "onPremisesExtensionAttributes", Required = Required.Default)]
public OnPremisesExtensionAttributes OnPremisesExtensionAttributes { get; set; }

Now, the challenge is to extend the UI to have a choice field/Dropdown to select extensionAttribute1-15 and to name this mapping so that it can be stored.

@Yvand and @sujeetsharma Do you have any suggestions?

Just followed up on my yesterday's findings and was looking into an easy solution. The easy solution would add the 15 extension attributes to the enumeration AzureADObjectProperty.

https://github.com/Yvand/AzureCP/blob/243ba6d94975dba9afe3d10e537a04e1f201b3d8/AzureCP/AzureCPConfig.cs#L1143-L1184

Having these attributes available here will hit the same issue as above and as a quick workaround I would implement something like that in the check in https://github.com/Yvand/AzureCP/blob/243ba6d94975dba9afe3d10e537a04e1f201b3d8/AzureCP/TEMPLATE/ADMIN/AzureCP/ClaimTypesConfig.ascx.cs#L309-L317

 // Ensure property exists for the current object type
                if (azureObject.Value.EntityType == DirectoryObjectType.User)
                {
                    if (AzureCP.GetPropertyValue(new User(), prop.ToString()) == null && !prop.ToString().StartsWith("extensionAttribute"))
                    {
                        continue;
                    }
                }

Right now I'm struggling with where else is this information used. The enumeration values do not match the real property on the Microsoft graph user object.

e.g. extensionAttribute15 would need to mapped to userboject.OnPremisesExtensionAttributes.extensionAttribute15

Any help would be highly appreciated.

Yvand commented

@andikrueger I need to look at this in more detail before I can give you an answer, it is on my radar and I'll come back asap

I'll push my changes to this branch: https://github.com/andikrueger/AzureCP/tree/162-custom-claims

I found an easier option to add the extensionAttributes to the dropdowns and config pages. Now looking at the usage of these extensions within queries to MSGraph - if there are any.

Just some info what I added so far:

  • Option to select 15 extensionAttributes by exenting the enum for AzureADObjectProperty
  • New AD Connect Client ID field for tenants to allow the proper construction of the extensionAttributes within the tenant
  • New input field to set the AD Connect client ID Field
  • Replace logic for creating queries with the extension attributes

At the moment, the solution is untested for the query part. The other parts are tested within the UI.
image
image

Yvand commented

@andikrueger thank you for your work.
Can you make sure to cherry picks those 3 commits from the dev branch?
Once you think there is something ready to test, feel free to open a PR to merge on dev branch and I'll test it on my side.

171 and 172 should already be part of my branc. I’ll do some rebasing as soon as I’m confident with the changes made.

I faced one issue while upgrading the solution. I had to remove the current current configuration - I don’t know if this was something within my environment or something to expect. The stsadm command did the trick in the end.

Just pushed an additional commit to my branch with a minor change. Right now, we struggle due to #164 to verify the solution.

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.