fix(oauth): make the token-status route's canRefresh match the real predicate - #6160
Open
pedrofrxncx wants to merge 1 commit into
Open
fix(oauth): make the token-status route's canRefresh match the real predicate#6160pedrofrxncx wants to merge 1 commit into
pedrofrxncx wants to merge 1 commit into
Conversation
…redicate The /oauth-token/status route hand-rolled canRefresh as refreshToken && tokenEndpoint, omitting the clientId check that oauth/token-refresh.ts's canRefresh() (the predicate the actual refresh path uses) requires. A token saved with a refresh token and endpoint but no clientId would report canRefresh: true here while a real refresh attempt fails immediately in refresh-access-token.ts with "No client ID available" — the status endpoint lies about whether the token is actually refreshable. Reuse the canonical canRefresh() instead of a second, drifted copy.
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.
Source: bug found while auditing the per-user OAuth flow (lazy-client/virtual-mcp/mcp-oauth) —
GET /api/:org/connections/:connectionId/oauth-token/statusinapps/api/src/api/routes/downstream-token.ts.Payoff: the status route computed
canRefreshinline as!!token.refreshToken && !!token.tokenEndpoint, but the actual refresh path (oauth/token-refresh.ts's exportedcanRefresh(), used bygetValidDownstreamAccessToken) additionally requiresclientId— andrefresh-access-token.tsfails immediately with "No client ID available" when it's missing. A token saved with a refresh token + endpoint but noclientIdwould reportcanRefresh: truefrom this endpoint while a real refresh attempt is guaranteed to fail. Any caller of this status endpoint (current or future UI, or an external app integrating per the documented API) gets a wrong signal about whether the connection can self-heal.Fix: import and reuse the canonical
canRefresh()fromoauth/token-refresh.tsinstead of a second, drifted copy of the same predicate.Reviewer check:
grep -n canRefresh apps/api/src/api/routes/downstream-token.ts apps/api/src/oauth/token-refresh.ts— one definition, one call site.Verified locally:
bun run fmt,cd apps/api && bunx tsc --noEmit(clean),bunx oxlint apps/api/src/api/routes/downstream-token.ts(0 warnings/errors). No local Postgres available to run the route's integration test tier; full CI covers that.Summary by cubic
Aligns the OAuth token status route with the real refresh predicate so
canRefreshno longer returns a false positive whenclientIdis missing. Previously, the route reportedcanRefresh: trueifrefreshTokenandtokenEndpointexisted; now it uses the canonicalcanRefresh(token)that also requiresclientId, matching the actual refresh path.GET /api/:org/connections/:connectionId/oauth-token/statusonly; response shape is unchanged, but tokens withoutclientIdnow returncanRefresh: false.canRefreshfromoauth/token-refreshto keep a single source of truth.Written for commit 480efe4. Summary will update on new commits.