diff --git a/e2e/selfhost/mcp-oauth-resource-challenge.test.ts b/e2e/selfhost/mcp-oauth-resource-challenge.test.ts new file mode 100644 index 000000000..18d4bea06 --- /dev/null +++ b/e2e/selfhost/mcp-oauth-resource-challenge.test.ts @@ -0,0 +1,82 @@ +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { connectEmulator } from "@executor-js/emulate"; +import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; +import { IntegrationSlug } from "@executor-js/sdk/shared"; + +import { createEmulatorInstance } from "../src/emulator-instance"; +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; + +const api = composePluginApi([mcpHttpPlugin()] as const); + +scenario( + "MCP OAuth · the endpoint challenge selects the resource used by browser authorization", + { timeout: 180_000 }, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: makeApiClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeApiClient(api, identity); + const baseUrl = yield* createEmulatorInstance("mcp", "resource-challenge"); + const emulator = yield* Effect.promise(() => connectEmulator({ baseUrl, service: "mcp" })); + const slug = IntegrationSlug.make(`resource_challenge_${randomBytes(4).toString("hex")}`); + + // The published emulator advertises root metadata in its Bearer challenge, + // while its path-scoped document describes a different resource (/mcp). + const probe = yield* client.oauth.probe({ payload: { url: `${baseUrl}/mcp` } }); + expect(probe.resource).toBe(baseUrl); + + yield* client.mcp.addServer({ + payload: { + transport: "remote", + name: "Resource challenge MCP", + endpoint: `${baseUrl}/mcp`, + slug, + authenticationTemplate: [{ kind: "oauth2" }], + }, + }); + yield* Effect.addFinalizer(() => + client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore), + ); + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the protected integration", async () => { + await visit(page, `/integrations/${slug}`); + await page.getByRole("button", { name: "Add connection" }).waitFor(); + }); + await step("Connect using the resource advertised by the server", async () => { + await page.getByRole("button", { name: "Add connection" }).click(); + const popupPromise = page.waitForEvent("popup", { timeout: 30_000 }); + await page.getByRole("button", { name: "Connect", exact: true }).click(); + const popup = await popupPromise; + await popup.waitForURL((url) => url.pathname.endsWith("/authorize"), { timeout: 60_000 }); + expect(new URL(popup.url()).searchParams.get("resource")).toBe(baseUrl); + // The hosted emulator's button omits its required login field. + // Authorize the synthetic account through its HTTP form contract. + const authorization = new URL(popup.url()); + const approved = await popup.request.post(`${baseUrl}/authorize/approve`, { + form: { ...Object.fromEntries(authorization.searchParams), login: "admin" }, + maxRedirects: 0, + }); + expect(approved.status()).toBe(302); + const callback = approved.headers()["location"]; + if (!callback) throw new Error("The emulator did not return an OAuth callback"); + await popup.goto(callback); + await popup.getByRole("heading", { name: "Connected" }).waitFor({ timeout: 30_000 }); + }); + }); + const clients = yield* client.oauth.listClients(); + expect(clients.some((app) => app.resource === baseUrl)).toBe(true); + const ledger = yield* Effect.promise(() => emulator.ledger.list()); + expect(ledger.some((entry) => entry.path === "/token" && entry.response.status === 200)).toBe( + true, + ); + }), + ), +); diff --git a/packages/core/sdk/src/insufficient-scope.ts b/packages/core/sdk/src/insufficient-scope.ts index 8ea3155e0..90c9b1083 100644 --- a/packages/core/sdk/src/insufficient-scope.ts +++ b/packages/core/sdk/src/insufficient-scope.ts @@ -24,6 +24,8 @@ // // A miss is benign: the failure stays on the existing classification. +import { parseChallenges } from "./www-authenticate"; + export type InsufficientScopeDetection = { /** Scopes the upstream named as required, when it named any (RFC 6750's * `scope` attribute). Empty when the provider only signalled the class of @@ -36,172 +38,6 @@ const MAX_DEPTH = 8; const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); -/** Parser for the whole WWW-Authenticate header per RFC 7235 §2.1: a - * comma-separated #list of challenges, each - * `scheme [ 1*SP ( token68 / #auth-param ) ]`. Implemented as an explicit - * per-challenge state machine so params can never attach across challenge - * boundaries or to a token68 credential: - * - * - "scheme": just read a scheme; accepts a token68 OR a first auth-param - * (space-separated, no comma). - * - "params": accepts further auth-params ONLY after a comma. - * - "token68": accepts nothing; any trailing param is malformed. - * - * Auth-params allow BWS around `=` (RFC 7230). Quoted-strings consume - * quoted-pairs whole and must end at a separator. ANY malformed shape — - * scheme-less params, space-separated param runs, params after token68, - * stray quotes/bytes — returns null and never classifies: a miss is benign, - * a false positive strips a valid recovery path. */ -type Challenge = { readonly scheme: string; readonly params: Map }; - -// HTTP `token` alphabet (RFC 7230 §3.2.6) — schemes and auth-param names. -const TOKEN_RE = /[A-Za-z0-9!#$%&'*+.^_`|~-]/; -// token68 alphabet (RFC 7235 §2.1), padding `=` handled separately. -const TOKEN68_RE = /[A-Za-z0-9._~+/-]/; -// Superset used by the word reader; each use site validates against the -// context-specific alphabet after reading. -const WORD_RE = /[A-Za-z0-9!#$%&'*+.^_`|~/-]/; - -const isToken = (word: string): boolean => [...word].every((ch) => TOKEN_RE.test(ch)); -// Unquoted URL values some providers emit (scheme://host/path?query): URI -// characters per RFC 3986, no whitespace/comma/quotes. -const isUrlish = (word: string): boolean => /^[A-Za-z][A-Za-z0-9+.-]*:\/\/[^\s,"]+$/.test(word); -const isToken68 = (word: string): boolean => [...word].every((ch) => TOKEN68_RE.test(ch)); - -const parseChallenges = (header: string): readonly Challenge[] | null => { - const len = header.length; - const challenges: Challenge[] = []; - let current: Challenge | null = null; - let state: "boundary" | "scheme" | "token68" | "params" = "boundary"; - let sawComma = true; // header start counts as a list boundary - let i = 0; - - const readWord = (): string => { - const start = i; - while (i < len && WORD_RE.test(header[i]!)) i += 1; - return header.slice(start, i); - }; - // Returns null on an unterminated quote or a quote run into the next token. - const readQuoted = (): string | null => { - let value = ""; - i += 1; // opening quote - while (i < len) { - const ch = header[i]!; - if (ch === '"') { - i += 1; - return i >= len || /[\s,]/.test(header[i]!) ? value : null; - } - if (ch === "\\" && i + 1 < len) { - value += header[i + 1]; - i += 2; - continue; - } - value += ch; - i += 1; - } - return null; // unterminated - }; - - while (i < len) { - while (i < len && /\s/.test(header[i]!)) i += 1; - if (i >= len) break; - if (header[i] === ",") { - sawComma = true; - i += 1; - continue; - } - if (!WORD_RE.test(header[i]!)) return null; // stray quote/byte: malformed - const word = readWord(); - // Look ahead through BWS for `=` to classify the word. - let j = i; - while (j < len && /[ \t]/.test(header[j]!)) j += 1; - const isPaddingRun = (() => { - // An `=`-run directly on the word (no BWS) that is followed (after - // optional whitespace) by a comma or the end of input is token68 - // padding. An `=` followed by a value — even across BWS — is an - // auth-param (RFC 7230 allows BWS around `=`). - if (header[i] !== "=") return false; - let k = i; - while (k < len && header[k] === "=") k += 1; - while (k < len && /[ \t]/.test(header[k]!)) k += 1; - return k >= len || header[k] === ","; - })(); - - if (isPaddingRun) { - // token68 with padding — only legal directly after a scheme. - if (state !== "scheme" || sawComma) return null; - if (!isToken68(word)) return null; - while (i < len && header[i] === "=") i += 1; - state = "token68"; - sawComma = false; - continue; - } - - if (header[j] === "=") { - // auth-param: `word BWS = BWS value`. - if (!isToken(word)) return null; // param name must be an HTTP token - if (current === null) return null; // scheme-less param - if (state === "token68") return null; // params after token68 - if (state === "scheme" && sawComma) return null; // "Bearer, a=b" - if (state === "params" && !sawComma) return null; // space-separated run - i = j + 1; - while (i < len && /[ \t]/.test(header[i]!)) i += 1; - let value: string; - if (header[i] === '"') { - const quoted = readQuoted(); - if (quoted === null) return null; - value = quoted; - } else { - const start = i; - while (i < len && !/[\s,]/.test(header[i]!)) i += 1; - value = header.slice(start, i); - // An unquoted value must be an HTTP token (`realm =,` / `realm=;` - // are malformed) — EXCEPT that real providers emit unquoted URLs for - // resource_metadata (observed live: Stripe), so URL-safe characters - // are tolerated there. The signal params (`error`, `scope`) stay - // token-strict. - if (value.length === 0) return null; - const lowerName = word.toLowerCase(); - if (!isToken(value) && !(lowerName === "resource_metadata" && isUrlish(value))) { - return null; - } - } - // Duplicate SIGNAL params (`error`, `scope`) within one challenge mean - // a header playing games — never classify. Other duplicates are - // tolerated first-wins: real providers emit them (observed live: - // Sentry duplicates resource_metadata). - const key = word.toLowerCase(); - if (current.params.has(key)) { - if (key === "error" || key === "scope") return null; - } else { - current.params.set(key, value); - } - state = "params"; - sawComma = false; - continue; - } - - // Bare word: a new challenge's scheme at a list boundary, a token68 - // directly after a scheme, malformed anywhere else. - if (sawComma) { - if (!isToken(word)) return null; // a scheme must be an HTTP token - current = { scheme: word.toLowerCase(), params: new Map() }; - challenges.push(current); - state = "scheme"; - sawComma = false; - continue; - } - if (state === "scheme") { - if (!isToken68(word)) return null; - state = "token68"; - continue; - } - return null; - } - - return challenges; -}; - const detectFromChallenge = (header: string): InsufficientScopeDetection | null => { const challenges = parseChallenges(header); if (challenges === null) return null; diff --git a/packages/core/sdk/src/oauth-discovery.ts b/packages/core/sdk/src/oauth-discovery.ts index bb30fec79..260bfc625 100644 --- a/packages/core/sdk/src/oauth-discovery.ts +++ b/packages/core/sdk/src/oauth-discovery.ts @@ -29,6 +29,7 @@ import { createPkceCodeVerifier, type OAuthEndpointUrlPolicy, } from "./oauth-helpers"; +import { parseChallenges } from "./www-authenticate"; // --------------------------------------------------------------------------- // Errors @@ -250,8 +251,8 @@ const executeText = ( // --------------------------------------------------------------------------- // RFC 9728 — Protected Resource Metadata // -// Not covered by `oauth4webapi`. Hand-rolled probe: try the path-scoped -// well-known first, then the origin-scoped fallback. +// Follow the protected endpoint's advertised metadata URL before trying +// path-scoped and origin-scoped well-known locations (RFC 9728 section 5). // --------------------------------------------------------------------------- const buildResourceMetadataUrls = (resourceUrl: string): string[] => { @@ -278,6 +279,61 @@ const withResourceQueryParams = ( return parsed.toString(); }; +const discoverResourceMetadataChallenge = ( + resourceUrl: string, + options: DiscoveryRequestOptions, +): Effect.Effect => + provideHttpClient( + Effect.gen(function* () { + yield* validateEndpointUrl(resourceUrl, "resource", options.endpointUrlPolicy); + let request = HttpClientRequest.get( + withResourceQueryParams(resourceUrl, options.resourceQueryParams), + ).pipe(HttpClientRequest.setHeader("accept", "application/json")); + for (const [name, value] of Object.entries(options.resourceHeaders ?? {})) { + request = HttpClientRequest.setHeader(request, name, value); + } + if (options.mcpProtocolVersion) { + request = HttpClientRequest.setHeader( + request, + MCP_PROTOCOL_VERSION_HEADER, + options.mcpProtocolVersion, + ); + } + const client = yield* HttpClient.HttpClient; + // Read headers only: an MCP GET can open a long-lived event stream. + const response = yield* HttpClient.withScope(client) + .execute(request) + .pipe( + Effect.timeout(Duration.millis(options.timeoutMs ?? OAUTH2_DEFAULT_TIMEOUT_MS)), + Effect.mapError( + (cause) => + new OAuthDiscoveryError({ + message: "Failed to discover the protected resource authentication challenge", + cause, + }), + ), + ); + if (response.status !== 401 && response.status !== 403) return null; + const header = response.headers["www-authenticate"]; + if (header === undefined) return null; + const challenges = parseChallenges(header); + if (challenges === null) return null; + for (const challenge of challenges) { + if (challenge.scheme !== "bearer") continue; + const metadataUrl = challenge.params.get("resource_metadata"); + if (metadataUrl === undefined) continue; + return yield* validateEndpointUrl( + metadataUrl, + "resource_metadata", + options.endpointUrlPolicy, + ); + } + return null; + }).pipe(Effect.scoped), + options, + ); + +/** Discover RFC 9728 metadata, preferring an explicit Bearer challenge URL. */ export const discoverProtectedResourceMetadata = ( resourceUrl: string, options: DiscoveryRequestOptions = {}, @@ -286,12 +342,22 @@ export const discoverProtectedResourceMetadata = ( OAuthDiscoveryError > => Effect.gen(function* () { - for (const url of buildResourceMetadataUrls(resourceUrl)) { - const requestUrl = withResourceQueryParams(url, options.resourceQueryParams); + const advertisedUrl = yield* discoverResourceMetadataChallenge(resourceUrl, options); + const metadataUrls = + advertisedUrl === null ? buildResourceMetadataUrls(resourceUrl) : [advertisedUrl]; + for (const url of metadataUrls) { + // A challenge may name another origin. Never forward resource credentials there. + const sameOrigin = new URL(url).origin === new URL(resourceUrl).origin; + const requestUrl = withResourceQueryParams( + url, + sameOrigin ? options.resourceQueryParams : undefined, + ); let request = HttpClientRequest.get(requestUrl).pipe( HttpClientRequest.setHeader("accept", "application/json"), ); - for (const [name, value] of Object.entries(options.resourceHeaders ?? {})) { + for (const [name, value] of Object.entries( + sameOrigin ? (options.resourceHeaders ?? {}) : {}, + )) { request = HttpClientRequest.setHeader(request, name, value); } if (options.mcpProtocolVersion) { diff --git a/packages/core/sdk/src/www-authenticate.test.ts b/packages/core/sdk/src/www-authenticate.test.ts new file mode 100644 index 000000000..224034fb0 --- /dev/null +++ b/packages/core/sdk/src/www-authenticate.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from "@effect/vitest"; + +import { parseChallenges } from "./www-authenticate"; + +describe("authentication challenge metadata", () => { + it("keeps a Bearer metadata URL separate from other schemes and quoted text", () => { + const challenges = parseChallenges( + 'Basic realm="resource_metadata=wrong", resource_metadata="https://wrong.example", Bearer realm="OAuth", resource_metadata="https://api.example/metadata"', + ); + expect( + challenges + ?.find((challenge) => challenge.scheme === "bearer") + ?.params.get("resource_metadata"), + ).toBe("https://api.example/metadata"); + }); + + it("accepts the unquoted URL form emitted by providers", () => { + const challenges = parseChallenges("bearer resource_metadata=https://api.example/metadata"); + expect(challenges?.[0]?.params.get("resource_metadata")).toBe("https://api.example/metadata"); + }); + + it("rejects unterminated quoted metadata", () => { + expect(parseChallenges('Bearer resource_metadata="https://api.example/metadata')).toBeNull(); + }); +}); diff --git a/packages/core/sdk/src/www-authenticate.ts b/packages/core/sdk/src/www-authenticate.ts new file mode 100644 index 000000000..19ba84ff5 --- /dev/null +++ b/packages/core/sdk/src/www-authenticate.ts @@ -0,0 +1,166 @@ +/** Parser for the whole WWW-Authenticate header per RFC 7235 §2.1: a + * comma-separated #list of challenges, each + * `scheme [ 1*SP ( token68 / #auth-param ) ]`. Implemented as an explicit + * per-challenge state machine so params can never attach across challenge + * boundaries or to a token68 credential: + * + * - "scheme": just read a scheme; accepts a token68 OR a first auth-param + * (space-separated, no comma). + * - "params": accepts further auth-params ONLY after a comma. + * - "token68": accepts nothing; any trailing param is malformed. + * + * Auth-params allow BWS around `=` (RFC 7230). Quoted-strings consume + * quoted-pairs whole and must end at a separator. ANY malformed shape — + * scheme-less params, space-separated param runs, params after token68, + * stray quotes/bytes — returns null and never classifies: a miss is benign, + * a false positive strips a valid recovery path. */ +type Challenge = { readonly scheme: string; readonly params: Map }; + +// HTTP `token` alphabet (RFC 7230 §3.2.6) — schemes and auth-param names. +const TOKEN_RE = /[A-Za-z0-9!#$%&'*+.^_`|~-]/; +// token68 alphabet (RFC 7235 §2.1), padding `=` handled separately. +const TOKEN68_RE = /[A-Za-z0-9._~+/-]/; +// Superset used by the word reader; each use site validates against the +// context-specific alphabet after reading. +const WORD_RE = /[A-Za-z0-9!#$%&'*+.^_`|~/-]/; + +const isToken = (word: string): boolean => [...word].every((ch) => TOKEN_RE.test(ch)); +// Unquoted URL values some providers emit (scheme://host/path?query): URI +// characters per RFC 3986, no whitespace/comma/quotes. +const isUrlish = (word: string): boolean => /^[A-Za-z][A-Za-z0-9+.-]*:\/\/[^\s,"]+$/.test(word); +const isToken68 = (word: string): boolean => [...word].every((ch) => TOKEN68_RE.test(ch)); + +/** Parse authentication challenges without joining parameters across schemes. */ +export const parseChallenges = (header: string): readonly Challenge[] | null => { + const len = header.length; + const challenges: Challenge[] = []; + let current: Challenge | null = null; + let state: "boundary" | "scheme" | "token68" | "params" = "boundary"; + let sawComma = true; // header start counts as a list boundary + let i = 0; + + const readWord = (): string => { + const start = i; + while (i < len && WORD_RE.test(header[i]!)) i += 1; + return header.slice(start, i); + }; + // Returns null on an unterminated quote or a quote run into the next token. + const readQuoted = (): string | null => { + let value = ""; + i += 1; // opening quote + while (i < len) { + const ch = header[i]!; + if (ch === '"') { + i += 1; + return i >= len || /[\s,]/.test(header[i]!) ? value : null; + } + if (ch === "\\" && i + 1 < len) { + value += header[i + 1]; + i += 2; + continue; + } + value += ch; + i += 1; + } + return null; // unterminated + }; + + while (i < len) { + while (i < len && /\s/.test(header[i]!)) i += 1; + if (i >= len) break; + if (header[i] === ",") { + sawComma = true; + i += 1; + continue; + } + if (!WORD_RE.test(header[i]!)) return null; // stray quote/byte: malformed + const word = readWord(); + // Look ahead through BWS for `=` to classify the word. + let j = i; + while (j < len && /[ \t]/.test(header[j]!)) j += 1; + const isPaddingRun = (() => { + // An `=`-run directly on the word (no BWS) that is followed (after + // optional whitespace) by a comma or the end of input is token68 + // padding. An `=` followed by a value — even across BWS — is an + // auth-param (RFC 7230 allows BWS around `=`). + if (header[i] !== "=") return false; + let k = i; + while (k < len && header[k] === "=") k += 1; + while (k < len && /[ \t]/.test(header[k]!)) k += 1; + return k >= len || header[k] === ","; + })(); + + if (isPaddingRun) { + // token68 with padding — only legal directly after a scheme. + if (state !== "scheme" || sawComma) return null; + if (!isToken68(word)) return null; + while (i < len && header[i] === "=") i += 1; + state = "token68"; + sawComma = false; + continue; + } + + if (header[j] === "=") { + // auth-param: `word BWS = BWS value`. + if (!isToken(word)) return null; // param name must be an HTTP token + if (current === null) return null; // scheme-less param + if (state === "token68") return null; // params after token68 + if (state === "scheme" && sawComma) return null; // "Bearer, a=b" + if (state === "params" && !sawComma) return null; // space-separated run + i = j + 1; + while (i < len && /[ \t]/.test(header[i]!)) i += 1; + let value: string; + if (header[i] === '"') { + const quoted = readQuoted(); + if (quoted === null) return null; + value = quoted; + } else { + const start = i; + while (i < len && !/[\s,]/.test(header[i]!)) i += 1; + value = header.slice(start, i); + // An unquoted value must be an HTTP token (`realm =,` / `realm=;` + // are malformed) — EXCEPT that real providers emit unquoted URLs for + // resource_metadata (observed live: Stripe), so URL-safe characters + // are tolerated there. The signal params (`error`, `scope`) stay + // token-strict. + if (value.length === 0) return null; + const lowerName = word.toLowerCase(); + if (!isToken(value) && !(lowerName === "resource_metadata" && isUrlish(value))) { + return null; + } + } + // Duplicate SIGNAL params (`error`, `scope`) within one challenge mean + // a header playing games — never classify. Other duplicates are + // tolerated first-wins: real providers emit them (observed live: + // Sentry duplicates resource_metadata). + const key = word.toLowerCase(); + if (current.params.has(key)) { + if (key === "error" || key === "scope") return null; + } else { + current.params.set(key, value); + } + state = "params"; + sawComma = false; + continue; + } + + // Bare word: a new challenge's scheme at a list boundary, a token68 + // directly after a scheme, malformed anywhere else. + if (sawComma) { + if (!isToken(word)) return null; // a scheme must be an HTTP token + current = { scheme: word.toLowerCase(), params: new Map() }; + challenges.push(current); + state = "scheme"; + sawComma = false; + continue; + } + if (state === "scheme") { + if (!isToken68(word)) return null; + state = "token68"; + continue; + } + return null; + } + + return challenges; +};