mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-03 22:32:42 +00:00
🔐 fix: Prefer WWW-Authenticate resource_metadata Hint for MCP OAuth (#12763)
* 📦 chore: Bump @modelcontextprotocol/sdk to v1.29.0 * ♻️ refactor: Extract WWW-Authenticate Probe Helper for MCP OAuth * 🔐 fix: Prefer WWW-Authenticate resource_metadata Hint for MCP OAuth Per RFC 9728 §5.1, the `resource_metadata=<url>` parameter in a 401 `WWW-Authenticate: Bearer` challenge is the authoritative protected-resource metadata source. Path-aware `.well-known` discovery was winning over the hint, so split deployments that serve valid-but-wrong metadata at the path-aware endpoint stranded OAuth at defunct authorization servers. Threads the hint through `discoverOAuthProtectedResourceMetadata` via `opts.resourceMetadataUrl` in both startup detection and the OAuth handler, matching the behavior of Claude Desktop, the MCP Inspector, OpenAI tooling, and Microsoft Copilot Studio. Fixes #12761. * 🧵 fix: Thread OAuth-Aware fetchFn Through Resource-Metadata Probe Without this, admin-configured `oauthHeaders` (e.g. a gateway API key that fronts the MCP endpoint) were stripped from the probe, causing the gateway to 401 for the wrong reason and masking the real `WWW-Authenticate` hint. The helper now accepts a FetchLike and defaults to global fetch, so the startup detection path is unchanged while the handler passes its OAuth- aware wrapper through. * 🧹 refactor: Address MCP OAuth Probe Review Findings - Thread `fetchFn` through `probeResourceMetadataHint` so admin-configured `oauthHeaders` reach the probe (a gateway API key that fronts the MCP endpoint would otherwise 401 us for the wrong reason and hide the real Bearer challenge). - Skip the redundant HEAD request in `checkAuthErrorFallback` when the probe already observed a 401/403; fall back to a fresh HEAD only when every probe attempt threw (transient network error). - Narrow the oauth barrel: drop `export * from './resourceHint'` so the helper stays an internal module. - Add `scope` extraction coverage (`Bearer scope="read write"`) and a 403-only observation path; isolate `MCP_OAUTH_ON_AUTH_ERROR=true` in a dedicated suite so precise-outcome tests aren't muddied by the safety net. * ✅ fix: Use Zod Schema in MCP Reconnection-Storm Test Tool MCP SDK 1.28 tightened `McpServer.tool()` to require Zod schemas instead of plain JSON-Schema objects. Swap the `{ message: { type: 'string' } }` shape for `z.string()` so the fixture server spins up under SDK 1.29. * 🛡️ fix: Harden MCP OAuth resource_metadata Hint Against SSRF The `resource_metadata` URL is echoed from an untrusted MCP server, so handing it straight to the SDK lets a malicious server redirect discovery at private IPs, the cloud metadata service, or any host the admin did not intend to reach. Caught by the Copilot review on #12763. - `handler.ts`: run the hint through the same `validateOAuthUrl` / `allowedDomains` gate that already guards the authorization-server URL; drop it and fall back to path-aware discovery on rejection. - `detectOAuth.ts`: no admin-scoped allowedDomains here, so apply a strict `isSSRFTarget` + DNS resolution check and silently discard any hint pointing at a private/loopback/metadata address. - Tests cover both the hostname-list and DNS-resolution rejection paths and assert the SDK falls back to path-aware discovery unharmed. * 🧪 test: Mock ~/auth in fallback Suite for Consistency Matches the main `detectOAuth.test.ts` mock so the SSRF guards added in the previous commit don't touch the real `~/auth` module at test time. * 🔍 fix: Scope OAuth Fallback to HEAD + Parse Multi-Scheme WWW-Authenticate Two codex findings on #12763: - **P1**: the merged `authChallenge` flag was letting POST-only 401/403 flip the `MCP_OAUTH_ON_AUTH_ERROR` fallback, misclassifying WAF/CSRF-hardened endpoints (HEAD 200 + POST 403) as OAuth-required. Rename to `headAuthChallenge` and derive it only from the HEAD probe, matching the legacy fallback's HEAD-only semantics. Add a regression test. - **P2**: the SDK's `extractWWWAuthenticateParams` only inspects the first scheme token, so multi-scheme headers like `Basic realm="api", Bearer resource_metadata="..."` silently dropped the authoritative Bearer hint. Fall back to a regex across the full header when the SDK returns nothing but Bearer is present. Add a regression test covering the multi-scheme case. * 🧽 refactor: Tighten MCP OAuth Probe Semantics Addresses the second external review pass plus codex P2: - Merge the two stacked JSDoc blocks on `probeResourceMetadataHint` into one with a proper `@returns` section. - Only short-circuit HEAD when it delivered the `resource_metadata` hint itself — a Bearer-without-params HEAD now lets POST run, since some servers surface their hint only on POST and we were missing it. - Drop the unused `scope` field from `ResourceHintProbeResult`; no caller read it, and YAGNI beats a reserved field. - Remove the redundant `OAUTH_ON_AUTH_ERROR` guard inside `checkAuthErrorFallback` — the only call site already gates on it. - Codex P2: signal "HEAD status unknown" via `null` when the HEAD probe threw and POST returned non-auth. Previously that combination leaked a `{headAuthChallenge: false}` result and silently skipped the fallback's retry HEAD, which could misclassify OAuth-required servers after a transient HEAD failure. Regression tests cover every path: Bearer-no-hint-on-HEAD + hint-on-POST, multi-scheme `Basic + Bearer` headers, HEAD-threw + POST-200 retry, and the WAF/CSRF-only POST 403 case. * 🪥 polish: Tighten Probe Null-Guard Ordering + Add Malformed-Hint Test Two NITs from the follow-up review: - Move `bearerChallenge` computation after the `!wwwAuth` guard so the variable is only derived when it can be meaningfully `true`. The early-return path is now a clean unconditional exit. - Add a regression test that asserts `Bearer resource_metadata="not-a-url"` yields `resourceMetadataUrl: undefined` without throwing, locking in the try/catch safety net in `extractHintFromHeader` and the SDK parser alike.
This commit is contained in:
Danny Avila
GitHub
parent
2427edef90
commit
f5c4deac28
11 changed files with 1084 additions and 164 deletions
|
|
@ -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",
|
||||
|
|
|
|||
10
package-lock.json
generated
10
package-lock.json
generated
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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<typeof startAuthorization>;
|
||||
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<TestServer> {
|
|||
|
||||
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<string, string>).message ?? '';
|
||||
return { content: [{ type: 'text', text: msg }] };
|
||||
});
|
||||
|
|
|
|||
147
packages/api/src/mcp/oauth/detectOAuth.fallback.test.ts
Normal file
147
packages/api/src/mcp/oauth/detectOAuth.fallback.test.ts
Normal file
|
|
@ -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<typeof fetch>;
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<typeof isSSRFTarget>;
|
||||
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/<path>` 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)),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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<OAuthDetectionResult> - OAuth requirement details
|
||||
*/
|
||||
export async function detectOAuthRequirement(serverUrl: string): Promise<OAuthDetectionResult> {
|
||||
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<OAuthDe
|
|||
// ------------------------ Private helper functions for OAuth detection -------------------------//
|
||||
////////////////////////////////////////////////////////////////////////////////////////////////////
|
||||
|
||||
// Checks for OAuth using standard protected resource metadata (RFC 9728)
|
||||
/**
|
||||
* Fetches RFC 9728 Protected Resource Metadata. When `resourceMetadataUrl` is provided
|
||||
* (extracted from the server's `WWW-Authenticate` 401 challenge), the SDK fetches that
|
||||
* URL directly; otherwise it falls back to path-aware `/.well-known/oauth-protected-resource`
|
||||
* discovery. The `method` reflects which source actually produced the metadata.
|
||||
*/
|
||||
async function checkProtectedResourceMetadata(
|
||||
serverUrl: string,
|
||||
resourceMetadataUrl?: URL,
|
||||
): Promise<OAuthDetectionResult | null> {
|
||||
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<OAuthDetectionResult | null> {
|
||||
// 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<OAuthDetectionResult | null> {
|
||||
async function validateHintUrl(hintUrl: URL): Promise<URL | undefined> {
|
||||
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<OAuthDetectionResult | null> {
|
||||
try {
|
||||
if (!mcpConfig.OAUTH_ON_AUTH_ERROR) return null;
|
||||
|
||||
const response = await fetch(serverUrl, {
|
||||
method: 'HEAD',
|
||||
signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT),
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
260
packages/api/src/mcp/oauth/resourceHint.test.ts
Normal file
260
packages/api/src/mcp/oauth/resourceHint.test.ts
Normal file
|
|
@ -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<typeof fetch>;
|
||||
|
||||
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,
|
||||
});
|
||||
});
|
||||
});
|
||||
139
packages/api/src/mcp/oauth/resourceHint.ts
Normal file
139
packages/api/src/mcp/oauth/resourceHint.ts
Normal file
|
|
@ -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<ResourceHintProbeResult | null> {
|
||||
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<ResourceHintProbeResult | null> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue