From 03bf0627b28479c899b444b83ba852fdad21b92d Mon Sep 17 00:00:00 2001 From: g97iulio1609 Date: Sat, 28 Feb 2026 07:15:56 +0100 Subject: [PATCH 1/2] fix: accumulate OAuth scopes across 401/403 responses for progressive authorization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace scope overwrite with union-based accumulation in StreamableHTTPClientTransport. Both the 401 and 403 (insufficient_scope) handlers now merge new scopes with previously-granted scopes, preventing the infinite re-authorization loop described in #1582. Adds mergeScopes() helper that computes the set union of space-delimited OAuth scope strings per RFC 6749 §3.3. Closes #1582 --- packages/client/src/client/streamableHttp.ts | 19 ++++- .../client/test/client/streamableHttp.test.ts | 82 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) 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; From d2d30d8cac77cd5dd9598a6c66fc6254dabc5ac4 Mon Sep 17 00:00:00 2001 From: giulio-leone <6887247+giulio-leone@users.noreply.github.com> Date: Fri, 6 Mar 2026 12:09:03 +0100 Subject: [PATCH 2/2] chore: add changeset for scope accumulation fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .changeset/fix-scope-accumulation.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-scope-accumulation.md 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.