DuendeSoftware / Support

Support for Duende Software products
20 stars 0 forks source link

Breaking changes in ITokenService and IRefreshTokenService when updating from v4.1 to v6.3 (ArgumentNullException) #1291

Open eddex opened 1 month ago

eddex commented 1 month ago

Which version of Duende IdentityServer are you using? 6.3.9

Which version of .NET are you using? 6

Describe the bug After migrating my project from v4.1 to v6.3, I was facing two problems. Both problems resulted in an ArgumentNullException during token creation. I wasn't able to find any documentation about these breaking changes and it took me quite a while to figure out how to fix them. That's why I'm leaving this issue here.

I am using the default ITokenService and IRefreshTokenService (no custom implementations).

To Reproduce

Problem 1: When I tried to create a new access-token, there was an ArgumentNullException: "Value cannot be null. (Parameter 'value')" when calling ITokenService.CreateAccessTokenAsync(tokenCreationRequest). The exception happened when parsing claims. The 'iss' claim was in the list of claims but the value was null.

// this is how I called the method in v4.1
var validatedRequest = new ValidatedRequest()
{
    Subject = subject,
    ClientClaims = identityUser.AdditionalClaims,
    SessionId = Guid.NewGuid().ToString()
};
validatedRequest.SetClient(client);
// ...
var tokenRequest = new TokenCreationRequest()
{
    Subject = identityUser.CreatePrincipal(),
    ValidatedResources = resources,
    IncludeAllIdentityClaims = false,
    ValidatedRequest = validatedRequest,
};

The solution was to set the property TokenCreationRequest.ValidatedRequest.IssuerName. This used to work without setting this property manually in v4.1.

// With v6.3 I had to set the IssuerName to prevent the ArgumentNullException
var validatedRequest = new ValidatedRequest()
{
    Subject = subject,
    ClientClaims = identityUser.AdditionalClaims,
    SessionId = Guid.NewGuid().ToString(),
    // If this is not added here, the 'iss' claim won't be added to the claims of the token.
    // This causes issues with IdentityServer v6 and above because the 'iss' claim can't be null.
    IssuerName = issuer
};

Problem 2: After fixing problem 1, I was able to successfully create an access-token and a refresh-token and use them to access my APIs. However, once the access-token expired and I tried to create a new access-token using the refresh-token, there was again an ArgumentNullException: "Value cannot be null. (Parameter 'scopeValues')". This time it happened when calling IRefreshTokenService.CreateRefreshTokenAsync(refreshTokenCreationRequest).

// this is how I called the method in v4.1
var refreshToken = await this.refreshTokenService.CreateRefreshTokenAsync(new RefreshTokenCreationRequest
{
    Subject = subject,
    AccessToken = accessToken,
    Client = client
});

The solution was to set the RefreshTokenCreationRequest.AuthorizedScopes manually. This used to work without setting this property manually in v4.1. What's weird is that this information is already present in RefreshTokenCreationRequest.Client.AllowedScopes and I'm not sure why IS6.3 doesn't just take this information from there. Is there a reason for that? Am I using this method wrongly?

// This is how I was able to fix it in v6.3
var refreshToken = await this.refreshTokenService.CreateRefreshTokenAsync(new RefreshTokenCreationRequest
{
    Subject = subject,
    AccessToken = accessToken,
    Client = client,
    AuthorizedScopes = client.AllowedScopes
});

Expected behavior

In my opinion, this should be mentioned in one of the migration guides. I had a look at all the guides from 4.1->6.0, 6.0->6.1, 6.1->6.2 and 6.2->6.3 but didn't find any mention of these changes. Did I miss something?

AndersAbel commented 7 hours ago

What's weird is that this information is already present in RefreshTokenCreationRequest.Client.AllowedScopes and I'm not sure why IS6.3 doesn't just take this information from there.

Just because the client application is configured to be allowed to use the scopes doesn't mean that the current user session allows them. It could be that the client application didn't request all scopes in the call to the authorize endpoint. In that case we can only give access to the scopes that were requests. Or if the client application requested all scopes the user might have opted out of some of them on the consent screen.

This code has had some changes to it since version 4.1. While these services are normally not called directly from outside code, they are public and available. We will have to investigate if we need to update the documentation.