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.

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

Unable to read non-standard claims from OpenID IdP #15808

Closed
cbadger-montecitobank opened this issue Apr 22, 2024 · 6 comments
Closed

Unable to read non-standard claims from OpenID IdP #15808

cbadger-montecitobank opened this issue Apr 22, 2024 · 6 comments
Labels

Comments

@cbadger-montecitobank
Copy link

cbadger-montecitobank commented Apr 22, 2024

I'm not sure if this is a bug, feature request, or just my misunderstanding of how Orchard's OpenID integration works. I'm successfully using the OpenID Client module to log into Orchard using Identity Server as the external provider. It returns some standard claims (e.g. name, email, etc.) that Orchard recognizes and uses to register and log in the user. So far so good.

The problem arises when I try to read other (non-standard?) claims returned from Identity Server within the user login script, with the hope of using those claims to make decision about which Orchard roles the authenticating user should belong to. These additional claims are not in the context.ExternalClaims collection where I'd expect them to be.

At the very least I was hoping Identity Server could provide "role" claims that I could read and use to set the user's Orchard roles. Being able to read any claim coming back from Identity Server would be even better.

Below is my login script, which looks for a claim of type "role" with value "AdminRole", but it never finds one. For debuggin, I've logged the entire contents of context.externalClaims to see if it contains any role claims, but it does not.

switch (context.loginProvider) {
    case "OpenIdConnect":
        context.externalClaims.forEach(claim => {
            if (claim.type === "role") {
                switch (claim.value) {
                    case "AdminRole":
                        context.rolesToAdd.push("Administrator");
                        break;
                    default:
                        log("Warning", "Claim type: " + claim.type + " with value: " + claim.value + " was not recognized");
                        break;
                }
            }
        });
        break;
    default:
        log("Warning", "Provider " + context.loginProvider + " was not handled");
        break;
}

I've confirmed that I'm requesting the appropriate scopes that would result in the desired claims being included in the Identity Server response. Requesting the same scopes from another application, using the same configured clientid, shows the claims be returned.

Any help would be appreciated.

Thank you!

@MikeAlhayek
Copy link
Member

@cbadger-montecitobank Did you check the options Use a script to set user roles based on external provider claims in the User Login settings?

That would ensure that your script has effect on the process. With that enabled + your script If the external server assign AdminRole to the user, then the user in OC will have Administrator role assigned too them.

I also think you'll need Role scope needs to be allowed to ensure that the server share the role with your OC app.

@cbadger-montecitobank
Copy link
Author

@MikeAlhayek appreciate your response. Yes, my login script is enabled, and it definitely runs. I can add log statements to output the contents of the context.externalClaims collection and see that my role claims (or other custom claims) are not present. It seems as though only a certain set of standard claims are getting propagated into this collection, regardless of what comes back from the identity provider.

I've also added "roles" to my list of scopes, among other scopes that contain additional claims. Sadly this doesn't result in any of them appearing in the externalClaims collection.

@MikeAlhayek
Copy link
Member

@cbadger-montecitobank if you are able to replicate the issue into OC solution, you should be able to debug it. The following code is what evaluates the

var loginSettings = (await _siteService.GetSiteSettingsAsync()).As<LoginSettings>();
if (loginSettings.UseScriptToSyncRoles)
{
var script = $"js: function syncRoles(context) {{\n{loginSettings.SyncRolesScript}\n}}\nvar context={JConvert.SerializeObject(context, JOptions.CamelCase)};\nsyncRoles(context);\nreturn context;";
dynamic evaluationResult = _scriptingManager.Evaluate(script, null, null, null);
context.RolesToAdd.AddRange((evaluationResult.rolesToAdd as object[]).Select(i => i.ToString()));
context.RolesToRemove.AddRange((evaluationResult.rolesToRemove as object[]).Select(i => i.ToString()));
}
}

At that point, RolesToAdd should have correct records and RolesToRemove should not have anything.

The results are then called and added to the User here

await _userManager.AddToRolesAsync(user, context.RolesToAdd.Distinct());
await _userManager.RemoveFromRolesAsync(user, context.RolesToRemove.Distinct());

@cbadger-montecitobank
Copy link
Author

@MikeAlhayek I'm able to replicate the issue in the OC solution and debug. The login script runs, and if I have it add something to the context.RolesToAdd collection it successfully adds the user to those Orchard roles. The issue is specifically with the context.externalClaims collection not having all my claims in it. I would like to use some external claims in the script to determine which roles I want to include in the context.RolesToAdd collection.

I've traced the code back up to the AccountController.cs -> ExternalLoginCallback() method. This seems to handle the callback after the external authentication completes. In here it does:

var info = await _signInManager.GetExternalLoginInfoAsync();

Inspecting info.Principle.Claims shows a standard set of 10 claims, but none of the extra ones I'd expect to be in this collection. This is where I'm not certain how things should work. Should all of the claims returned from the identity provider exist here, or only a certain subset that Orchard recognizes?

Appreciate the help!

@MikeAlhayek
Copy link
Member

@kevinchalet at what point in OpenIddict we populate the external login claims? What could be preventing these external claims from coming through?

@kevinchalet
Copy link
Member

@MikeAlhayek OpenIddict is not used for the client part of the OpenID module (there are plans to change that, but we'd like to use the secrets module for key management: #14021).

@cbadger-montecitobank are you sure the claims are not mapped to their WS-Federation/ClaimTypes equivalent? Typically, I'd expect role to be mapped to ClaimTypes.Role using the default configuration: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/b352bcce8cf2c1676f6ef13558aff29b7766602f/src/Microsoft.IdentityModel.JsonWebTokens/ClaimTypeMapping.cs#L85-L86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants