diff --git a/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx b/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx index 8014aa08..4a440c8a 100644 --- a/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx +++ b/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx @@ -307,15 +307,16 @@ export function ClientSettingsForm({ {settings.cimdEnabled && ( <> - The metadata document must be served over HTTPS and list - this redirect URI:{" "} + The metadata document must be served over HTTPS — or over + plain HTTP from localhost, 127.0.0.1 or [::1], for a local + fixture — and list this redirect URI:{" "} {typeof window !== "undefined" ? `${window.location.origin}/oauth/callback` : "http://localhost:6274/oauth/callback"} patch({ clientMetadataUrl: e.currentTarget.value }) diff --git a/clients/web/src/test/core/client/config.test.ts b/clients/web/src/test/core/client/config.test.ts index bb2b7712..3b6e2663 100644 --- a/clients/web/src/test/core/client/config.test.ts +++ b/clients/web/src/test/core/client/config.test.ts @@ -206,6 +206,19 @@ describe("client config", () => { ).toThrow(/HTTPS/); }); + it("parseClientConfig accepts a loopback http CIMD URL", () => { + for (const clientMetadataUrl of [ + "http://localhost:8090/client-metadata.json", + "http://127.0.0.1:8090/client-metadata.json", + "http://[::1]:8090/client-metadata.json", + ]) { + const config = parseClientConfig({ + cimd: { enabled: true, clientMetadataUrl }, + }); + expect(config.cimd?.clientMetadataUrl).toBe(clientMetadataUrl); + } + }); + it("parseClientConfig rejects CIMD URL without path", () => { expect(() => parseClientConfig({ @@ -299,6 +312,50 @@ describe("client config", () => { CIMD_METADATA_URL_PATH_ERROR, ); }); + + it("accepts plain http on the three exempt loopback literals", () => { + expect( + getCimdClientMetadataUrlError( + "http://localhost:8090/client-metadata.json", + ), + ).toBeUndefined(); + expect( + getCimdClientMetadataUrlError( + "http://127.0.0.1:8090/client-metadata.json", + ), + ).toBeUndefined(); + expect( + getCimdClientMetadataUrlError("http://[::1]:8090/client-metadata.json"), + ).toBeUndefined(); + }); + + it("still requires a path on an exempt loopback host", () => { + expect(getCimdClientMetadataUrlError("http://localhost:8090")).toBe( + CIMD_METADATA_URL_PATH_ERROR, + ); + }); + + it("does not widen the exemption beyond the three literals", () => { + // `localhost.` and `*.localhost` resolve to loopback on every resolver, + // but they are outside the SDK's allow-list — so they are outside ours + // too, deliberately, rather than the two disagreeing about one URL. + for (const value of [ + "http://tenant.app.localhost:3300/client-metadata.json", + "http://127.0.0.2:8090/client-metadata.json", + "http://localhost.example.com/client-metadata.json", + ]) { + expect(getCimdClientMetadataUrlError(value)).toBe( + CIMD_METADATA_URL_HTTPS_ERROR, + ); + } + // The root-anchored spelling never reaches the protocol check at all: + // `isAbsoluteHttpUrl` rejects its trailing empty label first, so it is + // flagged as unparseable rather than as non-HTTPS. Pinned so a later + // change to either check has to decide about this case on purpose. + expect( + getCimdClientMetadataUrlError("http://localhost./client-metadata.json"), + ).toBe(CIMD_METADATA_URL_INVALID_ERROR); + }); }); it("formatClientConfigLoadError summarizes Zod validation failures", () => { diff --git a/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts b/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts index 759d1d2a..96f30192 100644 --- a/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts +++ b/clients/web/src/test/integration/mcp/oauth-cimd-fixture.test.ts @@ -10,6 +10,10 @@ import { loadConfig, resolveConfig, } from "@modelcontextprotocol/inspector-test-server"; +import { + getCimdClientMetadataUrlError, + parseClientConfig, +} from "@inspector/core/client/config-parse.js"; /** * Live coverage of `test-servers/configs/oauth-cimd-http.json` — the fixture @@ -34,11 +38,13 @@ import { * - the document's `client_id` equals the URL it was fetched from, which is * what makes it a legal CIMD client id rather than an arbitrary blob. * - * The document is served over plain HTTP here. That is deliberate and is *not* - * a usable `clientMetadataUrl` for the Inspector, which requires HTTPS with no - * loopback exemption (#2305) — driving the UI needs a self-signed HTTPS - * listener, as `docs/test-servers.md` describes. This endpoint exists for the - * authorization-server side of the flow and for exactly these assertions. + * The document is served over plain HTTP here, on `localhost`. Since #2305 that + * *is* a usable `clientMetadataUrl`: the Inspector's HTTPS requirement now + * exempts the same three loopback literals the SDK exempts for token endpoints, + * so the fixture can be driven from the web client with no self-signed HTTPS + * listener. The last case below pins that, because it is the property the + * fixture's whole reason for existing rests on — a re-tightened validator would + * put the manual repro back out of reach without failing anything else here. */ describe("CIMD showcase fixture (#2242)", () => { let server: TestServerHttp | null = null; @@ -121,6 +127,22 @@ describe("CIMD showcase fixture (#2242)", () => { expect(doc.token_endpoint_auth_method).toBe("none"); }); + it("serves the document at a URL the Inspector accepts as a clientMetadataUrl", async () => { + const started = await startShowcase(); + const documentUrl = `${originOf(started)}/client-metadata.json`; + + // The #2305 property: no self-signed HTTPS listener, no + // NODE_TLS_REJECT_UNAUTHORIZED=0 — the served URL is legal config as-is, + // both inline in the settings form and in `client.json` on disk. + expect(new URL(documentUrl).protocol).toBe("http:"); + expect(getCimdClientMetadataUrlError(documentUrl)).toBeUndefined(); + expect( + parseClientConfig({ + cimd: { enabled: true, clientMetadataUrl: documentUrl }, + }).cimd?.clientMetadataUrl, + ).toBe(documentUrl); + }); + it("preserves a query-bearing document URL in the client_id it publishes", async () => { const started = await startShowcase(); const documentUrl = `${originOf(started)}/client-metadata.json?profile=a`; diff --git a/core/client/config-parse.ts b/core/client/config-parse.ts index 82c66f1f..fe783664 100644 --- a/core/client/config-parse.ts +++ b/core/client/config-parse.ts @@ -58,9 +58,48 @@ const HttpUrlStringSchema = z export const CIMD_METADATA_URL_INVALID_ERROR = "Must be a valid URL, like https://example.com/oauth/client.json"; -/** Field-level error when a CIMD metadata URL is not HTTPS. */ +/** + * Field-level error when a CIMD metadata URL is neither HTTPS nor served from + * one of the exempt loopback literals. The message names the exemption rather + * than saying only "must use HTTPS", so a developer pointing the field at a + * local fixture is told which spellings work instead of concluding that nothing + * local can (the same phrasing lesson as the token-endpoint notice in + * `core/auth/oauthUx.ts`). + */ export const CIMD_METADATA_URL_HTTPS_ERROR = - "CIMD client metadata URL must use HTTPS"; + "CIMD client metadata URL must use HTTPS, except on localhost, 127.0.0.1 or [::1]"; + +/** + * Hosts for which a plain `http:` CIMD metadata URL is accepted. + * + * SEP-991 expects HTTPS in production for a real reason: a CIMD `client_id` is + * a URL the authorization server dereferences, and over plain HTTP the document + * it gets back is attacker-modifiable in transit. Loopback is the documented + * exception to exactly that reasoning — there is no network segment to sit on — + * which is why the SDK exempts these same three literals from its token-endpoint + * TLS assertion, and why `core/auth/cimd.ts` already notes that an + * already-stored `client_id` may be an `http://` URL "used by local dev/test + * metadata servers". Without this, the runtime tolerated a value the config + * validation would not let anyone enter, and CIMD could not be driven against + * any fixture in this repo (#2305). + * + * ⚠️ This is the SDK's *three-literal* list, deliberately, and not the broader + * `isLoopbackHost` in `core/node/hostUrl.ts` — which imports `node:net` and so + * cannot be reached from this browser-safe module. Matching the SDK literal for + * literal also keeps the two allow-lists from disagreeing about the same URL. + * `::1` arrives from `URL.hostname` bracketed, which is the form stored here. + */ +const CIMD_HTTP_EXEMPT_HOSTS = new Set(["localhost", "127.0.0.1", "[::1]"]); + +/** + * True when a plain-`http:` CIMD metadata URL on this host is acceptable. Takes + * the bare host (`URL.hostname`), never a `host:port`. Not exported: the only + * caller is the validator below, and the exemption is exercised through it + * rather than through a second public surface that could drift from it. + */ +function isCimdHttpExemptHost(hostname: string): boolean { + return CIMD_HTTP_EXEMPT_HOSTS.has(hostname); +} /** Field-level error when a CIMD metadata URL has no path segment. */ export const CIMD_METADATA_URL_PATH_ERROR = @@ -81,7 +120,7 @@ export function getCimdClientMetadataUrlError( } try { const url = new URL(trimmed); - if (url.protocol !== "https:") { + if (url.protocol !== "https:" && !isCimdHttpExemptHost(url.hostname)) { return CIMD_METADATA_URL_HTTPS_ERROR; } if (url.pathname === "/" || url.pathname === "") { diff --git a/docs/test-servers.md b/docs/test-servers.md index 55afa07a..806cace2 100644 --- a/docs/test-servers.md +++ b/docs/test-servers.md @@ -560,16 +560,23 @@ from **Client settings**, not from a server's own OAuth settings. A `clientMetad catalog entry's `oauth` block is silently ignored — and with `supportDCR: true` the connection then succeeds *via DCR*, which looks like CIMD working until you read the client id. -⚠️ **The Inspector requires that URL to be HTTPS, and there is no loopback exemption** +**Use this server's own document as the `clientMetadataUrl`** — `/client-metadata.json` on the origin +the server announced on startup (`http://127.0.0.1:8092/client-metadata.json` when it got its +configured port; read the announced URL, since this fixture walks upward on `EADDRINUSE` like every +other one here). The Inspector requires HTTPS *except* on the three +loopback literals `localhost`, `127.0.0.1` and `[::1]` (`getCimdClientMetadataUrlError` in `core/client/config-parse.ts`, applied to `client.json` on disk as -well as to the settings form). So this server's own `http://` document is **not** usable as a -`clientMetadataUrl`: it exists for the authorization-server side of the flow and for tests that drive -the AS directly. To drive the Inspector end to end you need the document served over HTTPS — -`https://127.0.0.1:8443/client-metadata.json` from a throwaway self-signed listener works, with -`NODE_TLS_REJECT_UNAUTHORIZED=0` in the *test server's* environment so its own fetch of that document -succeeds. That asymmetry is tracked in -[#2305](https://github.com/modelcontextprotocol/inspector/issues/2305); it is the same over-narrow -allow-list shape as the token-endpoint exemption above. +well as to the settings form), which is the same exemption the SDK applies to token endpoints and what +the runtime already tolerated for an already-stored `client_id`. Give the URL the host the server is +actually listening on; a host outside those three — `localhost.` and `tenant.app.localhost` included, +even though both resolve to loopback — is still rejected, deliberately, so this allow-list and the +SDK's cannot disagree about one URL. + +⚠️ **Before [#2305](https://github.com/modelcontextprotocol/inspector/issues/2305) there was no +loopback exemption at all**, so driving this flow meant standing up a throwaway self-signed HTTPS +listener to hold the document and setting `NODE_TLS_REJECT_UNAUTHORIZED=0` in the *test server's* +environment so its own fetch of that document would succeed. That workaround is no longer needed — if +you find it in a script or an older note, delete it. With that in place: set the metadata URL in Client settings, connect, and open **Connection Info**. It should read `Client registration — Client ID Metadata (CIMD)` with the **client id equal to the