diff --git a/.changeset/fix-scope-accumulation.md b/.changeset/fix-scope-accumulation.md new file mode 100644 index 000000000..afe502273 --- /dev/null +++ b/.changeset/fix-scope-accumulation.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Accumulate OAuth scopes across 401/403 responses for progressive authorization. When an MCP server returns a 401 or 403 with a new required scope, the client now merges the new scope into the existing set rather than replacing it, preventing authorization loops where gaining one scope loses another. diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index 79a20adfc..826aed97e 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -16,6 +16,21 @@ import { EventSourceParserStream } from 'eventsource-parser/stream'; import type { AuthResult, OAuthClientProvider } from './auth.js'; import { auth, extractWWWAuthenticateParams, UnauthorizedError } from './auth.js'; +/** + * Merges OAuth scopes by computing the union of existing and new space-delimited scope strings. + * Per RFC 6749 ยง3.3, scopes are space-delimited, case-sensitive strings. + */ +function mergeScopes(existing: string | undefined, incoming: string | undefined): string | undefined { + if (!incoming) return existing; + if (!existing) return incoming; + + const existingSet = new Set(existing.split(' ')); + for (const s of incoming.split(' ')) { + existingSet.add(s); + } + return [...existingSet].join(' '); +} + // Default reconnection options for StreamableHTTP connections const DEFAULT_STREAMABLE_HTTP_RECONNECTION_OPTIONS: StreamableHTTPReconnectionOptions = { initialReconnectionDelay: 1000, @@ -504,7 +519,7 @@ export class StreamableHTTPClientTransport implements Transport { const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); this._resourceMetadataUrl = resourceMetadataUrl; - this._scope = scope; + this._scope = mergeScopes(this._scope, scope); const result = await auth(this._authProvider, { serverUrl: this._url, @@ -537,7 +552,7 @@ export class StreamableHTTPClientTransport implements Transport { } if (scope) { - this._scope = scope; + this._scope = mergeScopes(this._scope, scope); } if (resourceMetadataUrl) { diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 8a550feae..c12301faf 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -727,6 +727,88 @@ describe('StreamableHTTPClientTransport', () => { authSpy.mockRestore(); }); + it('accumulates scopes across multiple 403 responses for progressive authorization', async () => { + const message1: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'tools/list', + params: {}, + id: 'test-1' + }; + const message2: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'tools/call', + params: {}, + id: 'test-2' + }; + + const fetchMock = globalThis.fetch as Mock; + + // Spy on the imported auth function and mock successful authorization + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + // First request: 403 requesting "mcp:tools:read", then success + fetchMock + .mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="mcp:tools:read"' + }), + text: () => Promise.resolve('Insufficient scope') + }) + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + await transport.send(message1); + + // Verify auth was called with the first scope + expect(authSpy).toHaveBeenCalledWith( + mockAuthProvider, + expect.objectContaining({ + scope: 'mcp:tools:read' + }) + ); + + // Reset upscoping circuit breaker for next send + fetchMock.mockReset(); + authSpy.mockClear(); + + // Second request: 403 requesting "mcp:tools:write", then success + fetchMock + .mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="mcp:tools:write"' + }), + text: () => Promise.resolve('Insufficient scope') + }) + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + await transport.send(message2); + + // Verify auth was called with ACCUMULATED scopes (both read and write) + expect(authSpy).toHaveBeenCalledWith( + mockAuthProvider, + expect.objectContaining({ + scope: 'mcp:tools:read mcp:tools:write' + }) + ); + + authSpy.mockRestore(); + }); + describe('Reconnection Logic', () => { let transport: StreamableHTTPClientTransport;