fix(client): accumulate scopes (union) on step-up authorization challenges (SEP-2350)#2265
Open
mattzcarey wants to merge 1 commit into
Open
fix(client): accumulate scopes (union) on step-up authorization challenges (SEP-2350)#2265mattzcarey wants to merge 1 commit into
mattzcarey wants to merge 1 commit into
Conversation
…enges (SEP-2350) Per the draft authorization spec's step-up authorization flow, scope accumulation is a client-side responsibility: when re-authorizing after a 403 insufficient_scope challenge, the client SHOULD request the union of previously requested/granted scopes and the newly challenged scopes. Previously the StreamableHTTP transport replaced the requested scope with the challenged scopes only, dropping previously granted permissions. - Add exported unionScopes() helper (opaque-string, order-preserving, deduped union of space-delimited scope strings) - Union stored token scope + previously requested scope + challenged scope in the 403 insufficient_scope retry path - Tests for the step-up union, dedup, and no-prior-scope cases plus unionScopes unit tests Closes #2200
🦋 Changeset detectedLatest commit: 7bb9636 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
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
1 participant
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.
Implements SEP-2350 (tracking issue #2200): client-side scope accumulation in step-up authorization.
Spec
The draft authorization spec (
basic/authorization/index.mdx, "Scope Challenge Handling" note and "Step-Up Authorization Flow" step 2) makes scope accumulation a client-side responsibility:Servers stay stateless and emit only per-operation scopes in
WWW-Authenticate: Bearer error="insufficient_scope", scope="..."challenges.Before / after
Before: the
403 insufficient_scoperetry path inStreamableHTTPClientTransportdidthis._scope = scope— the challenged scopes replaced the requested scope, so the re-authorization dropped everything previously granted. A client holdingread writethat hit a challenge foradminwould re-authorize with onlyadmin.After: the client requests the union of (1) the scope on the stored tokens (previously granted), (2) the previously requested scope, and (3) the newly challenged scope. The same client now re-authorizes with
read write admin. The union is order-preserving and exact-string-deduped — scopes are treated as opaque strings per the spec, so no hierarchy-aware deduplication (repodoes not absorbrepo:read).The helper is exported as
unionScopes(...scopeStrings: Array<string | undefined>): string | undefinedfrom@modelcontextprotocol/client.Retry limiting is unchanged: the existing
_lastUpscopingHeaderguard still prevents infinite upscoping loops.Decision note for reviewers: 401 path left as-is
The
401path (handleOAuthUnauthorized/ thethis._scope = scopeassignments in the 401 branches ofstreamableHttp.tsandsse.ts) is intentionally unchanged. The spec's union language is specifically about re-authorization during step-up after aninsufficient_scopechallenge; a 401 carries initial-auth semantics (no valid grant to preserve), and applying the union there is debatable. Flagging explicitly in case reviewers want union-on-401 too — happy to follow up.Relatedly,
sse.tshas no403 insufficient_scopehandler at all (only 401 paths), so there was nothing to mirror there.Validation
pnpm build:all✅pnpm typecheck:all✅pnpm lint:all✅pnpm --filter @modelcontextprotocol/client test— 378/378 ✅ (new: 3 step-up integration tests instreamableHttp.test.tscovering union, dedup-on-repeated-scope, and no-prior-scope; 8unionScopesunit tests inauth.test.ts)@modelcontextprotocol/clientDownstream impact: cloudflare/agents needs no changes — its DurableObjectOAuthClientProvider persists token scope, so the union picks previously granted scopes up automatically.
A follow-up PR implementing SEP-2468 (iss validation) touches the same transport retry region and will be rebased over this.
Closes #2200