From 7bb9636ecf87d2828d1e1abf4c66a3e2fd9ffb71 Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Tue, 9 Jun 2026 21:33:33 +0100 Subject: [PATCH] fix(client): accumulate scopes (union) on step-up authorization challenges (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/sep-2350-scope-union-step-up.md | 5 + packages/client/src/client/auth.ts | 28 ++++++ packages/client/src/client/streamableHttp.ts | 10 +- packages/client/src/index.ts | 1 + packages/client/test/client/auth.test.ts | 41 ++++++++ .../client/test/client/streamableHttp.test.ts | 93 +++++++++++++++++++ 6 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 .changeset/sep-2350-scope-union-step-up.md diff --git a/.changeset/sep-2350-scope-union-step-up.md b/.changeset/sep-2350-scope-union-step-up.md new file mode 100644 index 0000000000..24f4465ffa --- /dev/null +++ b/.changeset/sep-2350-scope-union-step-up.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Accumulate scopes (union) when re-authorizing after a `403 insufficient_scope` step-up challenge (SEP-2350). Previously the challenged scopes replaced the requested scope, so per-operation challenges dropped previously granted permissions. The client now requests the union of previously granted scopes (from stored tokens), previously requested scopes, and the newly challenged scopes. Adds an exported `unionScopes` helper to `@modelcontextprotocol/client`. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 5f55fb7a08..1532aa3e40 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -598,6 +598,34 @@ export function determineScope(options: { return effectiveScope; } +/** + * Computes the union of one or more space-delimited OAuth scope strings (SEP-2350). + * + * Per the MCP authorization spec's step-up authorization flow, scope accumulation is a + * client-side responsibility: when re-authorizing after an `insufficient_scope` challenge, + * clients SHOULD request the union of previously requested/granted scopes and the newly + * challenged scopes, so that per-operation challenges don't drop previously granted + * permissions. + * + * Scopes are treated as opaque strings (no hierarchy-aware deduplication). The result + * preserves first-seen order and removes exact duplicates. Returns `undefined` when no + * scopes are present in any input. + */ +export function unionScopes(...scopeStrings: Array): string | undefined { + const seen = new Set(); + for (const scopeString of scopeStrings) { + if (!scopeString) { + continue; + } + for (const scope of scopeString.split(/\s+/)) { + if (scope) { + seen.add(scope); + } + } + } + return seen.size > 0 ? [...seen].join(' ') : undefined; +} + async function authInternal( provider: OAuthClientProvider, { diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index 3b8ddafe5a..68051b0554 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -16,7 +16,7 @@ import { import { EventSourceParserStream } from 'eventsource-parser/stream'; import type { AuthProvider, OAuthClientProvider } from './auth.js'; -import { adaptOAuthProvider, auth, extractWWWAuthenticateParams, isOAuthClientProvider, UnauthorizedError } from './auth.js'; +import { adaptOAuthProvider, auth, extractWWWAuthenticateParams, isOAuthClientProvider, UnauthorizedError, unionScopes } from './auth.js'; // Default reconnection options for StreamableHTTP connections const DEFAULT_STREAMABLE_HTTP_RECONNECTION_OPTIONS: StreamableHTTPReconnectionOptions = { @@ -609,7 +609,13 @@ export class StreamableHTTPClientTransport implements Transport { } if (scope) { - this._scope = scope; + // SEP-2350: scope accumulation is a client-side responsibility. When + // re-authorizing after a scope challenge, request the union of + // previously granted scopes (from the stored tokens), previously + // requested scopes, and the newly challenged scopes, so per-operation + // challenges don't drop previously granted permissions. + const grantedTokens = await this._oauthProvider.tokens(); + this._scope = unionScopes(grantedTokens?.scope, this._scope, scope); } if (resourceMetadataUrl) { diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 8a08e8fd79..bb93dd1c73 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -35,6 +35,7 @@ export { selectResourceURL, startAuthorization, UnauthorizedError, + unionScopes, validateClientMetadataUrl } from './client/auth.js'; export type { diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 04d7f4a3fb..0b6156743e 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -19,6 +19,7 @@ import { registerClient, selectClientAuthMethod, startAuthorization, + unionScopes, validateClientMetadataUrl } from '../../src/client/auth.js'; import { createPrivateKeyJwtAuth } from '../../src/client/authExtensions.js'; @@ -4131,3 +4132,43 @@ describe('OAuth Authorization', () => { }); }); }); + +describe('unionScopes (SEP-2350)', () => { + it('returns undefined when called with no arguments', () => { + expect(unionScopes()).toBeUndefined(); + }); + + it('returns undefined when all inputs are undefined or empty', () => { + expect(unionScopes(undefined, undefined)).toBeUndefined(); + expect(unionScopes('', undefined, '')).toBeUndefined(); + expect(unionScopes(' ')).toBeUndefined(); + }); + + it('returns a single scope string unchanged', () => { + expect(unionScopes('read')).toBe('read'); + expect(unionScopes('read write')).toBe('read write'); + }); + + it('unions multiple scope strings preserving first-seen order', () => { + expect(unionScopes('read write', 'admin')).toBe('read write admin'); + expect(unionScopes('admin', 'read write')).toBe('admin read write'); + }); + + it('deduplicates repeated scopes across inputs', () => { + expect(unionScopes('read write', 'write admin')).toBe('read write admin'); + expect(unionScopes('read', 'read', 'read')).toBe('read'); + }); + + it('skips undefined and empty entries between scope strings', () => { + expect(unionScopes(undefined, 'read', undefined, 'write')).toBe('read write'); + expect(unionScopes('', 'admin')).toBe('admin'); + }); + + it('normalizes extra whitespace between scopes', () => { + expect(unionScopes('read write', ' admin ')).toBe('read write admin'); + }); + + it('does not perform hierarchy-aware deduplication (scopes are opaque strings)', () => { + expect(unionScopes('repo', 'repo:read')).toBe('repo repo:read'); + }); +}); diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 0edf8b75ac..4303e3d297 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -3,6 +3,7 @@ import { OAuthError, OAuthErrorCode, SdkErrorCode, SdkHttpError } from '@modelco import type { Mock, Mocked } from 'vitest'; import type { OAuthClientProvider } from '../../src/client/auth.js'; +import * as authModule from '../../src/client/auth.js'; import { UnauthorizedError } from '../../src/client/auth.js'; import type { ReconnectionScheduler, StartSSEOptions, StreamableHTTPReconnectionOptions } from '../../src/client/streamableHttp.js'; import { StreamableHTTPClientTransport } from '../../src/client/streamableHttp.js'; @@ -876,6 +877,98 @@ describe('StreamableHTTPClientTransport', () => { authSpy.mockRestore(); }); + describe('scope accumulation on step-up authorization (SEP-2350)', () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const mock403Then202 = (challengeScope: string) => { + (globalThis.fetch as Mock) + .mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': `Bearer error="insufficient_scope", scope="${challengeScope}"` + }), + text: () => Promise.resolve('Insufficient scope') + }) + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + }; + + it('requests the union of previously granted scopes and the challenged scope', async () => { + mockAuthProvider.tokens.mockResolvedValue({ + access_token: 'test-token', + token_type: 'Bearer', + scope: 'read write' + }); + mock403Then202('admin'); + + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + expect(authSpy).toHaveBeenCalledWith( + mockAuthProvider, + expect.objectContaining({ + scope: 'read write admin' + }) + ); + + authSpy.mockRestore(); + }); + + it('deduplicates when the challenge repeats an already-granted scope', async () => { + mockAuthProvider.tokens.mockResolvedValue({ + access_token: 'test-token', + token_type: 'Bearer', + scope: 'read write' + }); + mock403Then202('write admin'); + + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + expect(authSpy).toHaveBeenCalledWith( + mockAuthProvider, + expect.objectContaining({ + scope: 'read write admin' + }) + ); + + authSpy.mockRestore(); + }); + + it('uses only the challenged scope when there is no prior scope', async () => { + mockAuthProvider.tokens.mockResolvedValue(undefined); + mock403Then202('admin'); + + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + expect(authSpy).toHaveBeenCalledWith( + mockAuthProvider, + expect.objectContaining({ + scope: 'admin' + }) + ); + + authSpy.mockRestore(); + }); + }); + describe('Reconnection Logic', () => { let transport: StreamableHTTPClientTransport;