stacklok / toolhive / 33531463669

70%

Build Type

push

github

Committed by web-flow

Commit Message

fix(authserver): scope upstream-token callback cleanup to the chain's providers (#6476)

* fix(authserver): scope upstream-token callback cleanup to the chain's providers

The /oauth/callback error paths cleaned up "the tokens this authorization
collected" by calling the session-wide DeleteUpstreamTokens(ctx, sessionID),
which removes every provider row under that session. That is correct for the
native (session, provider) storage scheme, where each authorization mints its
own session.

It is wrong under a storage decorator that re-keys upstream tokens by
(gateway, user) so a session-independent caller (e.g. a user-facing UI) can
find a user's tokens. Under that scheme every one of a user's connector tokens
shares the session slot, so a session-wide delete on any failing login leg
deletes the user's entire connector set — a transient Directory/storage blip on
the most-traversed leg (chain resolution) wipes a user's whole historical token
set, and correlated failures widen that to every user logging in during the
window. It fails safe (never grants access) and is largely self-healing, but the
availability hit is real.

The callback is the only production caller of the session-wide delete, so:

- Route all nine callback cleanup sites through a new cleanupUpstreamTokens
  helper that deletes per provider via DeleteUpstreamTokensForProvider (already
  on the storage interface, implemented by memory and redis), sourcing the
  provider list from the chain config — never a storage read, which a
  broadening decorator cannot scope. Sites with the resolved chain pass it;
  pre-resolve sites pass pending.ChainUpstreams plus the current leg.
- A user-keyed decorator can now scope each delete to exactly this chain's
  providers, so a connector outside the chain is never touched.

Behavior note: a stale subsequent-leg pending with no ChainUpstreams can only
name the leg just processed; an earlier leg it cannot name is left to expire by
TTL rather than risk a sessi... (continued)

Coverage Stats

22 of 23 new or added lines in 1 file covered. (95.65%)

22 existing lines in 7 files now uncovered.

76703 of 109917 relevant lines covered (69.78%)

91.63 hits per line