DuendeSoftware / IdentityServer

The most flexible and standards-compliant OpenID Connect and OAuth 2.x framework for ASP.NET Core
https://duendesoftware.com/products/identityserver
Other
1.49k stars 348 forks source link

Upgrade from IS4 to IS7 can break RFC 7662 #1628

Open eduardsocea opened 3 days ago

eduardsocea commented 3 days ago

Which version of Duende IdentityServer are you using? v7.0.6

Which version of .NET are you using? v8.0.303

Describe the bug When upgrading from IdentityServer4 (IS4) to IdentityServer7 (IS7), some existing tokens generated with IS4 may cause issues after refresh. Once a IS4 token is refreshed, the newly generated IS7 token may break the introspection response based on RFC-7662 specification.

As per RFC 7662, iat is optional and an Integer timestamp. Right after the upgrade, for some tokens which are refreshed, the returned iat is an array of Integer type.

To Reproduce

  1. Setup Identity Server 4.
  2. Configure a Client with offline_access scope and updateAccessTokenClaimsOnRefresh: false.
  3. Perform authorization and generate a pair of access_token & refresh_token and keep the refresh_token
  4. Upgrade the system to Identity Server 7
  5. Using the refresh_token from step 3, generate a new pair of access_token & refresh token
  6. Call /introspect endpoint with the refresh_token from step 5.
  7. iat will be returned as an Array of Integer

Root cause:

Expected behavior

  1. Upgrade documentation should contain information about this breaking change.
  2. The method ToClaimsDictionary creates a dynamic response based on the enumeration of the Claims. By not having any sanitizer, it is easy to break RFC 7662. I would expect to have a sanitizer in order to make sure that RFC 7662 is followed and proper Warnings / Errors to be logged. Otherwise, the error is failing silently and noticed by the consumer (Client).

Additional context I found another github issue posted in 2021 which looks similar. From what I see, the other user experienced similar behavior (though there was an exception) and once all the broken refresh tokens were refreshed correctly, the user no longer experienced this issue. https://github.com/DuendeSoftware/IdentityServer/issues/549

With the current behavior, the error is silent and is may cause users to be disconnected because the consumer cannot parse the response of introspection endpoint.

RolandGuijt commented 2 days ago

Transferring this to IdentityServer's repo where it can be triaged.