From 4a6a71132eafa7fe8c81dce271cbe9468daed0c2 Mon Sep 17 00:00:00 2001 From: Sergei Demin Date: Wed, 28 Oct 2026 13:11:26 +0300 Subject: [PATCH 01/10] [Feat] Support custom MCP OAuth callback paths and WWW-Authenticate discovery Some authorization servers register static clients with a callback path other than /callback. The redirectUri option now accepts any path except the root path. The local callback server listens on that path. Some MCP servers host the protected resource metadata outside the RFC 9728 well-known URLs. When the well-known lookup fails, discovery now reads the resource_metadata URL from the 401 WWW-Authenticate header. Co-Authored-By: Claude Opus 5.5 --- src/services/mcp/McpOAuthClientProvider.ts | 18 ++++--- .../__tests__/McpOAuthClientProvider.spec.ts | 29 +++++++++-- .../utils/__tests__/callbackServer.spec.ts | 17 +++++++ .../mcp/utils/__tests__/oauth.spec.ts | 48 +++++++++++++++++- src/services/mcp/utils/callbackServer.ts | 4 +- src/services/mcp/utils/oauth.ts | 49 +++++++++++++++++-- 6 files changed, 151 insertions(+), 14 deletions(-) diff --git a/src/services/mcp/McpOAuthClientProvider.ts b/src/services/mcp/McpOAuthClientProvider.ts index 1882f6d690..ee3277be4c 100644 --- a/src/services/mcp/McpOAuthClientProvider.ts +++ b/src/services/mcp/McpOAuthClientProvider.ts @@ -243,11 +243,12 @@ export class McpOAuthClientProvider implements OAuthClientProvider { private async _doStartCallbackServer(): Promise { this._closed = false - const { port: configuredPort, host } = getCallbackAddress(this._configuredRedirectUri) + const { port: configuredPort, host, path } = getCallbackAddress(this._configuredRedirectUri) const { server, port, result, cancel } = await startCallbackServer( configuredPort ?? this._port, this._state, host, + path, ) this._server = server this._port = port @@ -670,8 +671,12 @@ function resolveResourceIndicator(configured: McpOAuthResource | undefined, disc return discovered ?? null } -function getCallbackAddress(redirectUri?: string): { port?: number; host: "localhost" | "127.0.0.1" } { - if (!redirectUri) return { host: "127.0.0.1" } +function getCallbackAddress(redirectUri?: string): { + port?: number + host: "localhost" | "127.0.0.1" + path: string +} { + if (!redirectUri) return { host: "127.0.0.1", path: "/callback" } const url = new URL(redirectUri) if (url.hostname !== "localhost" && url.hostname !== "127.0.0.1") { throw new Error("MCP OAuth redirectUri must use a loopback host") @@ -679,8 +684,9 @@ function getCallbackAddress(redirectUri?: string): { port?: number; host: "local if (url.protocol !== "http:") { throw new Error("MCP OAuth redirectUri must use HTTP on a loopback host") } - if (url.pathname !== "/callback") { - throw new Error("MCP OAuth redirectUri path must be /callback") + // Authorization servers with pre-registered clients may require a specific path. + if (url.pathname === "/") { + throw new Error("MCP OAuth redirectUri must specify a callback path") } if (url.username || url.password || url.search || url.hash) { throw new Error("MCP OAuth redirectUri must not contain credentials, a query, or a fragment") @@ -689,5 +695,5 @@ function getCallbackAddress(redirectUri?: string): { port?: number; host: "local if (!Number.isInteger(port) || port < 1 || port > 65535) { throw new Error("MCP OAuth redirectUri must specify a valid port") } - return { port, host: url.hostname } + return { port, host: url.hostname, path: url.pathname } } diff --git a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts index d677e88ac1..22e59a8847 100644 --- a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts +++ b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts @@ -231,16 +231,39 @@ describe("McpOAuthClientProvider", () => { await provider.openBrowser() const openedUri = vi.mocked(vscode.env.openExternal).mock.calls[0][0].toString() expect(new URL(openedUri).searchParams.get("scope")).toBe("openid profile offline_access") - expect(startCallbackServer).toHaveBeenCalledWith(33418, expect.any(String), callbackHost) + expect(startCallbackServer).toHaveBeenCalledWith(33418, expect.any(String), callbackHost, "/callback") await provider.close() }, ) + it("should listen on the configured callback path for a static client", async () => { + setupCallbackServerMock() + const redirectUri = "http://127.0.0.1:19876/mcp/oauth/callback" + const provider = await McpOAuthClientProvider.create( + "https://example.com/mcp", + createMockSecretStorage(), + undefined, + { clientId: "registered-client", redirectUri }, + ) + + expect(provider.redirectUrl).toBe(redirectUri) + expect(await provider.clientInformation()).toMatchObject({ redirect_uris: [redirectUri] }) + + await provider.redirectToAuthorization(new URL("https://auth.example.com/authorize")) + expect(startCallbackServer).toHaveBeenCalledWith( + 19876, + expect.any(String), + "127.0.0.1", + "/mcp/oauth/callback", + ) + await provider.close() + }) + it.each([ ["https://example.com:33418/callback", "must use a loopback host"], ["http://example.com:33418/callback", "must use a loopback host"], ["https://localhost:33418/callback", "must use HTTP on a loopback host"], - ["http://localhost:33418/wrong", "path must be /callback"], + ["http://localhost:33418/", "must specify a callback path"], ["http://localhost:33418/callback?x=1", "must not contain credentials, a query, or a fragment"], ["http://localhost:33418/callback#fragment", "must not contain credentials, a query, or a fragment"], ["http://u:p@localhost:33418/callback", "must not contain credentials, a query, or a fragment"], @@ -1398,7 +1421,7 @@ describe("McpOAuthClientProvider", () => { // reads redirect_uris, so the advertised URI carries the real port and never :0. expect(startCallbackServer).toHaveBeenCalledTimes(1) // Port 0 asks the OS for an ephemeral port; the mock binds it to 12345. - expect(startCallbackServer).toHaveBeenCalledWith(0, expect.any(String), "127.0.0.1") + expect(startCallbackServer).toHaveBeenCalledWith(0, expect.any(String), "127.0.0.1", "/callback") expect(provider.redirectUrl).toBe("http://127.0.0.1:12345/callback") expect(provider.redirectUrl).not.toContain(":0/") expect(provider.clientMetadata.redirect_uris).toEqual(["http://127.0.0.1:12345/callback"]) diff --git a/src/services/mcp/utils/__tests__/callbackServer.spec.ts b/src/services/mcp/utils/__tests__/callbackServer.spec.ts index 1a3e6a5be6..6196e5e3a2 100644 --- a/src/services/mcp/utils/__tests__/callbackServer.spec.ts +++ b/src/services/mcp/utils/__tests__/callbackServer.spec.ts @@ -84,6 +84,23 @@ describe("startCallbackServer", () => { await cancellation }) + it("should resolve only on the configured callback path", async () => { + const mockServer = createMockServer() + + vi.mocked(http.createServer).mockReturnValue(mockServer as unknown as http.Server) + + const { result } = await startCallbackServer(19876, undefined, "127.0.0.1", "/mcp/oauth/callback") + const requestCall = mockServer.on.mock.calls.find((call) => call[0] === "request") + const requestHandler = requestCall ? requestCall[1] : vi.fn() + + const defaultPathRes = createMockResponse() + requestHandler({ url: "/callback?code=wrong-path", method: "GET" }, defaultPathRes) + expect(defaultPathRes.writeHead).toHaveBeenCalledWith(404) + + requestHandler({ url: "/mcp/oauth/callback?code=test-code", method: "GET" }, createMockResponse()) + expect((await result).code).toBe("test-code") + }) + it("should reject invalid state", async () => { const mockServer = createMockServer() diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index b276f60fd4..a6d321e09f 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -3,9 +3,13 @@ import { describe, it, expect, vi, beforeEach, afterAll } from "vitest" // Mock the SDK's discoverOAuthProtectedResourceMetadata vi.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ discoverOAuthProtectedResourceMetadata: vi.fn(), + extractResourceMetadataUrl: vi.fn(), })) -import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/client/auth.js" +import { + discoverOAuthProtectedResourceMetadata, + extractResourceMetadataUrl, +} from "@modelcontextprotocol/sdk/client/auth.js" import { fetchOAuthAuthServerMetadata } from "../oauth" const mockFetch = vi.fn() @@ -33,9 +37,51 @@ describe("fetchOAuthAuthServerMetadata", () => { it("returns null when discoverOAuthProtectedResourceMetadata throws", async () => { ;(discoverOAuthProtectedResourceMetadata as any).mockRejectedValue(new Error("network error")) + mockFetch.mockResolvedValueOnce({ status: 200, body: null }) const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") expect(result).toBeNull() + expect(extractResourceMetadataUrl).not.toHaveBeenCalled() + }) + + it("falls back to the resource_metadata URL from the 401 WWW-Authenticate header", async () => { + const resourceMetadataUrl = new URL("https://example.com/core/.well-known/oauth-protected-resource/mcp") + vi.mocked(discoverOAuthProtectedResourceMetadata) + .mockRejectedValueOnce( + new Error("Resource server does not implement OAuth 2.0 Protected Resource Metadata."), + ) + .mockResolvedValueOnce({ + resource: "https://example.com/core/mcp", + authorization_servers: ["https://auth.example.com"], + }) + vi.mocked(extractResourceMetadataUrl).mockReturnValueOnce(resourceMetadataUrl) + const meta = { issuer: "https://auth.example.com" } + mockFetch + .mockResolvedValueOnce({ status: 401, body: null }) + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(meta) }) + + const result = await fetchOAuthAuthServerMetadata("https://example.com/core/mcp") + + expect(mockFetch).toHaveBeenNthCalledWith( + 1, + "https://example.com/core/mcp", + expect.objectContaining({ method: "POST" }), + ) + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenLastCalledWith("https://example.com/core/mcp", { + resourceMetadataUrl, + }) + expect(result).toEqual({ authServerMeta: meta, resourceIndicator: "https://example.com/core/mcp" }) + }) + + it("returns null when the 401 response has no resource_metadata URL", async () => { + vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) + vi.mocked(extractResourceMetadataUrl).mockReturnValueOnce(undefined) + mockFetch.mockResolvedValueOnce({ status: 401, body: null }) + + const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") + + expect(result).toBeNull() + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1) }) it("constructs the RFC 8414 discovery URL correctly for an issuer with a path", async () => { diff --git a/src/services/mcp/utils/callbackServer.ts b/src/services/mcp/utils/callbackServer.ts index 34c2f3a91d..22e98d337e 100644 --- a/src/services/mcp/utils/callbackServer.ts +++ b/src/services/mcp/utils/callbackServer.ts @@ -19,12 +19,14 @@ type CallbackHost = "localhost" | "127.0.0.1" * @param port Optional port to use (defaults to random available port) * @param expectedState Optional expected state for CSRF protection * @param host Loopback host used by the OAuth redirect URI + * @param callbackPath Path of the OAuth redirect URI that receives the authorization code * @returns Promise<{server: http.Server, port: number, result: Promise}> */ export function startCallbackServer( port?: number, expectedState?: string, host: CallbackHost = "127.0.0.1", + callbackPath = "/callback", ): Promise<{ server: http.Server port: number @@ -87,7 +89,7 @@ export function startCallbackServer( const url = new URL(req.url || "", `http://localhost:${actualPort}`) const pathname = url.pathname - if (pathname === "/callback") { + if (pathname === callbackPath) { resolved = true clearTimeout(timeout) diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index e205626c00..1bc073665e 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -1,4 +1,8 @@ -import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/client/auth.js" +import { + discoverOAuthProtectedResourceMetadata, + extractResourceMetadataUrl, +} from "@modelcontextprotocol/sdk/client/auth.js" +import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/shared/auth.js" /** * Result of a successful OAuth discovery for an MCP server. @@ -34,7 +38,9 @@ export interface OAuthDiscoveryResult { * * Performs two discovery steps: * 1. RFC 9728 – fetches the Protected Resource Metadata to find the issuer URL - * and the RFC 8707 resource indicator. + * and the RFC 8707 resource indicator. When the well-known URLs fail, falls + * back to the `resource_metadata` URL from the server's 401 `WWW-Authenticate` + * header (RFC 9728 §5.1), for servers that host the metadata elsewhere. * 2. RFC 8414 §3.1 – constructs the well-known discovery URL by inserting * `/.well-known/oauth-authorization-server` *between* the host and the issuer * path (not appended after the path). @@ -52,7 +58,7 @@ export async function fetchOAuthAuthServerMetadata(serverUrl: string): Promise((_, reject) => setTimeout(() => reject(new Error("OAuth discovery timeout")), DISCOVERY_TIMEOUT_MS), ), @@ -85,3 +91,40 @@ export async function fetchOAuthAuthServerMetadata(serverUrl: string): Promise { + try { + return await discoverOAuthProtectedResourceMetadata(serverUrl) + } catch (wellKnownError) { + const resourceMetadataUrl = await probeResourceMetadataUrl(serverUrl) + if (!resourceMetadataUrl) throw wellKnownError + return discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }) + } +} + +/** + * Sends an unauthenticated MCP `initialize` request and returns the + * `resource_metadata` URL from the 401 `WWW-Authenticate` header, if any. + */ +async function probeResourceMetadataUrl(serverUrl: string): Promise { + const response = await fetch(serverUrl, { + method: "POST", + headers: { + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 0, + method: "initialize", + params: { + protocolVersion: "2025-06-18", + capabilities: {}, + clientInfo: { name: "oauth-discovery", version: "1.0.0" }, + }, + }), + signal: AbortSignal.timeout(DISCOVERY_TIMEOUT_MS), + }) + await response.body?.cancel().catch(() => {}) + return response.status === 401 ? extractResourceMetadataUrl(response) : undefined +} From b0b86afd5953a02a677818729116b2d840c59c41 Mon Sep 17 00:00:00 2001 From: Sergei Demin Date: Wed, 28 Oct 2026 23:04:04 +0300 Subject: [PATCH 02/10] [Fix] Give MCP OAuth discovery fallback its own timeout and parse resource_metadata case-insensitively Each protected resource metadata lookup now races its own timeout, so a slow well-known lookup no longer consumes the budget of the WWW-Authenticate fallback. The resource_metadata parameter of the Bearer challenge is matched case-insensitively, as RFC 9110 requires for authentication parameters. The probe test now asserts the initialize payload and MCP headers. Co-Authored-By: Claude Opus 5.5 --- .../mcp/utils/__tests__/oauth.spec.ts | 95 +++++++++++++++---- src/services/mcp/utils/oauth.ts | 51 +++++++--- 2 files changed, 112 insertions(+), 34 deletions(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index a6d321e09f..2a09037bfb 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -3,19 +3,23 @@ import { describe, it, expect, vi, beforeEach, afterAll } from "vitest" // Mock the SDK's discoverOAuthProtectedResourceMetadata vi.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ discoverOAuthProtectedResourceMetadata: vi.fn(), - extractResourceMetadataUrl: vi.fn(), })) -import { - discoverOAuthProtectedResourceMetadata, - extractResourceMetadataUrl, -} from "@modelcontextprotocol/sdk/client/auth.js" +import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/client/auth.js" import { fetchOAuthAuthServerMetadata } from "../oauth" const mockFetch = vi.fn() const originalFetch = global.fetch global.fetch = mockFetch +const unauthorized = (wwwAuthenticate?: string) => + new Response(null, { + status: 401, + headers: wwwAuthenticate ? { "WWW-Authenticate": wwwAuthenticate } : {}, + }) + +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) + describe("fetchOAuthAuthServerMetadata", () => { beforeEach(() => { vi.clearAllMocks() @@ -37,15 +41,18 @@ describe("fetchOAuthAuthServerMetadata", () => { it("returns null when discoverOAuthProtectedResourceMetadata throws", async () => { ;(discoverOAuthProtectedResourceMetadata as any).mockRejectedValue(new Error("network error")) - mockFetch.mockResolvedValueOnce({ status: 200, body: null }) + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") expect(result).toBeNull() - expect(extractResourceMetadataUrl).not.toHaveBeenCalled() + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1) }) - it("falls back to the resource_metadata URL from the 401 WWW-Authenticate header", async () => { - const resourceMetadataUrl = new URL("https://example.com/core/.well-known/oauth-protected-resource/mcp") + it.each([ + 'Bearer resource_metadata="https://example.com/core/.well-known/oauth-protected-resource/mcp"', + 'Bearer RESOURCE_METADATA="https://example.com/core/.well-known/oauth-protected-resource/mcp"', + 'bearer error="invalid_token", resource_metadata = "https://example.com/core/.well-known/oauth-protected-resource/mcp"', + ])("falls back to the resource_metadata URL from the 401 header: %s", async (wwwAuthenticate) => { vi.mocked(discoverOAuthProtectedResourceMetadata) .mockRejectedValueOnce( new Error("Resource server does not implement OAuth 2.0 Protected Resource Metadata."), @@ -54,29 +61,34 @@ describe("fetchOAuthAuthServerMetadata", () => { resource: "https://example.com/core/mcp", authorization_servers: ["https://auth.example.com"], }) - vi.mocked(extractResourceMetadataUrl).mockReturnValueOnce(resourceMetadataUrl) const meta = { issuer: "https://auth.example.com" } mockFetch - .mockResolvedValueOnce({ status: 401, body: null }) + .mockResolvedValueOnce(unauthorized(wwwAuthenticate)) .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(meta) }) const result = await fetchOAuthAuthServerMetadata("https://example.com/core/mcp") - expect(mockFetch).toHaveBeenNthCalledWith( - 1, - "https://example.com/core/mcp", - expect.objectContaining({ method: "POST" }), - ) + const [probeUrl, probeInit] = mockFetch.mock.calls[0] as [string, RequestInit] + expect(probeUrl).toBe("https://example.com/core/mcp") + expect(probeInit).toMatchObject({ + method: "POST", + headers: { "Content-Type": "application/json", Accept: "application/json, text/event-stream" }, + }) + expect(JSON.parse(probeInit.body as string)).toMatchObject({ jsonrpc: "2.0", method: "initialize" }) expect(discoverOAuthProtectedResourceMetadata).toHaveBeenLastCalledWith("https://example.com/core/mcp", { - resourceMetadataUrl, + resourceMetadataUrl: new URL("https://example.com/core/.well-known/oauth-protected-resource/mcp"), }) expect(result).toEqual({ authServerMeta: meta, resourceIndicator: "https://example.com/core/mcp" }) }) - it("returns null when the 401 response has no resource_metadata URL", async () => { + it.each([ + ["no WWW-Authenticate header", undefined], + ["no resource_metadata parameter", 'Bearer realm="mcp"'], + ["a non-Bearer challenge", 'Basic resource_metadata="https://example.com/metadata"'], + ["an invalid URL", 'Bearer resource_metadata="not a url"'], + ])("returns null when the 401 response has %s", async (_case, wwwAuthenticate) => { vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) - vi.mocked(extractResourceMetadataUrl).mockReturnValueOnce(undefined) - mockFetch.mockResolvedValueOnce({ status: 401, body: null }) + mockFetch.mockResolvedValueOnce(unauthorized(wwwAuthenticate)) const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") @@ -84,6 +96,49 @@ describe("fetchOAuthAuthServerMetadata", () => { expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1) }) + it("gives the fallback its own timeout budget after a slow well-known lookup", async () => { + vi.useFakeTimers() + try { + vi.mocked(discoverOAuthProtectedResourceMetadata) + .mockImplementationOnce(() => delay(4_500).then(() => Promise.reject(new Error("not found")))) + .mockImplementationOnce(() => + delay(1_000).then(() => ({ + resource: "https://example.com/mcp", + authorization_servers: ["https://auth.example.com"], + })), + ) + const meta = { issuer: "https://auth.example.com" } + mockFetch + .mockResolvedValueOnce(unauthorized('Bearer resource_metadata="https://example.com/metadata"')) + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(meta) }) + + const result = fetchOAuthAuthServerMetadata("https://example.com/mcp") + await vi.advanceTimersByTimeAsync(6_000) + + await expect(result).resolves.toEqual({ + authServerMeta: meta, + resourceIndicator: "https://example.com/mcp", + }) + } finally { + vi.useRealTimers() + } + }) + + it("returns null when a protected resource metadata lookup exceeds its timeout", async () => { + vi.useFakeTimers() + try { + vi.mocked(discoverOAuthProtectedResourceMetadata).mockImplementationOnce(() => new Promise(() => {})) + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) + + const result = fetchOAuthAuthServerMetadata("https://example.com/mcp") + await vi.advanceTimersByTimeAsync(5_000) + + await expect(result).resolves.toBeNull() + } finally { + vi.useRealTimers() + } + }) + it("constructs the RFC 8414 discovery URL correctly for an issuer with a path", async () => { ;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({ resource: "https://mcp.kapa.ai/", diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index 1bc073665e..a89f313136 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -1,7 +1,4 @@ -import { - discoverOAuthProtectedResourceMetadata, - extractResourceMetadataUrl, -} from "@modelcontextprotocol/sdk/client/auth.js" +import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/client/auth.js" import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/shared/auth.js" /** @@ -56,13 +53,7 @@ export async function fetchOAuthAuthServerMetadata(serverUrl: string): Promise((_, reject) => - setTimeout(() => reject(new Error("OAuth discovery timeout")), DISCOVERY_TIMEOUT_MS), - ), - ]) + const resourceMeta = await discoverProtectedResourceMetadata(serverUrl) const authServers = resourceMeta.authorization_servers if (!authServers?.length) return null @@ -94,11 +85,29 @@ export async function fetchOAuthAuthServerMetadata(serverUrl: string): Promise { try { - return await discoverOAuthProtectedResourceMetadata(serverUrl) + return await withDiscoveryTimeout(discoverOAuthProtectedResourceMetadata(serverUrl)) } catch (wellKnownError) { const resourceMetadataUrl = await probeResourceMetadataUrl(serverUrl) if (!resourceMetadataUrl) throw wellKnownError - return discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }) + return withDiscoveryTimeout(discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl })) + } +} + +/** + * The SDK does not accept an AbortSignal, so each lookup races its own timeout. + * A slow well-known lookup therefore cannot consume the fallback's budget. + */ +async function withDiscoveryTimeout(promise: Promise): Promise { + let timer: ReturnType | undefined + try { + return await Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error("OAuth discovery timeout")), DISCOVERY_TIMEOUT_MS) + }), + ]) + } finally { + clearTimeout(timer) } } @@ -126,5 +135,19 @@ async function probeResourceMetadataUrl(serverUrl: string): Promise {}) - return response.status === 401 ? extractResourceMetadataUrl(response) : undefined + return response.status === 401 ? extractResourceMetadataUrl(response.headers.get("WWW-Authenticate")) : undefined +} + +/** + * Reads the `resource_metadata` parameter of a Bearer challenge. Unlike the SDK + * helper, matches the parameter name case-insensitively (RFC 9110 §11.2). + */ +function extractResourceMetadataUrl(header: string | null): URL | undefined { + const match = header && /^bearer\s+(?:.*[\s,])?resource_metadata\s*=\s*"([^"]*)"/i.exec(header) + if (!match) return undefined + try { + return new URL(match[1]) + } catch { + return undefined + } } From 834134536343faed0c0900edf1100df41e4d50c6 Mon Sep 17 00:00:00 2001 From: Sergei Demin Date: Wed, 28 Oct 2026 23:25:43 +0300 Subject: [PATCH 03/10] [Fix] Validate MCP protected resource metadata and abort timed-out lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protected resource metadata whose resource does not match the server URL is now rejected on both the well-known and the WWW-Authenticate fallback paths (RFC 9728 §3.3), as the SDK does in its own auth flow. The resource is sent as the RFC 8707 resource indicator, so a mismatched value must not be used. Each lookup timeout now aborts the SDK's requests through its fetch hook instead of leaving them running. Co-Authored-By: Claude Opus 5.5 --- .../__tests__/McpOAuthClientProvider.spec.ts | 4 +- .../mcp/utils/__tests__/oauth.spec.ts | 53 ++++++++++++++++--- src/services/mcp/utils/oauth.ts | 47 +++++++++++++--- 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts index 22e59a8847..43e9e1ad18 100644 --- a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts +++ b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts @@ -586,7 +586,7 @@ describe("McpOAuthClientProvider", () => { const secretStorage = createMockSecretStorage() const serverUrl = "https://example.com/mcp" const clientId = "registered-client" - const discoveredResource = "https://discovered.example.com/" + const discoveredResource = "https://example.com/mcp" await secretStorage.saveOAuthData( serverUrl, { @@ -637,7 +637,7 @@ describe("McpOAuthClientProvider", () => { }, clientId, ) - mockOAuthDiscovery("https://discovered.example.com/") + mockOAuthDiscovery("https://example.com/mcp") const provider = await McpOAuthClientProvider.create(serverUrl, secretStorage, undefined, { clientId, resource: explicitResource, diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index 2a09037bfb..347e0de4b7 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -75,9 +75,11 @@ describe("fetchOAuthAuthServerMetadata", () => { headers: { "Content-Type": "application/json", Accept: "application/json, text/event-stream" }, }) expect(JSON.parse(probeInit.body as string)).toMatchObject({ jsonrpc: "2.0", method: "initialize" }) - expect(discoverOAuthProtectedResourceMetadata).toHaveBeenLastCalledWith("https://example.com/core/mcp", { - resourceMetadataUrl: new URL("https://example.com/core/.well-known/oauth-protected-resource/mcp"), - }) + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenLastCalledWith( + "https://example.com/core/mcp", + { resourceMetadataUrl: new URL("https://example.com/core/.well-known/oauth-protected-resource/mcp") }, + expect.any(Function), + ) expect(result).toEqual({ authServerMeta: meta, resourceIndicator: "https://example.com/core/mcp" }) }) @@ -124,21 +126,60 @@ describe("fetchOAuthAuthServerMetadata", () => { } }) - it("returns null when a protected resource metadata lookup exceeds its timeout", async () => { + it("aborts a protected resource metadata lookup that exceeds its timeout", async () => { vi.useFakeTimers() try { - vi.mocked(discoverOAuthProtectedResourceMetadata).mockImplementationOnce(() => new Promise(() => {})) - mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) + vi.mocked(discoverOAuthProtectedResourceMetadata).mockImplementationOnce(async (url, _opts, fetchFn) => { + await fetchFn!(url) + throw new Error("unreachable") + }) + let lookupSignal: AbortSignal | undefined + mockFetch + .mockImplementationOnce((_url: string, init: RequestInit) => { + lookupSignal = init.signal ?? undefined + return new Promise(() => {}) + }) + .mockResolvedValueOnce(new Response(null, { status: 200 })) const result = fetchOAuthAuthServerMetadata("https://example.com/mcp") await vi.advanceTimersByTimeAsync(5_000) await expect(result).resolves.toBeNull() + expect(lookupSignal?.aborted).toBe(true) } finally { vi.useRealTimers() } }) + it("rejects well-known metadata whose resource does not match the server URL", async () => { + vi.mocked(discoverOAuthProtectedResourceMetadata).mockResolvedValueOnce({ + resource: "https://other.example.com/mcp", + authorization_servers: ["https://auth.example.com"], + }) + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) + + const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") + + expect(result).toBeNull() + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + + it("rejects fallback metadata whose resource does not match the server URL", async () => { + vi.mocked(discoverOAuthProtectedResourceMetadata) + .mockRejectedValueOnce(new Error("not found")) + .mockResolvedValueOnce({ + resource: "https://example.com/other", + authorization_servers: ["https://auth.example.com"], + }) + mockFetch.mockResolvedValueOnce(unauthorized('Bearer resource_metadata="https://example.com/metadata"')) + + const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") + + expect(result).toBeNull() + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(2) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + it("constructs the RFC 8414 discovery URL correctly for an issuer with a path", async () => { ;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({ resource: "https://mcp.kapa.ai/", diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index a89f313136..26485e547e 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -1,4 +1,5 @@ import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/client/auth.js" +import { checkResourceAllowed } from "@modelcontextprotocol/sdk/shared/auth-utils.js" import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/shared/auth.js" /** @@ -85,25 +86,57 @@ export async function fetchOAuthAuthServerMetadata(serverUrl: string): Promise { try { - return await withDiscoveryTimeout(discoverOAuthProtectedResourceMetadata(serverUrl)) + return assertResourceMatches( + serverUrl, + await withDiscoveryTimeout((fetchFn) => + discoverOAuthProtectedResourceMetadata(serverUrl, undefined, fetchFn), + ), + ) } catch (wellKnownError) { const resourceMetadataUrl = await probeResourceMetadataUrl(serverUrl) if (!resourceMetadataUrl) throw wellKnownError - return withDiscoveryTimeout(discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl })) + return assertResourceMatches( + serverUrl, + await withDiscoveryTimeout((fetchFn) => + discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn), + ), + ) } } /** - * The SDK does not accept an AbortSignal, so each lookup races its own timeout. - * A slow well-known lookup therefore cannot consume the fallback's budget. + * RFC 9728 §3.3: the metadata must describe the resource it was fetched for. + * Its `resource` is later sent as the RFC 8707 resource indicator. */ -async function withDiscoveryTimeout(promise: Promise): Promise { +function assertResourceMatches( + serverUrl: string, + metadata: OAuthProtectedResourceMetadata, +): OAuthProtectedResourceMetadata { + if ( + metadata.resource && + !checkResourceAllowed({ requestedResource: serverUrl, configuredResource: metadata.resource }) + ) { + throw new Error(`Protected resource metadata for ${metadata.resource} does not match ${serverUrl}`) + } + return metadata +} + +/** + * Gives each lookup its own timeout, so a slow well-known lookup cannot consume + * the fallback's budget. The timeout aborts the lookup's requests through the + * SDK fetch hook. + */ +async function withDiscoveryTimeout(lookup: (fetchFn: typeof fetch) => Promise): Promise { + const controller = new AbortController() let timer: ReturnType | undefined try { return await Promise.race([ - promise, + lookup((input, init) => fetch(input, { ...init, signal: controller.signal })), new Promise((_, reject) => { - timer = setTimeout(() => reject(new Error("OAuth discovery timeout")), DISCOVERY_TIMEOUT_MS) + timer = setTimeout(() => { + controller.abort() + reject(new Error("OAuth discovery timeout")) + }, DISCOVERY_TIMEOUT_MS) }), ]) } finally { From c665b26ca1d81b90e797a20f27798f5f7ebcdcfc Mon Sep 17 00:00:00 2001 From: Sergei Demin Date: Thu, 29 Oct 2026 09:41:21 +0300 Subject: [PATCH 04/10] [Fix] Keep MCP OAuth when the published resource does not match the server URL A protected resource whose URL did not pass checkResourceAllowed made discovery return null, so the server looked like it had no OAuth. A trailing slash alone was enough: https://x/mcp/ for https://x/mcp. A trailing slash on the published resource is now ignored. Any other mismatch drops only the RFC 8707 resource indicator, and the server still uses OAuth. Co-Authored-By: Claude Opus 5.5 --- .../mcp/utils/__tests__/oauth.spec.ts | 48 ++++++++++++------- src/services/mcp/utils/oauth.ts | 42 ++++++++-------- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index 347e0de4b7..67a524d2ea 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, afterAll } from "vitest" +import { describe, it, expect, vi, beforeEach, afterEach, afterAll } from "vitest" // Mock the SDK's discoverOAuthProtectedResourceMetadata vi.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ @@ -25,6 +25,10 @@ describe("fetchOAuthAuthServerMetadata", () => { vi.clearAllMocks() }) + afterEach(() => { + vi.restoreAllMocks() + }) + afterAll(() => { global.fetch = originalFetch }) @@ -151,33 +155,45 @@ describe("fetchOAuthAuthServerMetadata", () => { } }) - it("rejects well-known metadata whose resource does not match the server URL", async () => { - vi.mocked(discoverOAuthProtectedResourceMetadata).mockResolvedValueOnce({ - resource: "https://other.example.com/mcp", - authorization_servers: ["https://auth.example.com"], - }) - mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) + it.each([ + ["the server URL with a trailing slash", "https://example.com/mcp/", "https://example.com/mcp/"], + ["a same-origin ancestor", "https://example.com/", "https://example.com/"], + ["a child path", "https://example.com/mcp/child", null], + ["another origin", "https://other.example.com/mcp", null], + ["an invalid URL", "not a url", null], + ])( + "keeps OAuth when the metadata resource is %s and sends only a matching indicator", + async (_case, resource, expected) => { + vi.spyOn(console, "warn").mockImplementation(() => {}) + vi.mocked(discoverOAuthProtectedResourceMetadata).mockResolvedValueOnce({ + resource, + authorization_servers: ["https://auth.example.com"], + }) + const meta = { issuer: "https://auth.example.com" } + mockFetch.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(meta) }) - const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") + const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") - expect(result).toBeNull() - expect(mockFetch).toHaveBeenCalledTimes(1) - }) + expect(result).toEqual({ authServerMeta: meta, resourceIndicator: expected }) + }, + ) - it("rejects fallback metadata whose resource does not match the server URL", async () => { + it("keeps OAuth but drops a mismatched resource indicator from fallback metadata", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}) vi.mocked(discoverOAuthProtectedResourceMetadata) .mockRejectedValueOnce(new Error("not found")) .mockResolvedValueOnce({ resource: "https://example.com/other", authorization_servers: ["https://auth.example.com"], }) - mockFetch.mockResolvedValueOnce(unauthorized('Bearer resource_metadata="https://example.com/metadata"')) + const meta = { issuer: "https://auth.example.com" } + mockFetch + .mockResolvedValueOnce(unauthorized('Bearer resource_metadata="https://example.com/metadata"')) + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(meta) }) const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") - expect(result).toBeNull() - expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(2) - expect(mockFetch).toHaveBeenCalledTimes(1) + expect(result).toEqual({ authServerMeta: meta, resourceIndicator: null }) }) it("constructs the RFC 8414 discovery URL correctly for an issuer with a path", async () => { diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index 26485e547e..463e804cdc 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -62,7 +62,9 @@ export async function fetchOAuthAuthServerMetadata(serverUrl: string): Promise { try { - return assertResourceMatches( - serverUrl, - await withDiscoveryTimeout((fetchFn) => - discoverOAuthProtectedResourceMetadata(serverUrl, undefined, fetchFn), - ), + return await withDiscoveryTimeout((fetchFn) => + discoverOAuthProtectedResourceMetadata(serverUrl, undefined, fetchFn), ) } catch (wellKnownError) { const resourceMetadataUrl = await probeResourceMetadataUrl(serverUrl) if (!resourceMetadataUrl) throw wellKnownError - return assertResourceMatches( - serverUrl, - await withDiscoveryTimeout((fetchFn) => - discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn), - ), + return withDiscoveryTimeout((fetchFn) => + discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn), ) } } /** * RFC 9728 §3.3: the metadata must describe the resource it was fetched for. - * Its `resource` is later sent as the RFC 8707 resource indicator. + * Like the SDK, accepts the server URL or a same-origin ancestor of it. A + * trailing slash on the published resource is ignored, so `https://x/mcp/` + * matches `https://x/mcp`. A mismatched resource is not sent as the RFC 8707 + * resource indicator, but the server still uses OAuth. */ -function assertResourceMatches( - serverUrl: string, - metadata: OAuthProtectedResourceMetadata, -): OAuthProtectedResourceMetadata { - if ( - metadata.resource && - !checkResourceAllowed({ requestedResource: serverUrl, configuredResource: metadata.resource }) - ) { - throw new Error(`Protected resource metadata for ${metadata.resource} does not match ${serverUrl}`) +function resourceMatches(serverUrl: string, resource: string): boolean { + try { + const configuredResource = new URL(resource) + configuredResource.pathname = configuredResource.pathname.replace(/\/+$/, "") + if (checkResourceAllowed({ requestedResource: serverUrl, configuredResource })) return true + } catch { + // An invalid resource URL is treated as a mismatch. } - return metadata + console.warn(`[MCP OAuth] Ignoring resource indicator ${resource}: it does not match ${serverUrl}`) + return false } /** From bd92158eeb1ffc33aeccc4f0154be2b8526bfddb Mon Sep 17 00:00:00 2001 From: Sergei Demin Date: Thu, 29 Oct 2026 10:29:05 +0300 Subject: [PATCH 05/10] [Fix] Find the Bearer resource_metadata after other WWW-Authenticate challenges A 401 header such as `Basic realm="mcp", Bearer resource_metadata="..."` lists the Bearer challenge second. The parser only matched a header that starts with Bearer, so the fallback was skipped. It now also matches a Bearer challenge that follows a comma. Adds a test for a resource with multiple trailing slashes. Co-Authored-By: Claude Opus 5.5 --- src/services/mcp/utils/__tests__/oauth.spec.ts | 2 ++ src/services/mcp/utils/oauth.ts | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index 67a524d2ea..d51b8d688c 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -56,6 +56,7 @@ describe("fetchOAuthAuthServerMetadata", () => { 'Bearer resource_metadata="https://example.com/core/.well-known/oauth-protected-resource/mcp"', 'Bearer RESOURCE_METADATA="https://example.com/core/.well-known/oauth-protected-resource/mcp"', 'bearer error="invalid_token", resource_metadata = "https://example.com/core/.well-known/oauth-protected-resource/mcp"', + 'Basic realm="mcp", Bearer resource_metadata="https://example.com/core/.well-known/oauth-protected-resource/mcp"', ])("falls back to the resource_metadata URL from the 401 header: %s", async (wwwAuthenticate) => { vi.mocked(discoverOAuthProtectedResourceMetadata) .mockRejectedValueOnce( @@ -157,6 +158,7 @@ describe("fetchOAuthAuthServerMetadata", () => { it.each([ ["the server URL with a trailing slash", "https://example.com/mcp/", "https://example.com/mcp/"], + ["the server URL with multiple trailing slashes", "https://example.com/mcp//", "https://example.com/mcp//"], ["a same-origin ancestor", "https://example.com/", "https://example.com/"], ["a child path", "https://example.com/mcp/child", null], ["another origin", "https://other.example.com/mcp", null], diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index 463e804cdc..ebeb1767cb 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -171,10 +171,11 @@ async function probeResourceMetadataUrl(serverUrl: string): Promise Date: Thu, 29 Oct 2026 10:43:13 +0300 Subject: [PATCH 06/10] [Fix] Read resource_metadata only from the Bearer challenge's own parameters The parser could skip past the Bearer challenge into a later challenge. For `Bearer realm="mcp", Basic resource_metadata="..."` it returned the Basic URL. After Bearer it now skips only auth-params, so a later challenge stops the match. Co-Authored-By: Claude Opus 5.5 --- src/services/mcp/utils/__tests__/oauth.spec.ts | 4 ++++ src/services/mcp/utils/oauth.ts | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index d51b8d688c..617bcca283 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -92,6 +92,10 @@ describe("fetchOAuthAuthServerMetadata", () => { ["no WWW-Authenticate header", undefined], ["no resource_metadata parameter", 'Bearer realm="mcp"'], ["a non-Bearer challenge", 'Basic resource_metadata="https://example.com/metadata"'], + [ + "resource_metadata only in a later non-Bearer challenge", + 'Bearer realm="mcp", Basic resource_metadata="https://other.example/metadata"', + ], ["an invalid URL", 'Bearer resource_metadata="not a url"'], ])("returns null when the 401 response has %s", async (_case, wwwAuthenticate) => { vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index ebeb1767cb..610ac73810 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -175,7 +175,13 @@ async function probeResourceMetadataUrl(serverUrl: string): Promise Date: Thu, 29 Oct 2026 10:56:28 +0300 Subject: [PATCH 07/10] [Fix] Parse WWW-Authenticate without a backtracking regular expression CodeQL flagged the resource_metadata regular expression: a crafted 401 header could make it backtrack exponentially. The parser now splits the header on commas outside quoted strings and matches each item against a single auth-param pattern, tracking the challenge it belongs to. Behavior is unchanged. Adds the CodeQL example header as a test case. Co-Authored-By: Claude Opus 5.5 --- .../mcp/utils/__tests__/oauth.spec.ts | 1 + src/services/mcp/utils/oauth.ts | 34 +++++++++++-------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index 617bcca283..43e1f2294b 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -96,6 +96,7 @@ describe("fetchOAuthAuthServerMetadata", () => { "resource_metadata only in a later non-Bearer challenge", 'Bearer realm="mcp", Basic resource_metadata="https://other.example/metadata"', ], + ["a header that backtracks a naive parser", ",bearer\t!=" + "\t,!=".repeat(10_000)], ["an invalid URL", 'Bearer resource_metadata="not a url"'], ])("returns null when the 401 response has %s", async (_case, wwwAuthenticate) => { vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index 610ac73810..090f8fa69a 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -169,23 +169,29 @@ async function probeResourceMetadataUrl(serverUrl: string): Promise Date: Thu, 29 Oct 2026 11:17:15 +0300 Subject: [PATCH 08/10] [Fix] Handle escaped quotes and invalid URLs in WWW-Authenticate parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A quoted auth-param can contain an escaped quote or a comma (RFC 9110 §5.6.4), which broke the split before a later resource_metadata. The header is now split by a loop that tracks quoted strings and escapes. An invalid resource_metadata URL no longer stops parsing, so a later Bearer challenge with a valid URL is still used. Co-Authored-By: Claude Opus 5.5 --- .../mcp/utils/__tests__/oauth.spec.ts | 2 ++ src/services/mcp/utils/oauth.ts | 34 ++++++++++++++----- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index 43e1f2294b..1026c65027 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -57,6 +57,8 @@ describe("fetchOAuthAuthServerMetadata", () => { 'Bearer RESOURCE_METADATA="https://example.com/core/.well-known/oauth-protected-resource/mcp"', 'bearer error="invalid_token", resource_metadata = "https://example.com/core/.well-known/oauth-protected-resource/mcp"', 'Basic realm="mcp", Bearer resource_metadata="https://example.com/core/.well-known/oauth-protected-resource/mcp"', + 'Bearer error_description="bad \\"token\\", retry", resource_metadata="https://example.com/core/.well-known/oauth-protected-resource/mcp"', + 'Bearer resource_metadata="not a url", Bearer resource_metadata="https://example.com/core/.well-known/oauth-protected-resource/mcp"', ])("falls back to the resource_metadata URL from the 401 header: %s", async (wwwAuthenticate) => { vi.mocked(discoverOAuthProtectedResourceMetadata) .mockRejectedValueOnce( diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index 090f8fa69a..b714257bd4 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -170,7 +170,7 @@ async function probeResourceMetadataUrl(serverUrl: string): Promise Date: Thu, 29 Oct 2026 11:38:57 +0300 Subject: [PATCH 09/10] [Fix] Restrict header-supplied MCP OAuth metadata to the server origin The WWW-Authenticate fallback fetched any resource_metadata URL, so a server could direct the client to a loopback, private, or other host. It now fetches only metadata on the MCP server's own origin. Servers with cross-origin metadata keep the behavior they had before the fallback existed. A well-known lookup that timed out may still be running, so it no longer starts the fallback. Adds tests for a failed probe, a failed or timed-out fallback lookup, no fallback after a timed-out first lookup, and rejected destinations. Co-Authored-By: Claude Opus 5.5 --- .../mcp/utils/__tests__/oauth.spec.ts | 75 +++++++++++++++---- src/services/mcp/utils/oauth.ts | 14 +++- 2 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index 1026c65027..e9669e2dd5 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -100,6 +100,8 @@ describe("fetchOAuthAuthServerMetadata", () => { ], ["a header that backtracks a naive parser", ",bearer\t!=" + "\t,!=".repeat(10_000)], ["an invalid URL", 'Bearer resource_metadata="not a url"'], + ["a URL on another origin", 'Bearer resource_metadata="https://other.example.com/metadata"'], + ["a loopback URL", 'Bearer resource_metadata="http://127.0.0.1:8080/admin"'], ])("returns null when the 401 response has %s", async (_case, wwwAuthenticate) => { vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) mockFetch.mockResolvedValueOnce(unauthorized(wwwAuthenticate)) @@ -138,26 +140,73 @@ describe("fetchOAuthAuthServerMetadata", () => { } }) - it("aborts a protected resource metadata lookup that exceeds its timeout", async () => { + it("returns null when the probe request fails", async () => { + vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) + mockFetch.mockRejectedValueOnce(new Error("connection refused")) + + const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") + + expect(result).toBeNull() + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1) + }) + + it("returns null when the fallback lookup fails", async () => { + vi.mocked(discoverOAuthProtectedResourceMetadata) + .mockRejectedValueOnce(new Error("not found")) + .mockRejectedValueOnce(new Error("invalid metadata")) + mockFetch.mockResolvedValueOnce(unauthorized('Bearer resource_metadata="https://example.com/metadata"')) + + const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") + + expect(result).toBeNull() + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(2) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + + /** Makes the next SDK lookup fetch through its hook and never settle; returns the request signal. */ + const hangNextLookup = () => { + const signal: { current?: AbortSignal } = {} + vi.mocked(discoverOAuthProtectedResourceMetadata).mockImplementationOnce(async (url, _opts, fetchFn) => { + await fetchFn!(url) + throw new Error("unreachable") + }) + mockFetch.mockImplementationOnce((_url: string, init: RequestInit) => { + signal.current = init.signal ?? undefined + return new Promise(() => {}) + }) + return signal + } + + it("aborts a timed-out well-known lookup and does not start the fallback", async () => { vi.useFakeTimers() try { - vi.mocked(discoverOAuthProtectedResourceMetadata).mockImplementationOnce(async (url, _opts, fetchFn) => { - await fetchFn!(url) - throw new Error("unreachable") - }) - let lookupSignal: AbortSignal | undefined - mockFetch - .mockImplementationOnce((_url: string, init: RequestInit) => { - lookupSignal = init.signal ?? undefined - return new Promise(() => {}) - }) - .mockResolvedValueOnce(new Response(null, { status: 200 })) + const lookupSignal = hangNextLookup() + + const result = fetchOAuthAuthServerMetadata("https://example.com/mcp") + await vi.advanceTimersByTimeAsync(5_000) + + await expect(result).resolves.toBeNull() + expect(lookupSignal.current?.aborted).toBe(true) + expect(mockFetch).toHaveBeenCalledTimes(1) + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it("aborts a timed-out fallback lookup", async () => { + vi.useFakeTimers() + try { + vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) + mockFetch.mockResolvedValueOnce(unauthorized('Bearer resource_metadata="https://example.com/metadata"')) + const lookupSignal = hangNextLookup() const result = fetchOAuthAuthServerMetadata("https://example.com/mcp") await vi.advanceTimersByTimeAsync(5_000) await expect(result).resolves.toBeNull() - expect(lookupSignal?.aborted).toBe(true) + expect(lookupSignal.current?.aborted).toBe(true) + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(2) } finally { vi.useRealTimers() } diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index b714257bd4..de0d2e750a 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -92,8 +92,12 @@ async function discoverProtectedResourceMetadata(serverUrl: string): Promise discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn), ) @@ -119,6 +123,12 @@ function resourceMatches(serverUrl: string, resource: string): boolean { return false } +class DiscoveryTimeoutError extends Error { + constructor() { + super("OAuth discovery timeout") + } +} + /** * Gives each lookup its own timeout, so a slow well-known lookup cannot consume * the fallback's budget. The timeout aborts the lookup's requests through the @@ -133,7 +143,7 @@ async function withDiscoveryTimeout(lookup: (fetchFn: typeof fetch) => Promis new Promise((_, reject) => { timer = setTimeout(() => { controller.abort() - reject(new Error("OAuth discovery timeout")) + reject(new DiscoveryTimeoutError()) }, DISCOVERY_TIMEOUT_MS) }), ]) From 6756e2f3b84106c3b0f81d03d1847fc543d420e1 Mon Sep 17 00:00:00 2001 From: Sergei Demin Date: Thu, 29 Oct 2026 12:02:02 +0300 Subject: [PATCH 10/10] [Fix] Do not follow redirects in the MCP OAuth WWW-Authenticate fallback The same-origin check covered only the resource_metadata URL. A 3xx response could still lead the initialize probe or the metadata fetch to a loopback, private, or other host. Both requests now use redirect: "manual", so a redirect fails the fallback. Well-known lookups are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../mcp/utils/__tests__/oauth.spec.ts | 32 ++++++++++++++----- src/services/mcp/utils/oauth.ts | 15 ++++++--- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/services/mcp/utils/__tests__/oauth.spec.ts b/src/services/mcp/utils/__tests__/oauth.spec.ts index e9669e2dd5..7f422410f5 100644 --- a/src/services/mcp/utils/__tests__/oauth.spec.ts +++ b/src/services/mcp/utils/__tests__/oauth.spec.ts @@ -79,6 +79,7 @@ describe("fetchOAuthAuthServerMetadata", () => { expect(probeUrl).toBe("https://example.com/core/mcp") expect(probeInit).toMatchObject({ method: "POST", + redirect: "manual", headers: { "Content-Type": "application/json", Accept: "application/json, text/event-stream" }, }) expect(JSON.parse(probeInit.body as string)).toMatchObject({ jsonrpc: "2.0", method: "initialize" }) @@ -163,30 +164,44 @@ describe("fetchOAuthAuthServerMetadata", () => { expect(mockFetch).toHaveBeenCalledTimes(1) }) - /** Makes the next SDK lookup fetch through its hook and never settle; returns the request signal. */ + it("returns null when the probe is redirected", async () => { + vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) + mockFetch.mockResolvedValueOnce( + new Response(null, { status: 307, headers: { Location: "http://127.0.0.1:8080/mcp" } }), + ) + + const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp") + + expect(result).toBeNull() + expect(mockFetch).toHaveBeenCalledTimes(1) + expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1) + }) + + /** Makes the next SDK lookup fetch through its hook and never settle; returns the request init. */ const hangNextLookup = () => { - const signal: { current?: AbortSignal } = {} + const request: { init?: RequestInit } = {} vi.mocked(discoverOAuthProtectedResourceMetadata).mockImplementationOnce(async (url, _opts, fetchFn) => { await fetchFn!(url) throw new Error("unreachable") }) mockFetch.mockImplementationOnce((_url: string, init: RequestInit) => { - signal.current = init.signal ?? undefined + request.init = init return new Promise(() => {}) }) - return signal + return request } it("aborts a timed-out well-known lookup and does not start the fallback", async () => { vi.useFakeTimers() try { - const lookupSignal = hangNextLookup() + const lookup = hangNextLookup() const result = fetchOAuthAuthServerMetadata("https://example.com/mcp") await vi.advanceTimersByTimeAsync(5_000) await expect(result).resolves.toBeNull() - expect(lookupSignal.current?.aborted).toBe(true) + expect(lookup.init?.signal?.aborted).toBe(true) + expect(lookup.init?.redirect).toBeUndefined() expect(mockFetch).toHaveBeenCalledTimes(1) expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(1) } finally { @@ -199,13 +214,14 @@ describe("fetchOAuthAuthServerMetadata", () => { try { vi.mocked(discoverOAuthProtectedResourceMetadata).mockRejectedValueOnce(new Error("not found")) mockFetch.mockResolvedValueOnce(unauthorized('Bearer resource_metadata="https://example.com/metadata"')) - const lookupSignal = hangNextLookup() + const lookup = hangNextLookup() const result = fetchOAuthAuthServerMetadata("https://example.com/mcp") await vi.advanceTimersByTimeAsync(5_000) await expect(result).resolves.toBeNull() - expect(lookupSignal.current?.aborted).toBe(true) + expect(lookup.init?.signal?.aborted).toBe(true) + expect(lookup.init?.redirect).toBe("manual") expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalledTimes(2) } finally { vi.useRealTimers() diff --git a/src/services/mcp/utils/oauth.ts b/src/services/mcp/utils/oauth.ts index de0d2e750a..3feb9290e4 100644 --- a/src/services/mcp/utils/oauth.ts +++ b/src/services/mcp/utils/oauth.ts @@ -98,8 +98,10 @@ async function discoverProtectedResourceMetadata(serverUrl: string): Promise - discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn), + // A redirect could still lead off-origin, so the metadata fetch does not follow one. + return withDiscoveryTimeout( + (fetchFn) => discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn), + { redirect: "manual" }, ) } } @@ -134,12 +136,15 @@ class DiscoveryTimeoutError extends Error { * the fallback's budget. The timeout aborts the lookup's requests through the * SDK fetch hook. */ -async function withDiscoveryTimeout(lookup: (fetchFn: typeof fetch) => Promise): Promise { +async function withDiscoveryTimeout( + lookup: (fetchFn: typeof fetch) => Promise, + requestInit?: RequestInit, +): Promise { const controller = new AbortController() let timer: ReturnType | undefined try { return await Promise.race([ - lookup((input, init) => fetch(input, { ...init, signal: controller.signal })), + lookup((input, init) => fetch(input, { ...init, ...requestInit, signal: controller.signal })), new Promise((_, reject) => { timer = setTimeout(() => { controller.abort() @@ -173,6 +178,8 @@ async function probeResourceMetadataUrl(serverUrl: string): Promise {})