DuendeSoftware / Support

Support for Duende Software products
21 stars 0 forks source link

Claims missing in Subject property for UserInfoEndpoint in IProfileService.GetProfileDataAsync #1345

Closed mikegarciam closed 1 month ago

mikegarciam commented 3 months ago

Which version of Duende IdentityServer are you using? 7.0.5 Which version of .NET are you using? 8.0.303

Describe the bug The Subject property in ProfileDataRequestContext does not have the claims contained in the access token when Caller is UserInfoEndpoint in IProfileService.GetProfileDataAsync

To Reproduce

  1. Issue an access token with a custom claim in IProfileService.GetProfileDataAsync for ProfileDataCallers.ClaimsProviderAccessToken:
if (context.Caller == IdentityServerConstants.ProfileDataCallers.ClaimsProviderAccessToken)
{
    context.IssuedClaims.Add(new Claim("custom_claim", "abc123"));
    _logger.LogInformation($"Issued custom claim for access token");
}
  1. Verify that issued claim does not exist in Subject, and that it does exist in the access token when Caller is UserInfoEndpoint:

    if (context.Caller == IdentityServerConstants.ProfileDataCallers.UserInfoEndpoint)
    {
    if(context.Subject.Claims.Any(c => c.Type == "custom_claim"))
    {
        _logger.LogInformation($"Custom Claim Exists in Subject");
    }
    else
    {
        _logger.LogInformation($"Custom Claim does not exist in Subject");
    }
    
    var httpContext = _httpContextAccessor.HttpContext;
    var access_token = httpContext.Request.Headers[HeaderNames.Authorization].ToString().Replace("Bearer ", "");
    
    var handler = new JwtSecurityTokenHandler();
    var jwtToken = handler.ReadJwtToken(access_token);
    if(jwtToken.Claims.Any(claim => claim.Type == "custom_claim"))
    {
        _logger.LogInformation($"Custom Claim Exists in Access token");
    }
    else 
    {
        _logger.LogInformation($"Custom Claim does not exist in Access token");
    }
    }

Expected behavior

As stated in the documentation:

Subject

The ClaimsPrincipal modeling the user associated with this request for profile data. When the profile service is invoked for tokens, the Subject property will contain the principal that was issued during user sign-in. When the profile service is called for requests to the userinfo endpoint, the Subject property will contain a claims principal populated with the claims in the access token used to authorize the userinfo call.

Additional context The Claims populated in the Subject property when Caller is UserInfoEndpoint are actually the claims issued when the user signed-in originally, which is curious if this endpoint is called via a back channel.

This was working as expected in a previous version (6.3 and net7.0) and changed when upgrading to net8.0 and v7. It might be related to a breaking change introduced in net8.0.

mikegarciam commented 3 months ago

Update: This only occurs when Server Side Sessions are enabled.

mikegarciam commented 2 months ago

Wondering if there's a workaround or a configuration issue I may have overlooked?

Is there any additional info I can provide to help assess this issue?

RolandGuijt commented 2 months ago

This behavior has indeed changed in v7. The PR for that is here. The documentation is not taking the change into account. We will fix that. This issue gave us the heads-up. Thank you for that.

Is the change a problem for you? If so in what way?

mikegarciam commented 2 months ago

Thank you for your reply, and for pointing me to the PR.

The way in which this affects my current use case is that I am issuing claims based on the AcrValues received in the Authentication Request, specifically the tenant value. These claims are added to the access token.

In the Client, I have the setting to request the claims from the user info endpoint and, since the access token is used on this call, I am able to retrieve the previously-issued claims and also handle the call based on the result of the original request (and its AcrValues)

Not having the access token claims available in the UserInfo endpoint call causes a disconnect between the claims principal that the web client stores in the authentication cookie (when requesting it via the userinfo endpoint) and the claims principal represented in the access token.

The work around would be to not use the userinfo endpoint and map the claims in some other way (based on the identity token or access token), or disable server side sessions altogether, but this solves the issue at the expense of other features.

Not sure if there's another way of handling this requirement that doesn't require to disable features or having to reconstruct the claims principal on the profile service from the access token.

Any guidance on this is appreciated.

RolandGuijt commented 2 months ago

FYI: thanks to your heads-up we corrected the documentation.

And also thanks for the additional information about your issue. Instead of or in addition to putting the claims in the access token can they be added to the session?

mikegarciam commented 2 months ago

Thank you for the suggestion. Indeed, the claims can be added to the session, and it will certainly solve the issue. But these claims are client-specific, they're only used within the context of the client application, so it feels like they shouldn't be a concern of the IdP, beyond issuing them when logging in via the OIDC protocol. Especially since the IdP will deal with multiple clients, each requesting its own set of claims.

Now that we've clarified how the calls to the Profile Service are handled when Server Sessions are enabled, I can adjust the logic and workaround the issue I was having.

It would be helpful, in any case, to have the claims from the access token accessible from Profile Service when called via the User Info endpoint. Perhaps as another property in the context object, separate from the current Subject. Could this be turned into a feature request?

Thank you again

RolandGuijt commented 2 months ago

They way I see it they are a concern of the IdP now since you are returning them from the userinfo endpoint. The endpoint meant to be an extension of what is put in the tokens. Besides that: In my view TenantId is a claim that could be relevant to the whole application landscape. So I don't see it as a problem to put it in the session. But I say that without knowing the details around your scenario of course. Alternatively you could consider creating a separate authorization API (access token protected) that returns TenantId along with other tenant and/or client specific claims and/or user settings. This pattern is typically used if additional claims are needed beyond the generic IdP ones.

RolandGuijt commented 1 month ago

@mikegarciam Do you have enough information or would you like to add anything? If not I'd like to close this issue.

mikegarciam commented 1 month ago

Thank you for the information provided @RolandGuijt. I have what I need and we can close this ticket.