fix(oauth): stop leaking access-token format across requests (#139)#157
Merged
Merged
Conversation
AccessTokenTypeHandler switched JWT clients onto self-contained tokens by setting context.Options.UseReferenceAccessTokens = false on a ProcessSignInContext handler. But context.Options is the process-wide OpenIddictServerOptions singleton (OpenIddictServerFactory assigns IOptionsMonitor.CurrentValue, the same cached instance every request), and the handler never restored it. The first JWT client to authenticate flipped the global reference default off process-wide, so every Reference-configured client that authenticated afterwards silently received a self-contained JWT instead of an opaque reference token — losing instant revocation — and the shared write races concurrent requests. Move the handler to GenerateTokenContext (a per-request context) and set the per-request IsReferenceToken + PersistTokenPayload switches for JWT clients on the access token only. context.Options is never mutated. Refresh tokens are untouched. Adds ReferenceClient_TokenFormat_IsNotLeaked_By_A_Prior_JwtClient: drives a JWT client first (which flipped the shared flag under the old code) then a reference client, asserting the reference token stays opaque. Fails pre-fix (JWT at pos ~110), passes post-fix. Unit 1395/1395, integration 517/517 green. Also updates docs/integrate/oauth.md, whose AccessTokenTypeHandler snippet showed the old ProcessSignInContext + context.Options mutation — now the per-request GenerateTokenContext form, with a note on why Options must not be mutated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
AccessTokenTypeHandlerswitched JWT clients onto self-contained tokens bysetting
context.Options.UseReferenceAccessTokens = falseon aProcessSignInContexthandler. Butcontext.Optionsis the process-wideOpenIddictServerOptionssingleton —OpenIddictServerFactoryassignsOptions = IOptionsMonitor.CurrentValue, the same cached instance every request(verified by decompiling OpenIddict 7.5). The handler mutated that singleton and
never restored it (early-return on
!= Jwt).Effect: the first JWT client to authenticate flipped the global reference
default off process-wide, so every
AccessTokenType.Referenceclient thatauthenticated afterwards silently received a self-contained JWT instead of an
opaque reference token — losing instant revocation. The shared write also
races concurrent requests. It stayed invisible because
/connect/userinfoand/connect/introspectwork identically for JWT and reference tokens.This is the root cause behind the ordering-dependent token-format observation
flagged while building #139 (reference token opaque in a solo run, JWT-shaped in
the full class run).
Fix
Move
AccessTokenTypeHandlerfromProcessSignInContexttoGenerateTokenContext(a per-request context) and set the per-requestIsReferenceToken/PersistTokenPayloadswitches for JWT clients on theaccess token only.
context.Optionsis never mutated. Refresh tokens areuntouched. Ordered just ahead of
CreateTokenEntryso every downstream handler(notably
AttachTokenPayload) sees the override.Test
Adds
ReferenceClient_TokenFormat_IsNotLeaked_By_A_Prior_JwtClient: drives a JWTclient first (which flipped the shared flag under the old code) then a reference
client, asserting the reference token stays opaque (an OpenIddict reference
access token is
Base64Url(256 random bytes)— never a dot; a dot ⟹ JWT).Fails pre-fix (JWT at pos ~110), passes post-fix.
Docs
Updates
docs/integrate/oauth.md, whoseAccessTokenTypeHandlersnippet showedthe old
ProcessSignInContext+context.Optionsmutation — now theper-request
GenerateTokenContextform, with a note on whyOptions(the sharedsingleton) must not be mutated.
Verification
🤖 Generated with Claude Code