Auth: Fix re-entrant /token call after OAuth code exchange#22097
Merged
leekelleher merged 1 commit intomainfrom Mar 11, 2026
Merged
Auth: Fix re-entrant /token call after OAuth code exchange#22097leekelleher merged 1 commit intomainfrom
leekelleher merged 1 commit intomainfrom
Conversation
Move #inSessionUpdateCallback guard into #setSessionLocally() so all callers are protected, not just makeRefreshTokenRequest()'s lock callback. Previously, completeAuthorizationRequest() called #setSessionLocally() directly without setting the flag. With keepUserLoggedIn=true and a short TimeOut, session$ observers fired synchronously inside #setSessionLocally, triggering #onSessionExpiring → validateToken() → makeRefreshTokenRequest() before #inSessionUpdateCallback was ever set — causing a second /token call immediately after the initial code exchange 200. The no-Web-Locks fallback path in makeRefreshTokenRequest() had the same gap. Moving the flag into #setSessionLocally() covers all call sites. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an auth flow re-entrancy bug where session observers could synchronously trigger a second /token request immediately after an OAuth code exchange (and in the no-Web-Locks fallback), by ensuring the re-entrancy guard applies to all session-setting call paths.
Changes:
- Moves the
#inSessionUpdateCallbacktry/finally guard into#setSessionLocally()so it covers all callers. - Removes the now-redundant guard wrapper around
#updateSession()in the Web Locks refresh callback.
leekelleher
approved these changes
Mar 11, 2026
leekelleher
deleted the
v17/bugfix/fix-reentrant-token-call-after-code-exchange
branch
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.
What
Moves the
#inSessionUpdateCallbackre-entrancy guard into#setSessionLocally()so all callers are covered, not just the Web Locks lock callback.Why (the bug)
PR #22087 added
#inSessionUpdateCallbackinmakeRefreshTokenRequest()'s lock callback to prevent a double/tokencall whensession$fired synchronously inside#updateSession(). ButcompleteAuthorizationRequest()calls#setSessionLocally()directly — without ever setting the flag.With
KeepUserLoggedIn=trueand a shortTimeOut(e.g.00:01:00, givingexpiresIn=15s),secondsUntilWarning=0so#onSessionExpiring()fires synchronously from inside#setSessionLocally(). This path was unguarded, producing a second/tokenPOST immediately after the code exchange 200.The same gap existed in the no-Web-Locks fallback path in
makeRefreshTokenRequest().Fix
Move the
try/finallyguard into#setSessionLocally()itself. Remove the now-redundant wrapper inmakeRefreshTokenRequest()'s lock callback. The guard at the top ofmakeRefreshTokenRequest()(if (this.#inSessionUpdateCallback) return true) is unchanged — it still works because the flag is now set beforesession$emits.makeRefreshTokenRequest()lock callback →#updateSession()→#setSessionLocally()#setSessionLocally()completeAuthorizationRequest()→#setSessionLocally()makeRefreshTokenRequest()no-locks fallback →#updateSession()→#setSessionLocally()Testing
Verified with
KeepUserLoggedIn=true+TimeOut=00:01:00:/token [200]after code exchange (was 2)ID2019errors[Auth] Session expiring, auto-refreshingimmediately after login/token [200]per cycle🤖 Generated with Claude Code