From ccccb4b7ccb98b06b3537bf9eebc29bfc3c00368 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Mon, 16 Jun 2025 21:34:06 +0100 Subject: [PATCH] simplify PR / only keep verification in demo inmemory oauth provider Co-Authored-By: Claude --- .../server/demoInMemoryOAuthProvider.test.ts | 175 ++++++++- .../server/demoInMemoryOAuthProvider.ts | 107 +++++- src/examples/server/simpleStreamableHttp.ts | 7 +- .../auth/handlers/authorize.config.test.ts | 360 ------------------ src/server/auth/handlers/authorize.ts | 36 +- src/server/auth/handlers/token.ts | 58 +-- src/server/auth/types.ts | 33 -- 7 files changed, 290 insertions(+), 486 deletions(-) delete mode 100644 src/server/auth/handlers/authorize.config.test.ts diff --git a/src/examples/server/demoInMemoryOAuthProvider.test.ts b/src/examples/server/demoInMemoryOAuthProvider.test.ts index 49c6f69b..852f0c98 100644 --- a/src/examples/server/demoInMemoryOAuthProvider.test.ts +++ b/src/examples/server/demoInMemoryOAuthProvider.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach } from '@jest/globals'; -import { DemoInMemoryAuthProvider, DemoInMemoryClientsStore } from './demoInMemoryOAuthProvider.js'; -import { InvalidTargetError } from '../../server/auth/errors.js'; +import { DemoInMemoryAuthProvider, DemoInMemoryClientsStore, DemoOAuthProviderConfig } from './demoInMemoryOAuthProvider.js'; +import { InvalidTargetError, InvalidRequestError } from '../../server/auth/errors.js'; import { OAuthClientInformationFull } from '../../shared/auth.js'; import { Response } from 'express'; @@ -215,4 +215,175 @@ describe('DemoInMemoryOAuthProvider - RFC 8707 Resource Validation', () => { }, mockResponse as Response)).rejects.toThrow(InvalidTargetError); }); }); + + describe('Server URL validation configuration', () => { + it('should throw error when validateResourceMatchesServer is true but serverUrl is not set', () => { + const invalidConfig: DemoOAuthProviderConfig = { + validateResourceMatchesServer: true + // serverUrl is missing + }; + + expect(() => { + new DemoInMemoryAuthProvider(invalidConfig); + }).toThrow("serverUrl must be configured when validateResourceMatchesServer is true"); + }); + + describe('with server URL validation enabled', () => { + let strictProvider: DemoInMemoryAuthProvider; + + beforeEach(() => { + const config: DemoOAuthProviderConfig = { + serverUrl: 'https://api.example.com/mcp', + validateResourceMatchesServer: true + }; + strictProvider = new DemoInMemoryAuthProvider(config); + + strictProvider.clientsStore.registerClient(mockClient); + }); + + it('should reject authorization without resource parameter', async () => { + await expect(strictProvider.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + scopes: ['mcp:tools'] + // resource is missing + }, mockResponse as Response)).rejects.toThrow(InvalidRequestError); + }); + + it('should reject authorization with non-matching resource', async () => { + await expect(strictProvider.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + resource: 'https://different.api.com/mcp', + scopes: ['mcp:tools'] + }, mockResponse as Response)).rejects.toThrow(InvalidTargetError); + }); + + it('should accept authorization with matching resource', async () => { + await expect(strictProvider.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + resource: 'https://api.example.com/mcp', + scopes: ['mcp:tools'] + }, mockResponse as Response)).resolves.not.toThrow(); + + expect(mockResponse.redirect).toHaveBeenCalled(); + }); + + it('should handle server URL with fragment correctly', async () => { + const configWithFragment: DemoOAuthProviderConfig = { + serverUrl: 'https://api.example.com/mcp#fragment', + validateResourceMatchesServer: true + }; + const providerWithFragment = new DemoInMemoryAuthProvider(configWithFragment); + + await providerWithFragment.clientsStore.registerClient(mockClient); + + // Should accept resource without fragment + await expect(providerWithFragment.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + resource: 'https://api.example.com/mcp', + scopes: ['mcp:tools'] + }, mockResponse as Response)).resolves.not.toThrow(); + }); + + it('should reject token exchange without resource parameter', async () => { + // First authorize with resource + mockResponse.redirect = jest.fn(); + await strictProvider.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + resource: 'https://api.example.com/mcp', + scopes: ['mcp:tools'] + }, mockResponse as Response); + + const redirectCall = (mockResponse.redirect as jest.Mock).mock.calls[0][0]; + const url = new URL(redirectCall); + const authCode = url.searchParams.get('code')!; + + await expect(strictProvider.exchangeAuthorizationCode( + mockClient, + authCode, + undefined, + undefined + // resource is missing + )).rejects.toThrow(InvalidRequestError); + }); + + it('should reject refresh token without resource parameter', async () => { + await expect(strictProvider.exchangeRefreshToken( + mockClient, + 'refresh-token', + undefined + // resource is missing + )).rejects.toThrow(InvalidRequestError); + }); + }); + + describe('with server URL validation disabled (warning mode)', () => { + let warnProvider: DemoInMemoryAuthProvider; + let consoleWarnSpy: jest.SpyInstance; + + beforeEach(() => { + consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + warnProvider = new DemoInMemoryAuthProvider(); // No config = warnings enabled + + warnProvider.clientsStore.registerClient(mockClient); + }); + + afterEach(() => { + consoleWarnSpy.mockRestore(); + }); + + it('should log warning when resource is missing from authorization', async () => { + await warnProvider.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + scopes: ['mcp:tools'] + // resource is missing + }, mockResponse as Response); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('test-client is missing the resource parameter') + ); + }); + + it('should not log warning when resource is present', async () => { + await warnProvider.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + resource: 'https://api.example.com/mcp', + scopes: ['mcp:tools'] + }, mockResponse as Response); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should log warning when resource is missing from token exchange', async () => { + // First authorize without resource + await warnProvider.authorize(mockClient, { + codeChallenge: 'test-challenge', + redirectUri: mockClient.redirect_uris[0], + scopes: ['mcp:tools'] + }, mockResponse as Response); + + const redirectCall = (mockResponse.redirect as jest.Mock).mock.calls[0][0]; + const url = new URL(redirectCall); + const authCode = url.searchParams.get('code')!; + + await warnProvider.exchangeAuthorizationCode( + mockClient, + authCode, + undefined, + undefined + // resource is missing + ); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('test-client is missing the resource parameter') + ); + }); + }); + }); }); \ No newline at end of file diff --git a/src/examples/server/demoInMemoryOAuthProvider.ts b/src/examples/server/demoInMemoryOAuthProvider.ts index 66583e49..2f0e3539 100644 --- a/src/examples/server/demoInMemoryOAuthProvider.ts +++ b/src/examples/server/demoInMemoryOAuthProvider.ts @@ -5,7 +5,8 @@ import { OAuthClientInformationFull, OAuthMetadata, OAuthTokens } from '../../sh import express, { Request, Response } from "express"; import { AuthInfo } from '../../server/auth/types.js'; import { createOAuthMetadata, mcpAuthRouter } from '../../server/auth/router.js'; -import { InvalidTargetError } from '../../server/auth/errors.js'; +import { InvalidTargetError, InvalidRequestError } from '../../server/auth/errors.js'; +import { resourceUrlFromServerUrl } from '../../shared/auth-utils.js'; interface ExtendedClientInformation extends OAuthClientInformationFull { @@ -48,19 +49,79 @@ interface ExtendedAuthInfo extends AuthInfo { type?: string; } +/** + * Configuration options for the demo OAuth provider + */ +export interface DemoOAuthProviderConfig { + /** + * The canonical URL of this MCP server. When provided, the provider will validate + * that the resource parameter in OAuth requests matches this URL. + * + * This should be the full URL that clients use to connect to this server, + * without any fragment component (e.g., "https://api.example.com/mcp"). + * + * Required when validateResourceMatchesServer is true. + */ + serverUrl?: string; + + /** + * If true, validates that the resource parameter matches the configured serverUrl. + * + * When enabled: + * - serverUrl must be configured (throws error if not) + * - resource parameter is required on all requests + * - resource must exactly match serverUrl (after fragment removal) + * - requests without resource parameter will be rejected with invalid_request error + * - requests with non-matching resource will be rejected with invalid_target error + * + * When disabled: + * - warnings are logged when resource parameter is missing (for migration tracking) + * + * @default false + */ + validateResourceMatchesServer?: boolean; +} + export class DemoInMemoryAuthProvider implements OAuthServerProvider { clientsStore = new DemoInMemoryClientsStore(); private codes = new Map(); private tokens = new Map(); + private config?: DemoOAuthProviderConfig; + + constructor(config?: DemoOAuthProviderConfig) { + if (config?.validateResourceMatchesServer && !config?.serverUrl) { + throw new Error("serverUrl must be configured when validateResourceMatchesServer is true"); + } + this.config = config; + } async authorize( client: OAuthClientInformationFull, params: AuthorizationParams, res: Response ): Promise { - // Validate resource parameter if provided + // Validate resource parameter based on configuration + if (this.config?.validateResourceMatchesServer) { + if (!params.resource) { + throw new InvalidRequestError("Resource parameter is required when server URL validation is enabled"); + } + + // Remove fragment from server URL if present (though it shouldn't have one) + const canonicalServerUrl = resourceUrlFromServerUrl(this.config.serverUrl!); + + if (params.resource !== canonicalServerUrl) { + throw new InvalidTargetError( + `Resource parameter '${params.resource}' does not match this server's URL '${canonicalServerUrl}'` + ); + } + } else if (!params.resource) { + // Always log warning if resource is missing (unless validation is enabled) + console.warn(`Authorization request from client ${client.client_id} is missing the resource parameter. Consider migrating to RFC 8707.`); + } + + // Additional validation: check if client is allowed to access the resource if (params.resource) { await this.validateResource(client, params.resource); } @@ -116,6 +177,24 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider { throw new Error(`Authorization code was not issued to this client, ${codeData.client.client_id} != ${client.client_id}`); } + // Validate resource parameter based on configuration + if (this.config?.validateResourceMatchesServer) { + if (!resource) { + throw new InvalidRequestError("Resource parameter is required when server URL validation is enabled"); + } + + const canonicalServerUrl = resourceUrlFromServerUrl(this.config.serverUrl!); + + if (resource !== canonicalServerUrl) { + throw new InvalidTargetError( + `Resource parameter '${resource}' does not match this server's URL '${canonicalServerUrl}'` + ); + } + } else if (!resource) { + // Always log warning if resource is missing (unless validation is enabled) + console.warn(`Token exchange request from client ${client.client_id} is missing the resource parameter. Consider migrating to RFC 8707.`); + } + // Validate that the resource matches what was authorized if (resource !== codeData.params.resource) { throw new InvalidTargetError('Resource parameter does not match the authorized resource'); @@ -154,7 +233,25 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider { _scopes?: string[], resource?: string ): Promise { - // Validate resource parameter if provided + // Validate resource parameter based on configuration + if (this.config?.validateResourceMatchesServer) { + if (!resource) { + throw new InvalidRequestError("Resource parameter is required when server URL validation is enabled"); + } + + const canonicalServerUrl = resourceUrlFromServerUrl(this.config.serverUrl!); + + if (resource !== canonicalServerUrl) { + throw new InvalidTargetError( + `Resource parameter '${resource}' does not match this server's URL '${canonicalServerUrl}'` + ); + } + } else if (!resource) { + // Always log warning if resource is missing (unless validation is enabled) + console.warn(`Token refresh request from client ${client.client_id} is missing the resource parameter. Consider migrating to RFC 8707.`); + } + + // Additional validation: check if client is allowed to access the resource if (resource) { await this.validateResource(client, resource); } @@ -204,13 +301,13 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider { } -export const setupAuthServer = (authServerUrl: URL): OAuthMetadata => { +export const setupAuthServer = (authServerUrl: URL, config?: DemoOAuthProviderConfig): OAuthMetadata => { // Create separate auth server app // NOTE: This is a separate app on a separate port to illustrate // how to separate an OAuth Authorization Server from a Resource // server in the SDK. The SDK is not intended to be provide a standalone // authorization server. - const provider = new DemoInMemoryAuthProvider(); + const provider = new DemoInMemoryAuthProvider(config); const authApp = express(); authApp.use(express.json()); // For introspection requests diff --git a/src/examples/server/simpleStreamableHttp.ts b/src/examples/server/simpleStreamableHttp.ts index ebe31920..65b6263e 100644 --- a/src/examples/server/simpleStreamableHttp.ts +++ b/src/examples/server/simpleStreamableHttp.ts @@ -282,7 +282,12 @@ if (useOAuth) { const mcpServerUrl = new URL(`http://localhost:${MCP_PORT}`); const authServerUrl = new URL(`http://localhost:${AUTH_PORT}`); - const oauthMetadata: OAuthMetadata = setupAuthServer(authServerUrl); + // Configure the demo auth provider to validate resources match this server + const demoProviderConfig = { + serverUrl: mcpServerUrl.href, + validateResourceMatchesServer: false // Set to true to enable strict validation + }; + const oauthMetadata: OAuthMetadata = setupAuthServer(authServerUrl, demoProviderConfig); const tokenVerifier = { verifyAccessToken: async (token: string) => { diff --git a/src/server/auth/handlers/authorize.config.test.ts b/src/server/auth/handlers/authorize.config.test.ts deleted file mode 100644 index f0736da2..00000000 --- a/src/server/auth/handlers/authorize.config.test.ts +++ /dev/null @@ -1,360 +0,0 @@ -import express from "express"; -import request from "supertest"; -import { authorizationHandler } from "./authorize.js"; -import { OAuthServerProvider } from "../provider.js"; -import { OAuthServerConfig } from "../types.js"; - -describe("Authorization handler with config", () => { - let app: express.Application; - let mockProvider: jest.Mocked; - - beforeEach(() => { - app = express(); - - const mockClientsStore = { - getClient: jest.fn(), - registerClient: jest.fn(), - }; - - mockProvider = { - clientsStore: mockClientsStore, - authorize: jest.fn(), - exchangeAuthorizationCode: jest.fn(), - exchangeRefreshToken: jest.fn(), - challengeForAuthorizationCode: jest.fn(), - verifyAccessToken: jest.fn(), - } as jest.Mocked; - }); - - describe("validateResourceMatchesServer configuration", () => { - it("should throw error when validateResourceMatchesServer is true but serverUrl is not set", () => { - const invalidConfig: OAuthServerConfig = { - validateResourceMatchesServer: true - // serverUrl is missing - }; - - expect(() => { - authorizationHandler({ - provider: mockProvider, - config: invalidConfig - }); - }).toThrow("serverUrl must be configured when validateResourceMatchesServer is true"); - }); - }); - - describe("server URL validation (validateResourceMatchesServer: true)", () => { - const serverValidationConfig: OAuthServerConfig = { - serverUrl: "https://api.example.com/mcp", - validateResourceMatchesServer: true - }; - - beforeEach(() => { - app.use("/oauth/authorize", authorizationHandler({ - provider: mockProvider, - config: serverValidationConfig - })); - }); - - it("should reject requests without resource parameter", async () => { - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - - const response = await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state" - }); - - expect(response.status).toBe(302); - expect(response.headers.location).toContain("error=invalid_request"); - expect(response.headers.location).toContain("Resource+parameter+is+required+when+server+URL+validation+is+enabled"); - }); - - it("should accept requests with resource parameter", async () => { - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - mockProvider.authorize.mockImplementation(async (client, params, res) => { - res.redirect(`https://example.com/callback?code=auth-code&state=${params.state}`); - }); - - const response = await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state", - resource: "https://api.example.com/mcp" - }); - - expect(response.status).toBe(302); - expect(response.headers.location).toContain("code=auth-code"); - expect(mockProvider.authorize).toHaveBeenCalledWith( - mockClient, - expect.objectContaining({ - resource: "https://api.example.com/mcp" - }), - expect.any(Object) - ); - }); - }); - - describe("warning mode (default behavior)", () => { - const warnConfig: OAuthServerConfig = { - // No configuration needed - warnings are always enabled by default - }; - - let consoleWarnSpy: jest.SpyInstance; - - beforeEach(() => { - consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(); - app.use("/oauth/authorize", authorizationHandler({ - provider: mockProvider, - config: warnConfig - })); - }); - - afterEach(() => { - consoleWarnSpy.mockRestore(); - }); - - it("should log warning when resource is missing", async () => { - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - mockProvider.authorize.mockImplementation(async (client, params, res) => { - res.redirect(`https://example.com/callback?code=auth-code&state=${params.state}`); - }); - - await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state" - }); - - expect(consoleWarnSpy).toHaveBeenCalledWith( - expect.stringContaining("test-client is missing the resource parameter") - ); - }); - - it("should not log warning when resource is present", async () => { - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - mockProvider.authorize.mockImplementation(async (client, params, res) => { - res.redirect(`https://example.com/callback?code=auth-code&state=${params.state}`); - }); - - await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state", - resource: "https://api.example.com/mcp" - }); - - expect(consoleWarnSpy).not.toHaveBeenCalled(); - }); - }); - - // Note: No silent mode test anymore - warnings are always enabled - - describe("server URL validation (validateResourceMatchesServer: true)", () => { - const serverValidationConfig: OAuthServerConfig = { - serverUrl: "https://api.example.com/mcp", - validateResourceMatchesServer: true - }; - - beforeEach(() => { - app.use("/oauth/authorize", authorizationHandler({ - provider: mockProvider, - config: serverValidationConfig - })); - }); - - it("should accept requests when resource matches server URL", async () => { - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - mockProvider.authorize.mockImplementation(async (client, params, res) => { - res.redirect(`https://example.com/callback?code=auth-code&state=${params.state}`); - }); - - const response = await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state", - resource: "https://api.example.com/mcp" - }); - - expect(response.status).toBe(302); - expect(response.headers.location).toContain("code=auth-code"); - }); - - it("should reject requests when resource does not match server URL", async () => { - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - - const response = await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state", - resource: "https://different.api.com/mcp" - }); - - expect(response.status).toBe(302); - expect(response.headers.location).toContain("error=invalid_target"); - expect(response.headers.location).toContain("does+not+match+this+server"); - }); - - it("should reject requests without resource parameter when validation is enabled", async () => { - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - - const response = await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state" - }); - - expect(response.status).toBe(302); - expect(response.headers.location).toContain("error=invalid_request"); - expect(response.headers.location).toContain("Resource+parameter+is+required+when+server+URL+validation+is+enabled"); - }); - - it("should handle server URL with fragment correctly", async () => { - // Reconfigure with a server URL that has a fragment (though it shouldn't) - const configWithFragment: OAuthServerConfig = { - serverUrl: "https://api.example.com/mcp#fragment", - validateResourceMatchesServer: true - }; - - app = express(); - app.use("/oauth/authorize", authorizationHandler({ - provider: mockProvider, - config: configWithFragment - })); - - const mockClient = { - client_id: "test-client", - client_name: "Test Client", - redirect_uris: ["https://example.com/callback"], - grant_types: ["authorization_code"], - response_types: ["code"], - scope: "read write", - token_endpoint_auth_method: "none", - }; - - (mockProvider.clientsStore.getClient as jest.Mock).mockResolvedValue(mockClient); - mockProvider.authorize.mockImplementation(async (client, params, res) => { - res.redirect(`https://example.com/callback?code=auth-code&state=${params.state}`); - }); - - const response = await request(app) - .get("/oauth/authorize") - .query({ - client_id: "test-client", - redirect_uri: "https://example.com/callback", - response_type: "code", - code_challenge: "test-challenge", - code_challenge_method: "S256", - state: "test-state", - resource: "https://api.example.com/mcp" // No fragment - }); - - expect(response.status).toBe(302); - expect(response.headers.location).toContain("code=auth-code"); - }); - }); -}); \ No newline at end of file diff --git a/src/server/auth/handlers/authorize.ts b/src/server/auth/handlers/authorize.ts index 946c46c9..f6c862ac 100644 --- a/src/server/auth/handlers/authorize.ts +++ b/src/server/auth/handlers/authorize.ts @@ -8,13 +8,10 @@ import { InvalidRequestError, InvalidClientError, InvalidScopeError, - InvalidTargetError, ServerError, TooManyRequestsError, OAuthError } from "../errors.js"; -import { OAuthServerConfig } from "../types.js"; -import { resourceUrlFromServerUrl } from "../../../shared/auth-utils.js"; export type AuthorizationHandlerOptions = { provider: OAuthServerProvider; @@ -23,10 +20,6 @@ export type AuthorizationHandlerOptions = { * Set to false to disable rate limiting for this endpoint. */ rateLimit?: Partial | false; - /** - * OAuth server configuration options - */ - config?: OAuthServerConfig; }; // Parameters that must be validated in order to issue redirects. @@ -45,12 +38,7 @@ const RequestAuthorizationParamsSchema = z.object({ resource: z.string().url().optional(), }); -export function authorizationHandler({ provider, rateLimit: rateLimitConfig, config }: AuthorizationHandlerOptions): RequestHandler { - // Validate configuration - if (config?.validateResourceMatchesServer && !config?.serverUrl) { - throw new Error("serverUrl must be configured when validateResourceMatchesServer is true"); - } - +export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: AuthorizationHandlerOptions): RequestHandler { // Create a router to apply middleware const router = express.Router(); router.use(allowedMethods(["GET", "POST"])); @@ -131,26 +119,8 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig, con const { scope, code_challenge, resource } = parseResult.data; state = parseResult.data.state; - // If validateResourceMatchesServer is enabled, resource is required and must match - if (config?.validateResourceMatchesServer) { - if (!resource) { - throw new InvalidRequestError("Resource parameter is required when server URL validation is enabled"); - } - - // Remove fragment from server URL if present (though it shouldn't have one) - const canonicalServerUrl = resourceUrlFromServerUrl(config.serverUrl!); - - if (resource !== canonicalServerUrl) { - throw new InvalidTargetError( - `Resource parameter '${resource}' does not match this server's URL '${canonicalServerUrl}'` - ); - } - } else { - // Always log warning if resource is missing (unless validation is enabled) - if (!resource) { - console.warn(`Authorization request from client ${client_id} is missing the resource parameter. Consider migrating to RFC 8707.`); - } - } + // Pass through the resource parameter to the provider + // The provider can decide how to validate it // Validate scopes let requestedScopes: string[] = []; diff --git a/src/server/auth/handlers/token.ts b/src/server/auth/handlers/token.ts index 7af42d7a..92fe9921 100644 --- a/src/server/auth/handlers/token.ts +++ b/src/server/auth/handlers/token.ts @@ -12,11 +12,8 @@ import { UnsupportedGrantTypeError, ServerError, TooManyRequestsError, - OAuthError, - InvalidTargetError + OAuthError } from "../errors.js"; -import { OAuthServerConfig } from "../types.js"; -import { resourceUrlFromServerUrl } from "../../../shared/auth-utils.js"; export type TokenHandlerOptions = { provider: OAuthServerProvider; @@ -25,10 +22,6 @@ export type TokenHandlerOptions = { * Set to false to disable rate limiting for this endpoint. */ rateLimit?: Partial | false; - /** - * OAuth server configuration options - */ - config?: OAuthServerConfig; }; const TokenRequestSchema = z.object({ @@ -48,12 +41,7 @@ const RefreshTokenGrantSchema = z.object({ resource: z.string().url().optional(), }); -export function tokenHandler({ provider, rateLimit: rateLimitConfig, config }: TokenHandlerOptions): RequestHandler { - // Validate configuration - if (config?.validateResourceMatchesServer && !config?.serverUrl) { - throw new Error("serverUrl must be configured when validateResourceMatchesServer is true"); - } - +export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHandlerOptions): RequestHandler { // Nested router so we can configure middleware and restrict HTTP method const router = express.Router(); @@ -105,25 +93,8 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig, config }: T const { code, code_verifier, redirect_uri, resource } = parseResult.data; - // If validateResourceMatchesServer is enabled, resource is required and must match - if (config?.validateResourceMatchesServer) { - if (!resource) { - throw new InvalidRequestError("Resource parameter is required when server URL validation is enabled"); - } - - const canonicalServerUrl = resourceUrlFromServerUrl(config.serverUrl!); - - if (resource !== canonicalServerUrl) { - throw new InvalidTargetError( - `Resource parameter '${resource}' does not match this server's URL '${canonicalServerUrl}'` - ); - } - } else { - // Always log warning if resource is missing (unless validation is enabled) - if (!resource) { - console.warn(`Token exchange request from client ${client.client_id} is missing the resource parameter. Consider migrating to RFC 8707.`); - } - } + // Pass through the resource parameter to the provider + // The provider can decide how to validate it const skipLocalPkceValidation = provider.skipLocalPkceValidation; @@ -156,25 +127,8 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig, config }: T const { refresh_token, scope, resource } = parseResult.data; - // If validateResourceMatchesServer is enabled, resource is required and must match - if (config?.validateResourceMatchesServer) { - if (!resource) { - throw new InvalidRequestError("Resource parameter is required when server URL validation is enabled"); - } - - const canonicalServerUrl = resourceUrlFromServerUrl(config.serverUrl!); - - if (resource !== canonicalServerUrl) { - throw new InvalidTargetError( - `Resource parameter '${resource}' does not match this server's URL '${canonicalServerUrl}'` - ); - } - } else { - // Always log warning if resource is missing (unless validation is enabled) - if (!resource) { - console.warn(`Token refresh request from client ${client.client_id} is missing the resource parameter. Consider migrating to RFC 8707.`); - } - } + // Pass through the resource parameter to the provider + // The provider can decide how to validate it const scopes = scope?.split(" "); const tokens = await provider.exchangeRefreshToken(client, refresh_token, scopes, resource); diff --git a/src/server/auth/types.ts b/src/server/auth/types.ts index 33ba3f86..c25c2b60 100644 --- a/src/server/auth/types.ts +++ b/src/server/auth/types.ts @@ -27,37 +27,4 @@ export interface AuthInfo { * This field should be used for any additional data that needs to be attached to the auth info. */ extra?: Record; -} - -/** - * Configuration options for OAuth server behavior - */ -export interface OAuthServerConfig { - /** - * The canonical URL of this MCP server. When provided, the server will validate - * that the resource parameter in OAuth requests matches this URL. - * - * This should be the full URL that clients use to connect to this server, - * without any fragment component (e.g., "https://api.example.com/mcp"). - * - * Required when validateResourceMatchesServer is true. - */ - serverUrl?: string; - - /** - * If true, validates that the resource parameter matches the configured serverUrl. - * - * When enabled: - * - serverUrl must be configured (throws error if not) - * - resource parameter is required on all requests - * - resource must exactly match serverUrl (after fragment removal) - * - requests without resource parameter will be rejected with invalid_request error - * - requests with non-matching resource will be rejected with invalid_target error - * - * When disabled: - * - warnings are logged when resource parameter is missing (for migration tracking) - * - * @default false - */ - validateResourceMatchesServer?: boolean; } \ No newline at end of file