diff --git a/api/package.json b/api/package.json index 61a65429b7..b0e4fcb14d 100644 --- a/api/package.json +++ b/api/package.json @@ -48,7 +48,7 @@ "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", - "@modelcontextprotocol/sdk": "^1.27.1", + "@modelcontextprotocol/sdk": "^1.29.0", "@node-saml/passport-saml": "^5.1.0", "@smithy/node-http-handler": "^4.4.5", "ai-tokenizer": "^1.0.6", diff --git a/package-lock.json b/package-lock.json index 7af4d14874..80fbd45b19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -63,7 +63,7 @@ "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", - "@modelcontextprotocol/sdk": "^1.27.1", + "@modelcontextprotocol/sdk": "^1.29.0", "@node-saml/passport-saml": "^5.1.0", "@smithy/node-http-handler": "^4.4.5", "ai-tokenizer": "^1.0.6", @@ -12042,9 +12042,9 @@ } }, "node_modules/@modelcontextprotocol/sdk": { - "version": "1.27.1", - "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.27.1.tgz", - "integrity": "sha512-sr6GbP+4edBwFndLbM60gf07z0FQ79gaExpnsjMGePXqFcSSb7t6iscpjk9DhFhwd+mTEQrzNafGP8/iGGFYaA==", + "version": "1.29.0", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.29.0.tgz", + "integrity": "sha512-zo37mZA9hJWpULgkRpowewez1y6ML5GsXJPY8FI0tBBCd77HEvza4jDqRKOXgHNn867PVGCyTdzqpz0izu5ZjQ==", "license": "MIT", "dependencies": { "@hono/node-server": "^1.19.9", @@ -44196,7 +44196,7 @@ "@langchain/core": "^0.3.80", "@librechat/agents": "^3.1.68", "@librechat/data-schemas": "*", - "@modelcontextprotocol/sdk": "^1.27.1", + "@modelcontextprotocol/sdk": "^1.29.0", "@smithy/node-http-handler": "^4.4.5", "ai-tokenizer": "^1.0.6", "axios": "^1.15.0", diff --git a/packages/api/package.json b/packages/api/package.json index 2dae50b846..62884cae4e 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -97,7 +97,7 @@ "@langchain/core": "^0.3.80", "@librechat/agents": "^3.1.68", "@librechat/data-schemas": "*", - "@modelcontextprotocol/sdk": "^1.27.1", + "@modelcontextprotocol/sdk": "^1.29.0", "@smithy/node-http-handler": "^4.4.5", "ai-tokenizer": "^1.0.6", "axios": "^1.15.0", diff --git a/packages/api/src/mcp/__tests__/handler.test.ts b/packages/api/src/mcp/__tests__/handler.test.ts index aa19eec9b0..6e0c1961bf 100644 --- a/packages/api/src/mcp/__tests__/handler.test.ts +++ b/packages/api/src/mcp/__tests__/handler.test.ts @@ -18,6 +18,7 @@ jest.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({ discoverOAuthProtectedResourceMetadata: jest.fn(), registerClient: jest.fn(), exchangeAuthorization: jest.fn(), + extractWWWAuthenticateParams: jest.fn(() => ({})), })); jest.mock('../../mcp/oauth/tokens', () => ({ @@ -26,6 +27,10 @@ jest.mock('../../mcp/oauth/tokens', () => ({ }, })); +jest.mock('../../mcp/oauth/resourceHint', () => ({ + probeResourceMetadataHint: jest.fn().mockResolvedValue(null), +})); + import { startAuthorization, discoverAuthorizationServerMetadata, @@ -34,6 +39,7 @@ import { exchangeAuthorization, } from '@modelcontextprotocol/sdk/client/auth.js'; import { MCPTokenStorage } from '../../mcp/oauth/tokens'; +import { probeResourceMetadataHint } from '../../mcp/oauth/resourceHint'; import { FlowStateManager } from '../../flow/manager'; const mockStartAuthorization = startAuthorization as jest.MockedFunction; @@ -53,6 +59,9 @@ const mockGetClientInfoAndMetadata = MCPTokenStorage.getClientInfoAndMetadata as jest.MockedFunction< typeof MCPTokenStorage.getClientInfoAndMetadata >; +const mockProbeResourceMetadataHint = probeResourceMetadataHint as jest.MockedFunction< + typeof probeResourceMetadataHint +>; describe('MCPOAuthHandler - Configurable OAuth Metadata', () => { const mockServerName = 'test-server'; @@ -2408,4 +2417,188 @@ describe('MCPOAuthHandler - Configurable OAuth Metadata', () => { expect(result.authorizationUrl).not.toContain('resource='); }); }); + + describe('WWW-Authenticate resource_metadata hint (RFC 9728 §5.1 / issue #12761)', () => { + const serverUrl = 'https://example.com/mcp'; + const hintUrl = 'https://example.com/.well-known/oauth-protected-resource'; + + beforeEach(() => { + // Default the probe to "no hint" so earlier suites that don't set it aren't affected. + mockProbeResourceMetadataHint.mockResolvedValue(null); + }); + + it('threads the hint URL into discoverOAuthProtectedResourceMetadata when present', async () => { + mockProbeResourceMetadataHint.mockResolvedValueOnce({ + resourceMetadataUrl: new URL(hintUrl), + bearerChallenge: true, + headAuthChallenge: true, + }); + + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: serverUrl, + authorization_servers: ['https://auth.example.com'], + }); + + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + registration_endpoint: 'https://auth.example.com/register', + response_types_supported: ['code'], + } as AuthorizationServerMetadata); + + mockRegisterClient.mockResolvedValueOnce({ + client_id: 'new-client-id', + redirect_uris: ['http://localhost:3080/api/mcp/test-server/oauth/callback'], + logo_uri: undefined, + tos_uri: undefined, + }); + + mockStartAuthorization.mockResolvedValueOnce({ + authorizationUrl: new URL('https://auth.example.com/authorize?client_id=new-client-id'), + codeVerifier: 'test-code-verifier', + }); + + await MCPOAuthHandler.initiateOAuthFlow('test-server', serverUrl, 'user-123', {}, undefined); + + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1); + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + serverUrl, + expect.objectContaining({ resourceMetadataUrl: new URL(hintUrl) }), + expect.any(Function), + ); + }); + + it('passes undefined resourceMetadataUrl when no hint is available', async () => { + mockProbeResourceMetadataHint.mockResolvedValueOnce(null); + + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: serverUrl, + authorization_servers: ['https://auth.example.com'], + }); + + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + registration_endpoint: 'https://auth.example.com/register', + response_types_supported: ['code'], + } as AuthorizationServerMetadata); + + mockRegisterClient.mockResolvedValueOnce({ + client_id: 'new-client-id', + redirect_uris: ['http://localhost:3080/api/mcp/test-server/oauth/callback'], + logo_uri: undefined, + tos_uri: undefined, + }); + + mockStartAuthorization.mockResolvedValueOnce({ + authorizationUrl: new URL('https://auth.example.com/authorize?client_id=new-client-id'), + codeVerifier: 'test-code-verifier', + }); + + await MCPOAuthHandler.initiateOAuthFlow('test-server', serverUrl, 'user-123', {}, undefined); + + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + serverUrl, + expect.objectContaining({ resourceMetadataUrl: undefined }), + expect.any(Function), + ); + }); + + it('prefers the hint over path-aware metadata when they diverge', async () => { + // The regression scenario from issue #12761: path-aware discovery would return + // stale metadata pointing at a defunct authorization server. The hint URL must + // take precedence so the SDK fetches the authoritative document instead. + mockProbeResourceMetadataHint.mockResolvedValueOnce({ + resourceMetadataUrl: new URL(hintUrl), + bearerChallenge: true, + headAuthChallenge: true, + }); + + // Whatever the hint URL returns is what reaches the handler — stale path-aware + // data never gets a chance to be used, because the SDK follows the hint instead. + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: serverUrl, + authorization_servers: ['https://auth.example.com'], + }); + + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + registration_endpoint: 'https://auth.example.com/register', + response_types_supported: ['code'], + } as AuthorizationServerMetadata); + + mockRegisterClient.mockResolvedValueOnce({ + client_id: 'new-client-id', + redirect_uris: ['http://localhost:3080/api/mcp/test-server/oauth/callback'], + logo_uri: undefined, + tos_uri: undefined, + }); + + mockStartAuthorization.mockResolvedValueOnce({ + authorizationUrl: new URL('https://auth.example.com/authorize?client_id=new-client-id'), + codeVerifier: 'test-code-verifier', + }); + + const result = await MCPOAuthHandler.initiateOAuthFlow( + 'test-server', + serverUrl, + 'user-123', + {}, + undefined, + ); + + expect(result.authorizationUrl).toContain('auth.example.com'); + // Exactly one SDK call — no separate path-aware retry. + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1); + }); + + it('invokes the probe with the OAuth-aware fetch so oauthHeaders reach the server', async () => { + // Regression guard: without the wrapper, admin-configured `oauthHeaders` (e.g. a + // gateway API key that fronts the MCP endpoint) would be stripped from the probe, + // causing the gateway to 401 us for the wrong reason and masking the real hint. + mockProbeResourceMetadataHint.mockResolvedValueOnce(null); + + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: serverUrl, + authorization_servers: ['https://auth.example.com'], + }); + + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + registration_endpoint: 'https://auth.example.com/register', + response_types_supported: ['code'], + } as AuthorizationServerMetadata); + + mockRegisterClient.mockResolvedValueOnce({ + client_id: 'new-client-id', + redirect_uris: ['http://localhost:3080/api/mcp/test-server/oauth/callback'], + logo_uri: undefined, + tos_uri: undefined, + }); + + mockStartAuthorization.mockResolvedValueOnce({ + authorizationUrl: new URL('https://auth.example.com/authorize?client_id=new-client-id'), + codeVerifier: 'test-code-verifier', + }); + + await MCPOAuthHandler.initiateOAuthFlow( + 'test-server', + serverUrl, + 'user-123', + { 'X-Gateway-Key': 'secret' }, + undefined, + ); + + expect(mockProbeResourceMetadataHint).toHaveBeenCalledTimes(1); + // Second argument must be a fetchFn (the OAuth-aware wrapper), not `undefined`. + const fetchFnArg = mockProbeResourceMetadataHint.mock.calls[0][1]; + expect(typeof fetchFnArg).toBe('function'); + }); + }); }); diff --git a/packages/api/src/mcp/__tests__/reconnection-storm.test.ts b/packages/api/src/mcp/__tests__/reconnection-storm.test.ts index e073dca8a3..c61e6659b8 100644 --- a/packages/api/src/mcp/__tests__/reconnection-storm.test.ts +++ b/packages/api/src/mcp/__tests__/reconnection-storm.test.ts @@ -8,6 +8,7 @@ import http from 'http'; import { randomUUID } from 'crypto'; import express from 'express'; +import { z } from 'zod'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { isInitializeRequest } from '@modelcontextprotocol/sdk/types.js'; import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; @@ -62,7 +63,7 @@ function startMCPServer(): Promise { function createServer(): McpServer { const server = new McpServer({ name: 'test-server', version: '1.0.0' }); - server.tool('echo', 'echoes input', { message: { type: 'string' } as never }, async (args) => { + server.tool('echo', 'echoes input', { message: z.string() }, async (args) => { const msg = (args as Record).message ?? ''; return { content: [{ type: 'text', text: msg }] }; }); diff --git a/packages/api/src/mcp/oauth/detectOAuth.fallback.test.ts b/packages/api/src/mcp/oauth/detectOAuth.fallback.test.ts new file mode 100644 index 0000000000..324fca8df6 --- /dev/null +++ b/packages/api/src/mcp/oauth/detectOAuth.fallback.test.ts @@ -0,0 +1,147 @@ +import { detectOAuthRequirement } from './detectOAuth'; + +jest.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({ + ...jest.requireActual('@modelcontextprotocol/sdk/client/auth.js'), + discoverOAuthProtectedResourceMetadata: jest.fn(), +})); + +jest.mock('~/auth', () => ({ + isSSRFTarget: jest.fn(() => false), + resolveHostnameSSRF: jest.fn(async () => false), +})); + +/** + * Exercises the `MCP_OAUTH_ON_AUTH_ERROR=true` path in isolation — the main + * `detectOAuth.test.ts` disables it to assert on precise detection outcomes, so + * the fallback's behavior (and its avoidance of a redundant HEAD request) lives + * here with the config forced on. + */ +jest.mock('../mcpConfig', () => ({ + mcpConfig: { + OAUTH_ON_AUTH_ERROR: true, + OAUTH_DETECTION_TIMEOUT: 5000, + }, +})); + +import { discoverOAuthProtectedResourceMetadata } from '@modelcontextprotocol/sdk/client/auth.js'; + +const mockDiscoverOAuthProtectedResourceMetadata = + discoverOAuthProtectedResourceMetadata as jest.MockedFunction< + typeof discoverOAuthProtectedResourceMetadata + >; + +describe('detectOAuthRequirement — MCP_OAUTH_ON_AUTH_ERROR fallback', () => { + const originalFetch = global.fetch; + const mockFetch = jest.fn() as unknown as jest.MockedFunction; + + beforeEach(() => { + jest.clearAllMocks(); + global.fetch = mockFetch; + mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue( + new Error('No protected resource metadata'), + ); + }); + + afterAll(() => { + global.fetch = originalFetch; + }); + + it('honors a 401 observed by the probe without issuing a second HEAD', async () => { + // Server responds 401 on HEAD with neither Bearer nor resource_metadata (Basic). + // Before the authChallenge optimization, detectOAuth would fire another HEAD via + // checkAuthErrorFallback; now it reuses the probe's observation. + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), + } as Response); + // POST still probed because the HEAD carried no useful hint. + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result).toEqual({ + requiresOAuth: true, + method: 'no-metadata-found', + metadata: null, + }); + // Exactly 2 fetches — HEAD + POST from the probe. No redundant fallback HEAD. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('honors a 403 observed by the probe without issuing a second HEAD', async () => { + mockFetch.mockResolvedValue({ status: 403, headers: new Headers() } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('no-metadata-found'); + // HEAD + POST from the probe; no extra fallback HEAD. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('retries via HEAD when the probe threw (transient network error)', async () => { + // Probe crashes on both HEAD and POST (network down). + mockFetch.mockRejectedValueOnce(new Error('ECONNREFUSED')); + mockFetch.mockRejectedValueOnce(new Error('ECONNREFUSED')); + // Fallback HEAD succeeds and returns 401 — we honor it. + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers(), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('no-metadata-found'); + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + + it('does not fire fallback when the probe observed a clean 200', async () => { + mockFetch.mockResolvedValue({ status: 200, headers: new Headers() } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(false); + expect(result.method).toBe('no-metadata-found'); + // HEAD + POST from the probe, both 200 — fallback must not fire. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('retries via fallback HEAD when only HEAD threw (transient failure)', async () => { + // If HEAD transiently fails (timeout/ECONNRESET) but POST responds non-auth, the + // probe must treat HEAD status as "unknown" so the fallback still gets a chance + // to classify 401/403 servers correctly. + mockFetch + .mockRejectedValueOnce(new Error('ETIMEDOUT')) + .mockResolvedValueOnce({ status: 200, headers: new Headers() } as Response); + // Fallback HEAD finally succeeds and returns 401. + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers(), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('no-metadata-found'); + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + + it('does not fire fallback when HEAD was 200 but POST returned 403 (WAF/CSRF)', async () => { + // A server that isn't OAuth-protected but 403s body-less POSTs for WAF/CSRF reasons + // must NOT be misclassified as OAuth-required. The fallback is scoped to HEAD status. + mockFetch + .mockResolvedValueOnce({ status: 200, headers: new Headers() } as Response) + .mockResolvedValueOnce({ status: 403, headers: new Headers() } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(false); + expect(result.method).toBe('no-metadata-found'); + // Only the probe ran — no extra fallback HEAD fired. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/api/src/mcp/oauth/detectOAuth.test.ts b/packages/api/src/mcp/oauth/detectOAuth.test.ts index 8164adddaf..cfcbf9a692 100644 --- a/packages/api/src/mcp/oauth/detectOAuth.test.ts +++ b/packages/api/src/mcp/oauth/detectOAuth.test.ts @@ -1,15 +1,37 @@ +import { discoverOAuthProtectedResourceMetadata } from '@modelcontextprotocol/sdk/client/auth.js'; +import { isSSRFTarget, resolveHostnameSSRF } from '~/auth'; import { detectOAuthRequirement } from './detectOAuth'; jest.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({ + ...jest.requireActual('@modelcontextprotocol/sdk/client/auth.js'), discoverOAuthProtectedResourceMetadata: jest.fn(), })); -import { discoverOAuthProtectedResourceMetadata } from '@modelcontextprotocol/sdk/client/auth.js'; +jest.mock('~/auth', () => ({ + isSSRFTarget: jest.fn(() => false), + resolveHostnameSSRF: jest.fn(async () => false), +})); + +/** + * Disable the `MCP_OAUTH_ON_AUTH_ERROR` fallback by default so tests assert on the + * precise detection outcome without the "any 401/403 = OAuth" safety net rewriting + * the result. The fallback is exercised directly in its own describe block. + */ +jest.mock('../mcpConfig', () => ({ + mcpConfig: { + OAUTH_ON_AUTH_ERROR: false, + OAUTH_DETECTION_TIMEOUT: 5000, + }, +})); const mockDiscoverOAuthProtectedResourceMetadata = discoverOAuthProtectedResourceMetadata as jest.MockedFunction< typeof discoverOAuthProtectedResourceMetadata >; +const mockIsSSRFTarget = isSSRFTarget as jest.MockedFunction; +const mockResolveHostnameSSRF = resolveHostnameSSRF as jest.MockedFunction< + typeof resolveHostnameSSRF +>; describe('detectOAuthRequirement', () => { const originalFetch = global.fetch; @@ -18,6 +40,7 @@ describe('detectOAuthRequirement', () => { beforeEach(() => { jest.clearAllMocks(); global.fetch = mockFetch; + // Default: path-aware / hint discovery returns no metadata unless a test overrides. mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue( new Error('No protected resource metadata'), ); @@ -28,14 +51,12 @@ describe('detectOAuthRequirement', () => { }); describe('POST fallback when HEAD fails', () => { - it('should try POST when HEAD returns 405 Method Not Allowed', async () => { - // HEAD returns 405 (Method Not Allowed) + it('tries POST when HEAD returns 405 Method Not Allowed', async () => { mockFetch.mockResolvedValueOnce({ status: 405, headers: new Headers(), } as Response); - // POST returns 401 with Bearer mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'Bearer' }), @@ -46,11 +67,7 @@ describe('detectOAuthRequirement', () => { expect(result.requiresOAuth).toBe(true); expect(result.method).toBe('401-challenge-metadata'); expect(mockFetch).toHaveBeenCalledTimes(2); - - // Verify HEAD was called first expect(mockFetch.mock.calls[0][1]).toEqual(expect.objectContaining({ method: 'HEAD' })); - - // Verify POST was called second with proper headers and body expect(mockFetch.mock.calls[1][1]).toEqual( expect.objectContaining({ method: 'POST', @@ -60,14 +77,12 @@ describe('detectOAuthRequirement', () => { ); }); - it('should try POST when HEAD returns non-401 status', async () => { - // HEAD returns 200 OK (no auth required for HEAD) + it('tries POST when HEAD returns non-401 status', async () => { mockFetch.mockResolvedValueOnce({ status: 200, headers: new Headers(), } as Response); - // POST returns 401 with Bearer mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'Bearer' }), @@ -79,23 +94,49 @@ describe('detectOAuthRequirement', () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); - it('should not try POST if HEAD returns 401', async () => { - // HEAD returns 401 with Bearer + it('short-circuits POST when HEAD already delivers the resource_metadata hint', async () => { + // Only `resource_metadata` on HEAD is strong enough to skip POST — Bearer-only + // still lets POST run in case the server surfaces its hint only on POST. + const hintUrl = 'https://example.com/.well-known/oauth-protected-resource'; mockFetch.mockResolvedValueOnce({ status: 401, - headers: new Headers({ 'www-authenticate': 'Bearer' }), + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${hintUrl}"`, + }), } as Response); + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: 'https://mcp.example.com', + authorization_servers: ['https://auth.example.com'], + }); + const result = await detectOAuthRequirement('https://mcp.example.com'); expect(result.requiresOAuth).toBe(true); - // Only HEAD should be called since it returned 401 expect(mockFetch).toHaveBeenCalledTimes(1); }); + + it('probes POST even when HEAD returns Bearer without a hint', async () => { + mockFetch + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response) + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); }); describe('Bearer detection without resource_metadata URL', () => { - it('should detect OAuth when 401 has WWW-Authenticate: Bearer (case insensitive)', async () => { + it('detects OAuth when 401 has WWW-Authenticate: Bearer (case insensitive)', async () => { mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'bearer' }), @@ -108,7 +149,7 @@ describe('detectOAuthRequirement', () => { expect(result.metadata).toBeNull(); }); - it('should detect OAuth when 401 has WWW-Authenticate: BEARER (uppercase)', async () => { + it('detects OAuth when 401 has WWW-Authenticate: BEARER (uppercase)', async () => { mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'BEARER' }), @@ -120,7 +161,7 @@ describe('detectOAuthRequirement', () => { expect(result.method).toBe('401-challenge-metadata'); }); - it('should detect OAuth when Bearer is part of a larger header value', async () => { + it('detects OAuth when Bearer is part of a larger header value', async () => { mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'Bearer realm="api"' }), @@ -131,13 +172,11 @@ describe('detectOAuthRequirement', () => { expect(result.requiresOAuth).toBe(true); }); - it('should not detect OAuth when 401 has no WWW-Authenticate header', async () => { + it('does not detect OAuth when 401 has no WWW-Authenticate header', async () => { mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers(), } as Response); - - // POST also returns 401 without header mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers(), @@ -149,13 +188,11 @@ describe('detectOAuthRequirement', () => { expect(result.method).toBe('no-metadata-found'); }); - it('should not detect OAuth when 401 has non-Bearer auth scheme', async () => { + it('does not detect OAuth when 401 has non-Bearer auth scheme', async () => { mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), } as Response); - - // POST also returns 401 with Basic mockFetch.mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), @@ -168,69 +205,183 @@ describe('detectOAuthRequirement', () => { }); describe('resource_metadata URL in WWW-Authenticate', () => { - it('should prefer resource_metadata URL when provided with Bearer', async () => { + it('passes the WWW-Authenticate hint URL to the SDK and returns its metadata', async () => { const metadataUrl = 'https://auth.example.com/.well-known/oauth-protected-resource'; - mockFetch - // HEAD request - 401 with resource_metadata URL - .mockResolvedValueOnce({ - status: 401, - headers: new Headers({ - 'www-authenticate': `Bearer resource_metadata="${metadataUrl}"`, - }), - } as Response) - // Metadata fetch - .mockResolvedValueOnce({ - ok: true, - json: async () => ({ - authorization_servers: ['https://auth.example.com'], - }), - } as Response); + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${metadataUrl}"`, + }), + } as Response); + + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: 'https://mcp.example.com', + authorization_servers: ['https://auth.example.com'], + }); const result = await detectOAuthRequirement('https://mcp.example.com'); expect(result.requiresOAuth).toBe(true); expect(result.method).toBe('401-challenge-metadata'); - expect(result.metadata).toEqual({ + expect(result.metadata).toMatchObject({ authorization_servers: ['https://auth.example.com'], }); + + // The SDK must be called with the hint so that it fetches the authoritative URL + // instead of the path-aware `/.well-known/oauth-protected-resource/` variant. + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + 'https://mcp.example.com', + expect.objectContaining({ resourceMetadataUrl: new URL(metadataUrl) }), + ); }); - it('should fall back to Bearer detection if metadata fetch fails', async () => { + it('falls back to Bearer-only detection when hinted metadata fetch fails', async () => { const metadataUrl = 'https://auth.example.com/.well-known/oauth-protected-resource'; - mockFetch - // HEAD request - 401 with resource_metadata URL - .mockResolvedValueOnce({ - status: 401, - headers: new Headers({ - 'www-authenticate': `Bearer resource_metadata="${metadataUrl}"`, - }), - } as Response) - // Metadata fetch fails - .mockRejectedValueOnce(new Error('Network error')); + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${metadataUrl}"`, + }), + } as Response); + + // Hinted discovery throws (e.g. 404 at the hinted URL). + mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValueOnce( + new Error('Resource server does not implement OAuth 2.0 Protected Resource Metadata.'), + ); const result = await detectOAuthRequirement('https://mcp.example.com'); - // Should still detect OAuth via Bearer expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); + expect(result.metadata).toBeNull(); + }); + + it('prefers the 401 hint even when path-aware metadata exists (RFC 9728 §5.1)', async () => { + // This is the bug from issue #12761: when a 401 WWW-Authenticate header advertises + // a `resource_metadata` URL, that URL must win over any path-aware metadata. + const metadataUrl = 'https://mcp.example.com/.well-known/oauth-protected-resource'; + + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${metadataUrl}"`, + }), + } as Response); + + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: 'https://mcp.example.com/mcp', + authorization_servers: ['https://auth.example.com/'], + }); + + const result = await detectOAuthRequirement('https://mcp.example.com/mcp'); + + expect(result.metadata).toMatchObject({ + authorization_servers: ['https://auth.example.com/'], + }); + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1); + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + 'https://mcp.example.com/mcp', + expect.objectContaining({ resourceMetadataUrl: new URL(metadataUrl) }), + ); + }); + }); + + describe('SSRF hardening of the resource_metadata hint', () => { + // A malicious MCP server can advertise a `resource_metadata=` URL that points at a + // private IP, loopback, or cloud metadata service. Blindly handing that URL to the + // SDK would let the server weaponize detection as an SSRF vector, so we validate it + // first and silently fall back to path-aware discovery on rejection. + + it('drops a hint URL whose hostname matches an SSRF target list entry', async () => { + const maliciousHint = 'http://169.254.169.254/latest/meta-data/'; + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${maliciousHint}"`, + }), + } as Response); + + mockIsSSRFTarget.mockImplementation((hostname: string) => hostname === '169.254.169.254'); + + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: 'https://mcp.example.com', + authorization_servers: ['https://auth.example.com'], + }); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + 'https://mcp.example.com', + expect.objectContaining({ resourceMetadataUrl: undefined }), + ); + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('protected-resource-metadata'); + }); + + it('drops a hint URL whose hostname resolves to a private address', async () => { + const maliciousHint = 'https://internal.local/.well-known/oauth-protected-resource'; + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${maliciousHint}"`, + }), + } as Response); + + mockResolveHostnameSSRF.mockImplementation( + async (hostname: string) => hostname === 'internal.local', + ); + + mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValueOnce( + new Error('Resource server does not implement OAuth 2.0 Protected Resource Metadata.'), + ); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + // Hint dropped → SDK called with undefined → path-aware falls through → Bearer-only. + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + 'https://mcp.example.com', + expect.objectContaining({ resourceMetadataUrl: undefined }), + ); + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); expect(result.metadata).toBeNull(); }); }); - describe('StackOverflow-like server behavior', () => { - it('should detect OAuth for servers that return 405 for HEAD and 401+Bearer for POST', async () => { - // This mimics StackOverflow's actual behavior: - // HEAD -> 405 Method Not Allowed - // POST -> 401 with WWW-Authenticate: Bearer + describe('path-aware discovery without a hint', () => { + it('uses path-aware discovery when the server returns no 401 challenge', async () => { + // HEAD and POST both return 200 — no challenge, but the server may still advertise + // `.well-known/oauth-protected-resource` (uncommon but spec-allowed). + mockFetch.mockResolvedValue({ + status: 200, + headers: new Headers(), + } as Response); + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: 'https://mcp.example.com', + authorization_servers: ['https://auth.example.com'], + }); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('protected-resource-metadata'); + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + 'https://mcp.example.com', + expect.objectContaining({ resourceMetadataUrl: undefined }), + ); + }); + }); + + describe('StackOverflow-like server behavior', () => { + it('detects OAuth for servers that return 405 for HEAD and 401+Bearer for POST', async () => { mockFetch - // HEAD returns 405 .mockResolvedValueOnce({ status: 405, headers: new Headers(), } as Response) - // POST returns 401 with Bearer .mockResolvedValueOnce({ status: 401, headers: new Headers({ 'www-authenticate': 'Bearer' }), @@ -245,7 +396,7 @@ describe('detectOAuthRequirement', () => { }); describe('error handling', () => { - it('should return no OAuth required when all checks fail', async () => { + it('returns no OAuth required when all checks fail', async () => { mockFetch.mockRejectedValue(new Error('Network error')); const result = await detectOAuthRequirement('https://unreachable.example.com'); @@ -254,7 +405,7 @@ describe('detectOAuthRequirement', () => { expect(result.method).toBe('no-metadata-found'); }); - it('should handle timeout gracefully', async () => { + it('handles timeout gracefully', async () => { mockFetch.mockImplementation( () => new Promise((_, reject) => setTimeout(() => reject(new Error('Timeout')), 100)), ); diff --git a/packages/api/src/mcp/oauth/detectOAuth.ts b/packages/api/src/mcp/oauth/detectOAuth.ts index 84d2066f4c..33391b181d 100644 --- a/packages/api/src/mcp/oauth/detectOAuth.ts +++ b/packages/api/src/mcp/oauth/detectOAuth.ts @@ -6,6 +6,8 @@ // Manual testing ensures the OAuth detection still works against real MCP servers. import { discoverOAuthProtectedResourceMetadata } from '@modelcontextprotocol/sdk/client/auth.js'; +import { isSSRFTarget, resolveHostnameSSRF } from '~/auth'; +import { probeResourceMetadataHint } from './resourceHint'; import { mcpConfig } from '../mcpConfig'; export interface OAuthDetectionResult { @@ -17,25 +19,65 @@ export interface OAuthDetectionResult { /** * Detects if an MCP server requires OAuth authentication using proactive discovery methods. * - * This function implements a comprehensive OAuth detection strategy: - * 1. Standard Protected Resource Metadata (RFC 9728) - checks /.well-known/oauth-protected-resource - * 2. 401 Challenge Method - checks WWW-Authenticate header for resource_metadata URL - * 3. Optional fallback: treat any 401/403 response as OAuth requirement (if MCP_OAUTH_ON_AUTH_ERROR=true) + * Strategy (RFC 9728 §5.1 aligned): + * 1. Probe the server for a 401 challenge and extract the `resource_metadata` URL from + * the `WWW-Authenticate` header, if any. + * 2. Call the SDK's Protected Resource Metadata discovery. When the hint is present it + * overrides the path-aware well-known endpoint, matching the behavior of Claude + * Desktop / MCP Inspector / Copilot and avoiding stale path-aware metadata. + * 3. If no metadata was found but the server advertised `Bearer`, report OAuth-required + * without metadata (legacy servers without `.well-known` still need auth). + * 4. Optional fallback: treat any 401/403 as an OAuth requirement when + * `MCP_OAUTH_ON_AUTH_ERROR=true`. * * @param serverUrl - The MCP server URL to check for OAuth requirements - * @returns Promise - OAuth requirement details */ export async function detectOAuthRequirement(serverUrl: string): Promise { - const protectedResourceResult = await checkProtectedResourceMetadata(serverUrl); - if (protectedResourceResult) return protectedResourceResult; + const hint = await probeResourceMetadataHint(serverUrl); - const challengeResult = await check401ChallengeMetadata(serverUrl); - if (challengeResult) return challengeResult; + /** + * The `resource_metadata` URL is attacker-controlled (it's echoed from the MCP + * server's own 401 challenge). Reject hints pointing at private/loopback/metadata + * addresses before the SDK fetches them, so a malicious server cannot weaponize + * detection as an SSRF vector against the LibreChat host or its internal network. + */ + const safeHintUrl = hint?.resourceMetadataUrl + ? await validateHintUrl(hint.resourceMetadataUrl) + : undefined; - const fallbackResult = await checkAuthErrorFallback(serverUrl); - if (fallbackResult) return fallbackResult; + const metadataResult = await checkProtectedResourceMetadata(serverUrl, safeHintUrl); + if (metadataResult) return metadataResult; + + if (hint?.bearerChallenge) { + return { + requiresOAuth: true, + method: '401-challenge-metadata', + metadata: null, + }; + } + + /** + * `MCP_OAUTH_ON_AUTH_ERROR` fallback: honor a 401/403 already observed by the HEAD + * probe instead of issuing a duplicate HEAD. POST-only 401/403 is intentionally + * excluded — WAF/CSRF rules commonly 403 a body-less JSON POST on endpoints that + * have nothing to do with OAuth, and those must not flip detection. A `null` probe + * means every attempt threw (transient network error); retry once via HEAD so a + * blip doesn't flip detection to "no OAuth required" for a server that needs it. + */ + if (mcpConfig.OAUTH_ON_AUTH_ERROR) { + if (hint?.headAuthChallenge) { + return { + requiresOAuth: true, + method: 'no-metadata-found', + metadata: null, + }; + } + if (hint === null) { + const fallbackResult = await checkAuthErrorFallback(serverUrl); + if (fallbackResult) return fallbackResult; + } + } - // No OAuth detected return { requiresOAuth: false, method: 'no-metadata-found', @@ -47,18 +89,26 @@ export async function detectOAuthRequirement(serverUrl: string): Promise { try { - const resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl); + const resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { + resourceMetadataUrl, + }); if (!resourceMetadata?.authorization_servers?.length) return null; return { requiresOAuth: true, - method: 'protected-resource-metadata', + method: resourceMetadataUrl ? '401-challenge-metadata' : 'protected-resource-metadata', metadata: resourceMetadata, }; } catch { @@ -67,90 +117,26 @@ async function checkProtectedResourceMetadata( } /** - * Checks for OAuth using 401 challenge with resource metadata URL or Bearer token. - * Tries HEAD first, then falls back to POST if HEAD doesn't return 401. - * Some servers (like StackOverflow) only return 401 for POST requests. + * SSRF-guards an attacker-controlled `resource_metadata` hint before the SDK follows it. + * `detectOAuthRequirement` runs without admin-scoped `allowedDomains`, so the rejection + * policy here is stricter than the handler's: any private/loopback/metadata-service + * target is dropped, regardless of origin relative to the MCP server. On rejection the + * caller continues with path-aware discovery (safe, since it targets the server itself). */ -async function check401ChallengeMetadata(serverUrl: string): Promise { - // Try HEAD first (lighter weight) - const headResult = await check401WithMethod(serverUrl, 'HEAD'); - if (headResult) return headResult; - - // Fall back to POST if HEAD didn't return 401 (some servers don't support HEAD) - const postResult = await check401WithMethod(serverUrl, 'POST'); - if (postResult) return postResult; - - return null; -} - -async function check401WithMethod( - serverUrl: string, - method: 'HEAD' | 'POST', -): Promise { +async function validateHintUrl(hintUrl: URL): Promise { try { - const fetchOptions: RequestInit = { - method, - signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT), - }; - - // POST requests need headers and body for MCP servers - if (method === 'POST') { - fetchOptions.headers = { 'Content-Type': 'application/json' }; - fetchOptions.body = JSON.stringify({}); - } - - const response = await fetch(serverUrl, fetchOptions); - - if (response.status !== 401) return null; - - const wwwAuth = response.headers.get('www-authenticate'); - const metadataUrl = wwwAuth?.match(/resource_metadata="([^"]+)"/)?.[1]; - - if (metadataUrl) { - try { - // Try to fetch resource metadata from the provided URL - const metadataResponse = await fetch(metadataUrl, { - signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT), - }); - const metadata = await metadataResponse.json(); - - if (metadata?.authorization_servers?.length) { - return { - requiresOAuth: true, - method: '401-challenge-metadata', - metadata, - }; - } - } catch { - // Metadata fetch failed, continue to Bearer check below - } - } - - /** - * If we got a 401 with WWW-Authenticate containing "Bearer" (case-insensitive), - * the server requires OAuth authentication even without discovery metadata. - * This handles "legacy" OAuth servers (like StackOverflow's MCP) that use standard - * OAuth endpoints (/authorize, /token, /register) without .well-known metadata. - */ - if (wwwAuth && /bearer/i.test(wwwAuth)) { - return { - requiresOAuth: true, - method: '401-challenge-metadata', - metadata: null, - }; - } - - return null; + if (isSSRFTarget(hintUrl.hostname)) return undefined; + if (await resolveHostnameSSRF(hintUrl.hostname)) return undefined; + return hintUrl; } catch { - return null; + // If validation itself fails (e.g. DNS lookup threw), be conservative and drop the hint. + return undefined; } } -// Fallback method: treats any auth error as OAuth requirement if configured +// Fallback: only called when probing threw. Caller already gates on `OAUTH_ON_AUTH_ERROR`. async function checkAuthErrorFallback(serverUrl: string): Promise { try { - if (!mcpConfig.OAUTH_ON_AUTH_ERROR) return null; - const response = await fetch(serverUrl, { method: 'HEAD', signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT), diff --git a/packages/api/src/mcp/oauth/handler.ts b/packages/api/src/mcp/oauth/handler.ts index b7e98fbce5..acd31a1fca 100644 --- a/packages/api/src/mcp/oauth/handler.ts +++ b/packages/api/src/mcp/oauth/handler.ts @@ -30,6 +30,7 @@ import { inferClientAuthMethod, } from './methods'; import { isSSRFTarget, resolveHostnameSSRF, isOAuthUrlAllowed } from '~/auth'; +import { probeResourceMetadataHint } from './resourceHint'; import { MCPTokenStorage } from './tokens'; import { sanitizeUrlForLogging } from '~/mcp/utils'; @@ -144,12 +145,54 @@ export class MCPOAuthHandler { const fetchFn = this.createOAuthFetch(oauthHeaders); + /** + * RFC 9728 §5.1: when the server's 401 `WWW-Authenticate` header advertises a + * `resource_metadata` URL, use that URL as the authoritative source. Path-aware + * `.well-known` discovery is a fallback for when the hint is absent — not the + * other way round — or a split deployment can serve stale/wrong metadata at the + * path-aware endpoint and strand the flow at a defunct authorization server. + * + * Reuse `fetchFn` so admin-configured `oauthHeaders` (e.g. a gateway API key + * required to reach the MCP endpoint at all) are attached to the probe — without + * them, the probe would 401 for the wrong reason and never see the real challenge. + */ + const hint = await probeResourceMetadataHint(serverUrl, fetchFn); + /** + * The hint URL is attacker-controlled (it comes from the MCP server's own 401 + * challenge). Validate it through the same SSRF/allowedDomains gate used for the + * authorization server — otherwise a malicious server could redirect discovery at + * a private IP, the metadata service, or a host the admin never intended to reach. + * On validation failure, discard the hint and fall back to path-aware discovery. + */ + let hintUrl: URL | undefined; + if (hint?.resourceMetadataUrl) { + try { + await this.validateOAuthUrl( + hint.resourceMetadataUrl.toString(), + 'resource_metadata', + allowedDomains, + ); + hintUrl = hint.resourceMetadataUrl; + logger.debug( + `[MCPOAuth] Using resource_metadata URL from WWW-Authenticate: ${sanitizeUrlForLogging(hintUrl.toString())}`, + ); + } catch (error) { + logger.warn( + `[MCPOAuth] Rejecting untrusted resource_metadata hint from ${sanitizeUrlForLogging(serverUrl)}; falling back to path-aware discovery`, + { error }, + ); + } + } + try { - // Try to discover resource metadata first logger.debug( - `[MCPOAuth] Attempting to discover protected resource metadata from ${serverUrl}`, + `[MCPOAuth] Attempting to discover protected resource metadata from ${sanitizeUrlForLogging(serverUrl)}`, + ); + resourceMetadata = await discoverOAuthProtectedResourceMetadata( + serverUrl, + { resourceMetadataUrl: hintUrl }, + fetchFn, ); - resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, {}, fetchFn); } catch (error) { logger.debug('[MCPOAuth] Resource metadata discovery failed, continuing with server URL', { error, diff --git a/packages/api/src/mcp/oauth/resourceHint.test.ts b/packages/api/src/mcp/oauth/resourceHint.test.ts new file mode 100644 index 0000000000..3ab1f214ed --- /dev/null +++ b/packages/api/src/mcp/oauth/resourceHint.test.ts @@ -0,0 +1,260 @@ +import { probeResourceMetadataHint } from './resourceHint'; + +jest.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({ + ...jest.requireActual('@modelcontextprotocol/sdk/client/auth.js'), +})); + +describe('probeResourceMetadataHint', () => { + const originalFetch = global.fetch; + const mockFetch = jest.fn() as unknown as jest.MockedFunction; + + beforeEach(() => { + jest.clearAllMocks(); + global.fetch = mockFetch; + }); + + afterAll(() => { + global.fetch = originalFetch; + }); + + it('returns the resource_metadata URL from a HEAD 401 challenge', async () => { + const hintUrl = 'https://example.com/.well-known/oauth-protected-resource'; + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${hintUrl}"`, + }), + } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toEqual({ + resourceMetadataUrl: new URL(hintUrl), + bearerChallenge: true, + headAuthChallenge: true, + }); + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(mockFetch.mock.calls[0][1]).toEqual(expect.objectContaining({ method: 'HEAD' })); + }); + + it('falls back to POST when HEAD does not return 401', async () => { + const hintUrl = 'https://example.com/.well-known/oauth-protected-resource'; + mockFetch + .mockResolvedValueOnce({ status: 405, headers: new Headers() } as Response) + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${hintUrl}"`, + }), + } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result?.resourceMetadataUrl?.toString()).toBe(hintUrl); + // POST's 401 must not set headAuthChallenge — HEAD was 405. + expect(result?.headAuthChallenge).toBe(false); + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(mockFetch.mock.calls[1][1]).toEqual( + expect.objectContaining({ + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({}), + }), + ); + }); + + it('still probes POST when HEAD returned Bearer without a hint', async () => { + // HEAD 401 with Bearer-but-no-params means the server definitely speaks OAuth, but + // some implementations surface `resource_metadata` only on POST responses. Letting + // POST run ensures the authoritative hint isn't dropped just because HEAD was first. + const hintUrl = 'https://example.com/.well-known/oauth-protected-resource'; + mockFetch + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response) + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${hintUrl}"`, + }), + } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result?.resourceMetadataUrl?.toString()).toBe(hintUrl); + expect(result?.headAuthChallenge).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('marks bearerChallenge even when no resource_metadata is advertised on either method', async () => { + mockFetch.mockResolvedValue({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer realm="api"' }), + } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toEqual({ + resourceMetadataUrl: undefined, + bearerChallenge: true, + headAuthChallenge: true, + }); + // HEAD Bearer without hint → POST runs too. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('ignores a malformed resource_metadata value without throwing', async () => { + // Defense: if the server advertises garbage in `resource_metadata=`, both the SDK + // parser and our regex fallback wrap `new URL()` in try/catch and yield `undefined`. + // Guard this behavior so a future refactor can't silently drop the safety net. + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': 'Bearer resource_metadata="not-a-url"', + }), + } as Response); + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': 'Bearer resource_metadata="not-a-url"', + }), + } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toEqual({ + resourceMetadataUrl: undefined, + bearerChallenge: true, + headAuthChallenge: true, + }); + }); + + it('extracts resource_metadata from multi-scheme challenges where Bearer is not first', async () => { + // RFC 7235 allows multiple schemes in one header. The SDK's `extractWWWAuthenticateParams` + // only parses the leading token, so a header like `Basic realm="api", Bearer resource_metadata="..."` + // would drop the authoritative hint — hence the local regex fallback. + const hintUrl = 'https://example.com/.well-known/oauth-protected-resource'; + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Basic realm="api", Bearer resource_metadata="${hintUrl}"`, + }), + } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result?.resourceMetadataUrl?.toString()).toBe(hintUrl); + expect(result?.bearerChallenge).toBe(true); + expect(result?.headAuthChallenge).toBe(true); + }); + + it('returns a no-challenge result when both probes receive clean 200s', async () => { + mockFetch.mockResolvedValue({ status: 200, headers: new Headers() } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toEqual({ + resourceMetadataUrl: undefined, + bearerChallenge: false, + headAuthChallenge: false, + }); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('returns null when the probe itself throws (e.g. network error)', async () => { + mockFetch.mockRejectedValue(new Error('Network error')); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toBeNull(); + }); + + it('uses the injected fetchFn so admin-configured oauthHeaders reach the probe', async () => { + // Simulates an MCP endpoint fronted by a gateway that requires a static API key + // header — without it, the gateway 401s before the MCP app ever sees the request, + // so the probe needs the OAuth-aware fetch wrapper to attach that header. + const customFetch = jest.fn(async () => { + return { + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response; + }) as unknown as typeof fetch; + + const result = await probeResourceMetadataHint('https://example.com/mcp', customFetch); + + expect(result).toEqual({ + resourceMetadataUrl: undefined, + bearerChallenge: true, + headAuthChallenge: true, + }); + // HEAD + POST both via customFetch (Bearer-no-hint doesn't short-circuit). + expect(customFetch).toHaveBeenCalledTimes(2); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('surfaces headAuthChallenge when a non-Bearer 401 is the only response', async () => { + // Basic-only 401 carries no OAuth hint, but callers still need to know a 401 was + // seen so the MCP_OAUTH_ON_AUTH_ERROR fallback can fire without a duplicate HEAD. + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), + } as Response); + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), + } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toEqual({ + resourceMetadataUrl: undefined, + bearerChallenge: false, + headAuthChallenge: true, + }); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('surfaces headAuthChallenge when only a 403 is observed on HEAD', async () => { + mockFetch.mockResolvedValue({ status: 403, headers: new Headers() } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toEqual({ + resourceMetadataUrl: undefined, + bearerChallenge: false, + headAuthChallenge: true, + }); + }); + + it('returns null when HEAD threw so callers can retry via the fallback', async () => { + // A transient HEAD failure followed by an uninformative POST used to leak a + // {bearerChallenge: false, headAuthChallenge: false} result, silently skipping the + // MCP_OAUTH_ON_AUTH_ERROR retry. Signal "unknown" via `null` instead. + mockFetch + .mockRejectedValueOnce(new Error('ETIMEDOUT')) + .mockResolvedValueOnce({ status: 200, headers: new Headers() } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toBeNull(); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('does not set headAuthChallenge when only POST returns 401/403 (WAF/CSRF case)', async () => { + // Classic WAF/CSRF posture: HEAD cleanly returns 200, but a body-less JSON POST + // trips a rule and gets 403. This is not an OAuth signal and must not flip the + // `MCP_OAUTH_ON_AUTH_ERROR` fallback, so `headAuthChallenge` stays false. + mockFetch + .mockResolvedValueOnce({ status: 200, headers: new Headers() } as Response) + .mockResolvedValueOnce({ status: 403, headers: new Headers() } as Response); + + const result = await probeResourceMetadataHint('https://example.com/mcp'); + + expect(result).toEqual({ + resourceMetadataUrl: undefined, + bearerChallenge: false, + headAuthChallenge: false, + }); + }); +}); diff --git a/packages/api/src/mcp/oauth/resourceHint.ts b/packages/api/src/mcp/oauth/resourceHint.ts new file mode 100644 index 0000000000..66eb66ce36 --- /dev/null +++ b/packages/api/src/mcp/oauth/resourceHint.ts @@ -0,0 +1,139 @@ +import { extractWWWAuthenticateParams } from '@modelcontextprotocol/sdk/client/auth.js'; +import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport'; +import { mcpConfig } from '../mcpConfig'; + +export interface ResourceHintProbeResult { + /** URL advertised via the `resource_metadata` parameter of a `WWW-Authenticate: Bearer` header, if any. */ + resourceMetadataUrl?: URL; + /** True when the server answered 401 with a `WWW-Authenticate: Bearer` challenge (with or without parameters). */ + bearerChallenge: boolean; + /** + * True when the *HEAD* probe specifically returned 401 or 403. Matches the semantics + * of the legacy `MCP_OAUTH_ON_AUTH_ERROR` HEAD-only fallback so the caller can skip a + * redundant HEAD. POST-only 401/403 is intentionally excluded — servers commonly 403 + * a body-less JSON POST for WAF/CSRF reasons unrelated to OAuth, and those should not + * flip the fallback. + */ + headAuthChallenge: boolean; +} + +/** + * Probes an MCP server for an OAuth 401 challenge and extracts the RFC 6750 + * `WWW-Authenticate` `resource_metadata` hint. Per RFC 9728 §5.1, clients SHOULD prefer + * that URL over path-aware well-known discovery; the value returned here is meant to be + * threaded into `discoverOAuthProtectedResourceMetadata` as `opts.resourceMetadataUrl`. + * + * Tries HEAD first (cheap). POST still runs unless HEAD already delivered the hint — + * some servers surface their Bearer challenge parameters only on POST (e.g. StackOverflow), + * so a HEAD-Bearer-without-hint is not enough to short-circuit discovery. + * + * When `fetchFn` is supplied (for example, the OAuth-aware wrapper built by the handler) + * it is used for both probes so admin-configured `oauthHeaders` are attached — a gateway + * that requires a static API key to reach the MCP endpoint would otherwise 401 us for the + * wrong reason and never surface the real Bearer challenge. + * + * @returns A `ResourceHintProbeResult` when at least one probe response was observed, or + * `null` when every attempt threw (DNS failure, timeout, etc.). Callers should treat + * `null` as "status unknown" and can choose to retry. + */ +export async function probeResourceMetadataHint( + serverUrl: string, + fetchFn: FetchLike = fetch, +): Promise { + const headResult = await probeWithMethod(serverUrl, 'HEAD', fetchFn); + /** + * Only short-circuit when HEAD already produced the authoritative hint. A Bearer + * challenge without `resource_metadata` is not enough: some servers emit the + * `resource_metadata` parameter only on POST responses, and we'd miss it by + * bailing here. + */ + if (headResult?.resourceMetadataUrl) return headResult; + + const postResult = await probeWithMethod(serverUrl, 'POST', fetchFn); + if (postResult?.resourceMetadataUrl || postResult?.bearerChallenge) { + // Carry HEAD's auth-challenge observation forward if we got one — the fallback + // decision is HEAD-only, so POST must not overwrite it back to false. + return { ...postResult, headAuthChallenge: !!headResult?.headAuthChallenge }; + } + + /** + * Invariant for callers: a non-null return means the HEAD probe actually observed + * the server. If HEAD threw (DNS, timeout, reset), signal "unknown" with `null` so + * the `MCP_OAUTH_ON_AUTH_ERROR` fallback can still retry a fresh HEAD — otherwise a + * transient HEAD failure plus a normal POST 200 would silently skip the fallback + * and misclassify an OAuth-required server as open. + */ + if (!headResult) return null; + if (postResult) return mergeProbes(headResult, postResult); + return headResult; +} + +function mergeProbes( + head: ResourceHintProbeResult, + post: ResourceHintProbeResult, +): ResourceHintProbeResult { + return { + resourceMetadataUrl: head.resourceMetadataUrl ?? post.resourceMetadataUrl, + bearerChallenge: head.bearerChallenge || post.bearerChallenge, + /** + * Only HEAD's observation feeds the fallback decision — POST-only 401/403 is too + * noisy a signal (WAF/CSRF rules routinely 403 a body-less JSON POST on endpoints + * that are not OAuth-protected at all). + */ + headAuthChallenge: head.headAuthChallenge, + }; +} + +async function probeWithMethod( + serverUrl: string, + method: 'HEAD' | 'POST', + fetchFn: FetchLike, +): Promise { + try { + const fetchOptions: RequestInit = { + method, + signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT), + }; + + if (method === 'POST') { + fetchOptions.headers = { 'Content-Type': 'application/json' }; + fetchOptions.body = JSON.stringify({}); + } + + const response = await fetchFn(serverUrl, fetchOptions); + const headAuthChallenge = + method === 'HEAD' && (response.status === 401 || response.status === 403); + + if (response.status !== 401) { + return { bearerChallenge: false, headAuthChallenge }; + } + + const wwwAuth = response.headers.get('www-authenticate'); + if (!wwwAuth) return { bearerChallenge: false, headAuthChallenge }; + + const bearerChallenge = /bearer/i.test(wwwAuth); + + /** + * The SDK's `extractWWWAuthenticateParams` checks only the *first* token of the + * header and returns `{}` for multi-scheme challenges like + * `Basic realm="api", Bearer resource_metadata="..."`. Fall back to our own regex + * across the whole header so those servers' authoritative hint isn't dropped. + */ + const sdkParsed = extractWWWAuthenticateParams(response); + const resourceMetadataUrl = sdkParsed.resourceMetadataUrl ?? extractHintFromHeader(wwwAuth); + + return { resourceMetadataUrl, bearerChallenge, headAuthChallenge }; + } catch { + return null; + } +} + +function extractHintFromHeader(header: string): URL | undefined { + const match = /resource_metadata="([^"]+)"/.exec(header); + if (!match?.[1]) return undefined; + try { + return new URL(match[1]); + } catch { + return undefined; + } +}