DuendeSoftware / Support

Support for Duende Software products
20 stars 0 forks source link

Session ID is lost during Token Exchange #1312

Open tos-ilex opened 2 weeks ago

tos-ilex commented 2 weeks ago

Which version of Duende IdentityServer are you using? 7.0.5

Which version of .NET are you using? 8

Describe the bug When calling /connect/token with grant_type urn:ietf:params:oauth:grant-type:token-exchange and an existing access token with a sid claim, the resulting access token does not have a sid claim. A resulting refresh token is not correlated with the session, and an entry with SessionId=null is added to the persisted grant store for each exchange.

This is causing issues for us because we exchange tokens quite frequently and each one creates a fairly long-lived, orphaned refresh token entry in the persisted grant database that remains after logout because its session id is null.

To Reproduce

  1. get access token with a session id in the sid claim and offline_access in its scopes.
  2. make token exchange.
  3. observe that resulting token has no sid claim.
  4. observe that persisted grant store has new refresh_token entry with SessionId=NULL.

Expected behavior

sid of given access token should be put into exchanged token. No extra refresh token should go into the persisted grant store, rather the existing one should be renewed or replaced.

Additional context Our TokenExchangeGrantValidator:

public async Task ValidateAsync(ExtensionGrantValidationContext context)
    {
        context.Result = new GrantValidationResult(TokenRequestErrors.InvalidRequest);

        ... //some validation that does nothing or lets the request fail

        string? subjectTokenType = context.Request.Raw.Get(OidcConstants.TokenRequest.SubjectTokenType);
        TokenValidationResult? validationResult;
        switch (subjectTokenType)
        {
            case OidcConstants.TokenTypeIdentifiers.AccessToken:
                validationResult = await _validator.ValidateAccessTokenAsync(subjectToken); // Default ITokenValidator
                break;
            default:
                context.Result.ErrorDescription = TokenConstants.Errors.UnsupportedTokenType;
                return;
        }

        string sub = validationResult.Claims.First(c => c.Type == JwtClaimTypes.Subject).Value;

        var customClaims = new List<Claim>();

        string? redacted = context.Request.Raw.Get("redacted");
        if (redacted == null) // issue happens in both cases.
        {
            context.Result = new GrantValidationResult(sub, GrantType, customClaims);
            return;
        }

        customClaims.Add(new Claim("redacted"));

        context.Result = new GrantValidationResult(sub, GrantType, customClaims);
    }
saithis commented 1 week ago

And this together with the slow token cleanup query is a bad combination https://github.com/DuendeSoftware/Support/issues/1304

josephdecock commented 4 days ago

Have you tried copying the session id into the custom claims? We don't automatically copy the sid forward into an exchanged token because depending on the semantics of the exchange that may or may not be appropriate. But I can't think of anything that would prevent that from working.

tos-ilex commented 4 days ago

I have added this to our IProfileService implementation:

if (context.ValidatedRequest.SessionId != null)
    claims.Add(new Claim("sid", context.ValidatedRequest.SessionId));

context.AddRequestedClaims(claims);

This does result in the exchanged token having a correct sid claim. However, a new refresh token is issued anyway, and its session id in the Persisted Grant store is null: image Screenshot from my local test db after I

Let me clarify that our main problem isn't the missing sid claim (I just thought it was a clear sign that something is wrong). It's that new, "orphaned" refresh tokens are written to the PG store, making our DB fill up rapidly because we do frequent token exchanges. This is causing performance issues.