From 38bab6b08d24e6c967f5c9456c3789bb070e4387 Mon Sep 17 00:00:00 2001 From: Nayana Parameswarappa Date: Thu, 13 Nov 2025 19:54:28 -0800 Subject: [PATCH 1/6] Adding 403 upscoping with resource url --- src/client/auth.test.ts | 137 +++++++++++++++++++++++++++++- src/client/auth.ts | 63 ++++++++++++-- src/client/streamableHttp.test.ts | 92 ++++++++++++++++++++ src/client/streamableHttp.ts | 40 ++++++++- 4 files changed, 320 insertions(+), 12 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 8124fe768..f3312b690 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -8,13 +8,14 @@ import { refreshAuthorization, registerClient, discoverOAuthProtectedResourceMetadata, + extractFieldFromWwwAuth, extractWWWAuthenticateParams, auth, type OAuthClientProvider, selectClientAuthMethod } from './auth.js'; import { ServerError } from '../server/auth/errors.js'; -import { AuthorizationServerMetadata } from '../shared/auth.js'; +import { AuthorizationServerMetadata, OAuthClientMetadata } from '../shared/auth.js'; // Mock fetch globally const mockFetch = jest.fn(); @@ -25,6 +26,50 @@ describe('OAuth Authorization', () => { mockFetch.mockReset(); }); + describe('extractFieldFromWwwAuth', () => { + function mockResponseWithWWWAuthenticate(headerValue: string): Response { + return { + headers: { + get: jest.fn(name => (name === 'WWW-Authenticate' ? headerValue : null)) + } + } as unknown as Response; + } + + it('returns the value of a quoted field', () => { + const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm="example", field="value"`); + expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('value'); + }); + + it('returns the value of an unquoted field', () => { + const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm=example, field=value`); + expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('value'); + }); + + it('returns the correct value when multiple parameters are present', () => { + const mockResponse = mockResponseWithWWWAuthenticate( + `Bearer realm="api", error="invalid_token", field="test_value", scope="admin"` + ); + expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('test_value'); + }); + + it('returns null if the field is not present', () => { + const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm="api", scope="admin"`); + expect(extractFieldFromWwwAuth(mockResponse, 'missing_field')).toBeNull(); + }); + + it('returns null if the WWW-Authenticate header is missing', () => { + const mockResponse = { headers: new Headers() } as unknown as Response; + expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBeNull(); + }); + + it('handles fields with special characters in quotes', () => { + const mockResponse = mockResponseWithWWWAuthenticate( + `Bearer error="invalid_token", error_description="The token has expired, please re-authenticate."` + ); + expect(extractFieldFromWwwAuth(mockResponse, 'error_description')).toBe('The token has expired, please re-authenticate.'); + }); + }); + describe('extractWWWAuthenticateParams', () => { it('returns resource metadata url when present', async () => { const resourceUrl = 'https://resource.example.com/.well-known/oauth-protected-resource'; @@ -1496,6 +1541,8 @@ describe('OAuth Authorization', () => { }); describe('auth function', () => { + let clientMetadataScope: string | undefined = undefined; + const mockProvider: OAuthClientProvider = { get redirectUrl() { return 'http://localhost:3000/callback'; @@ -1503,7 +1550,8 @@ describe('OAuth Authorization', () => { get clientMetadata() { return { redirect_uris: ['http://localhost:3000/callback'], - client_name: 'Test Client' + client_name: 'Test Client', + scope: clientMetadataScope }; }, clientInformation: jest.fn(), @@ -2284,6 +2332,91 @@ describe('OAuth Authorization', () => { // Verify custom fetch was called for AS metadata discovery expect(customFetch.mock.calls[1][0].toString()).toBe('https://auth.example.com/.well-known/oauth-authorization-server'); }); + + it('prioritizes provided scope over resourceMetadata.scope', async () => { + const providedScope = 'provided_scope'; + (mockProvider.clientMetadata as OAuthClientMetadata).scope = 'client_metadata_scope'; + + mockFetch.mockImplementation(url => { + if (url.toString().includes('/.well-known/oauth-protected-resource')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + resource: 'https://api.example.com/mcp-server', + scopes_supported: ['read', 'write'], + authorization_servers: ['https://auth.example.com'] + }) + }); + } + return Promise.resolve({ ok: false, status: 404 }); + }); + + await auth(mockProvider, { + serverUrl: 'https://api.example.com/mcp-server', + scope: providedScope + }); + + const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0]; + const authUrl: URL = redirectCall[0]; + expect(authUrl.searchParams.get('scope')).toBe(providedScope); + }); + + it('uses resourceMetadata.scope when provided scope is missing', async () => { + const resourceScope = 'resource_metadata_scope'; + (mockProvider.clientMetadata as OAuthClientMetadata).scope = 'client_metadata_scope'; + + mockFetch.mockImplementation(url => { + if (url.toString().includes('/.well-known/oauth-protected-resource')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + resource: 'https://api.example.com/mcp-server', + scopes_supported: ['resource_metadata_scope'], + authorization_servers: ['https://auth.example.com'] + }) + }); + } + return Promise.resolve({ ok: false, status: 404 }); + }); + + await auth(mockProvider, { + serverUrl: 'https://api.example.com/mcp-server' + }); + + const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0]; + const authUrl: URL = redirectCall[0]; + expect(authUrl.searchParams.get('scope')).toBe(resourceScope); + }); + + it('falls back to clientMetadata.scope when provided and resourceMetadata scopes are missing', async () => { + const expectedScope = 'client_metadata_scope'; + clientMetadataScope = expectedScope; + + mockFetch.mockImplementation(url => { + if (url.toString().includes('/.well-known/oauth-protected-resource')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + resource: 'https://api.example.com/mcp-server', + resource_metadata_scope: [], + authorization_servers: ['https://auth.example.com'] + }) + }); + } + return Promise.resolve({ ok: false, status: 404 }); + }); + + await auth(mockProvider, { + serverUrl: 'https://api.example.com/mcp-server' + }); + + const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0]; + const authUrl: URL = redirectCall[0]; + expect(authUrl.searchParams.get('scope')).toBe(clientMetadataScope); + }); }); describe('exchangeAuthorization with multiple client authentication methods', () => { diff --git a/src/client/auth.ts b/src/client/auth.ts index fba0e7bf7..207f8e429 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -348,7 +348,7 @@ async function authInternal( ): Promise { let resourceMetadata: OAuthProtectedResourceMetadata | undefined; let authorizationServerUrl: string | URL | undefined; - + try { resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn); if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) { @@ -447,7 +447,7 @@ async function authInternal( clientInformation, state, redirectUrl: provider.redirectUrl, - scope: scope || provider.clientMetadata.scope, + scope: selectScope(provider, resourceMetadata, scope), resource }); @@ -456,6 +456,31 @@ async function authInternal( return 'REDIRECT'; } +/** + * Selects the appropriate OAuth scope to use. + * + * The priority order is: + * 1. The provided `scope` argument (if available). The scope is usually provided by WWW-authenticate header. + * 2. Protected Resource Metadata scope (if available) + * 3. The `OAuthClientProvider.clientMetadata.scope` (if available) + */ +export function selectScope( + provider: OAuthClientProvider, + resourceMetadata?: OAuthProtectedResourceMetadata, + scope?: string +): string | undefined { + if (scope) { + return scope; + } + + const scopes = resourceMetadata?.scopes_supported; + if (scopes && scopes.length > 0) { + return scopes.join(' '); + } + + return provider.clientMetadata.scope; +} + export async function selectResourceURL( serverUrl: string | URL, provider: OAuthClientProvider, @@ -495,22 +520,18 @@ export function extractWWWAuthenticateParams(res: Response): { resourceMetadataU return {}; } - const resourceMetadataRegex = /resource_metadata="([^"]*)"/; - const resourceMetadataMatch = resourceMetadataRegex.exec(authenticateHeader); - - const scopeRegex = /scope="([^"]*)"/; - const scopeMatch = scopeRegex.exec(authenticateHeader); + const resourceMetadataMatch = extractFieldFromWwwAuth(res, 'resource_metadata') || undefined; let resourceMetadataUrl: URL | undefined; if (resourceMetadataMatch) { try { - resourceMetadataUrl = new URL(resourceMetadataMatch[1]); + resourceMetadataUrl = new URL(resourceMetadataMatch); } catch { // Ignore invalid URL } } - const scope = scopeMatch?.[1] || undefined; + const scope = extractFieldFromWwwAuth(res, 'scope') || undefined; return { resourceMetadataUrl, @@ -518,6 +539,30 @@ export function extractWWWAuthenticateParams(res: Response): { resourceMetadataU }; } +/** + * Extracts a specific field's value from the WWW-Authenticate header string. + * + * @param response The HTTP response object containing the headers. + * @param fieldName The name of the field to extract (e.g., "realm", "nonce"). + * @returns The field value + */ +export function extractFieldFromWwwAuth(response: Response, fieldName: string): string | null { + const wwwAuthHeader = response.headers.get('WWW-Authenticate'); + if (!wwwAuthHeader) { + return null; + } + + const pattern = new RegExp(`${fieldName}=(?:"([^"]+)"|([^\\s,]+))`); + const match = wwwAuthHeader.match(pattern); + + if (match) { + // Pattern matches: field_name="value" or field_name=value (unquoted) + return match[1] || match[2]; + } + + return null; +} + /** * Extract resource_metadata from response header. * @deprecated Use `extractWWWAuthenticateParams` instead. diff --git a/src/client/streamableHttp.test.ts b/src/client/streamableHttp.test.ts index 3c6a9ec4d..b59e77a16 100644 --- a/src/client/streamableHttp.test.ts +++ b/src/client/streamableHttp.test.ts @@ -592,6 +592,98 @@ describe('StreamableHTTPClientTransport', () => { expect(mockAuthProvider.redirectToAuthorization.mock.calls).toHaveLength(1); }); + it('attempts upscoping on 403 with WWW-Authenticate header', async () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + const fetchMock = global.fetch as jest.Mock; + fetchMock + // First call: returns 403 with insufficient_scope + .mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': + 'Bearer error="insufficient_scope", scope="new_scope", resource_metadata="http://example.com/resource"' + }), + text: () => Promise.resolve('Insufficient scope') + }) + // Second call: successful after upscoping + .mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers() + }); + + // Spy on the imported auth function and mock successful authorization + const authModule = await import('./auth.js'); + const authSpy = jest.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + await transport.send(message); + + // Verify fetch was called twice + expect(fetchMock).toHaveBeenCalledTimes(2); + + // Verify auth was called with the new scope + expect(authSpy).toHaveBeenCalledWith( + mockAuthProvider, + expect.objectContaining({ + scope: 'new_scope', + resourceMetadataUrl: new URL('http://example.com/resource') + }) + ); + + authSpy.mockRestore(); + }); + + it('prevents infinite upscoping on repeated 403', async () => { + const message: JSONRPCMessage = { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 'test-id' + }; + + // Mock fetch calls to always return 403 with insufficient_scope + const fetchMock = global.fetch as jest.Mock; + fetchMock.mockResolvedValue({ + ok: false, + status: 403, + statusText: 'Forbidden', + headers: new Headers({ + 'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="new_scope"' + }), + text: () => Promise.resolve('Insufficient scope') + }); + + // Spy on the imported auth function and mock successful authorization + const authModule = await import('./auth.js'); + const authSpy = jest.spyOn(authModule, 'auth'); + authSpy.mockResolvedValue('AUTHORIZED'); + + // First send: should trigger upscoping + await expect(transport.send(message)).rejects.toThrow('Server returned 403 after trying upscoping'); + + expect(fetchMock).toHaveBeenCalledTimes(2); // Initial call + one retry after auth + expect(authSpy).toHaveBeenCalledTimes(1); // Auth called once + + // Second send: should fail immediately without re-calling auth + fetchMock.mockClear(); + authSpy.mockClear(); + await expect(transport.send(message)).rejects.toThrow('Server returned 403 after trying upscoping'); + + expect(fetchMock).toHaveBeenCalledTimes(1); // Only one fetch call + expect(authSpy).not.toHaveBeenCalled(); // Auth not called again + + authSpy.mockRestore(); + }); + describe('Reconnection Logic', () => { let transport: StreamableHTTPClientTransport; diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index b57013c33..1e98712b8 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -1,6 +1,6 @@ import { Transport, FetchLike } from '../shared/transport.js'; import { isInitializedNotification, isJSONRPCRequest, isJSONRPCResponse, JSONRPCMessage, JSONRPCMessageSchema } from '../types.js'; -import { auth, AuthResult, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; +import { auth, AuthResult, extractFieldFromWwwAuth, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; import { EventSourceParserStream } from 'eventsource-parser/stream'; // Default reconnection options for StreamableHTTP connections @@ -133,6 +133,7 @@ export class StreamableHTTPClientTransport implements Transport { private _reconnectionOptions: StreamableHTTPReconnectionOptions; private _protocolVersion?: string; private _hasCompletedAuthFlow = false; // Circuit breaker: detect auth success followed by immediate 401 + private _hasTriedUpscoping = false; // Circuit breaker: detect upscoping attempt followed by immediate 403 onclose?: () => void; onerror?: (error: Error) => void; @@ -464,12 +465,49 @@ export class StreamableHTTPClientTransport implements Transport { return this.send(message); } + if (response.status === 403 && this._authProvider) { + const error = extractFieldFromWwwAuth(response, 'error'); + + if (error === 'insufficient_scope') { + // Prevent infinite recursion when upscoping was already tried. + if (this._hasTriedUpscoping) { + throw new StreamableHTTPError(403, 'Server returned 403 after trying upscoping'); + } + + const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); + + if (scope) { + this._scope = scope; + } + + if (resourceMetadataUrl) { + this._resourceMetadataUrl = resourceMetadataUrl; + } + + // Mark that upscoping was tried. + this._hasTriedUpscoping = true; + const result = await auth(this._authProvider, { + serverUrl: this._url, + resourceMetadataUrl: this._resourceMetadataUrl, + scope: this._scope, + fetchFn: this._fetch + }); + + if (result !== 'AUTHORIZED') { + throw new UnauthorizedError(); + } + + return this.send(message); + } + } + const text = await response.text().catch(() => null); throw new Error(`Error POSTing to endpoint (HTTP ${response.status}): ${text}`); } // Reset auth loop flag on successful response this._hasCompletedAuthFlow = false; + this._hasTriedUpscoping = false; // If the response is 202 Accepted, there's no body to process if (response.status === 202) { From fde7fc41d2fae6c1a9eef7cd4938a94c0e69fe07 Mon Sep 17 00:00:00 2001 From: Nayana Parameswarappa Date: Fri, 14 Nov 2025 10:34:57 -0800 Subject: [PATCH 2/6] Using lastUpscopingHeader to track last upscoping header to prevent infinite upscoping. --- src/client/auth.ts | 2 +- src/client/streamableHttp.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index 207f8e429..7eada30ac 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -348,7 +348,7 @@ async function authInternal( ): Promise { let resourceMetadata: OAuthProtectedResourceMetadata | undefined; let authorizationServerUrl: string | URL | undefined; - + try { resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn); if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) { diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 1e98712b8..4ae0527e4 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -133,7 +133,7 @@ export class StreamableHTTPClientTransport implements Transport { private _reconnectionOptions: StreamableHTTPReconnectionOptions; private _protocolVersion?: string; private _hasCompletedAuthFlow = false; // Circuit breaker: detect auth success followed by immediate 401 - private _hasTriedUpscoping = false; // Circuit breaker: detect upscoping attempt followed by immediate 403 + private _lastUpscopingHeader?: string; // Track last upscoping header to prevent infinite upscoping. onclose?: () => void; onerror?: (error: Error) => void; @@ -469,8 +469,10 @@ export class StreamableHTTPClientTransport implements Transport { const error = extractFieldFromWwwAuth(response, 'error'); if (error === 'insufficient_scope') { - // Prevent infinite recursion when upscoping was already tried. - if (this._hasTriedUpscoping) { + const wwwAuthHeader = response.headers.get('WWW-Authenticate'); + + // Check if we've already tried upscoping with this header to prevent infinite loops. + if (this._lastUpscopingHeader === wwwAuthHeader) { throw new StreamableHTTPError(403, 'Server returned 403 after trying upscoping'); } @@ -485,7 +487,7 @@ export class StreamableHTTPClientTransport implements Transport { } // Mark that upscoping was tried. - this._hasTriedUpscoping = true; + this._lastUpscopingHeader = wwwAuthHeader ?? undefined; const result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, @@ -507,7 +509,7 @@ export class StreamableHTTPClientTransport implements Transport { // Reset auth loop flag on successful response this._hasCompletedAuthFlow = false; - this._hasTriedUpscoping = false; + this._lastUpscopingHeader = undefined; // If the response is 202 Accepted, there's no body to process if (response.status === 202) { From c44eedc5ba1fcf4d9fa50af43df940ce1a9684d4 Mon Sep 17 00:00:00 2001 From: Nayana Parameswarappa Date: Fri, 14 Nov 2025 11:16:25 -0800 Subject: [PATCH 3/6] Fix merge conflict --- src/client/auth.test.ts | 8 ++++---- src/client/streamableHttp.test.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index f087aecda..0ae501337 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -39,7 +39,7 @@ describe('OAuth Authorization', () => { function mockResponseWithWWWAuthenticate(headerValue: string): Response { return { headers: { - get: jest.fn(name => (name === 'WWW-Authenticate' ? headerValue : null)) + get: vi.fn(name => (name === 'WWW-Authenticate' ? headerValue : null)) } } as unknown as Response; } @@ -2368,7 +2368,7 @@ describe('OAuth Authorization', () => { scope: providedScope }); - const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0]; + const redirectCall = (mockProvider.redirectToAuthorization as vi.Mock).mock.calls[0]; const authUrl: URL = redirectCall[0]; expect(authUrl.searchParams.get('scope')).toBe(providedScope); }); @@ -2396,7 +2396,7 @@ describe('OAuth Authorization', () => { serverUrl: 'https://api.example.com/mcp-server' }); - const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0]; + const redirectCall = (mockProvider.redirectToAuthorization as vi.Mock).mock.calls[0]; const authUrl: URL = redirectCall[0]; expect(authUrl.searchParams.get('scope')).toBe(resourceScope); }); @@ -2424,7 +2424,7 @@ describe('OAuth Authorization', () => { serverUrl: 'https://api.example.com/mcp-server' }); - const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0]; + const redirectCall = (mockProvider.redirectToAuthorization as vi.Mock).mock.calls[0]; const authUrl: URL = redirectCall[0]; expect(authUrl.searchParams.get('scope')).toBe(clientMetadataScope); }); diff --git a/src/client/streamableHttp.test.ts b/src/client/streamableHttp.test.ts index 2302fee6f..249997dbc 100644 --- a/src/client/streamableHttp.test.ts +++ b/src/client/streamableHttp.test.ts @@ -601,7 +601,7 @@ describe('StreamableHTTPClientTransport', () => { id: 'test-id' }; - const fetchMock = global.fetch as jest.Mock; + const fetchMock = global.fetch as vi.Mock; fetchMock // First call: returns 403 with insufficient_scope .mockResolvedValueOnce({ @@ -623,7 +623,7 @@ describe('StreamableHTTPClientTransport', () => { // Spy on the imported auth function and mock successful authorization const authModule = await import('./auth.js'); - const authSpy = jest.spyOn(authModule, 'auth'); + const authSpy = vi.spyOn(authModule, 'auth'); authSpy.mockResolvedValue('AUTHORIZED'); await transport.send(message); @@ -652,7 +652,7 @@ describe('StreamableHTTPClientTransport', () => { }; // Mock fetch calls to always return 403 with insufficient_scope - const fetchMock = global.fetch as jest.Mock; + const fetchMock = global.fetch as vi.Mock; fetchMock.mockResolvedValue({ ok: false, status: 403, @@ -665,7 +665,7 @@ describe('StreamableHTTPClientTransport', () => { // Spy on the imported auth function and mock successful authorization const authModule = await import('./auth.js'); - const authSpy = jest.spyOn(authModule, 'auth'); + const authSpy = vi.spyOn(authModule, 'auth'); authSpy.mockResolvedValue('AUTHORIZED'); // First send: should trigger upscoping From 37566c62eb04af6ff27b3600cc2e070d34ae0410 Mon Sep 17 00:00:00 2001 From: Nayana Parameswarappa Date: Fri, 14 Nov 2025 11:26:44 -0800 Subject: [PATCH 4/6] Fixing lint issues --- src/client/auth.test.ts | 6 +++--- src/client/streamableHttp.test.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 0ae501337..5c40a7bd2 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -2368,7 +2368,7 @@ describe('OAuth Authorization', () => { scope: providedScope }); - const redirectCall = (mockProvider.redirectToAuthorization as vi.Mock).mock.calls[0]; + const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0]; const authUrl: URL = redirectCall[0]; expect(authUrl.searchParams.get('scope')).toBe(providedScope); }); @@ -2396,7 +2396,7 @@ describe('OAuth Authorization', () => { serverUrl: 'https://api.example.com/mcp-server' }); - const redirectCall = (mockProvider.redirectToAuthorization as vi.Mock).mock.calls[0]; + const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0]; const authUrl: URL = redirectCall[0]; expect(authUrl.searchParams.get('scope')).toBe(resourceScope); }); @@ -2424,7 +2424,7 @@ describe('OAuth Authorization', () => { serverUrl: 'https://api.example.com/mcp-server' }); - const redirectCall = (mockProvider.redirectToAuthorization as vi.Mock).mock.calls[0]; + const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0]; const authUrl: URL = redirectCall[0]; expect(authUrl.searchParams.get('scope')).toBe(clientMetadataScope); }); diff --git a/src/client/streamableHttp.test.ts b/src/client/streamableHttp.test.ts index 249997dbc..a4f582cfc 100644 --- a/src/client/streamableHttp.test.ts +++ b/src/client/streamableHttp.test.ts @@ -601,7 +601,7 @@ describe('StreamableHTTPClientTransport', () => { id: 'test-id' }; - const fetchMock = global.fetch as vi.Mock; + const fetchMock = global.fetch as Mock; fetchMock // First call: returns 403 with insufficient_scope .mockResolvedValueOnce({ @@ -652,7 +652,7 @@ describe('StreamableHTTPClientTransport', () => { }; // Mock fetch calls to always return 403 with insufficient_scope - const fetchMock = global.fetch as vi.Mock; + const fetchMock = global.fetch as Mock; fetchMock.mockResolvedValue({ ok: false, status: 403, From 26362691134a8af94f4b162f11656bdcd5fdce30 Mon Sep 17 00:00:00 2001 From: Nayana Parameswarappa Date: Wed, 19 Nov 2025 21:25:31 -0800 Subject: [PATCH 5/6] Removing the duplicate logic --- src/client/auth.test.ts | 92 +---------------------------------------- src/client/auth.ts | 26 +----------- 2 files changed, 3 insertions(+), 115 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 769565316..90c10fad1 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -16,7 +16,7 @@ import { isHttpsUrl } from './auth.js'; import { InvalidClientMetadataError, ServerError } from '../server/auth/errors.js'; -import { AuthorizationServerMetadata, OAuthClientMetadata } from '../shared/auth.js'; +import { AuthorizationServerMetadata } from '../shared/auth.js'; import { expect, vi, type Mock } from 'vitest'; // Mock pkce-challenge @@ -1553,8 +1553,6 @@ describe('OAuth Authorization', () => { }); describe('auth function', () => { - let clientMetadataScope: string | undefined = undefined; - const mockProvider: OAuthClientProvider = { get redirectUrl() { return 'http://localhost:3000/callback'; @@ -1562,8 +1560,7 @@ describe('OAuth Authorization', () => { get clientMetadata() { return { redirect_uris: ['http://localhost:3000/callback'], - client_name: 'Test Client', - scope: clientMetadataScope + client_name: 'Test Client' }; }, clientInformation: vi.fn(), @@ -2473,91 +2470,6 @@ describe('OAuth Authorization', () => { // Verify custom fetch was called for AS metadata discovery expect(customFetch.mock.calls[1][0].toString()).toBe('https://auth.example.com/.well-known/oauth-authorization-server'); }); - - it('prioritizes provided scope over resourceMetadata.scope', async () => { - const providedScope = 'provided_scope'; - (mockProvider.clientMetadata as OAuthClientMetadata).scope = 'client_metadata_scope'; - - mockFetch.mockImplementation(url => { - if (url.toString().includes('/.well-known/oauth-protected-resource')) { - return Promise.resolve({ - ok: true, - status: 200, - json: async () => ({ - resource: 'https://api.example.com/mcp-server', - scopes_supported: ['read', 'write'], - authorization_servers: ['https://auth.example.com'] - }) - }); - } - return Promise.resolve({ ok: false, status: 404 }); - }); - - await auth(mockProvider, { - serverUrl: 'https://api.example.com/mcp-server', - scope: providedScope - }); - - const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0]; - const authUrl: URL = redirectCall[0]; - expect(authUrl.searchParams.get('scope')).toBe(providedScope); - }); - - it('uses resourceMetadata.scope when provided scope is missing', async () => { - const resourceScope = 'resource_metadata_scope'; - (mockProvider.clientMetadata as OAuthClientMetadata).scope = 'client_metadata_scope'; - - mockFetch.mockImplementation(url => { - if (url.toString().includes('/.well-known/oauth-protected-resource')) { - return Promise.resolve({ - ok: true, - status: 200, - json: async () => ({ - resource: 'https://api.example.com/mcp-server', - scopes_supported: ['resource_metadata_scope'], - authorization_servers: ['https://auth.example.com'] - }) - }); - } - return Promise.resolve({ ok: false, status: 404 }); - }); - - await auth(mockProvider, { - serverUrl: 'https://api.example.com/mcp-server' - }); - - const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0]; - const authUrl: URL = redirectCall[0]; - expect(authUrl.searchParams.get('scope')).toBe(resourceScope); - }); - - it('falls back to clientMetadata.scope when provided and resourceMetadata scopes are missing', async () => { - const expectedScope = 'client_metadata_scope'; - clientMetadataScope = expectedScope; - - mockFetch.mockImplementation(url => { - if (url.toString().includes('/.well-known/oauth-protected-resource')) { - return Promise.resolve({ - ok: true, - status: 200, - json: async () => ({ - resource: 'https://api.example.com/mcp-server', - resource_metadata_scope: [], - authorization_servers: ['https://auth.example.com'] - }) - }); - } - return Promise.resolve({ ok: false, status: 404 }); - }); - - await auth(mockProvider, { - serverUrl: 'https://api.example.com/mcp-server' - }); - - const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0]; - const authUrl: URL = redirectCall[0]; - expect(authUrl.searchParams.get('scope')).toBe(clientMetadataScope); - }); }); describe('exchangeAuthorization with multiple client authentication methods', () => { diff --git a/src/client/auth.ts b/src/client/auth.ts index d715d94ae..97e08935c 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -473,7 +473,7 @@ async function authInternal( clientInformation, state, redirectUrl: provider.redirectUrl, - scope: selectScope(provider, resourceMetadata, scope), + scope: scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope, resource }); @@ -483,30 +483,6 @@ async function authInternal( } /** - * Selects the appropriate OAuth scope to use. - * - * The priority order is: - * 1. The provided `scope` argument (if available). The scope is usually provided by WWW-authenticate header. - * 2. Protected Resource Metadata scope (if available) - * 3. The `OAuthClientProvider.clientMetadata.scope` (if available) - */ -export function selectScope( - provider: OAuthClientProvider, - resourceMetadata?: OAuthProtectedResourceMetadata, - scope?: string -): string | undefined { - if (scope) { - return scope; - } - - const scopes = resourceMetadata?.scopes_supported; - if (scopes && scopes.length > 0) { - return scopes.join(' '); - } - - return provider.clientMetadata.scope; - } - * SEP-991: URL-based Client IDs * Validate that the client_id is a valid URL with https scheme */ From 48f4470cce557473fe7ee0da813c66b31bb7b6f1 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Thu, 20 Nov 2025 19:34:30 +0000 Subject: [PATCH 6/6] Simplify WWW-Authenticate parsing by keeping extractFieldFromWwwAuth private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of exporting extractFieldFromWwwAuth as a separate function, keep it private and extend extractWWWAuthenticateParams to also return the 'error' field. This provides a cleaner API while maintaining all functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/client/auth.test.ts | 55 +++++++----------------------------- src/client/auth.ts | 10 ++++--- src/client/streamableHttp.ts | 6 ++-- 3 files changed, 18 insertions(+), 53 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 90c10fad1..0e3a544a2 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -8,7 +8,6 @@ import { refreshAuthorization, registerClient, discoverOAuthProtectedResourceMetadata, - extractFieldFromWwwAuth, extractWWWAuthenticateParams, auth, type OAuthClientProvider, @@ -36,50 +35,6 @@ describe('OAuth Authorization', () => { mockFetch.mockReset(); }); - describe('extractFieldFromWwwAuth', () => { - function mockResponseWithWWWAuthenticate(headerValue: string): Response { - return { - headers: { - get: vi.fn(name => (name === 'WWW-Authenticate' ? headerValue : null)) - } - } as unknown as Response; - } - - it('returns the value of a quoted field', () => { - const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm="example", field="value"`); - expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('value'); - }); - - it('returns the value of an unquoted field', () => { - const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm=example, field=value`); - expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('value'); - }); - - it('returns the correct value when multiple parameters are present', () => { - const mockResponse = mockResponseWithWWWAuthenticate( - `Bearer realm="api", error="invalid_token", field="test_value", scope="admin"` - ); - expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('test_value'); - }); - - it('returns null if the field is not present', () => { - const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm="api", scope="admin"`); - expect(extractFieldFromWwwAuth(mockResponse, 'missing_field')).toBeNull(); - }); - - it('returns null if the WWW-Authenticate header is missing', () => { - const mockResponse = { headers: new Headers() } as unknown as Response; - expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBeNull(); - }); - - it('handles fields with special characters in quotes', () => { - const mockResponse = mockResponseWithWWWAuthenticate( - `Bearer error="invalid_token", error_description="The token has expired, please re-authenticate."` - ); - expect(extractFieldFromWwwAuth(mockResponse, 'error_description')).toBe('The token has expired, please re-authenticate.'); - }); - }); - describe('extractWWWAuthenticateParams', () => { it('returns resource metadata url when present', async () => { const resourceUrl = 'https://resource.example.com/.well-known/oauth-protected-resource'; @@ -140,6 +95,16 @@ describe('OAuth Authorization', () => { expect(extractWWWAuthenticateParams(mockResponse)).toEqual({ scope: scope }); }); + + it('returns error when present', async () => { + const mockResponse = { + headers: { + get: vi.fn(name => (name === 'WWW-Authenticate' ? `Bearer error="insufficient_scope", scope="admin"` : null)) + } + } as unknown as Response; + + expect(extractWWWAuthenticateParams(mockResponse)).toEqual({ error: 'insufficient_scope', scope: 'admin' }); + }); }); describe('discoverOAuthProtectedResourceMetadata', () => { diff --git a/src/client/auth.ts b/src/client/auth.ts index 97e08935c..536ff6859 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -522,9 +522,9 @@ export async function selectResourceURL( } /** - * Extract resource_metadata and scope from WWW-Authenticate header. + * Extract resource_metadata, scope, and error from WWW-Authenticate header. */ -export function extractWWWAuthenticateParams(res: Response): { resourceMetadataUrl?: URL; scope?: string } { +export function extractWWWAuthenticateParams(res: Response): { resourceMetadataUrl?: URL; scope?: string; error?: string } { const authenticateHeader = res.headers.get('WWW-Authenticate'); if (!authenticateHeader) { return {}; @@ -547,10 +547,12 @@ export function extractWWWAuthenticateParams(res: Response): { resourceMetadataU } const scope = extractFieldFromWwwAuth(res, 'scope') || undefined; + const error = extractFieldFromWwwAuth(res, 'error') || undefined; return { resourceMetadataUrl, - scope + scope, + error }; } @@ -561,7 +563,7 @@ export function extractWWWAuthenticateParams(res: Response): { resourceMetadataU * @param fieldName The name of the field to extract (e.g., "realm", "nonce"). * @returns The field value */ -export function extractFieldFromWwwAuth(response: Response, fieldName: string): string | null { +function extractFieldFromWwwAuth(response: Response, fieldName: string): string | null { const wwwAuthHeader = response.headers.get('WWW-Authenticate'); if (!wwwAuthHeader) { return null; diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 0b2b04a69..3ca50b954 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -1,6 +1,6 @@ import { Transport, FetchLike, createFetchWithInit, normalizeHeaders } from '../shared/transport.js'; import { isInitializedNotification, isJSONRPCRequest, isJSONRPCResponse, JSONRPCMessage, JSONRPCMessageSchema } from '../types.js'; -import { auth, AuthResult, extractFieldFromWwwAuth, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; +import { auth, AuthResult, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; import { EventSourceParserStream } from 'eventsource-parser/stream'; // Default reconnection options for StreamableHTTP connections @@ -454,7 +454,7 @@ export class StreamableHTTPClientTransport implements Transport { } if (response.status === 403 && this._authProvider) { - const error = extractFieldFromWwwAuth(response, 'error'); + const { resourceMetadataUrl, scope, error } = extractWWWAuthenticateParams(response); if (error === 'insufficient_scope') { const wwwAuthHeader = response.headers.get('WWW-Authenticate'); @@ -464,8 +464,6 @@ export class StreamableHTTPClientTransport implements Transport { throw new StreamableHTTPError(403, 'Server returned 403 after trying upscoping'); } - const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); - if (scope) { this._scope = scope; }