Skip to content

Commit 38bab6b

Browse files
Nayana-ParameswarappaNayana-Parameswarappa
committed
Adding 403 upscoping with resource url
1 parent 324d471 commit 38bab6b

File tree

4 files changed

+320
-12
lines changed

4 files changed

+320
-12
lines changed

‎src/client/auth.test.ts‎

Lines changed: 135 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@ import {
88
refreshAuthorization,
99
registerClient,
1010
discoverOAuthProtectedResourceMetadata,
11+
extractFieldFromWwwAuth,
1112
extractWWWAuthenticateParams,
1213
auth,
1314
type OAuthClientProvider,
1415
selectClientAuthMethod
1516
} from './auth.js';
1617
import { ServerError } from '../server/auth/errors.js';
17-
import { AuthorizationServerMetadata } from '../shared/auth.js';
18+
import { AuthorizationServerMetadata, OAuthClientMetadata } from '../shared/auth.js';
1819

1920
// Mock fetch globally
2021
const mockFetch = jest.fn();
@@ -25,6 +26,50 @@ describe('OAuth Authorization', () => {
2526
mockFetch.mockReset();
2627
});
2728

29+
describe('extractFieldFromWwwAuth', () => {
30+
function mockResponseWithWWWAuthenticate(headerValue: string): Response {
31+
return {
32+
headers: {
33+
get: jest.fn(name => (name === 'WWW-Authenticate' ? headerValue : null))
34+
}
35+
} as unknown as Response;
36+
}
37+
38+
it('returns the value of a quoted field', () => {
39+
const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm="example", field="value"`);
40+
expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('value');
41+
});
42+
43+
it('returns the value of an unquoted field', () => {
44+
const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm=example, field=value`);
45+
expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('value');
46+
});
47+
48+
it('returns the correct value when multiple parameters are present', () => {
49+
const mockResponse = mockResponseWithWWWAuthenticate(
50+
`Bearer realm="api", error="invalid_token", field="test_value", scope="admin"`
51+
);
52+
expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBe('test_value');
53+
});
54+
55+
it('returns null if the field is not present', () => {
56+
const mockResponse = mockResponseWithWWWAuthenticate(`Bearer realm="api", scope="admin"`);
57+
expect(extractFieldFromWwwAuth(mockResponse, 'missing_field')).toBeNull();
58+
});
59+
60+
it('returns null if the WWW-Authenticate header is missing', () => {
61+
const mockResponse = { headers: new Headers() } as unknown as Response;
62+
expect(extractFieldFromWwwAuth(mockResponse, 'field')).toBeNull();
63+
});
64+
65+
it('handles fields with special characters in quotes', () => {
66+
const mockResponse = mockResponseWithWWWAuthenticate(
67+
`Bearer error="invalid_token", error_description="The token has expired, please re-authenticate."`
68+
);
69+
expect(extractFieldFromWwwAuth(mockResponse, 'error_description')).toBe('The token has expired, please re-authenticate.');
70+
});
71+
});
72+
2873
describe('extractWWWAuthenticateParams', () => {
2974
it('returns resource metadata url when present', async () => {
3075
const resourceUrl = 'https://resource.example.com/.well-known/oauth-protected-resource';
@@ -1496,14 +1541,17 @@ describe('OAuth Authorization', () => {
14961541
});
14971542

14981543
describe('auth function', () => {
1544+
let clientMetadataScope: string | undefined = undefined;
1545+
14991546
const mockProvider: OAuthClientProvider = {
15001547
get redirectUrl() {
15011548
return 'http://localhost:3000/callback';
15021549
},
15031550
get clientMetadata() {
15041551
return {
15051552
redirect_uris: ['http://localhost:3000/callback'],
1506-
client_name: 'Test Client'
1553+
client_name: 'Test Client',
1554+
scope: clientMetadataScope
15071555
};
15081556
},
15091557
clientInformation: jest.fn(),
@@ -2284,6 +2332,91 @@ describe('OAuth Authorization', () => {
22842332
// Verify custom fetch was called for AS metadata discovery
22852333
expect(customFetch.mock.calls[1][0].toString()).toBe('https://auth.example.com/.well-known/oauth-authorization-server');
22862334
});
2335+
2336+
it('prioritizes provided scope over resourceMetadata.scope', async () => {
2337+
const providedScope = 'provided_scope';
2338+
(mockProvider.clientMetadata as OAuthClientMetadata).scope = 'client_metadata_scope';
2339+
2340+
mockFetch.mockImplementation(url => {
2341+
if (url.toString().includes('/.well-known/oauth-protected-resource')) {
2342+
return Promise.resolve({
2343+
ok: true,
2344+
status: 200,
2345+
json: async () => ({
2346+
resource: 'https://api.example.com/mcp-server',
2347+
scopes_supported: ['read', 'write'],
2348+
authorization_servers: ['https://auth.example.com']
2349+
})
2350+
});
2351+
}
2352+
return Promise.resolve({ ok: false, status: 404 });
2353+
});
2354+
2355+
await auth(mockProvider, {
2356+
serverUrl: 'https://api.example.com/mcp-server',
2357+
scope: providedScope
2358+
});
2359+
2360+
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
2361+
const authUrl: URL = redirectCall[0];
2362+
expect(authUrl.searchParams.get('scope')).toBe(providedScope);
2363+
});
2364+
2365+
it('uses resourceMetadata.scope when provided scope is missing', async () => {
2366+
const resourceScope = 'resource_metadata_scope';
2367+
(mockProvider.clientMetadata as OAuthClientMetadata).scope = 'client_metadata_scope';
2368+
2369+
mockFetch.mockImplementation(url => {
2370+
if (url.toString().includes('/.well-known/oauth-protected-resource')) {
2371+
return Promise.resolve({
2372+
ok: true,
2373+
status: 200,
2374+
json: async () => ({
2375+
resource: 'https://api.example.com/mcp-server',
2376+
scopes_supported: ['resource_metadata_scope'],
2377+
authorization_servers: ['https://auth.example.com']
2378+
})
2379+
});
2380+
}
2381+
return Promise.resolve({ ok: false, status: 404 });
2382+
});
2383+
2384+
await auth(mockProvider, {
2385+
serverUrl: 'https://api.example.com/mcp-server'
2386+
});
2387+
2388+
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
2389+
const authUrl: URL = redirectCall[0];
2390+
expect(authUrl.searchParams.get('scope')).toBe(resourceScope);
2391+
});
2392+
2393+
it('falls back to clientMetadata.scope when provided and resourceMetadata scopes are missing', async () => {
2394+
const expectedScope = 'client_metadata_scope';
2395+
clientMetadataScope = expectedScope;
2396+
2397+
mockFetch.mockImplementation(url => {
2398+
if (url.toString().includes('/.well-known/oauth-protected-resource')) {
2399+
return Promise.resolve({
2400+
ok: true,
2401+
status: 200,
2402+
json: async () => ({
2403+
resource: 'https://api.example.com/mcp-server',
2404+
resource_metadata_scope: [],
2405+
authorization_servers: ['https://auth.example.com']
2406+
})
2407+
});
2408+
}
2409+
return Promise.resolve({ ok: false, status: 404 });
2410+
});
2411+
2412+
await auth(mockProvider, {
2413+
serverUrl: 'https://api.example.com/mcp-server'
2414+
});
2415+
2416+
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
2417+
const authUrl: URL = redirectCall[0];
2418+
expect(authUrl.searchParams.get('scope')).toBe(clientMetadataScope);
2419+
});
22872420
});
22882421

22892422
describe('exchangeAuthorization with multiple client authentication methods', () => {

‎src/client/auth.ts‎

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ async function authInternal(
348348
): Promise<AuthResult> {
349349
let resourceMetadata: OAuthProtectedResourceMetadata | undefined;
350350
let authorizationServerUrl: string | URL | undefined;
351-
351+
352352
try {
353353
resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn);
354354
if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) {
@@ -447,7 +447,7 @@ async function authInternal(
447447
clientInformation,
448448
state,
449449
redirectUrl: provider.redirectUrl,
450-
scope: scope || provider.clientMetadata.scope,
450+
scope: selectScope(provider, resourceMetadata, scope),
451451
resource
452452
});
453453

@@ -456,6 +456,31 @@ async function authInternal(
456456
return 'REDIRECT';
457457
}
458458

459+
/**
460+
* Selects the appropriate OAuth scope to use.
461+
*
462+
* The priority order is:
463+
* 1. The provided `scope` argument (if available). The scope is usually provided by WWW-authenticate header.
464+
* 2. Protected Resource Metadata scope (if available)
465+
* 3. The `OAuthClientProvider.clientMetadata.scope` (if available)
466+
*/
467+
export function selectScope(
468+
provider: OAuthClientProvider,
469+
resourceMetadata?: OAuthProtectedResourceMetadata,
470+
scope?: string
471+
): string | undefined {
472+
if (scope) {
473+
return scope;
474+
}
475+
476+
const scopes = resourceMetadata?.scopes_supported;
477+
if (scopes && scopes.length > 0) {
478+
return scopes.join(' ');
479+
}
480+
481+
return provider.clientMetadata.scope;
482+
}
483+
459484
export async function selectResourceURL(
460485
serverUrl: string | URL,
461486
provider: OAuthClientProvider,
@@ -495,29 +520,49 @@ export function extractWWWAuthenticateParams(res: Response): { resourceMetadataU
495520
return {};
496521
}
497522

498-
const resourceMetadataRegex = /resource_metadata="([^"]*)"/;
499-
const resourceMetadataMatch = resourceMetadataRegex.exec(authenticateHeader);
500-
501-
const scopeRegex = /scope="([^"]*)"/;
502-
const scopeMatch = scopeRegex.exec(authenticateHeader);
523+
const resourceMetadataMatch = extractFieldFromWwwAuth(res, 'resource_metadata') || undefined;
503524

504525
let resourceMetadataUrl: URL | undefined;
505526
if (resourceMetadataMatch) {
506527
try {
507-
resourceMetadataUrl = new URL(resourceMetadataMatch[1]);
528+
resourceMetadataUrl = new URL(resourceMetadataMatch);
508529
} catch {
509530
// Ignore invalid URL
510531
}
511532
}
512533

513-
const scope = scopeMatch?.[1] || undefined;
534+
const scope = extractFieldFromWwwAuth(res, 'scope') || undefined;
514535

515536
return {
516537
resourceMetadataUrl,
517538
scope
518539
};
519540
}
520541

542+
/**
543+
* Extracts a specific field's value from the WWW-Authenticate header string.
544+
*
545+
* @param response The HTTP response object containing the headers.
546+
* @param fieldName The name of the field to extract (e.g., "realm", "nonce").
547+
* @returns The field value
548+
*/
549+
export function extractFieldFromWwwAuth(response: Response, fieldName: string): string | null {
550+
const wwwAuthHeader = response.headers.get('WWW-Authenticate');
551+
if (!wwwAuthHeader) {
552+
return null;
553+
}
554+
555+
const pattern = new RegExp(`${fieldName}=(?:"([^"]+)"|([^\\s,]+))`);
556+
const match = wwwAuthHeader.match(pattern);
557+
558+
if (match) {
559+
// Pattern matches: field_name="value" or field_name=value (unquoted)
560+
return match[1] || match[2];
561+
}
562+
563+
return null;
564+
}
565+
521566
/**
522567
* Extract resource_metadata from response header.
523568
* @deprecated Use `extractWWWAuthenticateParams` instead.

‎src/client/streamableHttp.test.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,98 @@ describe('StreamableHTTPClientTransport', () => {
592592
expect(mockAuthProvider.redirectToAuthorization.mock.calls).toHaveLength(1);
593593
});
594594

595+
it('attempts upscoping on 403 with WWW-Authenticate header', async () => {
596+
const message: JSONRPCMessage = {
597+
jsonrpc: '2.0',
598+
method: 'test',
599+
params: {},
600+
id: 'test-id'
601+
};
602+
603+
const fetchMock = global.fetch as jest.Mock;
604+
fetchMock
605+
// First call: returns 403 with insufficient_scope
606+
.mockResolvedValueOnce({
607+
ok: false,
608+
status: 403,
609+
statusText: 'Forbidden',
610+
headers: new Headers({
611+
'WWW-Authenticate':
612+
'Bearer error="insufficient_scope", scope="new_scope", resource_metadata="http://example.com/resource"'
613+
}),
614+
text: () => Promise.resolve('Insufficient scope')
615+
})
616+
// Second call: successful after upscoping
617+
.mockResolvedValueOnce({
618+
ok: true,
619+
status: 202,
620+
headers: new Headers()
621+
});
622+
623+
// Spy on the imported auth function and mock successful authorization
624+
const authModule = await import('./auth.js');
625+
const authSpy = jest.spyOn(authModule, 'auth');
626+
authSpy.mockResolvedValue('AUTHORIZED');
627+
628+
await transport.send(message);
629+
630+
// Verify fetch was called twice
631+
expect(fetchMock).toHaveBeenCalledTimes(2);
632+
633+
// Verify auth was called with the new scope
634+
expect(authSpy).toHaveBeenCalledWith(
635+
mockAuthProvider,
636+
expect.objectContaining({
637+
scope: 'new_scope',
638+
resourceMetadataUrl: new URL('http://example.com/resource')
639+
})
640+
);
641+
642+
authSpy.mockRestore();
643+
});
644+
645+
it('prevents infinite upscoping on repeated 403', async () => {
646+
const message: JSONRPCMessage = {
647+
jsonrpc: '2.0',
648+
method: 'test',
649+
params: {},
650+
id: 'test-id'
651+
};
652+
653+
// Mock fetch calls to always return 403 with insufficient_scope
654+
const fetchMock = global.fetch as jest.Mock;
655+
fetchMock.mockResolvedValue({
656+
ok: false,
657+
status: 403,
658+
statusText: 'Forbidden',
659+
headers: new Headers({
660+
'WWW-Authenticate': 'Bearer error="insufficient_scope", scope="new_scope"'
661+
}),
662+
text: () => Promise.resolve('Insufficient scope')
663+
});
664+
665+
// Spy on the imported auth function and mock successful authorization
666+
const authModule = await import('./auth.js');
667+
const authSpy = jest.spyOn(authModule, 'auth');
668+
authSpy.mockResolvedValue('AUTHORIZED');
669+
670+
// First send: should trigger upscoping
671+
await expect(transport.send(message)).rejects.toThrow('Server returned 403 after trying upscoping');
672+
673+
expect(fetchMock).toHaveBeenCalledTimes(2); // Initial call + one retry after auth
674+
expect(authSpy).toHaveBeenCalledTimes(1); // Auth called once
675+
676+
// Second send: should fail immediately without re-calling auth
677+
fetchMock.mockClear();
678+
authSpy.mockClear();
679+
await expect(transport.send(message)).rejects.toThrow('Server returned 403 after trying upscoping');
680+
681+
expect(fetchMock).toHaveBeenCalledTimes(1); // Only one fetch call
682+
expect(authSpy).not.toHaveBeenCalled(); // Auth not called again
683+
684+
authSpy.mockRestore();
685+
});
686+
595687
describe('Reconnection Logic', () => {
596688
let transport: StreamableHTTPClientTransport;
597689

0 commit comments

Comments
 (0)