diff --git a/apps/cloud/src/app-paths.test.ts b/apps/cloud/src/app-paths.test.ts index dd9ff1af3c..024ec5091a 100644 --- a/apps/cloud/src/app-paths.test.ts +++ b/apps/cloud/src/app-paths.test.ts @@ -20,6 +20,7 @@ describe("isAppOwnedPath", () => { "/mcp", "/mcp/toolkits/deploy-kit", "/.well-known/oauth-protected-resource/mcp", + "/.well-known/oauth-protected-resource/_v1/mcp", "/.well-known/oauth-protected-resource/mcp/toolkits/deploy-kit", "/.well-known/oauth-authorization-server", // Org-pinned MCP: the org's URL slug (what the install card prints) and diff --git a/apps/cloud/src/mcp/mount.test.ts b/apps/cloud/src/mcp/mount.test.ts index dd9c0a3f49..c4d7b0af30 100644 --- a/apps/cloud/src/mcp/mount.test.ts +++ b/apps/cloud/src/mcp/mount.test.ts @@ -55,3 +55,48 @@ describe("cloud MCP toolkit route normalization", () => { ); }); }); + +// Executor v2 will own `executor.sh/mcp`'s root discovery documents, so plain +// `/mcp` names a v1-only alias of the bare document in its challenge. +describe("cloud MCP plain-/mcp resource metadata alias", () => { + it("challenges plain /mcp with the v1 alias, and org and toolkit URLs with their own documents", () => { + expect(protectedResourceMetadataUrlFor(null)).toBe( + "https://executor.sh/.well-known/oauth-protected-resource/_v1/mcp", + ); + expect(protectedResourceMetadataUrlFor("acme")).toBe( + "https://executor.sh/.well-known/oauth-protected-resource/acme/mcp", + ); + expect(protectedResourceMetadataUrlFor("org_01ABCDEF")).toBe( + "https://executor.sh/.well-known/oauth-protected-resource/org_01ABCDEF/mcp", + ); + }); + + it("classifies the alias as the bare document, not an org", () => { + expect(classifyMcpPath("/.well-known/oauth-protected-resource/_v1/mcp")).toEqual({ + kind: "oauth-protected-resource", + organizationId: null, + }); + expect(classifyMcpPath("/.well-known/oauth-protected-resource/mcp")).toEqual({ + kind: "oauth-protected-resource", + organizationId: null, + }); + // `_v1` only names the bare document; it is not an org selector. + expect(classifyMcpPath("/_v1/mcp")).toBeNull(); + expect( + classifyMcpPath("/.well-known/oauth-protected-resource/_v1/mcp/toolkits/deploy"), + ).toBeNull(); + }); + + it("rewrites the alias to the mounted bare document and drops any org header", () => { + const request = new Request( + "https://executor.sh/.well-known/oauth-protected-resource/_v1/mcp", + { headers: { "x-executor-mcp-organization": "spoofed" } }, + ); + + const rewritten = prepareMcpOrgScope(request); + + expect(new URL(rewritten.url).pathname).toBe("/.well-known/oauth-protected-resource/mcp"); + expect(rewritten.headers.has("x-executor-mcp-organization")).toBe(false); + expect(toolkitSlugFromRequest(rewritten)).toBeNull(); + }); +}); diff --git a/e2e/cloud/mcp-expired-token-rediscovery.test.ts b/e2e/cloud/mcp-expired-token-rediscovery.test.ts new file mode 100644 index 0000000000..db8d6f9e0d --- /dev/null +++ b/e2e/cloud/mcp-expired-token-rediscovery.test.ts @@ -0,0 +1,171 @@ +// Cloud: an MCP SDK client whose plain-`/mcp` token has expired follows the +// 401's `resource_metadata` back to this product's authorization server. +// +// Executor v2 will serve `executor.sh/mcp`'s root discovery documents +// (`/.well-known/oauth-protected-resource/mcp`, +// `/.well-known/oauth-authorization-server`) while v1 still answers plain +// `/mcp` requests that carry a v1 token. So the challenge must name a v1-only +// document, and a spec-following client must never consult the root ones on +// the way to re-authorizing. Driven with the real @modelcontextprotocol/sdk +// client; the run stops where the SDK would open the browser. +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; + +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { + UnauthorizedError, + type OAuthClientProvider, +} from "@modelcontextprotocol/sdk/client/auth.js"; +import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import type { + OAuthClientInformationMixed, + OAuthTokens, +} from "@modelcontextprotocol/sdk/shared/auth.js"; + +import { scenario } from "../src/scenario"; +import { Target } from "../src/services"; + +const V1_RESOURCE_METADATA_PATH = "/.well-known/oauth-protected-resource/_v1/mcp"; +const ROOT_DISCOVERY_PATHS = [ + "/.well-known/oauth-protected-resource", + "/.well-known/oauth-protected-resource/mcp", + "/.well-known/oauth-authorization-server", +]; + +const base64url = (value: unknown): string => + Buffer.from(JSON.stringify(value)).toString("base64url"); + +/** A JWT shaped like an expired access token from `issuer`. The signature is + * junk: the server must reject it either way, and the router only reads `iss`. */ +const expiredAccessToken = (issuer: string): string => + [ + base64url({ alg: "RS256", typ: "JWT" }), + base64url({ + iss: issuer, + sub: "user_e2e_expired", + exp: Math.floor(Date.now() / 1000) - 3600, + }), + base64url("expired"), + ].join("."); + +type Hop = { readonly method: string; readonly url: string; readonly status: number }; + +scenario( + "MCP protocol · an expired plain-/mcp token re-authorizes through the v1 discovery document", + {}, + Effect.gen(function* () { + const target = yield* Target; + const mcpUrl = new URL("/mcp", target.baseUrl); + + const bare = (yield* Effect.promise(async () => + (await fetch(new URL("/.well-known/oauth-protected-resource/mcp", target.baseUrl))).json(), + )) as { readonly authorization_servers: ReadonlyArray }; + const authorizationServer = bare.authorization_servers[0] ?? ""; + expect(authorizationServer, "the product advertises an authorization server").not.toBe(""); + + const hops: Array = []; + const recordingFetch: typeof fetch = async (input, init) => { + const request = new Request(input, init); + const response = await fetch(request); + hops.push({ method: request.method, url: request.url, status: response.status }); + return response; + }; + + let tokens: OAuthTokens | undefined = { + access_token: expiredAccessToken(authorizationServer), + token_type: "Bearer", + refresh_token: "refresh_e2e_revoked", + }; + let clientInformation: OAuthClientInformationMixed | undefined; + let authorizationUrl: URL | undefined; + const provider: OAuthClientProvider = { + redirectUrl: "http://127.0.0.1:1/callback", + clientMetadata: { + client_name: "executor-e2e-expired-token", + redirect_uris: ["http://127.0.0.1:1/callback"], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }, + clientInformation: () => clientInformation, + saveClientInformation: (info) => { + clientInformation = info; + }, + tokens: () => tokens, + saveTokens: (next) => { + tokens = next; + }, + invalidateCredentials: (scope) => { + if (scope === "all" || scope === "tokens") tokens = undefined; + if (scope === "all" || scope === "client") clientInformation = undefined; + }, + redirectToAuthorization: (url) => { + authorizationUrl = url; + }, + saveCodeVerifier: () => undefined, + codeVerifier: () => "", + }; + + const client = new Client({ name: "executor-e2e-expired-token", version: "0.0.1" }); + const transport = new StreamableHTTPClientTransport(mcpUrl, { + authProvider: provider, + fetch: recordingFetch, + }); + const connected = yield* Effect.promise(() => + client.connect(transport).then( + () => "connected" as const, + (error: unknown) => (error instanceof UnauthorizedError ? "unauthorized" : String(error)), + ), + ); + yield* Effect.promise(() => client.close()); + + yield* Effect.annotateCurrentSpan({ "e2e.hops": JSON.stringify(hops) }); + expect(connected, "the expired token cannot connect; the client must re-authorize").toBe( + "unauthorized", + ); + + const first = hops[0]; + expect(first?.url, "the client first calls plain /mcp with the expired token").toBe( + mcpUrl.toString(), + ); + expect(first?.status, "the expired token is challenged").toBe(401); + + const aliasUrl = new URL(V1_RESOURCE_METADATA_PATH, target.baseUrl).toString(); + expect( + hops.some((hop) => hop.url === aliasUrl && hop.status === 200), + "the client fetches the v1 discovery document the challenge named", + ).toBe(true); + for (const path of ROOT_DISCOVERY_PATHS) { + const rootUrl = new URL(path, target.baseUrl).toString(); + expect( + hops.some((hop) => hop.url === rootUrl), + `the client never consults the root discovery document ${path}`, + ).toBe(false); + } + + const authServerOrigin = new URL(authorizationServer).origin; + expect( + hops.some( + (hop) => + hop.url.startsWith(`${authServerOrigin}/.well-known/oauth-authorization-server`) && + hop.status === 200, + ), + "the client reads the advertised authorization server's metadata", + ).toBe(true); + const tokenEndpoint = (yield* Effect.promise(async () => + (await fetch(new URL("/.well-known/oauth-authorization-server", authorizationServer))).json(), + )) as { readonly token_endpoint: string }; + expect( + hops.some((hop) => hop.method === "POST" && hop.url === tokenEndpoint.token_endpoint), + "the client tries its refresh token at the advertised authorization server", + ).toBe(true); + expect( + authorizationUrl?.origin, + "the browser sign-in opens on the advertised authorization server", + ).toBe(authServerOrigin); + expect( + authorizationUrl?.searchParams.get("resource"), + "the sign-in asks for the plain MCP resource", + ).toBe(mcpUrl.toString()); + }), +); diff --git a/e2e/cloud/mcp-protocol.test.ts b/e2e/cloud/mcp-protocol.test.ts index c192dd1812..fe19639899 100644 --- a/e2e/cloud/mcp-protocol.test.ts +++ b/e2e/cloud/mcp-protocol.test.ts @@ -20,6 +20,10 @@ import { Mcp, Target } from "../src/services"; import type { Identity } from "../src/target"; const JSON_AND_SSE = "application/json, text/event-stream"; +// Plain `/mcp` challenges name this v1-only alias of the bare discovery +// document: once Executor v2 serves `executor.sh/mcp`'s root discovery, the +// path-derived `/.well-known/oauth-protected-resource/mcp` is v2's. +const V1_RESOURCE_METADATA_PATH = "/.well-known/oauth-protected-resource/_v1/mcp"; const PROTOCOL_VERSION = "2025-03-26"; const INITIALIZE_REQUEST = { jsonrpc: "2.0" as const, @@ -201,6 +205,34 @@ scenario( }), ); +scenario( + "MCP protocol · the v1 alias serves the same discovery document as the bare path", + {}, + Effect.gen(function* () { + const target = yield* Target; + const [bare, alias] = yield* Effect.promise(() => + Promise.all([ + fetch(new URL("/.well-known/oauth-protected-resource/mcp", target.baseUrl)), + fetch(new URL(V1_RESOURCE_METADATA_PATH, target.baseUrl), { + headers: { origin: "https://claude.ai" }, + }), + ]), + ); + expect(alias.status, "the alias is public").toBe(200); + expect( + alias.headers.get("access-control-allow-origin"), + "the alias is readable cross-origin", + ).toBe("*"); + const aliasBody = (yield* Effect.promise(() => alias.json())) as Record; + expect(aliasBody["resource"], "the alias names the plain MCP resource").toBe( + new URL("/mcp", target.baseUrl).toString(), + ); + expect(aliasBody, "the alias is the bare document").toEqual( + yield* Effect.promise(() => bare.json()), + ); + }), +); + scenario( "MCP protocol · a request without a bearer is challenged with the resource metadata", {}, @@ -212,8 +244,8 @@ scenario( expect(response.status, "anonymous requests are rejected").toBe(401); const wwwAuth = response.headers.get("www-authenticate") ?? ""; expect(wwwAuth, "the challenge is a Bearer challenge").toContain('Bearer resource_metadata="'); - expect(wwwAuth, "the challenge points at the discovery document").toContain( - new URL("/.well-known/oauth-protected-resource/mcp", target.baseUrl).toString(), + expect(wwwAuth, "the challenge points at v1's alias of the discovery document").toContain( + `resource_metadata="${new URL(V1_RESOURCE_METADATA_PATH, target.baseUrl).toString()}"`, ); expect(wwwAuth, "a missing token carries no error code (per RFC 6750)").not.toContain("error="); expect( @@ -241,8 +273,8 @@ scenario( expect(wwwAuth, "the challenge names the invalid_token error").toContain( 'error="invalid_token"', ); - expect(wwwAuth, "the challenge still points at the discovery document").toContain( - new URL("/.well-known/oauth-protected-resource/mcp", target.baseUrl).toString(), + expect(wwwAuth, "the challenge still points at v1's alias of the discovery document").toContain( + `resource_metadata="${new URL(V1_RESOURCE_METADATA_PATH, target.baseUrl).toString()}"`, ); expect(yield* Effect.promise(() => response.json()), "the body is a JSON-RPC error").toEqual({ jsonrpc: "2.0",