From f8a46726b7b6956410278c0a54522b8a52c9e79d Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 24 Aug 2026 19:48:40 -0400 Subject: [PATCH] fix(auth): pass the challenge's resource_metadata URL to SDK auth() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An RFC 9728 `WWW-Authenticate: Bearer resource_metadata="…"` challenge was parsed and then dropped by the normalized `AuthChallenge`, so every challenge-driven `mcpAuth()` call ran discovery without it and probed locations derived from the MCP server URL instead. OAuth failed outright against any server whose metadata document lives off the well-known path. The SDK handles this correctly inside its own transports, but the Inspector never reaches that path — it intercepts the 401 and drives `auth()` itself to capture the authorization URL and run the step-up scope union. Carry the advertised URL on `AuthChallenge` as a string (it crosses the web client's remote-backend boundary as JSON), convert it at the OAuth boundary, and pass it to the standard challenge leg, the forced step-up leg, and CIMD pre-registration — which runs before `auth()` and did its own discovery. A malformed value is ignored rather than surfaced, matching the SDK's parser. The callback leg needs nothing: SDK `auth()` persists the URL in its discovery state. Adds an `oauth.resourceMetadataPath` test-server mode that moves the document off the well-known path entirely, so the integration test proves discovery genuinely fails without the fix. Closes #2071 Signed-off-by: cliffhall --- AGENTS.md | 14 ++- README.md | 16 +++ .../web/src/test/core/auth/challenge.test.ts | 53 +++++++++ clients/web/src/test/core/auth/cimd.test.ts | 56 ++++++++- .../src/test/core/mcp/oauthManager.test.ts | 92 +++++++++++++++ .../oauth-resource-metadata-challenge.test.ts | 106 ++++++++++++++++++ core/auth/challenge.ts | 41 +++++++ core/auth/cimd.ts | 8 ++ core/mcp/oauthManager.ts | 10 ++ .../oauth-custom-resource-metadata-http.json | 19 ++++ test-servers/src/composable-test-server.ts | 13 +++ test-servers/src/load-config.ts | 11 ++ test-servers/src/test-server-fixtures.ts | 8 ++ test-servers/src/test-server-oauth.ts | 28 ++++- 14 files changed, 469 insertions(+), 6 deletions(-) create mode 100644 clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts create mode 100644 test-servers/configs/oauth-custom-resource-metadata-http.json diff --git a/AGENTS.md b/AGENTS.md index 67cd1596..f2416823 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,19 @@ v2/main/ ├── core/ # Shared core code (no package.json — consumed via the `@inspector/core` vite alias) │ ├── auth/ # OAuth: providers, discovery, OAuthStorage + persist backends; │ │ # mid-session recovery (challenge.ts WWW-Authenticate -│ │ # parsing, scopes.ts SEP-2350 scope union, oauthUx.ts +│ │ # parsing — including the RFC 9728 +│ │ # `resource_metadata` URL, carried on +│ │ # AuthChallenge as a STRING because the +│ │ # web client's challenge crosses the +│ │ # remote-backend boundary as JSON, and +│ │ # converted to a URL at the OAuth +│ │ # boundary by challengeResourceMetadataUrl +│ │ # (malformed values ignored, matching the +│ │ # SDK's own parser) so discovery targets +│ │ # the advertised document rather than a +│ │ # location derived from the MCP server +│ │ # URL — #2071; +│ │ # scopes.ts SEP-2350 scope union, oauthUx.ts │ │ # shared copy, mcpAuth.ts force-reauthorization, │ │ # issuerBinding.ts SEP-2352 callback-leg failure │ │ # classification — separates a recoverable diff --git a/README.md b/README.md index 49528454..702e5d49 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | `nullable-fields-http.json` | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) | | `rfc6570-templates-http.json` | Resources tab: RFC 6570 resource-template expansion | [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) | | `advertised-extensions-http.json` | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) | +| `oauth-custom-resource-metadata-http.json` **(legacy era)** | OAuth discovery driven by the challenge's `resource_metadata` | [#2071](https://github.com/modelcontextprotocol/inspector/issues/2071) | | `logging-{legacy,modern}-http.json` | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) | | `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) | | `tasks-{legacy,modern}-http.json` | Tasks, both eras | [#1631](https://github.com/modelcontextprotocol/inspector/issues/1631) | @@ -316,6 +317,21 @@ Requiredness is a property of the **expression**, not the variable: RFC 6570 dro This is the debugging knob for a server legitimately changing tool registration based on what the client advertises. Legacy stateful leg only — the modern per-request leg has no persistent `oninitialized`. +#### OAuth `resource_metadata` at a non-default path + +`oauth-custom-resource-metadata-http.json` is an OAuth-protected server (combined AS + resource, DCR enabled) that serves its RFC 9728 protected-resource metadata document **only** from `/custom/protected-resource`, and advertises it on every 401: + +```http +HTTP/1.1 401 Unauthorized +WWW-Authenticate: Bearer resource_metadata="http://127.0.0.1:8082/custom/protected-resource" +``` + +The default `/.well-known/oauth-protected-resource` route is deliberately left unserved, so a client that ignores the advertised URL cannot discover the document at all. Plain streamable-HTTP — connect with the **default (legacy)** protocol era. + +Add the server, click **Connect**, and watch the Inspector's first protected-resource metadata request in the Network tab: it must go to `/custom/protected-resource`. On the broken build the challenge's `resource_metadata` was parsed and then dropped before the SDK's `auth()` ever saw it ([#2071](https://github.com/modelcontextprotocol/inspector/issues/2071)), so discovery probed locations derived from the MCP server URL, 404'd, and authorization failed for any server that puts the document somewhere other than the well-known path. + +The value now rides the normalized `AuthChallenge` as a string — it has to be serializable, because the web client's challenge crosses the remote-backend boundary as JSON — and is converted to a `URL` at the OAuth boundary, where it is handed to `auth()` as `resourceMetadataUrl` and to the CIMD pre-registration probe, which runs *before* `auth()` and would otherwise do its own default-location discovery. A malformed value is ignored rather than surfaced, matching the SDK's own `WWW-Authenticate` parser: discovery falls back to the default locations instead of failing the whole authorization on a bad header. The callback leg needs nothing extra — SDK `auth()` persists the URL in its discovery state, so it survives both the web full-page redirect and the CLI/TUI loopback callback. + #### Logging, both eras `logging-legacy-http.json` and `logging-modern-http.json` both serve `logging: true` plus a `send_notification` tool that emits a `notifications/message` at a chosen level. The legacy one is a plain streamable-HTTP server; the modern one sets `transport.modern: true`. diff --git a/clients/web/src/test/core/auth/challenge.test.ts b/clients/web/src/test/core/auth/challenge.test.ts index ed252576..08c7a68a 100644 --- a/clients/web/src/test/core/auth/challenge.test.ts +++ b/clients/web/src/test/core/auth/challenge.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from "vitest"; import { AuthChallengeError, + challengeResourceMetadataUrl, AuthRecoveryRequiredError, findNestedAuthError, isAuthChallengeError, @@ -286,6 +287,58 @@ describe("parseAuthChallengeFromError", () => { }); }); +describe("resource_metadata (RFC 9728) preservation — #2071", () => { + const METADATA_URL = "http://127.0.0.1:3001/custom/protected-resource"; + const HEADER = `Bearer resource_metadata="${METADATA_URL}"`; + + it("keeps the advertised URL when parsing a response challenge", () => { + const response = new Response(null, { + status: 401, + headers: { "WWW-Authenticate": HEADER }, + }); + + expect(parseAuthChallengeFromResponse(response)).toEqual({ + reason: "token_expired", + resourceMetadataUrl: METADATA_URL, + raw: { httpStatus: 401, wwwAuthenticate: HEADER }, + }); + }); + + it("keeps the advertised URL when parsing an error challenge", () => { + expect( + parseAuthChallengeFromError({ status: 401, wwwAuthenticate: HEADER }), + ).toMatchObject({ resourceMetadataUrl: METADATA_URL }); + }); + + it("omits the field entirely when the challenge does not advertise one", () => { + const response = new Response(null, { + status: 401, + headers: { "WWW-Authenticate": "Bearer" }, + }); + + expect(parseAuthChallengeFromResponse(response)).not.toHaveProperty( + "resourceMetadataUrl", + ); + }); + + it("converts the stored string to a URL at the OAuth boundary", () => { + expect( + challengeResourceMetadataUrl({ resourceMetadataUrl: METADATA_URL })?.href, + ).toBe(METADATA_URL); + }); + + it.each([ + ["absent", undefined], + ["empty", ""], + ["whitespace", " "], + ["not a URL", "not-a-url"], + ])("ignores a %s resource_metadata value", (_label, value) => { + expect( + challengeResourceMetadataUrl({ resourceMetadataUrl: value }), + ).toBeUndefined(); + }); +}); + describe("isAuthChallengeError", () => { it("detects AuthChallengeError instances", () => { const err = new AuthChallengeError({ reason: "token_expired" }, 401); diff --git a/clients/web/src/test/core/auth/cimd.test.ts b/clients/web/src/test/core/auth/cimd.test.ts index afb7ac3a..caf2e30e 100644 --- a/clients/web/src/test/core/auth/cimd.test.ts +++ b/clients/web/src/test/core/auth/cimd.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; import type { OAuthStorage } from "@inspector/core/auth/storage.js"; import { BaseOAuthClientProvider } from "@inspector/core/auth/providers.js"; import { ensureCimdClientRegistration } from "@inspector/core/auth/cimd.js"; @@ -31,6 +31,10 @@ describe("ensureCimdClientRegistration", () => { } as unknown as OAuthStorage; }); + afterEach(() => { + vi.unstubAllGlobals(); + }); + it("pre-registers URL-based client id when AS supports CIMD", async () => { const fetchFn = vi.fn(async (input: RequestInfo | URL) => { const url = String(input); @@ -97,6 +101,56 @@ describe("ensureCimdClientRegistration", () => { expect(storage.saveClientInformation).not.toHaveBeenCalled(); }); + it("discovers protected-resource metadata at the challenge-advertised URL (#2071)", async () => { + const metadataUrl = new URL( + "http://127.0.0.1:9999/custom/protected-resource", + ); + // `ensureCimdClientRegistration` runs protected-resource discovery through + // the SDK's default fetch, so the advertised URL is observable only on the + // global — the AS-metadata leg is the one that takes `fetchFn`. + const globalFetch = vi.fn(async (input: RequestInfo | URL) => + String(input) === metadataUrl.href + ? new Response( + JSON.stringify({ + resource: SERVER_URL, + authorization_servers: ["http://127.0.0.1:9999"], + }), + ) + : new Response(null, { status: 404 }), + ); + vi.stubGlobal("fetch", globalFetch); + + const fetchFn = vi.fn( + async () => + new Response( + JSON.stringify({ + issuer: "http://127.0.0.1:9999", + authorization_endpoint: "http://127.0.0.1:9999/oauth/authorize", + token_endpoint: "http://127.0.0.1:9999/oauth/token", + response_types_supported: ["code"], + client_id_metadata_document_supported: true, + }), + ), + ); + + await ensureCimdClientRegistration({ + serverUrl: SERVER_URL, + provider: createProvider(storage), + fetchFn, + resourceMetadataUrl: metadataUrl, + }); + + expect(globalFetch).toHaveBeenCalled(); + for (const [input] of globalFetch.mock.calls) { + expect(String(input)).toBe(metadataUrl.href); + } + expect(storage.saveClientInformation).toHaveBeenCalledWith( + SERVER_URL, + { client_id: METADATA_URL }, + { registrationKind: "cimd" }, + ); + }); + it("no-ops when client information is already stored", async () => { storage.getClientInformation = vi.fn(async () => ({ client_id: "existing-client", diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 1fbdf8e9..3d5fc320 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -1729,6 +1729,98 @@ describe("OAuthManager", () => { }); }); + describe("resource_metadata forwarding (RFC 9728) — #2071", () => { + const METADATA_URL = "http://127.0.0.1:3001/custom/protected-resource"; + + it("forwards the advertised metadata URL to auth() as a URL", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const manager = new OAuthManager(createMockParams()); + + await manager.handleAuthChallenge({ + reason: "token_expired", + resourceMetadataUrl: METADATA_URL, + }); + + expect(mockedMcpAuth).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + resourceMetadataUrl: new URL(METADATA_URL), + }), + ); + }); + + it("leaves the metadata URL undefined when the challenge advertises none", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const manager = new OAuthManager(createMockParams()); + + await manager.handleAuthChallenge({ reason: "token_expired" }); + + expect(mockedMcpAuth).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ resourceMetadataUrl: undefined }), + ); + }); + + it("ignores a malformed metadata URL rather than failing authorization", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const manager = new OAuthManager(createMockParams()); + + const outcome = await manager.handleAuthChallenge({ + reason: "token_expired", + resourceMetadataUrl: "not-a-url", + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(mockedMcpAuth).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ resourceMetadataUrl: undefined }), + ); + }); + + it("forwards it on the forced step-up reauthorization leg too", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + const noScopeTokens = { + access_token: "a", + token_type: "Bearer", + scope: "", + }; + const midScopeTokens = { + access_token: "b", + token_type: "Bearer", + scope: "newscope", + }; + const grantedTokens = { + access_token: "d", + token_type: "Bearer", + scope: "granted:scope", + }; + storageOf(params) + .getTokens.mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(noScopeTokens) + .mockResolvedValueOnce(noScopeTokens) + .mockResolvedValueOnce(midScopeTokens) + .mockResolvedValueOnce(grantedTokens); + const manager = new OAuthManager(params); + + await manager.handleAuthChallenge({ + reason: "insufficient_scope", + resourceMetadataUrl: METADATA_URL, + }); + + expect(mockedMcpAuth).toHaveBeenCalledTimes(2); + expect(mockedMcpAuth).toHaveBeenNthCalledWith( + 2, + expect.anything(), + expect.objectContaining({ + forceReauthorization: true, + resourceMetadataUrl: new URL(METADATA_URL), + }), + ); + }); + }); + describe("handleAuthChallenge (additional branch coverage)", () => { it("resolves via the second satisfaction check inside the mutex", async () => { const params = createMockParams(); diff --git a/clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts b/clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts new file mode 100644 index 00000000..909ac187 --- /dev/null +++ b/clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts @@ -0,0 +1,106 @@ +/** + * RFC 9728 `resource_metadata` end-to-end, against a real OAuth test server + * whose protected-resource metadata document lives at a non-default path and + * whose well-known locations deliberately 404 (#2071). + * + * The unit tests prove the URL is parsed and handed to `mcpAuth()`. This file + * proves the two halves that only a real server can: that the challenge the + * server actually emits parses into `AuthChallenge.resourceMetadataUrl`, and + * that SDK discovery genuinely cannot find the document without it — so the + * plumbing is load-bearing rather than merely present. + */ + +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { fileURLToPath } from "node:url"; +import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/client"; +import { + TestServerHttp, + getDefaultServerConfig, + createOAuthTestServerConfig, + loadConfig, + resolveConfig, + waitForOAuthWellKnown, +} from "@modelcontextprotocol/inspector-test-server"; +import { + challengeResourceMetadataUrl, + parseAuthChallengeFromResponse, +} from "@inspector/core/auth/challenge.js"; + +const METADATA_PATH = "/custom/protected-resource"; + +describe("OAuth challenge resource_metadata (RFC 9728)", () => { + let mcpServer: TestServerHttp | null = null; + let serverUrl = ""; + + beforeAll(async () => { + mcpServer = new TestServerHttp({ + ...getDefaultServerConfig(), + serverType: "streamable-http" as const, + ...createOAuthTestServerConfig({ + requireAuth: true, + resourceMetadataPath: METADATA_PATH, + }), + }); + const port = await mcpServer.start(); + serverUrl = `http://localhost:${port}`; + await waitForOAuthWellKnown(serverUrl); + }, 30_000); + + afterAll(async () => { + await mcpServer?.stop(); + mcpServer = null; + }, 30_000); + + it("advertises the non-default metadata URL on the 401 challenge", async () => { + const response = await fetch(`${serverUrl}/mcp`, { method: "POST" }); + + expect(response.status).toBe(401); + const challenge = parseAuthChallengeFromResponse(response); + expect(challenge?.resourceMetadataUrl).toBe(`${serverUrl}${METADATA_PATH}`); + expect(challengeResourceMetadataUrl(challenge!)?.href).toBe( + `${serverUrl}${METADATA_PATH}`, + ); + }); + + it("serves the metadata document only from the advertised path", async () => { + const advertised = await fetch(`${serverUrl}${METADATA_PATH}`); + expect(advertised.status).toBe(200); + await expect(advertised.json()).resolves.toMatchObject({ + authorization_servers: [serverUrl], + }); + + const wellKnown = await fetch( + `${serverUrl}/.well-known/oauth-protected-resource`, + ); + expect(wellKnown.status).toBe(404); + }); + + it("SDK discovery resolves with the advertised URL and fails without it", async () => { + await expect( + discoverOAuthProtectedResourceMetadata(serverUrl, { + resourceMetadataUrl: new URL(`${serverUrl}${METADATA_PATH}`), + }), + ).resolves.toMatchObject({ authorization_servers: [serverUrl] }); + + // The pre-fix behavior: discovery derived from the MCP server URL alone. + await expect( + discoverOAuthProtectedResourceMetadata(serverUrl), + ).rejects.toThrow(); + }); + + it("keeps the showcase config valid and carrying the custom path", () => { + const configPath = fileURLToPath( + new URL( + "../../../../../../test-servers/configs/oauth-custom-resource-metadata-http.json", + import.meta.url, + ), + ); + + // loadConfig validates as it parses, so this is the regression test for + // the checked-in showcase config as much as for the new field. + expect(resolveConfig(loadConfig(configPath)).oauth).toMatchObject({ + enabled: true, + resourceMetadataPath: METADATA_PATH, + }); + }); +}); diff --git a/core/auth/challenge.ts b/core/auth/challenge.ts index 09d9ba2e..78092a9b 100644 --- a/core/auth/challenge.ts +++ b/core/auth/challenge.ts @@ -26,6 +26,17 @@ export interface AuthChallenge { /** Resource authorization server audience when known. */ audience?: string; + /** + * RFC 9728 `resource_metadata` advertised by the `WWW-Authenticate` challenge. + * + * Kept as a string so the challenge stays serializable — it crosses the web + * client's remote-backend boundary as JSON. Converted to a `URL` (and + * discarded if malformed) at the OAuth boundary, where it is handed to the + * SDK as `resourceMetadataUrl` so protected-resource discovery targets the + * advertised document instead of a location derived from the MCP server URL. + */ + resourceMetadataUrl?: string; + /** Optional human-readable detail from server or SDK (for UI, not parsing). */ message?: string; @@ -193,6 +204,30 @@ export function parseWwwAuthenticateBearer( }; } +/** + * Convert a challenge's advertised RFC 9728 `resource_metadata` value into the + * `URL` the SDK's `auth()` takes as `resourceMetadataUrl`. + * + * A malformed value is ignored rather than surfaced, matching the SDK's own + * `WWW-Authenticate` parser (`extractWWWAuthenticateParams`), which drops a + * value `new URL()` rejects. Discovery then falls back to the default + * RFC 9728 locations derived from the MCP server URL — the pre-existing + * behavior — instead of failing the whole authorization on a bad header. + */ +export function challengeResourceMetadataUrl( + challenge: Pick, +): URL | undefined { + const value = challenge.resourceMetadataUrl?.trim(); + if (!value) { + return undefined; + } + try { + return new URL(value); + } catch { + return undefined; + } +} + /** Split an OAuth scope string into individual scopes (space-separated). */ export function parseScopeString(scope: string | undefined): string[] { if (!scope?.trim()) { @@ -275,6 +310,9 @@ export function parseAuthChallengeFromResponse( return { reason: reasonFromHttpResponse(status, bearer), ...(requiredScopes.length > 0 ? { requiredScopes } : {}), + ...(bearer.resourceMetadata + ? { resourceMetadataUrl: bearer.resourceMetadata } + : {}), ...(bearer.errorDescription ? { message: bearer.errorDescription } : {}), ...(context ? { context } : {}), raw: { @@ -337,6 +375,9 @@ export function parseAuthChallengeFromError( return { reason: reasonFromHttpResponse(status, bearer), ...(requiredScopes.length > 0 ? { requiredScopes } : {}), + ...(bearer.resourceMetadata + ? { resourceMetadataUrl: bearer.resourceMetadata } + : {}), ...(context ? { context } : {}), raw: { httpStatus: status, diff --git a/core/auth/cimd.ts b/core/auth/cimd.ts index 76f125cb..1b140d2b 100644 --- a/core/auth/cimd.ts +++ b/core/auth/cimd.ts @@ -14,11 +14,18 @@ import type { BaseOAuthClientProvider } from "./providers.js"; * accepts an already-stored `client_id` (including `http://` URLs used by local * dev/test metadata servers). Production CIMD metadata documents should still * use HTTPS per SEP-991. + * + * `resourceMetadataUrl` is the RFC 9728 document advertised by the + * `WWW-Authenticate` challenge, when the caller has one. It matters here and + * not only in `auth()`: this runs *before* SDK `auth()`, so without it the + * pre-registration probe would do its own default-location discovery and miss + * a document served from a non-default path (#2071). */ export async function ensureCimdClientRegistration(params: { serverUrl: string; provider: BaseOAuthClientProvider; fetchFn?: typeof fetch; + resourceMetadataUrl?: URL; }): Promise { const clientMetadataUrl = params.provider.clientMetadataUrl?.trim(); if (!clientMetadataUrl) return; @@ -30,6 +37,7 @@ export async function ensureCimdClientRegistration(params: { try { resourceMetadata = await discoverOAuthProtectedResourceMetadata( params.serverUrl, + { resourceMetadataUrl: params.resourceMetadataUrl }, ); } catch { resourceMetadata = undefined; diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 59846e2a..6f032437 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -39,6 +39,7 @@ import type { HandleAuthChallengeOptions, } from "../auth/challenge.js"; import { + challengeResourceMetadataUrl, parseScopeString, unionAuthorizationScopes, } from "../auth/challenge.js"; @@ -729,10 +730,17 @@ export class OAuthManager { provider.clearCapturedAuthUrl(); + // RFC 9728: honor the metadata document the challenge advertised rather + // than probing the default locations derived from the MCP server URL + // (#2071). The SDK persists it in its discovery state, so the callback leg + // recovers it without our passing it again. + const resourceMetadataUrl = challengeResourceMetadataUrl(enriched); + await ensureCimdClientRegistration({ serverUrl, provider, fetchFn: this.params.effectiveAuthFetch, + resourceMetadataUrl, }); const scopeForAuth = @@ -750,6 +758,7 @@ export class OAuthManager { serverUrl, scope: scopeForAuth, fetchFn: this.params.effectiveAuthFetch, + resourceMetadataUrl, ...(enriched.reason === "insufficient_scope" && { forceReauthorization: isStrictScopeSuperset( scopeForAuth, @@ -879,6 +888,7 @@ export class OAuthManager { serverUrl, scope: scopeForAuth, fetchFn: this.params.effectiveAuthFetch, + resourceMetadataUrl: challengeResourceMetadataUrl(enriched), forceReauthorization: true, }); } finally { diff --git a/test-servers/configs/oauth-custom-resource-metadata-http.json b/test-servers/configs/oauth-custom-resource-metadata-http.json new file mode 100644 index 00000000..3e9ed3e6 --- /dev/null +++ b/test-servers/configs/oauth-custom-resource-metadata-http.json @@ -0,0 +1,19 @@ +{ + "serverInfo": { + "name": "oauth-custom-resource-metadata", + "version": "1.0.0" + }, + "tools": [{ "preset": "echo" }], + "oauth": { + "enabled": true, + "mode": "combined", + "requireAuth": true, + "resourceMetadataPath": "/custom/protected-resource", + "scopesSupported": ["mcp"], + "supportDCR": true + }, + "transport": { + "type": "streamable-http", + "port": 8082 + } +} diff --git a/test-servers/src/composable-test-server.ts b/test-servers/src/composable-test-server.ts index e5f8c77e..45d740a1 100644 --- a/test-servers/src/composable-test-server.ts +++ b/test-servers/src/composable-test-server.ts @@ -573,6 +573,19 @@ export interface ServerConfig { */ resource?: string; + /** + * Serve the RFC 9728 protected-resource metadata document from this + * non-default path *instead of* `/.well-known/oauth-protected-resource`, + * and advertise it via `WWW-Authenticate: Bearer resource_metadata="…"` + * on every 401. + * + * The default well-known routes are deliberately left unserved, so a + * client that ignores the advertised URL cannot discover the document at + * all — which is what makes this a regression test for #2071 rather than + * a path that merely happens to work. + */ + resourceMetadataPath?: string; + /** * OAuth authorization server issuer URL (combined mode AS metadata). * If not provided, defaults to the test server's base URL. diff --git a/test-servers/src/load-config.ts b/test-servers/src/load-config.ts index 73d1b427..c791eef2 100644 --- a/test-servers/src/load-config.ts +++ b/test-servers/src/load-config.ts @@ -19,6 +19,8 @@ export interface ConfigFileOAuth { mode?: "combined" | "protected-resource"; authorizationServers?: string[]; resource?: string; + /** Serve RFC 9728 metadata from this path and advertise it on 401 (#2071). */ + resourceMetadataPath?: string; issuerUrl?: string; accessTokenIssuers?: string[]; jwksUri?: string; @@ -200,6 +202,15 @@ function validateConfig( } } } + const metadataPath = oauth.resourceMetadataPath; + if ( + metadataPath !== undefined && + (typeof metadataPath !== "string" || !metadataPath.startsWith("/")) + ) { + throw new Error( + `Invalid config in ${filePath}: oauth.resourceMetadataPath must be an absolute path starting with "/"`, + ); + } if (transportType === "stdio" && oauth.enabled === true) { throw new Error( `Invalid config in ${filePath}: oauth requires streamable-http or sse transport`, diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index 5022e0f3..631f704b 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -2902,6 +2902,11 @@ export function createOAuthTestServerConfig(options: { supportCIMD?: boolean; tokenExpirationSeconds?: number; supportRefreshTokens?: boolean; + /** + * Move the RFC 9728 metadata document off the well-known path and advertise + * it via `WWW-Authenticate: Bearer resource_metadata="…"` (#2071). + */ + resourceMetadataPath?: string; }): Partial { return { oauth: { @@ -2909,6 +2914,9 @@ export function createOAuthTestServerConfig(options: { mode: "combined", requireAuth: options.requireAuth ?? false, scopesSupported: options.scopesSupported ?? ["mcp"], + ...(options.resourceMetadataPath + ? { resourceMetadataPath: options.resourceMetadataPath } + : {}), staticClients: options.staticClients, supportDCR: options.supportDCR ?? false, supportCIMD: options.supportCIMD ?? false, diff --git a/test-servers/src/test-server-oauth.ts b/test-servers/src/test-server-oauth.ts index 79200366..946f7030 100644 --- a/test-servers/src/test-server-oauth.ts +++ b/test-servers/src/test-server-oauth.ts @@ -27,6 +27,23 @@ export function getOAuthMode( return config.mode ?? "combined"; } +/** + * The `WWW-Authenticate` challenge sent with every 401. + * + * RFC 9728 §5.1: a resource server advertises where its protected-resource + * metadata lives via the `resource_metadata` parameter. Only emitted when the + * config moves that document off the well-known path — otherwise the bare + * `Bearer` challenge keeps the existing fixtures byte-identical. + */ +function bearerChallenge(config: OAuthConfig, req: Request): string { + const path = config.resourceMetadataPath; + if (!path) { + return "Bearer"; + } + const requestBaseUrl = `${req.protocol}://${req.get("host")}`; + return `Bearer resource_metadata="${new URL(path, requestBaseUrl).href}"`; +} + /** * Set up OAuth routes on an Express application * This adds all OAuth endpoints (authorization, token, metadata, etc.) @@ -76,7 +93,7 @@ export function createBearerTokenMiddleware( // For streamable-http, the SDK checks response status and throws StreamableHTTPError with code 401 res.status(401); res.setHeader("Content-Type", "application/json"); - res.setHeader("WWW-Authenticate", "Bearer"); + res.setHeader("WWW-Authenticate", bearerChallenge(config, req)); // Return a JSON-RPC error response format that the SDK will recognize res.json({ jsonrpc: "2.0", @@ -113,7 +130,7 @@ export function createBearerTokenMiddleware( // Return 401 - the SDK's transport should detect this and throw an error res.status(401); res.setHeader("Content-Type", "application/json"); - res.setHeader("WWW-Authenticate", "Bearer"); + res.setHeader("WWW-Authenticate", bearerChallenge(config, req)); // Return a JSON-RPC error response format that the SDK will recognize res.json({ jsonrpc: "2.0", @@ -184,9 +201,12 @@ function setupMetadataEndpoints( ); } - // OAuth Protected Resource Metadata + // OAuth Protected Resource Metadata. `resourceMetadataPath` moves the + // document off the well-known path entirely (rather than serving both), so + // a client that ignores the advertised `resource_metadata` URL gets a 404 + // — see the field's doc comment. app.get( - "/.well-known/oauth-protected-resource", + config.resourceMetadataPath ?? "/.well-known/oauth-protected-resource", (req: Request, res: Response) => { const requestBaseUrl = `${req.protocol}://${req.get("host")}`; const resourceUrl = config.resource ?? new URL("/", requestBaseUrl).href;