diff --git a/packages/client/src/client/sse.ts b/packages/client/src/client/sse.ts index 133aa0004..5ba72a5da 100644 --- a/packages/client/src/client/sse.ts +++ b/packages/client/src/client/sse.ts @@ -6,6 +6,21 @@ import { EventSource } from 'eventsource'; import type { AuthResult, OAuthClientProvider } from './auth.js'; import { auth, extractWWWAuthenticateParams, UnauthorizedError } from './auth.js'; +/** + * Merges two space-separated OAuth scope strings into a deduplicated union. + * Returns undefined when the resulting set is empty. + * Preserves insertion order of first occurrence for determinism. + */ +function mergeScopes(existing: string | undefined, incoming: string | undefined): string | undefined { + const existingTokens = existing?.split(/\s+/).filter(Boolean) ?? []; + const incomingTokens = incoming?.split(/\s+/).filter(Boolean) ?? []; + const merged = new Set([...existingTokens, ...incomingTokens]); + if (merged.size === 0) { + return undefined; + } + return [...merged].join(' '); +} + export class SseError extends Error { constructor( public readonly code: number | undefined, @@ -152,7 +167,7 @@ export class SSEClientTransport implements Transport { if (response.status === 401 && response.headers.has('www-authenticate')) { const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); this._resourceMetadataUrl = resourceMetadataUrl; - this._scope = scope; + this._scope = mergeScopes(this._scope, scope); } return response; @@ -266,7 +281,7 @@ export class SSEClientTransport implements Transport { if (response.status === 401 && this._authProvider) { 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, diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index dab9b37ab..67c3a3e5a 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 two space-separated OAuth scope strings into a deduplicated union. + * Returns undefined when the resulting set is empty. + * Preserves insertion order of first occurrence for determinism. + */ +function mergeScopes(existing: string | undefined, incoming: string | undefined): string | undefined { + const existingTokens = existing?.split(/\s+/).filter(Boolean) ?? []; + const incomingTokens = incoming?.split(/\s+/).filter(Boolean) ?? []; + const merged = new Set([...existingTokens, ...incomingTokens]); + if (merged.size === 0) { + return undefined; + } + return [...merged].join(' '); +} + // Default reconnection options for StreamableHTTP connections const DEFAULT_STREAMABLE_HTTP_RECONNECTION_OPTIONS: StreamableHTTPReconnectionOptions = { initialReconnectionDelay: 1000, @@ -505,7 +520,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, @@ -538,7 +553,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/sse.test.ts b/packages/client/test/client/sse.test.ts index 0b0aff67b..608f21ba2 100644 --- a/packages/client/test/client/sse.test.ts +++ b/packages/client/test/client/sse.test.ts @@ -1171,6 +1171,127 @@ describe('SSEClientTransport', () => { await expect(() => transport.start()).rejects.toMatchObject(expectedError); expect(mockAuthProvider.invalidateCredentials).toHaveBeenCalledWith('tokens'); }); + + it('accumulates scopes from sequential 401 responses in send()', async () => { + // Create server that accepts SSE connection but returns 401 on POST + // with different scopes on successive requests + resourceServer.close(); + authServer.close(); + + await new Promise(resolve => { + authServer.listen(0, '127.0.0.1', () => { + const addr = authServer.address() as AddressInfo; + authBaseUrl = new URL(`http://127.0.0.1:${addr.port}`); + resolve(); + }); + }); + + let postCallCount = 0; + resourceServer = createServer((req, res) => { + lastServerRequest = req; + + switch (req.method) { + case 'GET': { + if (req.url === '/.well-known/oauth-protected-resource') { + res.writeHead(200, { + 'Content-Type': 'application/json' + }).end( + JSON.stringify({ + resource: resourceBaseUrl.href, + authorization_servers: [`${authBaseUrl}`] + }) + ); + return; + } + + if (req.url !== '/') { + res.writeHead(404).end(); + return; + } + + res.writeHead(200, { + 'Content-Type': 'text/event-stream', + 'Cache-Control': 'no-cache, no-transform', + Connection: 'keep-alive' + }); + res.write('event: endpoint\n'); + res.write(`data: ${resourceBaseUrl.href}\n\n`); + break; + } + + case 'POST': { + postCallCount++; + if (postCallCount === 1) { + // First POST: 401 with scope="read:op1" + res.writeHead(401, { + 'WWW-Authenticate': 'Bearer scope="read:op1"' + }); + res.end(); + } else if (postCallCount === 2) { + // Second POST (after first auth): 401 with scope="read:op2" + res.writeHead(401, { + 'WWW-Authenticate': 'Bearer scope="read:op2"' + }); + res.end(); + } else { + // Third POST (after second auth): success + res.writeHead(200); + res.end(); + } + break; + } + } + }); + + resourceBaseUrl = await listenOnRandomPort(resourceServer); + + transport = new SSEClientTransport(resourceBaseUrl, { + authProvider: mockAuthProvider + }); + + // Spy on the auth function to track scope arguments + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.start(); + + const message: JSONRPCMessage = { + jsonrpc: '2.0', + id: '1', + method: 'test', + params: {} + }; + + await transport.send(message); + + // Auth should have been called twice + expect(authSpy).toHaveBeenCalledTimes(2); + + // First auth call should have scope "read:op1" + expect(authSpy).toHaveBeenNthCalledWith( + 1, + mockAuthProvider, + expect.objectContaining({ + scope: 'read:op1' + }) + ); + + // Second auth call should have accumulated scope containing both tokens + const secondCall = authSpy.mock.calls[1] as unknown as [unknown, { scope?: string }]; + const secondCallScope = secondCall[1].scope; + expect(secondCallScope).toBeDefined(); + const scopeTokens = String(secondCallScope).split(' ').toSorted(); + expect(scopeTokens).toEqual(['read:op1', 'read:op2']); + + // Verify _scope state after accumulation + const finalScope = (transport as unknown as { _scope: string | undefined })['_scope']; + expect(finalScope).toBeDefined(); + const finalScopeTokens = String(finalScope).split(' ').toSorted(); + expect(finalScopeTokens).toEqual(['read:op1', 'read:op2']); + + authSpy.mockRestore(); + }); }); describe('custom fetch in auth code paths', () => { diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 0398964d3..5c167e9f4 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -755,6 +755,331 @@ describe('StreamableHTTPClientTransport', () => { authSpy.mockRestore(); }); + describe('mergeScopes behavior (via transport)', () => { + it('accumulates scopes from a 401 followed by a 403 with different scopes', async () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const fetchMock = globalThis.fetch as Mock; + fetchMock + // First call: 401 with scope="read:op1" + .mockResolvedValueOnce({ + ok: false, + status: 401, + statusText: 'Unauthorized', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer scope="read:op1"' + }), + text: () => Promise.resolve('Unauthorized') + }) + // Second call (retry after auth): 403 insufficient_scope with scope="read:op2" + .mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="read:op2"' + }), + text: () => Promise.resolve('Insufficient scope') + }) + // Third call (retry after second auth): 200 OK + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // Verify auth was called twice + expect(authSpy).toHaveBeenCalledTimes(2); + + // First auth call should have scope "read:op1" + expect(authSpy).toHaveBeenNthCalledWith( + 1, + mockAuthProvider, + expect.objectContaining({ + scope: 'read:op1' + }) + ); + + // Second auth call should have accumulated scope "read:op1 read:op2" + const secondCall = authSpy.mock.calls[1] as unknown as [unknown, { scope?: string }]; + const secondCallScope = secondCall[1].scope; + expect(secondCallScope).toBeDefined(); + const scopeTokens = String(secondCallScope).split(' ').toSorted(); + expect(scopeTokens).toEqual(['read:op1', 'read:op2']); + + // Verify fetch was called three times + expect(fetchMock).toHaveBeenCalledTimes(3); + + // Verify _scope state after accumulation + const finalScope = (transport as unknown as { _scope: string | undefined })['_scope']; + expect(finalScope).toBeDefined(); + const finalScopeTokens = String(finalScope).split(' ').toSorted(); + expect(finalScopeTokens).toEqual(['read:op1', 'read:op2']); + + authSpy.mockRestore(); + }); + + it('deduplicates repeated scope tokens during accumulation', async () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const fetchMock = globalThis.fetch as Mock; + fetchMock + // First call: 401 with scope="read:op1 read:op2" + .mockResolvedValueOnce({ + ok: false, + status: 401, + statusText: 'Unauthorized', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer scope="read:op1 read:op2"' + }), + text: () => Promise.resolve('Unauthorized') + }) + // Second call: 403 with scope="read:op2" (duplicate) + .mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="read:op2"' + }), + text: () => Promise.resolve('Insufficient scope') + }) + // Third call: 200 OK + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // Second auth call scope should be deduplicated + const secondCall = authSpy.mock.calls[1] as unknown as [unknown, { scope?: string }]; + const secondCallScope = secondCall[1].scope; + expect(secondCallScope).toBeDefined(); + const scopeTokens = String(secondCallScope).split(' ').toSorted(); + expect(scopeTokens).toEqual(['read:op1', 'read:op2']); + + authSpy.mockRestore(); + }); + + it('preserves existing scope when incoming scope is undefined on 401', async () => { + // Pre-set a scope on the transport + (transport as unknown as { _scope: string | undefined })['_scope'] = 'read:op1'; + + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const fetchMock = globalThis.fetch as Mock; + fetchMock + // 401 with no scope parameter + .mockResolvedValueOnce({ + ok: false, + status: 401, + statusText: 'Unauthorized', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer' + }), + text: () => Promise.resolve('Unauthorized') + }) + // 200 OK after auth + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // Scope should still be "read:op1" (unchanged) + expect((transport as unknown as { _scope: string | undefined })['_scope']).toBe('read:op1'); + + authSpy.mockRestore(); + }); + + it('returns undefined scope when both existing and incoming are undefined', async () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const fetchMock = globalThis.fetch as Mock; + fetchMock + // 401 with no scope parameter + .mockResolvedValueOnce({ + ok: false, + status: 401, + statusText: 'Unauthorized', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer' + }), + text: () => Promise.resolve('Unauthorized') + }) + // 200 OK after auth + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // Scope should be undefined when both were undefined + expect((transport as unknown as { _scope: string | undefined })['_scope']).toBeUndefined(); + + authSpy.mockRestore(); + }); + + it('sets scope from incoming when existing is undefined', async () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const fetchMock = globalThis.fetch as Mock; + fetchMock + // 401 with scope="read:op1" + .mockResolvedValueOnce({ + ok: false, + status: 401, + statusText: 'Unauthorized', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer scope="read:op1"' + }), + text: () => Promise.resolve('Unauthorized') + }) + // 200 OK after auth + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // Scope should be "read:op1" + expect((transport as unknown as { _scope: string | undefined })['_scope']).toBe('read:op1'); + + authSpy.mockRestore(); + }); + + it('does not lose existing scope when 401 has no scope parameter', async () => { + // Simulate a scenario where the first 401 sets a scope, then a second + // 401 has no scope in the header. The existing scope must be preserved. + (transport as unknown as { _scope: string | undefined })['_scope'] = 'read:op1'; + + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const fetchMock = globalThis.fetch as Mock; + fetchMock + // 401 with no scope parameter in WWW-Authenticate + .mockResolvedValueOnce({ + ok: false, + status: 401, + statusText: 'Unauthorized', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer realm="example"' + }), + text: () => Promise.resolve('Unauthorized') + }) + // 200 OK after auth + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // Scope should still be "read:op1" — mergeScopes("read:op1", undefined) = "read:op1" + expect((transport as unknown as { _scope: string | undefined })['_scope']).toBe('read:op1'); + + authSpy.mockRestore(); + }); + }); + + it('circuit-breaker fires on repeated 403 with same WWW-Authenticate header', async () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + // Mock fetch to always return 403 with the same WWW-Authenticate header + const fetchMock = globalThis.fetch as Mock; + fetchMock.mockResolvedValue({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="read:op1"' + }), + text: () => Promise.resolve('Insufficient scope') + }); + + const authModule = await import('../../src/client/auth.js'); + const authSpy = vi.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + // First send triggers upscoping, second 403 with same header causes circuit-breaker + await expect(transport.send(message)).rejects.toThrow('Server returned 403 after trying upscoping'); + + // Auth should have been called once (for the first 403), then circuit-breaker fires on second + expect(authSpy).toHaveBeenCalledTimes(1); + + authSpy.mockRestore(); + }); + describe('Reconnection Logic', () => { let transport: StreamableHTTPClientTransport;