diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 846ba35c2..3d6b99320 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -8,7 +8,7 @@ import { refreshAuthorization, registerClient, discoverOAuthProtectedResourceMetadata, - extractResourceMetadataUrl, + extractWWWAuthenticateParams, auth, type OAuthClientProvider } from './auth.js'; @@ -24,7 +24,7 @@ describe('OAuth Authorization', () => { mockFetch.mockReset(); }); - describe('extractResourceMetadataUrl', () => { + describe('extractWWWAuthenticateParams', () => { it('returns resource metadata url when present', async () => { const resourceUrl = 'https://resource.example.com/.well-known/oauth-protected-resource'; const mockResponse = { @@ -33,39 +33,56 @@ describe('OAuth Authorization', () => { } } as unknown as Response; - expect(extractResourceMetadataUrl(mockResponse)).toEqual(new URL(resourceUrl)); + expect(extractWWWAuthenticateParams(mockResponse)).toEqual({ resourceMetadataUrl: new URL(resourceUrl) }); }); - it('returns undefined if not bearer', async () => { + it('returns scope when present', async () => { + const scope = 'read'; + const mockResponse = { + headers: { + get: jest.fn(name => (name === 'WWW-Authenticate' ? `Bearer realm="mcp", scope="${scope}"` : null)) + } + } as unknown as Response; + + expect(extractWWWAuthenticateParams(mockResponse)).toEqual({ scope: scope }); + }); + + it('returns empty object if not bearer', async () => { const resourceUrl = 'https://resource.example.com/.well-known/oauth-protected-resource'; + const scope = 'read'; const mockResponse = { headers: { - get: jest.fn(name => (name === 'WWW-Authenticate' ? `Basic realm="mcp", resource_metadata="${resourceUrl}"` : null)) + get: jest.fn(name => + name === 'WWW-Authenticate' ? `Basic realm="mcp", resource_metadata="${resourceUrl}", scope="${scope}"` : null + ) } } as unknown as Response; - expect(extractResourceMetadataUrl(mockResponse)).toBeUndefined(); + expect(extractWWWAuthenticateParams(mockResponse)).toEqual({}); }); - it('returns undefined if resource_metadata not present', async () => { + it('returns empty object if resource_metadata and scope not present', async () => { const mockResponse = { headers: { - get: jest.fn(name => (name === 'WWW-Authenticate' ? `Basic realm="mcp"` : null)) + get: jest.fn(name => (name === 'WWW-Authenticate' ? `Bearer realm="mcp"` : null)) } } as unknown as Response; - expect(extractResourceMetadataUrl(mockResponse)).toBeUndefined(); + expect(extractWWWAuthenticateParams(mockResponse)).toEqual({}); }); - it('returns undefined on invalid url', async () => { + it('returns undefined resourceMetadataUrl on invalid url', async () => { const resourceUrl = 'invalid-url'; + const scope = 'read'; const mockResponse = { headers: { - get: jest.fn(name => (name === 'WWW-Authenticate' ? `Basic realm="mcp", resource_metadata="${resourceUrl}"` : null)) + get: jest.fn(name => + name === 'WWW-Authenticate' ? `Bearer realm="mcp", resource_metadata="${resourceUrl}", scope="${scope}"` : null + ) } } as unknown as Response; - expect(extractResourceMetadataUrl(mockResponse)).toBeUndefined(); + expect(extractWWWAuthenticateParams(mockResponse)).toEqual({ scope: scope }); }); }); diff --git a/src/client/auth.ts b/src/client/auth.ts index d5d39cad4..b8c7342c8 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -463,30 +463,40 @@ export async function selectResourceURL( } /** - * Extract resource_metadata from response header. + * Extract resource_metadata and scope from WWW-Authenticate header. */ -export function extractResourceMetadataUrl(res: Response): URL | undefined { +export function extractWWWAuthenticateParams(res: Response): { resourceMetadataUrl?: URL; scope?: string } { const authenticateHeader = res.headers.get('WWW-Authenticate'); if (!authenticateHeader) { - return undefined; + return {}; } const [type, scheme] = authenticateHeader.split(' '); if (type.toLowerCase() !== 'bearer' || !scheme) { - return undefined; + return {}; } - const regex = /resource_metadata="([^"]*)"/; - const match = regex.exec(authenticateHeader); - if (!match) { - return undefined; - } + const resourceMetadataRegex = /resource_metadata="([^"]*)"/; + const resourceMetadataMatch = resourceMetadataRegex.exec(authenticateHeader); - try { - return new URL(match[1]); - } catch { - return undefined; + const scopeRegex = /scope="([^"]*)"/; + const scopeMatch = scopeRegex.exec(authenticateHeader); + + let resourceMetadataUrl: URL | undefined; + if (resourceMetadataMatch) { + try { + resourceMetadataUrl = new URL(resourceMetadataMatch[1]); + } catch { + // Ignore invalid URL + } } + + const scope = scopeMatch?.[1] || undefined; + + return { + resourceMetadataUrl, + scope + }; } /** diff --git a/src/client/middleware.test.ts b/src/client/middleware.test.ts index c0420514b..ecf799844 100644 --- a/src/client/middleware.test.ts +++ b/src/client/middleware.test.ts @@ -7,14 +7,14 @@ jest.mock('../client/auth.js', () => { return { ...actual, auth: jest.fn(), - extractResourceMetadataUrl: jest.fn() + extractWWWAuthenticateParams: jest.fn() }; }); -import { auth, extractResourceMetadataUrl } from './auth.js'; +import { auth, extractWWWAuthenticateParams } from './auth.js'; const mockAuth = auth as jest.MockedFunction; -const mockExtractResourceMetadataUrl = extractResourceMetadataUrl as jest.MockedFunction; +const mockExtractWWWAuthenticateParams = extractWWWAuthenticateParams as jest.MockedFunction; describe('withOAuth', () => { let mockProvider: jest.Mocked; @@ -129,8 +129,11 @@ describe('withOAuth', () => { mockFetch.mockResolvedValueOnce(unauthorizedResponse).mockResolvedValueOnce(successResponse); - const mockResourceUrl = new URL('https://oauth.example.com/.well-known/oauth-protected-resource'); - mockExtractResourceMetadataUrl.mockReturnValue(mockResourceUrl); + const mockWWWAuthenticateParams = { + resourceMetadataUrl: new URL('https://oauth.example.com/.well-known/oauth-protected-resource'), + scope: 'read' + }; + mockExtractWWWAuthenticateParams.mockReturnValue(mockWWWAuthenticateParams); mockAuth.mockResolvedValue('AUTHORIZED'); const enhancedFetch = withOAuth(mockProvider, 'https://api.example.com')(mockFetch); @@ -141,7 +144,8 @@ describe('withOAuth', () => { expect(mockFetch).toHaveBeenCalledTimes(2); expect(mockAuth).toHaveBeenCalledWith(mockProvider, { serverUrl: 'https://api.example.com', - resourceMetadataUrl: mockResourceUrl, + resourceMetadataUrl: mockWWWAuthenticateParams.resourceMetadataUrl, + scope: mockWWWAuthenticateParams.scope, fetchFn: mockFetch }); @@ -172,8 +176,11 @@ describe('withOAuth', () => { mockFetch.mockResolvedValueOnce(unauthorizedResponse).mockResolvedValueOnce(successResponse); - const mockResourceUrl = new URL('https://oauth.example.com/.well-known/oauth-protected-resource'); - mockExtractResourceMetadataUrl.mockReturnValue(mockResourceUrl); + const mockWWWAuthenticateParams = { + resourceMetadataUrl: new URL('https://oauth.example.com/.well-known/oauth-protected-resource'), + scope: 'read' + }; + mockExtractWWWAuthenticateParams.mockReturnValue(mockWWWAuthenticateParams); mockAuth.mockResolvedValue('AUTHORIZED'); // Test without baseUrl - should extract from request URL @@ -185,7 +192,8 @@ describe('withOAuth', () => { expect(mockFetch).toHaveBeenCalledTimes(2); expect(mockAuth).toHaveBeenCalledWith(mockProvider, { serverUrl: 'https://api.example.com', // Should be extracted from request URL - resourceMetadataUrl: mockResourceUrl, + resourceMetadataUrl: mockWWWAuthenticateParams.resourceMetadataUrl, + scope: mockWWWAuthenticateParams.scope, fetchFn: mockFetch }); @@ -203,7 +211,7 @@ describe('withOAuth', () => { }); mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 })); - mockExtractResourceMetadataUrl.mockReturnValue(undefined); + mockExtractWWWAuthenticateParams.mockReturnValue({}); mockAuth.mockResolvedValue('REDIRECT'); // Test without baseUrl @@ -222,7 +230,7 @@ describe('withOAuth', () => { }); mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 })); - mockExtractResourceMetadataUrl.mockReturnValue(undefined); + mockExtractWWWAuthenticateParams.mockReturnValue({}); mockAuth.mockRejectedValue(new Error('Network error')); const enhancedFetch = withOAuth(mockProvider, 'https://api.example.com')(mockFetch); @@ -239,7 +247,7 @@ describe('withOAuth', () => { // Always return 401 mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 })); - mockExtractResourceMetadataUrl.mockReturnValue(undefined); + mockExtractWWWAuthenticateParams.mockReturnValue({}); mockAuth.mockResolvedValue('AUTHORIZED'); const enhancedFetch = withOAuth(mockProvider, 'https://api.example.com')(mockFetch); @@ -345,7 +353,7 @@ describe('withOAuth', () => { mockFetch.mockResolvedValueOnce(unauthorizedResponse).mockResolvedValueOnce(successResponse); - mockExtractResourceMetadataUrl.mockReturnValue(undefined); + mockExtractWWWAuthenticateParams.mockReturnValue({}); mockAuth.mockResolvedValue('AUTHORIZED'); const enhancedFetch = withOAuth(mockProvider)(mockFetch); @@ -876,7 +884,10 @@ describe('Integration Tests', () => { mockFetch.mockResolvedValueOnce(unauthorizedResponse).mockResolvedValueOnce(successResponse); - mockExtractResourceMetadataUrl.mockReturnValue(new URL('https://auth.example.com/.well-known/oauth-protected-resource')); + mockExtractWWWAuthenticateParams.mockReturnValue({ + resourceMetadataUrl: new URL('https://auth.example.com/.well-known/oauth-protected-resource'), + scope: 'read' + }); mockAuth.mockResolvedValue('AUTHORIZED'); // Use custom logger to avoid console output @@ -896,6 +907,7 @@ describe('Integration Tests', () => { expect(mockAuth).toHaveBeenCalledWith(mockProvider, { serverUrl: 'https://mcp-server.example.com', resourceMetadataUrl: new URL('https://auth.example.com/.well-known/oauth-protected-resource'), + scope: 'read', fetchFn: mockFetch }); }); diff --git a/src/client/middleware.ts b/src/client/middleware.ts index a7cbc6c69..c8f7fdd3d 100644 --- a/src/client/middleware.ts +++ b/src/client/middleware.ts @@ -1,4 +1,4 @@ -import { auth, extractResourceMetadataUrl, OAuthClientProvider, UnauthorizedError } from './auth.js'; +import { auth, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; import { FetchLike } from '../shared/transport.js'; /** @@ -54,7 +54,7 @@ export const withOAuth = // Handle 401 responses by attempting re-authentication if (response.status === 401) { try { - const resourceMetadataUrl = extractResourceMetadataUrl(response); + const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); // Use provided baseUrl or extract from request URL const serverUrl = baseUrl || (typeof input === 'string' ? new URL(input).origin : input.origin); @@ -62,6 +62,7 @@ export const withOAuth = const result = await auth(provider, { serverUrl, resourceMetadataUrl, + scope, fetchFn: next }); diff --git a/src/client/sse.ts b/src/client/sse.ts index aa4942444..c54a03188 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -1,7 +1,7 @@ import { EventSource, type ErrorEvent, type EventSourceInit } from 'eventsource'; import { Transport, FetchLike } from '../shared/transport.js'; import { JSONRPCMessage, JSONRPCMessageSchema } from '../types.js'; -import { auth, AuthResult, extractResourceMetadataUrl, OAuthClientProvider, UnauthorizedError } from './auth.js'; +import { auth, AuthResult, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; export class SseError extends Error { constructor( @@ -64,6 +64,7 @@ export class SSEClientTransport implements Transport { private _abortController?: AbortController; private _url: URL; private _resourceMetadataUrl?: URL; + private _scope?: string; private _eventSourceInit?: EventSourceInit; private _requestInit?: RequestInit; private _authProvider?: OAuthClientProvider; @@ -77,6 +78,7 @@ export class SSEClientTransport implements Transport { constructor(url: URL, opts?: SSEClientTransportOptions) { this._url = url; this._resourceMetadataUrl = undefined; + this._scope = undefined; this._eventSourceInit = opts?.eventSourceInit; this._requestInit = opts?.requestInit; this._authProvider = opts?.authProvider; @@ -93,6 +95,7 @@ export class SSEClientTransport implements Transport { result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, + scope: this._scope, fetchFn: this._fetch }); } catch (error) { @@ -136,7 +139,9 @@ export class SSEClientTransport implements Transport { }); if (response.status === 401 && response.headers.has('www-authenticate')) { - this._resourceMetadataUrl = extractResourceMetadataUrl(response); + const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); + this._resourceMetadataUrl = resourceMetadataUrl; + this._scope = scope; } return response; @@ -213,6 +218,7 @@ export class SSEClientTransport implements Transport { serverUrl: this._url, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, + scope: this._scope, fetchFn: this._fetch }); if (result !== 'AUTHORIZED') { @@ -245,11 +251,14 @@ export class SSEClientTransport implements Transport { const response = await (this._fetch ?? fetch)(this._endpoint, init); if (!response.ok) { if (response.status === 401 && this._authProvider) { - this._resourceMetadataUrl = extractResourceMetadataUrl(response); + const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); + this._resourceMetadataUrl = resourceMetadataUrl; + this._scope = scope; const result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, + scope: this._scope, fetchFn: this._fetch }); if (result !== 'AUTHORIZED') { diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 12cb94864..de3e5aa6a 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, extractResourceMetadataUrl, 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 @@ -125,6 +125,7 @@ export class StreamableHTTPClientTransport implements Transport { private _abortController?: AbortController; private _url: URL; private _resourceMetadataUrl?: URL; + private _scope?: string; private _requestInit?: RequestInit; private _authProvider?: OAuthClientProvider; private _fetch?: FetchLike; @@ -140,6 +141,7 @@ export class StreamableHTTPClientTransport implements Transport { constructor(url: URL, opts?: StreamableHTTPClientTransportOptions) { this._url = url; this._resourceMetadataUrl = undefined; + this._scope = undefined; this._requestInit = opts?.requestInit; this._authProvider = opts?.authProvider; this._fetch = opts?.fetch; @@ -157,6 +159,7 @@ export class StreamableHTTPClientTransport implements Transport { result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, + scope: this._scope, fetchFn: this._fetch }); } catch (error) { @@ -381,6 +384,7 @@ export class StreamableHTTPClientTransport implements Transport { serverUrl: this._url, authorizationCode, resourceMetadataUrl: this._resourceMetadataUrl, + scope: this._scope, fetchFn: this._fetch }); if (result !== 'AUTHORIZED') { @@ -437,11 +441,14 @@ export class StreamableHTTPClientTransport implements Transport { throw new StreamableHTTPError(401, 'Server returned 401 after successful authentication'); } - this._resourceMetadataUrl = extractResourceMetadataUrl(response); + const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response); + this._resourceMetadataUrl = resourceMetadataUrl; + this._scope = scope; const result = await auth(this._authProvider, { serverUrl: this._url, resourceMetadataUrl: this._resourceMetadataUrl, + scope: this._scope, fetchFn: this._fetch }); if (result !== 'AUTHORIZED') { diff --git a/src/server/auth/handlers/authorize.test.ts b/src/server/auth/handlers/authorize.test.ts index 5dabd19e4..51ce111a0 100644 --- a/src/server/auth/handlers/authorize.test.ts +++ b/src/server/auth/handlers/authorize.test.ts @@ -218,38 +218,6 @@ describe('Authorization Handler', () => { }); }); - describe('Scope validation', () => { - it('validates requested scopes against client registered scopes', async () => { - const response = await supertest(app).get('/authorize').query({ - client_id: 'valid-client', - redirect_uri: 'https://example.com/callback', - response_type: 'code', - code_challenge: 'challenge123', - code_challenge_method: 'S256', - scope: 'profile email admin' // 'admin' not in client scopes - }); - - expect(response.status).toBe(302); - const location = new URL(response.header.location); - expect(location.searchParams.get('error')).toBe('invalid_scope'); - }); - - it('accepts valid scopes subset', async () => { - const response = await supertest(app).get('/authorize').query({ - client_id: 'valid-client', - redirect_uri: 'https://example.com/callback', - response_type: 'code', - code_challenge: 'challenge123', - code_challenge_method: 'S256', - scope: 'profile' // subset of client scopes - }); - - expect(response.status).toBe(302); - const location = new URL(response.header.location); - expect(location.searchParams.has('code')).toBe(true); - }); - }); - describe('Resource parameter validation', () => { it('propagates resource parameter', async () => { const mockProviderWithResource = jest.spyOn(mockProvider, 'authorize'); diff --git a/src/server/auth/handlers/authorize.ts b/src/server/auth/handlers/authorize.ts index 14c7121ae..ef15770b9 100644 --- a/src/server/auth/handlers/authorize.ts +++ b/src/server/auth/handlers/authorize.ts @@ -4,7 +4,7 @@ import express from 'express'; import { OAuthServerProvider } from '../provider.js'; import { rateLimit, Options as RateLimitOptions } from 'express-rate-limit'; import { allowedMethods } from '../middleware/allowedMethods.js'; -import { InvalidRequestError, InvalidClientError, InvalidScopeError, ServerError, TooManyRequestsError, OAuthError } from '../errors.js'; +import { InvalidRequestError, InvalidClientError, ServerError, TooManyRequestsError, OAuthError } from '../errors.js'; export type AuthorizationHandlerOptions = { provider: OAuthServerProvider; @@ -120,14 +120,6 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A let requestedScopes: string[] = []; if (scope !== undefined) { requestedScopes = scope.split(' '); - const allowedScopes = new Set(client.scope?.split(' ')); - - // Check each requested scope against allowed scopes - for (const scope of requestedScopes) { - if (!allowedScopes.has(scope)) { - throw new InvalidScopeError(`Client was not registered with scope ${scope}`); - } - } } // All validation passed, proceed with authorization