OpenID Server not adding claims to identity token based on requested scopes (e.g. profile, email)

[mvarblow]

Describe the bug

The OpenID Server module is not adding claims to the identity token based on requested scopes (profile, email) as it should.

To Reproduce

The setup for this is a bit complicated because it requires both an Orchard Core site acting as the OIDC Identity Server and another application acting as the client/relying party. Some notes on the setup:

• I'm using a plain Orchard Core 1.7 site with the Open ID Authorization Server feature enabled.
• I'm using the Authorization Code Flow and the JSON Web Token "Token Format" setting.
• I've added profile and email scopes through the management interface (seems like they should be included automatically, but I am not able to complete the next step without adding them myself as valid scopes).
• I've added an application and granted it access to the Authorization Code Flow and the profile and email scopes. (I also set the consent type to "Impicit consent" and provided the redirect URI.

It took a bit of sleuthing to determine how this all holds together, but here's what I was able to determine after stepping through the OrchardCore and OpenIddict code a few times.

My client (relying part) application redirects the user to the Open ID Authorization Server authorization endpoint (OrchardCore.OpenId.Controllers.AccessController.Authorize). The first time through, the user is redirected to the normal Orchard Core sign-in page and then redirected back to the Open ID authorization endpoint. This time, the controller first grabs the Orchard Core claims identity. It includes the following claims (AccessController.cs line 83):

The AccessController.Authorize method the supplements those claims by adding the granted scopes, resources, etc. Now on line 137 the claims collection looks like this:

The Authorize method has looped through all the claims to set a .destinations property on each claim. This property identifies which tokens should include the claim. The determination of token destinations for each claim is made by the GetDestinations method which is hard coded in AccessController.cs (I wish this were overrideable, oh well):

Note that both the name and email claims have the .destinations property set to include both id_token and access_token as expected.

The relying party (my client application) receives an authorization code and makes the call to the token endpoint. (The standard OIDC authorization code flow). The token endpoint (OrchardCore.OpenId.Controllers.AccessController.Token) sees that this is an authorization code flow so it calls ExchangeAuthorizationCodeOrRefreshTokenGrantType. That method first calls AuthenticateAsync using the OpenIddict authentication scheme and gets back a claims principle containing all the claims that were set earlier by the authorization endpoint. The claims collection on the principle looks like this:

Note that it includes the granted scopes (oi_scp claims) and the claims include the .destination property. The name and email claims both have the .destination property set correctly at this point to include both the id and access tokens.

Here's where we run into a problem. The AccessController.ExchangeAuthorizationCodeOrRefreshTokenGrantType method includes this interesting code which was added back in 2019:

Getting fresh data is good in case the sign-in happened a while ago. However, all the scopes and the properties (such as .destinations) which were added earlier to the claims list by the OpenIddict OAuth pipeline are lost! The claims list is rest back to only include the original claims from the Orchard identity principal.

The identity token received by the calling application does not include the name or email claims as it should. They only non-protocol claim we get is the subject (sub) claim because that one is always included. Claims for the requested scopes are not included because the code block in ExchangeAuthorizationCodeOrRefreshTokenGrantType stripped away all the scope claims and the destinations. The destinations are recomputed at the end of ExchangeAuthorizationCodeOrRefreshTokenGrantType but they are recomputed incorrectly because the claims for the granted scopes were removed by that "refresh" code block (see above).

Expected behavior

The identity token returned from the token endpoint using the authorization code flow should include the name claim if the profile scope is requested. It should include the email claim if the email scope is requested. And it should include role claims if the roles claim is requested.

Screenshots

See above.

[rafaelrph]

@sebastienros and @jtkech
This issue is very similar to the one that I'm having now after upgrading to 1.7.0

The Media Security doesn't work as it worked before the upgrade.
If I login to a userX, create a page with a media and get the media URL. The media is accessible for any authenticated users by the URL, even if the page is published or not, and also when the user doesn't have access to the page.

My application has its own controller to handle these media requests, which is almost the same as this file:
https://github.com/OrchardCMS/OrchardCore/blob/03dd2f72c82f85ce213642286f8165f24022baa2/src/OrchardCore.Modules/OrchardCore.Media/Controllers/AdminController.cs

When the request arrives, the HttpContext.User doesn't have any claims or any custom data.
The logic just by-passes the authentication and returns the media that it should not return.

[mvarblow]

@rafaelrph: this seems like a separate issue. The issue I reported here is specific to using the Open ID Server module with the authorization code flow.

I just submitted a pull request that seemed to fix the problem for me. The fix was to -- when exchanging an authorization code for a token -- copy the granted scopes and resources from the original authorization code to the new/refreshed principal.

cc @sebastienros and @jtkech

[jtkech]

@mvarblow Yes I saw your PR, I asked @kevinchalet for a review.

@rafaelrph I will try to repro your use case.

[rafaelrph]

@jtkech the issue has been fixed in our side. Thanks for the help

[jtkech]

Cool then, thanks