diff --git a/AGENTS.md b/AGENTS.md index 828636333..5dfdb2189 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -157,6 +157,30 @@ v2/main/ │ │ # metadata document, the one seam SDK v2 routes │ │ # BOTH endpoints through (neither reaches the │ │ # OAuthClientProvider) — #1906; +│ │ # oidcDiscoveryCompat.ts workaround for +│ │ # typescript-sdk#2733: the SDK picks the +│ │ # metadata schema from the well-known +│ │ # FILENAME, so a plain OAuth 2.0 AS +│ │ # publishing RFC 8414 metadata at +│ │ # /.well-known/openid-configuration (which +│ │ # RFC 8414 §5 permits) is parsed as an +│ │ # OpenID provider document and THROWS, +│ │ # aborting discovery rather than trying the +│ │ # next candidate. The wrapper fabricates +│ │ # NOTHING — on a failed RFC 8414 candidate +│ │ # it probes the OIDC candidates and, when +│ │ # one is RFC 8414 metadata that is not a +│ │ # valid OIDC document, serves that body as +│ │ # the RFC 8414 response so the SDK picks the +│ │ # right schema. GET-only, and the well-known +│ │ # match requires a `/` boundary, so a token +│ │ # endpoint living under that prefix keeps its +│ │ # own error. Sits on the BASE fetch like +│ │ # endpointOverrides — the one seam that also +│ │ # covers the discovery the SDK runs inside the +│ │ # transport — so the substituted response +│ │ # carries COMPAT_SOURCE_HEADER naming the URL +│ │ # its body came from — #2172; │ │ # secret-storage-info.ts browser-safe │ │ # descriptor of WHERE a typed secret lands │ │ # — kind/plaintext/durable plus the label, diff --git a/README.md b/README.md index 835c55fc9..b9efa7bf7 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | `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) | | `oauth-revocation-http.json` / `oauth-no-revocation-http.json` **(legacy era)** | RFC 7009 token revocation on clear, with and without a `revocation_endpoint` | [#2144](https://github.com/modelcontextprotocol/inspector/issues/2144) | +| `oauth-rfc8414-at-oidc-path-http.json` **(legacy era)** | Plain OAuth 2.0 AS metadata served at the OIDC well-known path | [#2172](https://github.com/modelcontextprotocol/inspector/issues/2172) | | `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) | | `subscriptions-never-acknowledged-http.json` | A `subscriptions/listen` answered with a bare result | [#2097](https://github.com/modelcontextprotocol/inspector/issues/2097) | @@ -500,6 +501,22 @@ Uncheck **Revoke tokens on clear** in the same panel (persisted as `oauth.revoke The same behavior is reachable from the other clients — the TUI's **Clear OAuth State**, and the CLI's `--relogin` (with `--no-revoke` as the per-run opt-out). +#### Plain OAuth 2.0 metadata at the OIDC well-known path + +`oauth-rfc8414-at-oidc-path-http.json` is an OAuth-protected server (combined AS + resource, DCR enabled) whose RFC 8414 authorization-server metadata is served **only** from `/.well-known/openid-configuration`. It is a plain OAuth 2.0 authorization server — no ID tokens, no `jwks_uri`, no `sub` claims — and RFC 8414 §5 explicitly permits that filename for general OAuth metadata. `/.well-known/oauth-authorization-server` is deliberately left unserved. Plain streamable-HTTP — connect with the **default (legacy)** protocol era. + +Add the server and click **Connect**: authorization must proceed normally. On the broken build it failed before the browser ever opened, with a `ZodError` naming three fields the server had no reason to publish: + +``` +"path":["jwks_uri"] … "path":["subject_types_supported"] … "path":["id_token_signing_alg_values_supported"] +``` + +The cause is upstream. `discoverAuthorizationServerMetadata` in `@modelcontextprotocol/client@2.0.0` picks its validation schema from the well-known **filename that resolved**, not from the document that came back — anything found at `openid-configuration` is parsed as OpenID Connect Discovery 1.0 provider metadata, which requires those three fields. And because the parse *throws* rather than continuing the candidate loop, discovery aborts outright instead of falling through ([#2172](https://github.com/modelcontextprotocol/inspector/issues/2172), filed upstream as [typescript-sdk#2733](https://github.com/modelcontextprotocol/typescript-sdk/issues/2733)). + +`core/auth/oidcDiscoveryCompat.ts` works around it without fabricating anything. When the RFC 8414 candidate comes back 4xx, it fetches the OIDC candidates the SDK would try next; if one returns a document that satisfies `OAuthMetadataSchema` but *fails* the OIDC schema, that document is returned as the response to the RFC 8414 request — so the SDK validates it under the schema that actually describes it. A genuine OpenID provider document is left alone and takes the SDK's normal OIDC leg. Issuer validation is untouched, since the substituted document is the one the server published, `issuer` included. + +The metadata shown in the Auth tab is exactly what the server sent — no invented `jwks_uri`. In the Network tab the substituted response is captured against the RFC 8414 URL (the wrapper sits on the base fetch, below the tracker, because that is the only seam that also covers the discovery the SDK runs from inside the transport), so it carries an `x-inspector-oauth-metadata-source` response header naming the URL its body was actually fetched from. The same URL is printed as a console warning. + #### 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/cli/__tests__/stored-auth.test.ts b/clients/cli/__tests__/stored-auth.test.ts index 3fb5c0b18..bea1ad499 100644 --- a/clients/cli/__tests__/stored-auth.test.ts +++ b/clients/cli/__tests__/stored-auth.test.ts @@ -873,3 +873,86 @@ describe("--wait-for-auth", () => { expect(result.stderr).toContain("positive number of seconds"); }); }); + +/** + * The stored-token refresh path calls SDK discovery directly rather than + * through `InspectorClient.effectiveAuthFetch`, so it carries its own copy of + * the #2172 compatibility wrapper. This test injects no `discover`, because + * the wrapper lives in the *default* — injecting one would bypass exactly what + * is under test. + */ +describe("refreshStoredAuthToken discovery compatibility (#2172)", () => { + const REFRESHED = { + access_token: "refreshed-access-token", + token_type: "Bearer", + refresh_token: "rotated-refresh-token", + expires_in: 3600, + }; + + it("refreshes against an AS publishing RFC 8414 metadata at the OIDC path", async () => { + let base = ""; + let rfc8414Probes = 0; + const server: Server = createServer((req, res) => { + const path = req.url ?? ""; + if (path.startsWith("/.well-known/oauth-authorization-server")) { + rfc8414Probes += 1; + res.writeHead(404).end(); + return; + } + // Plain OAuth 2.0: no jwks_uri, no subject_types_supported, no + // id_token_signing_alg_values_supported — the shape the SDK rejects when + // it finds it under this filename. + if (path === "/mcp/.well-known/openid-configuration") { + res.writeHead(200, { "content-type": "application/json" }); + res.end( + JSON.stringify({ + issuer: `${base}/mcp`, + authorization_endpoint: `${base}/oauth/authorize`, + token_endpoint: `${base}/oauth/token`, + response_types_supported: ["code"], + grant_types_supported: ["authorization_code", "refresh_token"], + token_endpoint_auth_methods_supported: ["client_secret_post"], + }), + ); + return; + } + if (path === "/oauth/token") { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(REFRESHED)); + return; + } + res.writeHead(404).end(); + }); + await new Promise((resolve) => { + server.listen(0, "127.0.0.1", resolve); + }); + const addr = server.address(); + base = + typeof addr === "object" && addr ? `http://127.0.0.1:${addr.port}` : ""; + const serverUrl = `${base}/mcp`; + const fixture = writeOAuthFixture({ + [serverUrl]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + clientInformation: { client_id: "cid", client_secret: "sec" }, + }, + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshStoredAuthToken(serverUrl, fixture)).resolves.toBe( + "refreshed-access-token", + ); + // The RFC 8414 location really was tried and really did 404, so the + // document could only have come from the OIDC path. + expect(rfc8414Probes).toBeGreaterThan(0); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("/mcp/.well-known/openid-configuration"), + ); + } finally { + warn.mockRestore(); + rmSync(dirname(fixture), { recursive: true, force: true }); + await new Promise((resolve) => { + server.close(() => resolve()); + }); + } + }); +}); diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index 902448356..17aa5ba08 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -52,6 +52,7 @@ import { getAuthorizationServerUrl, getAuthorizationServerUrlCandidates, } from "@inspector/core/auth/discovery.js"; +import { withRfc8414OidcCompat } from "@inspector/core/auth/oidcDiscoveryCompat.js"; import { writeStoreFile } from "@inspector/core/storage/store-io.js"; import { refreshAuthorization, @@ -341,7 +342,27 @@ export async function refreshStoredAuthToken( deps: RefreshStoredAuthDeps = {}, ): Promise { const refresh = deps.refresh ?? refreshAuthorization; - const discover = deps.discover ?? discoverAuthorizationServerMetadata; + // #2172: this path calls SDK discovery directly rather than through + // `InspectorClient.effectiveAuthFetch`, so it needs the same compatibility + // wrapper — otherwise a stored refresh token with no persisted + // `serverMetadata` still cannot refresh against an authorization server that + // publishes RFC 8414 metadata at the OIDC well-known path (Copilot). + // + // Built over `createProxyFetch()` for the same reason `environment.fetch` is + // (#2067): this whole function runs outside `InspectorClient`, so nothing + // else puts a proxy under it, and a server reachable only through + // `HTTPS_PROXY` would otherwise be probed directly. The same fetch is handed + // to the token request below, so neither leg bypasses the proxy (Copilot). + const storedAuthFetch = withRfc8414OidcCompat(createProxyFetch() ?? fetch); + const discover: typeof discoverAuthorizationServerMetadata = + deps.discover ?? + ((authorizationServerUrl, options) => + discoverAuthorizationServerMetadata(authorizationServerUrl, { + ...options, + // A caller-supplied fetch is left alone — it is theirs to compose. The + // walker below passes no options, so in practice this is ours. + fetchFn: options?.fetchFn ?? storedAuthFetch, + })); const snapshot = await readOAuthSnapshot(statePath); const servers = snapshot.servers as StoredServers; @@ -392,6 +413,7 @@ export async function refreshStoredAuthToken( clientInformation, refreshToken, resource: new URL(serverUrl), + fetchFn: storedAuthFetch, }); } catch (err) { const message = err instanceof Error ? err.message : String(err); diff --git a/clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts b/clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts new file mode 100644 index 000000000..e3bf9d4de --- /dev/null +++ b/clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts @@ -0,0 +1,441 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { buildDiscoveryUrls } from "@modelcontextprotocol/client"; +import { + COMPAT_SOURCE_HEADER, + oidcDiscoveryCandidates, + isRfc8414OnlyMetadata, + withRfc8414OidcCompat, +} from "@inspector/core/auth/oidcDiscoveryCompat.js"; + +/** Minimal RFC 8414 authorization-server metadata — no OIDC-only fields. */ +const RFC8414_DOC = { + issuer: "https://as.example.com/tenant", + authorization_endpoint: "https://as.example.com/tenant/authorize", + token_endpoint: "https://as.example.com/tenant/token", + response_types_supported: ["code"], +}; + +/** The same document plus the three fields OpenID Connect Discovery requires. */ +const OIDC_DOC = { + ...RFC8414_DOC, + jwks_uri: "https://as.example.com/tenant/jwks", + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], +}; + +const RFC8414_URL = + "https://as.example.com/.well-known/oauth-authorization-server/tenant"; +const OIDC_SUFFIXED = + "https://as.example.com/.well-known/openid-configuration/tenant"; +const OIDC_APPENDED = + "https://as.example.com/tenant/.well-known/openid-configuration"; + +function json(body: unknown, init?: ResponseInit): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + ...init, + }); +} + +function notFound(): Response { + return new Response("nope", { status: 404 }); +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("oidcDiscoveryCandidates", () => { + // The derivation is a hand-written mirror of the SDK's `buildDiscoveryUrls`, + // which a fetch wrapper cannot call (it sees a request URL, not the + // authorization-server URL). Pinning it against the SDK's own export is what + // keeps the two from drifting when the SDK changes its candidate list. + it.each([ + "https://as.example.com", + "https://as.example.com/", + "https://as.example.com/tenant", + "https://as.example.com/tenant/", + "https://misc.poodll.com/mod/minilesson/mcp.php", + ])("matches the SDK's OIDC candidates for %s", (authServerUrl) => { + const sdkUrls = buildDiscoveryUrls(authServerUrl); + const rfc8414 = sdkUrls.find((entry) => entry.type === "oauth"); + expect(rfc8414).toBeDefined(); + const expected = sdkUrls + .filter((entry) => entry.type === "oidc") + .map((entry) => entry.url.href); + expect(oidcDiscoveryCandidates(rfc8414!.url.href)).toEqual(expected); + }); + + it("returns nothing for a URL that is not an RFC 8414 candidate", () => { + expect(oidcDiscoveryCandidates("https://as.example.com/tokens")).toEqual( + [], + ); + }); + + it("returns nothing for an unparseable URL", () => { + expect(oidcDiscoveryCandidates("not a url")).toEqual([]); + }); + + it("does not claim a path that merely shares the well-known prefix", () => { + // A bare prefix match would treat this as a discovery request and replace + // its failed response with a document from a derived URL (Copilot). + expect( + oidcDiscoveryCandidates( + "https://as.example.com/.well-known/oauth-authorization-server-backup", + ), + ).toEqual([]); + }); +}); + +describe("isRfc8414OnlyMetadata", () => { + it("accepts plain RFC 8414 metadata", () => { + expect(isRfc8414OnlyMetadata(RFC8414_DOC)).toBe(true); + }); + + it("rejects a document that is a valid OpenID provider document", () => { + expect(isRfc8414OnlyMetadata(OIDC_DOC)).toBe(false); + }); + + it("rejects a body that is not authorization-server metadata at all", () => { + expect(isRfc8414OnlyMetadata({ hello: "world" })).toBe(false); + }); +}); + +describe("withRfc8414OidcCompat", () => { + it("passes a successful response through untouched", async () => { + const original = json(RFC8414_DOC); + const base = vi.fn().mockResolvedValue(original); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped(RFC8414_URL)).resolves.toBe(original); + expect(base).toHaveBeenCalledTimes(1); + }); + + it("passes a status the SDK will not walk past through untouched", async () => { + const original = new Response("boom", { status: 500 }); + const base = vi.fn().mockResolvedValue(original); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped(RFC8414_URL)).resolves.toBe(original); + expect(base).toHaveBeenCalledTimes(1); + }); + + it("does not probe when the failed request is not a discovery request", async () => { + const original = notFound(); + const base = vi.fn().mockResolvedValue(original); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped("https://as.example.com/tokens")).resolves.toBe( + original, + ); + expect(base).toHaveBeenCalledTimes(1); + }); + + it("serves an RFC 8414 document found on the appended OIDC path", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const base = vi.fn(async (input) => { + const url = String(input); + if (url === OIDC_APPENDED) return json(RFC8414_DOC); + return notFound(); + }); + const wrapped = withRfc8414OidcCompat(base); + + const response = await wrapped(RFC8414_URL); + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe("application/json"); + await expect(response.json()).resolves.toEqual(RFC8414_DOC); + // The substitution names where its body came from, so a captured Network + // entry does not read as a 200 from the RFC 8414 path. + expect(response.headers.get(COMPAT_SOURCE_HEADER)).toBe(OIDC_APPENDED); + // The failed original, then both OIDC candidates. + expect(base).toHaveBeenCalledTimes(3); + expect(warn).toHaveBeenCalledWith(expect.stringContaining(OIDC_APPENDED)); + }); + + it("does not touch a non-GET request to a matching path", async () => { + // An authorization or token endpoint may legally live under the RFC 8414 + // well-known prefix; its ordinary OAuth error must reach the caller intact + // rather than being replaced with a metadata document (Copilot). + const original = new Response(JSON.stringify({ error: "invalid_grant" }), { + status: 400, + headers: { "content-type": "application/json" }, + }); + const base = vi.fn().mockResolvedValue(original); + const wrapped = withRfc8414OidcCompat(base); + + await expect( + wrapped(`${RFC8414_URL}/token`, { method: "post" }), + ).resolves.toBe(original); + await expect( + wrapped(new Request(`${RFC8414_URL}/token`, { method: "POST" })), + ).resolves.toBe(original); + expect(base).toHaveBeenCalledTimes(2); + }); + + it("serves an RFC 8414 document found on the path-suffixed OIDC path", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const base = vi.fn(async (input) => { + const url = String(input); + if (url === OIDC_SUFFIXED) return json(RFC8414_DOC); + return notFound(); + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect((await wrapped(RFC8414_URL)).json()).resolves.toEqual( + RFC8414_DOC, + ); + // The failed original, then the first OIDC candidate — no need for a second. + expect(base).toHaveBeenCalledTimes(2); + }); + + it("leaves a genuine OpenID provider document to the SDK's own OIDC leg", async () => { + const original = notFound(); + const base = vi.fn(async (input) => { + if (String(input) === OIDC_SUFFIXED) return json(OIDC_DOC); + return original; + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped(RFC8414_URL)).resolves.toBe(original); + // Stops at the document discovery would have used; the appended candidate + // is never probed. + expect(base).toHaveBeenCalledTimes(2); + }); + + it("accepts a 2xx candidate whatever media type it carries", async () => { + // The SDK parses a 2xx discovery body regardless of `content-type`, so + // gating on it here could skip a document the SDK would have used (Copilot). + vi.spyOn(console, "warn").mockImplementation(() => {}); + const base = vi.fn(async (input) => { + if (String(input) === OIDC_SUFFIXED) { + return new Response(JSON.stringify(RFC8414_DOC), { + status: 200, + headers: { "content-type": "text/plain" }, + }); + } + return notFound(); + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect((await wrapped(RFC8414_URL)).json()).resolves.toEqual( + RFC8414_DOC, + ); + }); + + it("stops at a candidate whose body will not parse", async () => { + // Terminal for the SDK, so a later candidate must not be promoted over it + // — that would turn a failure into a success (Copilot). + const original = notFound(); + const base = vi.fn(async (input) => { + if (String(input) === OIDC_SUFFIXED) { + return new Response("{ not json", { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return original; + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped(RFC8414_URL)).resolves.toBe(original); + // The failed original and the first candidate only — the second is not + // reached. + expect(base).toHaveBeenCalledTimes(2); + }); + + it("releases the bodies it discards", async () => { + // A response nobody reads still holds its connection on Node/undici, and + // this loop runs on every OAuth attempt (Copilot). + vi.spyOn(console, "warn").mockImplementation(() => {}); + const cancelled: string[] = []; + const streamed = (name: string, body: string): Response => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(body)); + controller.close(); + }, + cancel() { + cancelled.push(name); + }, + }), + { status: name === "appended" ? 200 : 404 }, + ); + + const base = vi.fn(async (input) => { + const url = String(input); + if (url === OIDC_APPENDED) + return streamed("appended", JSON.stringify(RFC8414_DOC)); + if (url === OIDC_SUFFIXED) return streamed("suffixed", "nope"); + return streamed("original", "nope"); + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect((await wrapped(RFC8414_URL)).json()).resolves.toEqual( + RFC8414_DOC, + ); + // The 404 probe, and the original response the substitution replaces. + expect(cancelled.sort()).toEqual(["original", "suffixed"]); + }); + + it("names the URL the body came from after a redirect", async () => { + // `fetch` follows redirects, so the candidate is where the probe was aimed + // — `probe.url` is where the document actually lives (Copilot). + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const redirected = "https://as.example.com/tenant/oidc-metadata"; + const base = vi.fn(async (input) => { + if (String(input) !== OIDC_SUFFIXED) return notFound(); + const probe = json(RFC8414_DOC); + Object.defineProperty(probe, "url", { value: redirected }); + return probe; + }); + const wrapped = withRfc8414OidcCompat(base); + + const response = await wrapped(RFC8414_URL); + expect(response.headers.get(COMPAT_SOURCE_HEADER)).toBe(redirected); + expect(warn).toHaveBeenCalledWith(expect.stringContaining(redirected)); + }); + + it("walks past a candidate that 404s, as the SDK does", async () => { + const original = notFound(); + const base = vi.fn().mockResolvedValue(original); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped(RFC8414_URL)).resolves.toBe(original); + expect(base).toHaveBeenCalledTimes(3); + }); + + it("stops at a candidate returning a status the SDK would not walk past", async () => { + // A 500 on the first OIDC candidate is an outage the SDK surfaces; hiding + // it behind a second candidate's document would be worse than the bug + // being worked around (Copilot). + const original = notFound(); + const base = vi.fn(async (input) => { + const url = String(input); + if (url === OIDC_SUFFIXED) return new Response("boom", { status: 500 }); + if (url === OIDC_APPENDED) return json(RFC8414_DOC); + return original; + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped(RFC8414_URL)).resolves.toBe(original); + expect(base).toHaveBeenCalledTimes(2); + }); + + it("stops when a probe throws", async () => { + // The SDK swallows only the browser's CORS `TypeError`; elsewhere a network + // error propagates. Leave it to the SDK to make the same request and reach + // its own verdict (Copilot). + const original = notFound(); + const base = vi.fn(async (input) => { + if (String(input).includes("openid-configuration")) { + throw new TypeError("Failed to fetch"); + } + return original; + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect(wrapped(RFC8414_URL)).resolves.toBe(original); + expect(base).toHaveBeenCalledTimes(2); + }); + + it("probes on a 502, which the SDK also walks past", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const base = vi.fn(async (input) => { + if (String(input) === OIDC_APPENDED) return json(RFC8414_DOC); + return new Response("bad gateway", { status: 502 }); + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect((await wrapped(RFC8414_URL)).json()).resolves.toEqual( + RFC8414_DOC, + ); + }); + + it("carries the discovery headers from the failed request onto the probe", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const seen: Headers[] = []; + const base = vi.fn(async (input, init) => { + const url = String(input); + if (url.includes("openid-configuration")) { + seen.push(new Headers(init?.headers)); + return json(RFC8414_DOC); + } + return notFound(); + }); + const wrapped = withRfc8414OidcCompat(base); + + await wrapped(RFC8414_URL, { + headers: { + "MCP-Protocol-Version": "2025-11-25", + Accept: "application/json", + }, + }); + expect(seen[0]?.get("mcp-protocol-version")).toBe("2025-11-25"); + expect(seen[0]?.get("accept")).toBe("application/json"); + }); + + it("reads the headers off a Request input when there is no init", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const seen: Headers[] = []; + const base = vi.fn(async (input, init) => { + const url = String(input instanceof Request ? input.url : input); + if (url.includes("openid-configuration")) { + seen.push(new Headers(init?.headers)); + return json(RFC8414_DOC); + } + return notFound(); + }); + const wrapped = withRfc8414OidcCompat(base); + + await wrapped( + new Request(RFC8414_URL, { + headers: { "MCP-Protocol-Version": "2025-11-25" }, + }), + ); + expect(seen[0]?.get("mcp-protocol-version")).toBe("2025-11-25"); + }); + + it("accepts a URL instance as the request input", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const base = vi.fn(async (input) => { + if (String(input) === OIDC_APPENDED) return json(RFC8414_DOC); + return notFound(); + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect((await wrapped(new URL(RFC8414_URL))).json()).resolves.toEqual( + RFC8414_DOC, + ); + }); + + it("handles a root authorization server, which has a single OIDC candidate", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const rootDoc = { + issuer: "https://as.example.com", + authorization_endpoint: "https://as.example.com/authorize", + token_endpoint: "https://as.example.com/token", + response_types_supported: ["code"], + }; + const base = vi.fn(async (input) => { + if ( + String(input) === + "https://as.example.com/.well-known/openid-configuration" + ) { + return json(rootDoc); + } + return notFound(); + }); + const wrapped = withRfc8414OidcCompat(base); + + await expect( + ( + await wrapped( + "https://as.example.com/.well-known/oauth-authorization-server", + ) + ).json(), + ).resolves.toEqual(rootDoc); + expect(base).toHaveBeenCalledTimes(2); + }); +}); diff --git a/clients/web/src/test/integration/auth/rfc8414AtOidcPath.test.ts b/clients/web/src/test/integration/auth/rfc8414AtOidcPath.test.ts new file mode 100644 index 000000000..cbd06e52d --- /dev/null +++ b/clients/web/src/test/integration/auth/rfc8414AtOidcPath.test.ts @@ -0,0 +1,146 @@ +/** + * RFC 8414 authorization-server metadata served at the OpenID Connect + * well-known path, end to end against a real OAuth test server (#2172). + * + * The unit tests prove the wrapper substitutes the right document. This file + * proves the two halves only a real server can: that the SDK genuinely rejects + * a conforming plain-OAuth document found at `openid-configuration`, and that + * the wrapper recovers discovery against the same server without adding a + * field the server never published. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from "vitest"; +import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/client"; +import { + TestServerHttp, + getDefaultServerConfig, + createOAuthTestServerConfig, + waitForOAuthWellKnown, +} from "@modelcontextprotocol/inspector-test-server"; +import { + COMPAT_SOURCE_HEADER, + withRfc8414OidcCompat, +} from "@inspector/core/auth/oidcDiscoveryCompat.js"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; + +const OIDC_PATH = "/.well-known/openid-configuration"; + +/** The three fields OpenID Connect Discovery requires and RFC 8414 does not. */ +const OIDC_ONLY_FIELDS = [ + "jwks_uri", + "subject_types_supported", + "id_token_signing_alg_values_supported", +] as const; + +describe("RFC 8414 metadata at the OIDC well-known path (#2172)", () => { + let mcpServer: TestServerHttp | null = null; + let serverUrl = ""; + + beforeAll(async () => { + mcpServer = new TestServerHttp({ + ...getDefaultServerConfig(), + serverType: "streamable-http" as const, + ...createOAuthTestServerConfig({ + // Metadata routes are served either way; leaving auth unrequired lets + // the transport-wiring test below actually connect. + requireAuth: false, + asMetadataPath: OIDC_PATH, + }), + }); + const port = await mcpServer.start(); + serverUrl = `http://localhost:${port}`; + await waitForOAuthWellKnown(serverUrl, { metadataPath: OIDC_PATH }); + }, 30_000); + + afterAll(async () => { + await mcpServer?.stop(); + mcpServer = null; + }, 30_000); + + it("serves plain RFC 8414 metadata only from the OIDC path", async () => { + const oidc = await fetch(`${serverUrl}${OIDC_PATH}`); + expect(oidc.status).toBe(200); + const metadata = (await oidc.json()) as Record; + expect(metadata.issuer).toBe(serverUrl); + for (const field of OIDC_ONLY_FIELDS) { + expect(metadata).not.toHaveProperty(field); + } + + const rfc8414 = await fetch( + `${serverUrl}/.well-known/oauth-authorization-server`, + ); + expect(rfc8414.status).toBe(404); + }); + + it("is rejected by unwrapped SDK discovery", async () => { + // The upstream defect this module exists for + // (modelcontextprotocol/typescript-sdk#2733). If this ever starts + // resolving, the SDK has been fixed and `oidcDiscoveryCompat` can go. + await expect( + discoverAuthorizationServerMetadata(serverUrl), + ).rejects.toThrow(/jwks_uri/); + }); + + it("resolves through the compat wrapper without fabricating a field", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const metadata = await discoverAuthorizationServerMetadata(serverUrl, { + fetchFn: withRfc8414OidcCompat(fetch), + }); + + expect(metadata?.issuer).toBe(serverUrl); + expect(metadata?.authorization_endpoint).toBe( + `${serverUrl}/oauth/authorize`, + ); + expect(metadata?.token_endpoint).toBe(`${serverUrl}/oauth/token`); + for (const field of OIDC_ONLY_FIELDS) { + expect(metadata).not.toHaveProperty(field); + } + expect(warn).toHaveBeenCalledWith( + expect.stringContaining(`${serverUrl}${OIDC_PATH}`), + ); + } finally { + warn.mockRestore(); + } + }); + + it("hands the transport a fetch that carries the shim", async () => { + // The SDK also runs discovery from *inside* the transport, which receives + // `InspectorClient`'s base fetch directly — so a reconnect with an existing + // auth provider must not bypass the shim (Copilot). Capturing the fetch the + // transport was handed and driving it against this server is what proves + // the wiring, without depending on the SDK to trigger that leg. + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + let transportFetch: typeof fetch | undefined; + const capturingTransport: typeof createTransportNode = ( + config, + options, + ) => { + transportFetch = options?.fetchFn; + return createTransportNode(config, options); + }; + const client = new InspectorClient( + { type: "streamable-http", url: `${serverUrl}/mcp` }, + { environment: { transport: capturingTransport } }, + ); + try { + await client.connect(); + expect(transportFetch).toBeDefined(); + + const response = await transportFetch!( + `${serverUrl}/.well-known/oauth-authorization-server`, + ); + expect(response.status).toBe(200); + expect(response.headers.get(COMPAT_SOURCE_HEADER)).toBe( + `${serverUrl}${OIDC_PATH}`, + ); + await expect(response.json()).resolves.toMatchObject({ + issuer: serverUrl, + }); + } finally { + await client.disconnect(); + warn.mockRestore(); + } + }); +}); diff --git a/core/auth/index.ts b/core/auth/index.ts index 0ddad0c54..2ba73e832 100644 --- a/core/auth/index.ts +++ b/core/auth/index.ts @@ -45,6 +45,11 @@ export { withOAuthEndpointOverrides, } from "./endpointOverrides.js"; export type { OAuthEndpointOverrides } from "./endpointOverrides.js"; +export { + oidcDiscoveryCandidates, + isRfc8414OnlyMetadata, + withRfc8414OidcCompat, +} from "./oidcDiscoveryCompat.js"; // Storage export type { diff --git a/core/auth/oidcDiscoveryCompat.ts b/core/auth/oidcDiscoveryCompat.ts new file mode 100644 index 000000000..bb30e7222 --- /dev/null +++ b/core/auth/oidcDiscoveryCompat.ts @@ -0,0 +1,332 @@ +/** + * Compatibility shim for a plain OAuth 2.0 authorization server that publishes + * its RFC 8414 metadata at the OpenID Connect well-known path (#2172). + * + * ## The upstream defect + * + * Filed as modelcontextprotocol/typescript-sdk#2733. + * + * `discoverAuthorizationServerMetadata` in `@modelcontextprotocol/client@2.0.0` + * tries a fixed list of well-known URLs (`buildDiscoveryUrls`) and picks the + * validation schema from the **filename that resolved**, not from the document + * that came back: + * + * ```js + * const parsed = type === "oauth" + * ? OAuthMetadataSchema.parse(body) + * : OpenIdProviderDiscoveryMetadataSchema.parse(body); + * ``` + * + * `type` is `"oidc"` for every `openid-configuration` candidate, so a document + * served there is required to carry `jwks_uri`, `subject_types_supported` and + * `id_token_signing_alg_values_supported` — three fields OpenID Connect + * Discovery 1.0 requires and RFC 8414 does not. A plain OAuth 2.0 authorization + * server — no ID tokens, no JWKS, no `sub` — that publishes at that path is + * therefore rejected with a `ZodError`, and because the parse **throws** rather + * than continuing the loop, discovery aborts outright: the connection fails and + * no later candidate is tried. + * + * RFC 8414 §5 explicitly anticipates this deployment. `openid-configuration` is + * a permitted location for general OAuth authorization server metadata, and the + * OIDC-only fields are not part of the RFC 8414 document. So the server in + * #2172 is conforming and the client is wrong. + * + * ## What this wrapper does + * + * It never fabricates a field. It changes *which candidate URL* the document is + * handed back on, so the SDK validates it under the schema that actually + * describes it: + * + * 1. It watches only for the RFC 8414 candidate, + * `/.well-known/oauth-authorization-server[]`, and only when that + * request came back with a status the SDK treats as "try the next + * candidate" (4xx, or 502). + * 2. It then fetches the OIDC candidates the SDK would try next — derived from + * the request URL by the same rule `buildDiscoveryUrls` uses, which + * `oidcDiscoveryCandidates` mirrors and a test pins against the SDK's own + * exported function so the two cannot drift. + * 3. If one of them returns a document that satisfies `OAuthMetadataSchema` but + * **fails** `OpenIdProviderDiscoveryMetadataSchema` — i.e. it is RFC 8414 + * metadata and is not an OpenID provider document — that body is returned as + * the response to the RFC 8414 request. The SDK parses it with + * `OAuthMetadataSchema`, which is the correct schema for it, and discovery + * succeeds. + * + * A document that *does* satisfy the OIDC schema is left alone: the original + * response is returned unchanged and the SDK proceeds to its own OIDC candidate + * as usual. That costs one duplicate request on a genuine OIDC server, which is + * the price of not changing behavior for the case that already works. + * + * ## What it deliberately does not do + * + * - **It does not weaken issuer validation.** The substituted document is the + * one the server actually published, `issuer` included, so the SDK's RFC 8414 + * §3.3 issuer-echo check runs on it exactly as before. A document whose + * `issuer` does not match is still rejected. + * - **It does not invent `jwks_uri` (or any other field).** Back-filling the + * three OIDC-required fields would also make the parse succeed, but the + * Inspector would then be displaying, in the Auth and Network tabs, a + * metadata document the server never published — which is the one thing a + * debugging tool must not do. + * - **It does not present the substitution as the server's own answer.** It is + * installed on `InspectorClient`'s *base* fetch — below both fetch trackers, + * the same seam `withOAuthEndpointOverrides` uses — because that is the only + * place that also covers the discovery the SDK runs from **inside the + * transport**, which is handed the base fetch directly and whose tracker is + * built inside the transport where nothing here can reach above it + * (Copilot). One seam therefore covers every path, at the cost that a + * captured entry shows the substituted document rather than the real 404 — + * so the substituted response carries {@link COMPAT_SOURCE_HEADER} naming + * the URL the body actually came from, and the warning below says the same + * thing on the console. The probe itself is issued through the wrapped fetch + * and so is not separately tracked. + * + * ## Removing this + * + * This is a workaround for an upstream bug, not a feature. When + * modelcontextprotocol/typescript-sdk#2733 lands — the SDK selecting the schema + * from the document rather than from the filename, or treating a schema failure + * as a candidate to skip rather than as fatal — delete this module and its + * wiring in `core/mcp/inspectorClient.ts`. + */ + +import { + OAuthMetadataSchema, + OpenIdProviderDiscoveryMetadataSchema, +} from "@modelcontextprotocol/core"; + +/** The RFC 8414 well-known path, which every OAuth-typed candidate starts with. */ +const RFC8414_WELL_KNOWN = "/.well-known/oauth-authorization-server"; + +/** The OpenID Connect Discovery well-known path. */ +const OIDC_WELL_KNOWN = "/.well-known/openid-configuration"; + +/** + * Response header stamped on a substituted document, naming the URL the body + * was actually fetched from. Inspector-private (`x-inspector-`), never sent on + * a request — it exists so a captured Network entry is self-describing rather + * than appearing to be a 200 from the RFC 8414 path. + */ +export const COMPAT_SOURCE_HEADER = "x-inspector-oauth-metadata-source"; + +/** + * The OIDC discovery candidates the SDK would try after the RFC 8414 candidate + * `rfc8414Url` failed, in the SDK's own order. + * + * Derived from the RFC 8414 URL rather than from the authorization-server URL + * because that is all a `fetch` wrapper sees. The two are equivalent: the SDK + * builds the RFC 8414 candidate as + * `${origin}/.well-known/oauth-authorization-server${path}` with the + * authorization server's (trailing-slash-stripped) pathname as `path`, so + * recovering `path` recovers everything `buildDiscoveryUrls` keyed off. + * + * Returns an empty array when the URL is not an RFC 8414 candidate at all. + */ +export function oidcDiscoveryCandidates(rfc8414Url: string): string[] { + let url: URL; + try { + url = new URL(rfc8414Url); + } catch { + return []; + } + if (!url.pathname.startsWith(RFC8414_WELL_KNOWN)) return []; + const path = url.pathname.slice(RFC8414_WELL_KNOWN.length); + // A bare prefix match would also claim `/.well-known/oauth-authorization-server-backup` + // — a path this wrapper has no business rewriting, and one whose failed + // response it would replace with a document fetched from somewhere else + // entirely (Copilot). The SDK appends the authorization server's pathname, + // which always begins with `/`, so requiring the exact path or a `/` + // boundary is precisely the set it can emit. + if (path !== "" && !path.startsWith("/")) return []; + // The authorization server had no path, so the SDK emitted a single OIDC + // candidate at the origin. + if (path === "") return [new URL(OIDC_WELL_KNOWN, url.origin).href]; + // A path-suffixed RFC 8414 candidate always has two OIDC siblings: the + // path-suffixed form and the path-prefixed ("appended") form. + return [ + new URL(`${OIDC_WELL_KNOWN}${path}`, url.origin).href, + new URL(`${path}${OIDC_WELL_KNOWN}`, url.origin).href, + ]; +} + +/** + * Whether a status makes the SDK move on to the next discovery candidate. + * + * Mirrors `discoverAuthorizationServerMetadata`: a 4xx or a 502 continues the + * loop, anything else throws. Probing on a status the SDK will not walk past + * would be wasted work — the flow is already over. + */ +function continuesDiscovery(status: number): boolean { + return (status >= 400 && status < 500) || status === 502; +} + +/** + * Discard a response body nobody is going to read, so the connection under it + * is returned to the pool rather than held open. Best-effort: a body already + * consumed, locked, or absent is not an error here. + */ +async function releaseBody(response: Response): Promise { + await response.body?.cancel().catch(() => {}); +} + +/** + * Whether a parsed body is RFC 8414 authorization-server metadata that is *not* + * a valid OpenID provider document — the exact shape the upstream schema + * selection rejects. + * + * Both halves matter. Without the first, any JSON body would be substituted; + * without the second, a genuine OIDC document would be diverted onto the RFC + * 8414 path and silently change behavior for servers that work today. + */ +export function isRfc8414OnlyMetadata(value: unknown): boolean { + if (!OAuthMetadataSchema.safeParse(value).success) return false; + return !OpenIdProviderDiscoveryMetadataSchema.safeParse(value).success; +} + +/** The request URL a `fetch` call was made with, in any of its three forms. */ +function requestUrlOf(input: RequestInfo | URL): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.href; + return input.url; +} + +/** + * The headers the failed discovery request carried, so the probe is answered + * the same way the SDK's own request would have been (it sends + * `MCP-Protocol-Version` and `Accept`). Copied from the request rather than + * reconstructed, so a header the SDK adds later is carried without this module + * needing to know about it. + */ +function discoveryHeaders( + input: RequestInfo | URL, + init: RequestInit | undefined, +): Headers { + if (init?.headers) return new Headers(init.headers); + if (typeof input !== "string" && !(input instanceof URL)) { + return new Headers(input.headers); + } + return new Headers(); +} + +/** + * The effective HTTP method of a `fetch` call, normalized. `fetch` defaults to + * `GET` and treats the method case-insensitively. + */ +function requestMethodOf( + input: RequestInfo | URL, + init: RequestInit | undefined, +): string { + if (init?.method) return init.method.toUpperCase(); + if (typeof input !== "string" && !(input instanceof URL)) { + return input.method.toUpperCase(); + } + return "GET"; +} + +/** + * Wrap a `fetch` so a plain OAuth 2.0 authorization server publishing RFC 8414 + * metadata at `/.well-known/openid-configuration` is discoverable. + * + * Inert for every request that is not a *failed* RFC 8414 discovery request, so + * the overwhelmingly common case costs one status check and one string + * comparison. + */ +export function withRfc8414OidcCompat(fetchFn: typeof fetch): typeof fetch { + return async (input, init) => { + const response = await fetchFn(input, init); + if (response.ok) return response; + if (!continuesDiscovery(response.status)) return response; + // The URL shape alone does not prove this was a discovery request: nothing + // stops an authorization or token endpoint from living under + // `/.well-known/oauth-authorization-server/…`, and a `POST` there returning + // an ordinary OAuth `400 invalid_grant` must reach its caller intact rather + // than being replaced with a metadata document (Copilot). Metadata + // discovery is a `GET`, so anything else is not ours. + if (requestMethodOf(input, init) !== "GET") return response; + + const candidates = oidcDiscoveryCandidates(requestUrlOf(input)); + if (candidates.length === 0) return response; + + const headers = discoveryHeaders(input, init); + // The loop advances to the next candidate only where the SDK's own loop + // would. Anywhere else it hands the original response back and lets + // discovery run its normal course, because *promoting a later candidate + // over an earlier one the SDK would have stopped at* would turn a failure + // into a success — a much worse defect than the one being worked around + // (Copilot). Concretely: a 500 on the first OIDC candidate is an outage the + // SDK surfaces, a malformed JSON body there is terminal for it, and a + // network error propagates outside the browser's CORS case. In each of + // those, returning `response` leaves the SDK to make the same request and + // reach the same verdict it always would. + for (const candidate of candidates) { + let probe: Response; + try { + probe = await fetchFn(candidate, { headers }); + } catch { + return response; + } + if (!probe.ok) { + // A probe response nobody will read still holds its connection open on + // Node/undici, and this loop can run on every OAuth attempt — so + // release it rather than letting repeated discovery against a 404 + // candidate exhaust the origin's pool (Copilot). Same discipline as + // `core/mcp/node/authChallengeFetch.ts`. + await releaseBody(probe); + if (continuesDiscovery(probe.status)) continue; + return response; + } + + // Deliberately not gated on `content-type`: the SDK parses a 2xx + // discovery body whatever media type it carries, so gating here could + // skip a genuine OIDC document served with an odd one and substitute a + // later RFC-8414-only document in its place. A body that will not parse + // falls through to `return response` below, where the SDK re-fetches it + // and raises its own parse error. + let parsed: unknown; + let body: string; + try { + body = await probe.text(); + parsed = JSON.parse(body); + } catch { + return response; + } + if (!isRfc8414OnlyMetadata(parsed)) { + // Either not metadata at all, or a genuine OpenID provider document the + // SDK can already read. Stop looking: this is the document discovery + // would have used, and it needs no help. + return response; + } + + // `fetch` follows redirects, so the candidate is where the probe was + // *aimed*; `probe.url` is where the body actually came from. Report the + // latter, falling back for a synthesized response that carries no url + // (Copilot). + const source = probe.url || candidate; + // The original failed response is about to be dropped in favour of the + // substitution, so release its connection too. + await releaseBody(response); + console.warn( + `[oauth] ${source} returned RFC 8414 OAuth 2.0 authorization server ` + + `metadata, not an OpenID provider document. The MCP TypeScript SDK ` + + `validates that path as OpenID Connect Discovery and would reject it ` + + `(modelcontextprotocol/typescript-sdk#2733), so the Inspector is ` + + `handing it to discovery as the RFC 8414 document it is.`, + ); + return new Response(body, { + status: 200, + statusText: "OK", + headers: { + "content-type": "application/json", + // The substituted response is what the fetch trackers above this + // wrapper record, so it says where its body actually came from + // rather than letting the Network tab imply the RFC 8414 path + // answered. Named on the response, not logged only, so it survives + // into the captured entry. + [COMPAT_SOURCE_HEADER]: source, + }, + }); + } + + return response; + }; +} diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 5a1f1fa3a..8fdaebe94 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -238,6 +238,7 @@ import { type HandleAuthChallengeOptions, } from "../auth/challenge.js"; import { withOAuthEndpointOverrides } from "../auth/endpointOverrides.js"; +import { withRfc8414OidcCompat } from "../auth/oidcDiscoveryCompat.js"; import type { TokenRevocationOutcome } from "../auth/revocation.js"; import type { OAuthTokens } from "@modelcontextprotocol/client"; import { silentLogger, type InspectorLogger } from "../logging/logger.js"; @@ -795,6 +796,16 @@ export class InspectorClient extends InspectorClientEventTarget { this.fetchFn = withOAuthEndpointOverrides(this.fetchFn ?? fetch, () => this.oauthManager?.getEndpointOverrides(), ); + // #2172: recover discovery when a plain OAuth 2.0 authorization server + // publishes RFC 8414 metadata at `/.well-known/openid-configuration`, which + // the SDK validates as an OpenID provider document and rejects. Wraps the + // *base* fetch for the same reason the overrides above do: the SDK also + // runs discovery from inside the transport, which is handed `this.fetchFn` + // directly, so a reconnect with an existing auth provider would otherwise + // still hit the upstream failure. The substituted response is stamped with + // `COMPAT_SOURCE_HEADER` so a captured entry names the URL its body came + // from rather than appearing to be a 200 from the RFC 8414 path. + this.fetchFn = withRfc8414OidcCompat(this.fetchFn); this.effectiveAuthFetch = this.buildEffectiveAuthFetch(); this.sessionId = options.sessionId; diff --git a/test-servers/configs/oauth-rfc8414-at-oidc-path-http.json b/test-servers/configs/oauth-rfc8414-at-oidc-path-http.json new file mode 100644 index 000000000..a55edffd9 --- /dev/null +++ b/test-servers/configs/oauth-rfc8414-at-oidc-path-http.json @@ -0,0 +1,19 @@ +{ + "serverInfo": { + "name": "oauth-rfc8414-at-oidc-path", + "version": "1.0.0" + }, + "tools": [{ "preset": "echo" }], + "oauth": { + "enabled": true, + "mode": "combined", + "requireAuth": true, + "asMetadataPath": "/.well-known/openid-configuration", + "scopesSupported": ["mcp"], + "supportDCR": true + }, + "transport": { + "type": "streamable-http", + "port": 8083 + } +} diff --git a/test-servers/src/composable-test-server.ts b/test-servers/src/composable-test-server.ts index d9799bffd..eddb56d5f 100644 --- a/test-servers/src/composable-test-server.ts +++ b/test-servers/src/composable-test-server.ts @@ -613,6 +613,21 @@ export interface ServerConfig { */ resourceMetadataPath?: string; + /** + * Serve the RFC 8414 authorization-server metadata document from this + * non-default path *instead of* `/.well-known/oauth-authorization-server` + * (combined mode only). + * + * Set it to `/.well-known/openid-configuration` to reproduce #2172: a + * plain OAuth 2.0 authorization server — no `jwks_uri`, no + * `subject_types_supported`, no `id_token_signing_alg_values_supported` — + * publishing RFC 8414 metadata at the OIDC well-known path, which RFC 8414 + * §5 permits. As with `resourceMetadataPath`, the default route is left + * unserved, so a client that cannot read the document where it actually + * lives fails outright rather than quietly succeeding elsewhere. + */ + asMetadataPath?: 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 c154a6962..dc68bb8da 100644 --- a/test-servers/src/load-config.ts +++ b/test-servers/src/load-config.ts @@ -22,6 +22,8 @@ export interface ConfigFileOAuth { resource?: string; /** Serve RFC 9728 metadata from this path and advertise it on 401 (#2071). */ resourceMetadataPath?: string; + /** Serve RFC 8414 AS metadata from this path instead of the default (#2172). */ + asMetadataPath?: string; issuerUrl?: string; accessTokenIssuers?: string[]; jwksUri?: string; @@ -224,6 +226,12 @@ function validateConfig( `Invalid config in ${filePath}: oauth.resourceMetadataPath must be an origin-relative path (e.g. "/custom/protected-resource") — a value such as "//host/doc" would advertise a document the server does not serve`, ); } + const asPath = oauth.asMetadataPath; + if (asPath !== undefined && !isOriginRelativePath(asPath)) { + throw new Error( + `Invalid config in ${filePath}: oauth.asMetadataPath must be an origin-relative path (e.g. "/.well-known/openid-configuration") — a value such as "//host/doc" would move the document off this server entirely`, + ); + } 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-helpers.ts b/test-servers/src/test-helpers.ts index da73f6f28..90347015b 100644 --- a/test-servers/src/test-helpers.ts +++ b/test-servers/src/test-helpers.ts @@ -79,6 +79,13 @@ export interface WaitForOAuthWellKnownOptions { interval?: number; /** Max time per fetch attempt (so one hung request doesn't burn the whole timeout). Default 1000. */ requestTimeout?: number; + /** + * Origin-relative path the AS metadata document is served from. Defaults to + * the RFC 8414 well-known path; pass the configured `oauth.asMetadataPath` + * when the fixture moved the document (#2172), since polling a route the + * server deliberately does not serve can only ever time out. + */ + metadataPath?: string; } /** @@ -97,8 +104,9 @@ export async function waitForOAuthWellKnown( timeout = 5000, interval = 50, requestTimeout = 1000, + metadataPath = "/.well-known/oauth-authorization-server", } = options ?? {}; - const wellKnownUrl = `${serverBaseUrl.replace(/\/$/, "")}/.well-known/oauth-authorization-server`; + const wellKnownUrl = `${serverBaseUrl.replace(/\/$/, "")}${metadataPath}`; const start = Date.now(); let lastStatus: number | undefined; let lastError: unknown; diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index 40dc504d7..8bef04106 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -3009,6 +3009,13 @@ export function createOAuthTestServerConfig(options: { * it via `WWW-Authenticate: Bearer resource_metadata="…"` (#2071). */ resourceMetadataPath?: string; + /** + * Move the RFC 8414 authorization-server metadata document off the + * well-known path — set it to `/.well-known/openid-configuration` to serve + * plain OAuth 2.0 metadata where the SDK expects an OpenID provider + * document (#2172). + */ + asMetadataPath?: string; }): Partial { return { oauth: { @@ -3022,6 +3029,11 @@ export function createOAuthTestServerConfig(options: { ...(options.resourceMetadataPath !== undefined ? { resourceMetadataPath: options.resourceMetadataPath } : {}), + // Same `!== undefined` reasoning as above: an explicit `""` is invalid + // and must reach the server so it reports the bad fixture. + ...(options.asMetadataPath !== undefined + ? { asMetadataPath: options.asMetadataPath } + : {}), 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 a93099c22..6d06c8dac 100644 --- a/test-servers/src/test-server-oauth.ts +++ b/test-servers/src/test-server-oauth.ts @@ -83,6 +83,24 @@ function resourceMetadataPath(config: OAuthConfig): string | undefined { return path; } +/** + * The path the RFC 8414 authorization-server metadata document is served from, + * validated the same way `resourceMetadataPath` is. Defaults to the well-known + * location; `asMetadataPath` moves it (see the field's doc comment). + */ +function asMetadataPath(config: OAuthConfig): string { + const path = config.asMetadataPath; + if (path === undefined) { + return "/.well-known/oauth-authorization-server"; + } + if (!isOriginRelativePath(path)) { + throw new Error( + `oauth.asMetadataPath must be an origin-relative path (got ${JSON.stringify(path)})`, + ); + } + return path; +} + /** * The `WWW-Authenticate` challenge sent with every 401. * @@ -222,53 +240,47 @@ function setupMetadataEndpoints( if (mode === "combined") { // OAuth Authorization Server Metadata (local AS) - app.get( - "/.well-known/oauth-authorization-server", - (req: Request, res: Response) => { - const requestBaseUrl = `${req.protocol}://${req.get("host")}`; - const actualIssuerUrl = config.issuerUrl ?? new URL(requestBaseUrl); - const metadata = { - // RFC 8414 §3.3: the issuer MUST be identical to the base URL the - // well-known path was appended to — i.e. no trailing slash. SDK v2's - // client enforces this exactly (IssuerMismatchError otherwise). - issuer: actualIssuerUrl.href.replace(/\/$/, ""), - authorization_endpoint: new URL("/oauth/authorize", actualIssuerUrl) - .href, - token_endpoint: new URL("/oauth/token", actualIssuerUrl).href, - scopes_supported: scopes, - response_types_supported: ["code"], - grant_types_supported: ["authorization_code", "refresh_token"], - code_challenge_methods_supported: ["S256"], - token_endpoint_auth_methods_supported: [ + app.get(asMetadataPath(config), (req: Request, res: Response) => { + const requestBaseUrl = `${req.protocol}://${req.get("host")}`; + const actualIssuerUrl = config.issuerUrl ?? new URL(requestBaseUrl); + const metadata = { + // RFC 8414 §3.3: the issuer MUST be identical to the base URL the + // well-known path was appended to — i.e. no trailing slash. SDK v2's + // client enforces this exactly (IssuerMismatchError otherwise). + issuer: actualIssuerUrl.href.replace(/\/$/, ""), + authorization_endpoint: new URL("/oauth/authorize", actualIssuerUrl) + .href, + token_endpoint: new URL("/oauth/token", actualIssuerUrl).href, + scopes_supported: scopes, + response_types_supported: ["code"], + grant_types_supported: ["authorization_code", "refresh_token"], + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["client_secret_basic", "none"], + // RFC 9207 / SEP-2468: advertise iss on authorization responses so + // clients must validate (and our e2e can exercise reject paths). + authorization_response_iss_parameter_supported: true, + ...(config.supportRevocation !== false && { + // RFC 7009 (#2144). Advertised by default so the in-repo servers + // exercise the Inspector's revocation leg; set + // `oauth.supportRevocation: false` to reproduce an authorization + // server that offers none, where the Inspector must do nothing. + revocation_endpoint: new URL("/oauth/revoke", actualIssuerUrl).href, + revocation_endpoint_auth_methods_supported: [ "client_secret_basic", "none", ], - // RFC 9207 / SEP-2468: advertise iss on authorization responses so - // clients must validate (and our e2e can exercise reject paths). - authorization_response_iss_parameter_supported: true, - ...(config.supportRevocation !== false && { - // RFC 7009 (#2144). Advertised by default so the in-repo servers - // exercise the Inspector's revocation leg; set - // `oauth.supportRevocation: false` to reproduce an authorization - // server that offers none, where the Inspector must do nothing. - revocation_endpoint: new URL("/oauth/revoke", actualIssuerUrl).href, - revocation_endpoint_auth_methods_supported: [ - "client_secret_basic", - "none", - ], - }), - ...(config.supportDCR && { - registration_endpoint: new URL("/oauth/register", actualIssuerUrl) - .href, - }), - ...(config.supportCIMD && { - client_id_metadata_document_supported: true, - }), - }; + }), + ...(config.supportDCR && { + registration_endpoint: new URL("/oauth/register", actualIssuerUrl) + .href, + }), + ...(config.supportCIMD && { + client_id_metadata_document_supported: true, + }), + }; - res.json(metadata); - }, - ); + res.json(metadata); + }); } // OAuth Protected Resource Metadata. `resourceMetadataPath` moves the