fix(oauth): persist reconnect ID token to stop SSO re-exchange rotation storm#981
Closed
paurosello wants to merge 2 commits into
Closed
fix(oauth): persist reconnect ID token to stop SSO re-exchange rotation storm#981paurosello wants to merge 2 commits into
paurosello wants to merge 2 commits into
Conversation
…on storm A long-lived SSO session that reconnects after its login-time ID token has expired (e.g. after a pod restart) re-inits its SSO backend connections in initSSOForSession using the ID token from the live request context, but never persisted that token to the OAuth-proxy store. The background re-exchange / forwarding closure (getIDTokenForForwarding) runs on a detached context.Background() and can only read that store, so it found nothing and logged "no subject ID token available for re-exchange", then fell back to the in-process refresher (RefreshSession -> RefreshAccessToken), which rotates the client's mcp refresh token. On a token-exchange backend whose continuous-listen retries every ~1s, this rotated the refresh-token family ~56x/min until two rotations collided and OAuth 2.1 reuse detection revoked the whole family, deauthing the session. Persist the request-context ID token in initSSOForSession so the store stays populated for as long as the session keeps making authenticated requests, which lets the background re-exchange resolve a subject without triggering the refresher. storeIDTokenForSSO already no-ops on empty/unparseable tokens. This is independent of the mcp-oauth provider-token rotation race (giantswarm/giantswarm#37164); a follow-up should stop the background re-exchange from rotating the client-facing refresh token at all (an mcp-oauth provider-only refresh that fires TokenRefreshHandler without rotating).
Contributor
Author
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
A long-lived SSO session (human dex login) got deauthed on a management cluster despite the mcp-oauth rotation-race work. Root cause is a muster lifecycle gap, independent of the mcp-oauth provider-token rotation race (giantswarm/giantswarm#37164):
onAuthenticatedtakes theauthAlivebranch and re-inits SSO viainitSSOForSessionusing the ID token from the live request context — but never persists it to the OAuth-proxy store.getIDTokenForForwardingruns on a detachedcontext.Background()and can only read that store, so it finds nothing → logsno subject ID token available for re-exchange→ falls back to the in-process refresherRefreshSession→RefreshAccessToken, which rotates the client's mcp refresh token.Connect-time token exchange kept succeeding the whole time (it reads the live request context), which is why the failure looked intermittent.
Fix
Persist the request-context ID token in
initSSOForSession(via the existingstoreIDTokenForSSO, which already no-ops on empty/unparseable tokens). BecauseonAuthenticatedre-inits on active requests, the proxy store now stays fresh for as long as the session keeps making authenticated requests, so the background re-exchange resolves a subject from the store and never hits the rotating refresher.Scope / follow-up
This kills the trigger. It does not remove the underlying hazard that the background SSO re-exchange refresher (
RefreshSession) rotates the client-facing refresh token at all. A proper follow-up is an mcp-oauth provider-only refresh entry point that firesTokenRefreshHandler(repopulating the ID token) without rotating the client's mcp refresh token; muster's re-exchange path would use that instead. Tracked as a follow-up so this safe, well-scoped fix can land first.Not fixed by bumping mcp-oauth to v1.1.1 — that coordinates the upstream provider token, not muster's SSO ID-token lifecycle or the client-refresh-token rotation.
Tests
TestInitSSOForSession_PersistsIDToken(+ no-op-on-empty subtest).internal/aggregatorpackage passes with-race;go vetclean.