dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.44k stars 10.02k forks source link

Combining IdToken and AccessToken Claims To Build Principal In Authorization Code Flow #53712

Open amiru3f opened 9 months ago

amiru3f commented 9 months ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

Hey there,

Context

While using OpenIdConnectHandler to leverage OIDC for handling authentication via external Identity Providers (like Duende, Google, Linkedin, AzureAD etc), after successful login and callback, the handler verifies the IdToken and injects claims into the current request Principal which means if the IDP puts claims into the IdToken, the application can access it via request.Principal.Claims

Namely:

authenticationBuilder.AddOpenIdConnect(Schemes.AuthenticationSchemeInternal, "Sample", options =>
{
    options.Authority = //sth
    options.ClientId = //sth
    options.ClientSecret = //sth
    options.ResponseType = OpenIdConnectResponseType.Code;
    options.GetClaimsFromUserInfoEndpoint = //Will explain this later
});

The OpenIdConnectHandler checks if the callback contains authorization code which means the authentication was initiated in AuthorizationCode flow, fetches the IdToken and AccessToken and then extracts the IdToken claims. Then if GetClaimsFromUserInfoEndpoint is enabled uses the AccessToken to fetch the additional claims from the external IDP

Problem

IDPs like Duende and AzureAD do not put the claims inside the IdToken but AccessToken in case you use the authorization_code flow. Unlike if you use implicit, they put the claims into the IdToken and the abovementioned works without any additional costs or issues.

So if you use authorization_code, since the application does not read/care theAccessToken, it needs another call to UserInfoEndpoint to fetch the claims (Which might not be a good/performant case for all scenarios)

Describe the solution you'd like

My question/suggestion is why OpenIdConnectHandler does not combine the claims from IdToken and AccessToken together to build the Principal? Is this out of Standards/Protocols? Namely we can have a flag like options.AlsoCheckAccessTokenForClaims (default: false for avoiding breaking behaviours) so when you set the following flags it will get the claims from both and then no need to initiate another call to UserInfoEndpoint:

options.GetClaimsFromUserInfoEndpoint = false;
options.ResponseType = OpenIdConnectResponseType.Code;
options.AlsoCheckAccessTokenForClaims = true;

Can you please help me to know if I'm thinking mistakenly against of the Protocol or Standards or any other downsides?

Additional context

No response

merijndejonge commented 9 months ago

53765

MackinnonBuck commented 9 months ago

What claims are on the access token that aren't on the ID token?

ghost commented 9 months ago

Hi @amiru3f. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

ghost commented 9 months ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

amiru3f commented 9 months ago

@MackinnonBuck Well, it depends to the IdentityProvider implementation. As you know IdToken is for user Identification (Who is the user) however AccessToken is for Authorization (To check if the user is authorized based on some specific claims, etc). So depending on the way IdentityProvider manages, those claims can differ.

For example a Third Party (Or relying party) requests a specific scope then forwards the user to the IDP like:

https://{IDP_URL}?connect/authorize?grantType=authorization_code&scope=Scope1

Based on the Scope requested, related claims will be included in AccessToken but not IdToken at least in the implementations I've seen (Like Duende, AzureAD, ...)

For implementation details like Duende please check:

https://github.com/DuendeSoftware/IdentityServer/blob/main/src/IdentityServer/Services/Default/DefaultClaimsService.cs#L53

To me that's not a good idea to put the scope-related claims in the IdToken cuz Client won't use IdToken to access any Audiences (Owner of that scope). That's why in Duende the default behaviour is not doing that