diff --git a/src/__tests__/analytics-endpoints.test.ts b/src/__tests__/analytics-endpoints.test.ts index 7cd9b4a..52eeb3c 100644 --- a/src/__tests__/analytics-endpoints.test.ts +++ b/src/__tests__/analytics-endpoints.test.ts @@ -45,6 +45,7 @@ vi.mock("../config.js", () => ({ discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -83,6 +84,7 @@ const DEFAULT_TEST_CONFIG = { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -387,6 +389,7 @@ describe("analyticsAuth middleware", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -428,6 +431,7 @@ describe("analyticsAuth middleware", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -471,6 +475,7 @@ describe("analyticsAuth middleware", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -516,6 +521,7 @@ describe("analyticsAuth middleware", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", diff --git a/src/__tests__/analytics-server.test.ts b/src/__tests__/analytics-server.test.ts index 91aea23..7bd239e 100644 --- a/src/__tests__/analytics-server.test.ts +++ b/src/__tests__/analytics-server.test.ts @@ -33,6 +33,7 @@ vi.mock("../config.js", () => ({ discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -953,6 +954,7 @@ describe("Analytics server routes (HTTP-level)", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -994,6 +996,7 @@ describe("Analytics server routes (HTTP-level)", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -1021,6 +1024,7 @@ describe("Analytics server routes (HTTP-level)", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -1061,6 +1065,7 @@ describe("Analytics server routes (HTTP-level)", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -1115,6 +1120,7 @@ describe("Analytics server routes (HTTP-level)", () => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", diff --git a/src/__tests__/atlas-ratification-endpoints.test.ts b/src/__tests__/atlas-ratification-endpoints.test.ts index f3a2190..619d864 100644 --- a/src/__tests__/atlas-ratification-endpoints.test.ts +++ b/src/__tests__/atlas-ratification-endpoints.test.ts @@ -40,6 +40,7 @@ vi.mock("../config.js", async (importOriginal) => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -73,6 +74,7 @@ const DEFAULT_TEST_CONFIG = { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", diff --git a/src/__tests__/atlas-search-endpoint.test.ts b/src/__tests__/atlas-search-endpoint.test.ts index 7895f05..d65457e 100644 --- a/src/__tests__/atlas-search-endpoint.test.ts +++ b/src/__tests__/atlas-search-endpoint.test.ts @@ -73,6 +73,7 @@ vi.mock("../config.js", async (importOriginal) => { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -104,6 +105,7 @@ const DEFAULT_TEST_CONFIG = { discordPublicKey: "", notionToken: "", mcpJwtSecret: "x".repeat(32), + oauthConsentHmacKeys: ["a".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index 1233bc7..a7ed946 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -1136,6 +1136,7 @@ describe("config.ts", () => { process.env.GITHUB_TOKEN = "ghp_test"; process.env.GITHUB_WEBHOOK_SECRET = "secret"; process.env.MCP_JWT_SECRET = "x".repeat(64); + process.env.PATHFINDER_CONSENT_HMAC_KEY = "a".repeat(64); mockedExistsSync.mockReturnValue(true); mockedReadFileSync.mockReturnValue(makeYaml()); @@ -1465,6 +1466,133 @@ describe("config.ts", () => { }); }); + // ── oauthConsentHmacKeys ───────────────────────────────────────────────── + + describe("oauthConsentHmacKeys", () => { + beforeEach(() => { + process.env.PATHFINDER_CONFIG = "/tmp/test.yaml"; + process.env.DATABASE_URL = "postgresql://test"; + process.env.OPENAI_API_KEY = "sk-test"; + }); + + it("parses a single PATHFINDER_CONSENT_HMAC_KEY", async () => { + const k = "a".repeat(64); + process.env.PATHFINDER_CONSENT_HMAC_KEY = k; + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + + const { getConfig } = await freshImport(); + const cfg = getConfig(); + expect(cfg.oauthConsentHmacKeys).toEqual([k]); + }); + + it("parses comma-separated keys for rotation", async () => { + const k1 = "a".repeat(64); + const k2 = "b".repeat(48); + process.env.PATHFINDER_CONSENT_HMAC_KEY = `${k1},${k2}`; + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + + const { getConfig } = await freshImport(); + const cfg = getConfig(); + expect(cfg.oauthConsentHmacKeys).toEqual([k1, k2]); + }); + + it("trims whitespace around comma-separated entries", async () => { + const k1 = "a".repeat(64); + const k2 = "b".repeat(64); + process.env.PATHFINDER_CONSENT_HMAC_KEY = ` ${k1} , ${k2} `; + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + + const { getConfig } = await freshImport(); + const cfg = getConfig(); + expect(cfg.oauthConsentHmacKeys).toEqual([k1, k2]); + }); + + it("rejects keys shorter than 32 hex chars", async () => { + process.env.PATHFINDER_CONSENT_HMAC_KEY = "abcdef"; // way too short + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + + const { getConfig } = await freshImport(); + expect(() => getConfig()).toThrow( + /PATHFINDER_CONSENT_HMAC_KEY entries must be ≥32 hex chars/, + ); + }); + + it("rejects non-hex characters in keys", async () => { + // 64 chars but contains 'g' (not hex) + process.env.PATHFINDER_CONSENT_HMAC_KEY = "g".repeat(64); + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + + const { getConfig } = await freshImport(); + expect(() => getConfig()).toThrow( + /PATHFINDER_CONSENT_HMAC_KEY entries must be ≥32 hex chars/, + ); + }); + + it("rejects when any key in a rotation list is invalid", async () => { + const good = "a".repeat(64); + const bad = "short"; + process.env.PATHFINDER_CONSENT_HMAC_KEY = `${good},${bad}`; + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + + const { getConfig } = await freshImport(); + expect(() => getConfig()).toThrow( + /PATHFINDER_CONSENT_HMAC_KEY entries must be ≥32 hex chars/, + ); + }); + + it("throws in production when PATHFINDER_CONSENT_HMAC_KEY is unset", async () => { + process.env.NODE_ENV = "production"; + process.env.MCP_JWT_SECRET = "x".repeat(64); + delete process.env.PATHFINDER_CONSENT_HMAC_KEY; + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + + const { getConfig } = await freshImport(); + expect(() => getConfig()).toThrow( + /PATHFINDER_CONSENT_HMAC_KEY is required in production/, + ); + }); + + it("generates an ephemeral key in development with a WARN log", async () => { + process.env.NODE_ENV = "development"; + delete process.env.PATHFINDER_CONSENT_HMAC_KEY; + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const { getConfig } = await freshImport(); + const cfg = getConfig(); + expect(cfg.oauthConsentHmacKeys).toHaveLength(1); + expect(cfg.oauthConsentHmacKeys[0]).toMatch(/^[0-9a-f]{64}$/); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching( + /\[oauth\] PATHFINDER_CONSENT_HMAC_KEY not set — generated an ephemeral consent-nonce key for development\./, + ), + ); + warnSpy.mockRestore(); + }); + + it("treats whitespace-only PATHFINDER_CONSENT_HMAC_KEY as unset (dev fallback)", async () => { + process.env.NODE_ENV = "development"; + process.env.PATHFINDER_CONSENT_HMAC_KEY = " "; + mockedExistsSync.mockReturnValue(true); + mockedReadFileSync.mockReturnValue(makeYaml()); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const { getConfig } = await freshImport(); + const cfg = getConfig(); + expect(cfg.oauthConsentHmacKeys).toHaveLength(1); + expect(cfg.oauthConsentHmacKeys[0]).toMatch(/^[0-9a-f]{64}$/); + warnSpy.mockRestore(); + }); + }); + // ── Config caching ─────────────────────────────────────────────────────── describe("caching", () => { diff --git a/src/__tests__/oauth-consent-handler.test.ts b/src/__tests__/oauth-consent-handler.test.ts new file mode 100644 index 0000000..e4b44aa --- /dev/null +++ b/src/__tests__/oauth-consent-handler.test.ts @@ -0,0 +1,482 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +// Mock config — the consent handler reads getConfig().oauthConsentHmacKeys +// to verify the signed nonce. Use a stable key so tests can sign payloads +// with the same value via the real signConsentNonce. +vi.mock("../config.js", () => ({ + getConfig: vi.fn().mockReturnValue({ + port: 3001, + databaseUrl: "pglite:///tmp/test", + openaiApiKey: "", + githubToken: "", + githubWebhookSecret: "", + nodeEnv: "test", + logLevel: "info", + cloneDir: "/tmp/test", + slackBotToken: "", + slackSigningSecret: "", + discordBotToken: "", + discordPublicKey: "", + notionToken: "", + mcpJwtSecret: "a".repeat(64), + oauthConsentHmacKeys: ["a".repeat(64)], + p2pTelemetryUrl: undefined, + p2pTelemetryDisabled: false, + packageVersion: "test", + }), + getServerConfig: vi.fn(), + getAnalyticsConfig: vi.fn(), + hasSearchTools: vi.fn().mockReturnValue(false), + hasKnowledgeTools: vi.fn().mockReturnValue(false), + hasCollectTools: vi.fn().mockReturnValue(false), + hasBashSemanticSearch: vi.fn().mockReturnValue(false), +})); + +import { consentHandler } from "../oauth/consent-handler.js"; +import { + signConsentNonce, + type ConsentNoncePayload, +} from "../oauth/consent-nonce.js"; +import { clientStore, codeStore } from "../oauth/store.js"; +import { setTrustingProxy } from "../oauth/trusted-client-ip.js"; +import { consentLimiter } from "../oauth/rate-limiter.js"; + +const KEY = "a".repeat(64); +const TOKEN_SCOPE = "mcp"; + +function mockReq( + overrides: Record = {}, +): Record { + return { + headers: { + host: "mcp.example.com", + "x-forwarded-proto": "https", + }, + query: {}, + body: {}, + socket: { remoteAddress: "1.2.3.4" }, + ...overrides, + }; +} + +function mockRes() { + const json = vi.fn(); + const send = vi.fn(); + const redirect = vi.fn(); + const setHeader = vi.fn(); + const status = vi.fn().mockImplementation(() => ({ json, send })); + return { + json, + send, + redirect, + setHeader, + status, + get statusCode() { + return status.mock.calls.at(-1)?.[0]; + }, + }; +} + +function basePayload( + over: Partial = {}, +): ConsentNoncePayload { + return { + client_id: "cid", + redirect_uri: "https://legitimate.example/cb", + state: "state-abc", + code_challenge: "cc", + code_challenge_method: "S256", + response_type: "code", + scope: TOKEN_SCOPE, + resource: "", + exp: Date.now() + 600_000, + ...over, + }; +} + +function bodyFor(p: ConsentNoncePayload, nonce: string, decision: string) { + return { + nonce, + client_id: p.client_id, + redirect_uri: p.redirect_uri, + state: p.state, + code_challenge: p.code_challenge, + code_challenge_method: p.code_challenge_method, + response_type: p.response_type, + scope: p.scope, + resource: p.resource, + decision, + }; +} + +function resetClientStore() { + const cs = clientStore as unknown as { + clients: Map; + byIp: Map>; + clientIpOf: Map; + }; + cs.clients.clear(); + cs.byIp.clear(); + cs.clientIpOf.clear(); + const cds = codeStore as unknown as { codes: Map }; + cds.codes.clear(); +} + +function registerKnownClient(client_id: string, redirect_uri: string) { + // ClientStore.register generates its own client_id, so we poke the + // internal Map directly with the bound id from our payload — fast and + // keeps tests deterministic. + const cs = clientStore as unknown as { + clients: Map; + byIp: Map>; + clientIpOf: Map; + }; + const now = Date.now(); + cs.clients.set(client_id, { + client_id, + client_secret: "s", + client_id_issued_at: Math.floor(now / 1000), + client_secret_issued_at: Math.floor(now / 1000), + client_secret_expires_at: 0, + redirect_uris: [redirect_uri], + client_name: "Test App", + registeredAt: now, + lastUsedAt: now, + }); + cs.clientIpOf.set(client_id, "1.2.3.4"); + let set = cs.byIp.get("1.2.3.4"); + if (!set) { + set = new Set(); + cs.byIp.set("1.2.3.4", set); + } + set.add(client_id); +} + +beforeEach(() => { + resetClientStore(); + // Reset rate-limiter buckets so per-IP windows don't leak across tests + // (or across files under fork reuse). + ( + consentLimiter as unknown as { buckets: Map } + ).buckets.clear(); + setTrustingProxy(() => false); +}); + +afterEach(() => { + setTrustingProxy(() => false); +}); + +describe("consentHandler — approve", () => { + it("approve happy path → 302 to nonce-bound redirect_uri with code + state", () => { + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "approve") }); + const res = mockRes(); + + consentHandler(req as never, res as never); + + expect(res.redirect).toHaveBeenCalledTimes(1); + const target = new URL(res.redirect.mock.calls[0]![0] as string); + expect(target.hostname).toBe("legitimate.example"); + expect(target.pathname).toBe("/cb"); + expect(target.searchParams.get("state")).toBe("state-abc"); + expect(target.searchParams.get("code")).toBeTruthy(); + }); + + it("approve touches the client (lastUsedAt bumped)", () => { + // Use fake timers so we deterministically observe a strictly-later + // `lastUsedAt` than the registration timestamp without burning CPU + // and without flaking on slow runners. + vi.useFakeTimers(); + try { + const baseMs = new Date(2026, 0, 1, 0, 0, 0).getTime(); + vi.setSystemTime(new Date(baseMs)); + const p = basePayload({ exp: baseMs + 600_000 }); + registerKnownClient(p.client_id, p.redirect_uri); + const cs = clientStore as unknown as { + clients: Map; + }; + const beforeTs = cs.clients.get(p.client_id)!.lastUsedAt; + expect(beforeTs).toBe(baseMs); + // Advance the clock so consentHandler -> clientStore.touch() observes + // a strictly-later Date.now(). + vi.setSystemTime(new Date(baseMs + 5)); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(cs.clients.get(p.client_id)!.lastUsedAt).toBe(baseMs + 5); + expect(cs.clients.get(p.client_id)!.lastUsedAt).toBeGreaterThan(beforeTs); + } finally { + vi.useRealTimers(); + } + }); +}); + +describe("consentHandler — deny", () => { + it("deny → 302 to nonce-bound redirect_uri with error=access_denied + state", () => { + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "deny") }); + const res = mockRes(); + + consentHandler(req as never, res as never); + + expect(res.redirect).toHaveBeenCalledTimes(1); + const target = new URL(res.redirect.mock.calls[0]![0] as string); + expect(target.hostname).toBe("legitimate.example"); + expect(target.searchParams.get("error")).toBe("access_denied"); + expect(target.searchParams.get("state")).toBe("state-abc"); + expect(target.searchParams.get("code")).toBeNull(); + }); + + it("deny redirects to NONCE-BOUND redirect_uri even when form body matches (positive proof)", () => { + // Sign a nonce naming the legitimate URI; submit a form body that + // ALSO names the legitimate URI (otherwise step 3 fires first). The + // assertion is on the hostname the handler actually redirects to, + // proving that the deny branch reads p.redirect_uri (not body). + const p = basePayload({ redirect_uri: "https://legitimate.example/cb" }); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "deny") }); + const res = mockRes(); + + consentHandler(req as never, res as never); + + expect(res.redirect).toHaveBeenCalledTimes(1); + const target = new URL(res.redirect.mock.calls[0]![0] as string); + expect(target.hostname).toBe("legitimate.example"); + }); + + it("tampered form-body redirect_uri (different from nonce payload) → 400, NO redirect to tampered URI", () => { + // Negative proof: body.redirect_uri differs from payload.redirect_uri, + // step (3) fires before the decision branch is even reached, so the + // handler 400s and does NOT redirect anywhere — and crucially does + // not redirect to the attacker-controlled body URI. + const p = basePayload({ redirect_uri: "https://legitimate.example/cb" }); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const body = bodyFor(p, nonce, "deny"); + body.redirect_uri = "https://attacker.example/cb"; + const req = mockReq({ body }); + const res = mockRes(); + + consentHandler(req as never, res as never); + + expect(res.redirect).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(400); + }); +}); + +describe("consentHandler — field mismatch (step 3)", () => { + it("client_id swap in form body → 400 invalid_request field_mismatch", () => { + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const body = bodyFor(p, nonce, "approve"); + body.client_id = "other"; + const req = mockReq({ body }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.redirect).not.toHaveBeenCalled(); + }); + + it("state tampered → 400, no redirect (proves step 3 gates ALL flow incl. deny)", () => { + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const body = bodyFor(p, nonce, "deny"); + body.state = "tampered"; + const req = mockReq({ body }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.redirect).not.toHaveBeenCalled(); + }); +}); + +describe("consentHandler — nonce failures", () => { + it("forged MAC → 400 with reason hmac", () => { + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const goodNonce = signConsentNonce(p, [KEY]); + // Replace MAC half with garbage of the same length. + const [payloadB64] = goodNonce.split("."); + const forged = `${payloadB64}.${Buffer.from("x".repeat(32)).toString("base64url")}`; + const req = mockReq({ body: bodyFor(p, forged, "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + const arg = res.status.mock.results[0]!.value.json.mock.calls[0]![0]; + expect(String(arg.error_description)).toContain("hmac"); + }); + + it("expired nonce → 400 with reason expired", () => { + const p = basePayload({ exp: Date.now() - 1 }); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + const arg = res.status.mock.results[0]!.value.json.mock.calls[0]![0]; + expect(String(arg.error_description)).toContain("expired"); + }); + + it("malformed nonce token → 400 with reason format", () => { + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const req = mockReq({ body: bodyFor(p, "not-a-token", "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + }); +}); + +describe("consentHandler — client + policy", () => { + it("unknown client_id (deleted between sign and consent) → 400 unauthorized_client + logs consent_stale_client", () => { + const warnSpy = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + const p = basePayload({ client_id: "nonexistent" }); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + const arg = res.status.mock.results[0]!.value.json.mock.calls[0]![0]; + expect(arg.error).toBe("unauthorized_client"); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching( + /^\[oauth\] consent_stale_client .*client_id=nonexistent/, + ), + ); + warnSpy.mockRestore(); + }); + + it("scope mismatch (post-MAC) → 400 invalid_scope + logs consent_scope_mismatch", () => { + const warnSpy = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + // Sign a payload whose scope is NOT the supported value. Step 3 + // requires the form body to equal the payload, so the body carries + // the same bogus scope — the mismatch surfaces at step 6a. + const p = basePayload({ scope: "bogus" }); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + const arg = res.status.mock.results[0]!.value.json.mock.calls[0]![0]; + expect(arg.error).toBe("invalid_scope"); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching(/^\[oauth\] consent_scope_mismatch .*scope=bogus/), + ); + warnSpy.mockRestore(); + }); + + it("unsupported response_type or code_challenge_method → 400 + logs consent_param_unsupported", () => { + const warnSpy = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + const p = basePayload({ code_challenge_method: "plain" }); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + const arg = res.status.mock.results[0]!.value.json.mock.calls[0]![0]; + expect(arg.error).toBe("invalid_request"); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching( + /^\[oauth\] consent_param_unsupported .*code_challenge_method=plain/, + ), + ); + warnSpy.mockRestore(); + }); + + it("redirect_uri no longer in client's registered list → 400 invalid_redirect_uri", () => { + const p = basePayload(); + // Register client with a DIFFERENT redirect_uri so the nonce-bound + // URI isn't on the list. + registerKnownClient(p.client_id, "https://other.example/cb"); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "approve") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + const arg = res.status.mock.results[0]!.value.json.mock.calls[0]![0]; + expect(arg.error).toBe("invalid_redirect_uri"); + }); +}); + +describe("consentHandler — invalid decision", () => { + it("decision value neither approve nor deny → 400 + logs consent_unknown_decision", () => { + const warnSpy = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const req = mockReq({ body: bodyFor(p, nonce, "maybe") }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.redirect).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching( + /^\[oauth\] consent_unknown_decision .*decision=maybe/, + ), + ); + warnSpy.mockRestore(); + }); + + it("missing decision → 400", () => { + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + const body = bodyFor(p, nonce, "approve") as Record; + delete body.decision; + const req = mockReq({ body }); + const res = mockRes(); + consentHandler(req as never, res as never); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.redirect).not.toHaveBeenCalled(); + }); +}); + +describe("consentHandler — rate limit", () => { + it("exceeding consentLimiter (30/min) → 429 with Retry-After + logs rate_limited endpoint=consent", async () => { + const warnSpy = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + const p = basePayload(); + registerKnownClient(p.client_id, p.redirect_uri); + const nonce = signConsentNonce(p, [KEY]); + + // Burn through the limiter from a single IP. + let last: ReturnType | undefined; + for (let i = 0; i < 31; i++) { + const req = mockReq({ + body: bodyFor(p, nonce, "approve"), + socket: { remoteAddress: "9.9.9.9" }, + }); + last = mockRes(); + consentHandler(req as never, last as never); + } + expect(last!.status).toHaveBeenCalledWith(429); + expect(last!.setHeader).toHaveBeenCalledWith( + "Retry-After", + expect.any(String), + ); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching(/^\[oauth\] rate_limited .*endpoint=consent/), + ); + warnSpy.mockRestore(); + }); +}); diff --git a/src/__tests__/oauth-consent-nonce.test.ts b/src/__tests__/oauth-consent-nonce.test.ts new file mode 100644 index 0000000..13c182d --- /dev/null +++ b/src/__tests__/oauth-consent-nonce.test.ts @@ -0,0 +1,369 @@ +import { describe, it, expect } from "vitest"; +import { + signConsentNonce, + verifyConsentNonce, + __testOnly_canonicalBytes, + type ConsentNoncePayload, +} from "../oauth/consent-nonce.js"; + +const k1 = "a".repeat(64); +const k2 = "b".repeat(64); + +function payload(over: Partial = {}): ConsentNoncePayload { + return { + client_id: "cid", + redirect_uri: "https://example.com/cb", + state: "s", + code_challenge: "cc", + code_challenge_method: "S256", + response_type: "code", + scope: "mcp", + resource: "", + exp: Date.now() + 600_000, + ...over, + }; +} + +function decodePayload(token: string): Record { + const b64 = token.split(".")[0]!; + return JSON.parse(Buffer.from(b64, "base64url").toString("utf8")); +} + +function reencode( + decoded: Record, + originalToken: string, +): string { + const mac = originalToken.split(".")[1]!; + const b64 = Buffer.from(JSON.stringify(decoded), "utf8").toString( + "base64url", + ); + return `${b64}.${mac}`; +} + +describe("consent nonce", () => { + it("round-trips a valid payload", () => { + const tok = signConsentNonce(payload(), [k1]); + const r = verifyConsentNonce(tok, [k1]); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.payload.client_id).toBe("cid"); + expect(r.payload.redirect_uri).toBe("https://example.com/cb"); + expect(r.payload.scope).toBe("mcp"); + } + }); + + it("rejects tampered payload (any field flip → hmac fail)", () => { + const tok = signConsentNonce(payload({ client_id: "a" }), [k1]); + const decoded = decodePayload(tok); + decoded.client_id = "b"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("rejects expired nonce", () => { + const tok = signConsentNonce(payload({ exp: Date.now() - 1 }), [k1]); + const r = verifyConsentNonce(tok, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("expired"); + }); + + it("rejects wrong key", () => { + const tok = signConsentNonce(payload(), [k1]); + const r = verifyConsentNonce(tok, [k2]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("multi-key verify accepts old key (rotation)", () => { + const tok = signConsentNonce(payload(), [k1]); + const r = verifyConsentNonce(tok, [k2, k1]); + expect(r.ok).toBe(true); + }); + + it("multi-key verify: signer uses keys[0] (new primary)", () => { + // Sign with k2 first (primary), and verify still works when listed first. + const tok = signConsentNonce(payload(), [k2, k1]); + const r1 = verifyConsentNonce(tok, [k2]); + expect(r1.ok).toBe(true); + const r2 = verifyConsentNonce(tok, [k1]); + expect(r2.ok).toBe(false); // wasn't signed with k1 + }); + + it("rejects malformed format (no dot)", () => { + const r = verifyConsentNonce("notatoken", [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("format"); + }); + + it("rejects empty payload half", () => { + const r = verifyConsentNonce(".abc", [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("format"); + }); + + it("rejects empty mac half", () => { + const r = verifyConsentNonce("abc.", [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("format"); + }); + + it("rejects unparseable JSON payload", () => { + const garbage = Buffer.from("not json", "utf8").toString("base64url"); + const r = verifyConsentNonce(`${garbage}.YWJj`, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("format"); + }); + + it("rejects payload missing required fields (format)", () => { + // Build a valid-looking token but strip a required field. + const tok = signConsentNonce(payload(), [k1]); + const decoded = decodePayload(tok); + delete decoded.scope; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + // Missing string field fails format check (before MAC). + if (!r.ok) expect(r.reason).toBe("format"); + }); + + it("rejects payload with non-numeric exp (format)", () => { + const tok = signConsentNonce(payload(), [k1]); + const decoded = decodePayload(tok); + decoded.exp = "not a number"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("format"); + }); + + it("MAC-then-exp ordering: tampered MAC with valid exp → hmac, NOT expired", () => { + // Future exp is fine; flip a payload byte so MAC fails. We must + // get reason=hmac, not reason=expired (we must not leak expiry). + const tok = signConsentNonce(payload({ exp: Date.now() + 60_000 }), [k1]); + const decoded = decodePayload(tok); + decoded.client_id = "different"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("MAC-then-exp ordering: garbage MAC with expired exp → hmac, NOT expired", () => { + // Build a properly-shaped token but with a bogus MAC. With a now-1 + // exp, a verifier that checked exp first would return "expired" — + // we must return "hmac" because the MAC fails. + const validTok = signConsentNonce(payload({ exp: Date.now() - 1 }), [k1]); + const bogusMac = Buffer.from("0".repeat(32), "utf8").toString("base64url"); + const tampered = `${validTok.split(".")[0]}.${bogusMac}`; + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("response_type is part of the MAC", () => { + const tok = signConsentNonce(payload({ response_type: "code" }), [k1]); + const decoded = decodePayload(tok); + decoded.response_type = "token"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("code_challenge_method is part of the MAC", () => { + const tok = signConsentNonce(payload({ code_challenge_method: "S256" }), [ + k1, + ]); + const decoded = decodePayload(tok); + decoded.code_challenge_method = "plain"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("resource is part of the MAC (aud-swap prevention)", () => { + const tok = signConsentNonce( + payload({ resource: "https://x.example/r1" }), + [k1], + ); + const decoded = decodePayload(tok); + decoded.resource = "https://y.example/r2"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("redirect_uri is part of the MAC", () => { + const tok = signConsentNonce( + payload({ redirect_uri: "https://a.example/cb" }), + [k1], + ); + const decoded = decodePayload(tok); + decoded.redirect_uri = "https://attacker.example/cb"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("state is part of the MAC", () => { + const tok = signConsentNonce(payload({ state: "alpha" }), [k1]); + const decoded = decodePayload(tok); + decoded.state = "beta"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("scope is part of the MAC", () => { + const tok = signConsentNonce(payload({ scope: "mcp" }), [k1]); + const decoded = decodePayload(tok); + decoded.scope = "mcp admin"; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("exp is part of the MAC (cannot extend validity)", () => { + const tok = signConsentNonce(payload({ exp: Date.now() + 1_000 }), [k1]); + const decoded = decodePayload(tok); + decoded.exp = Date.now() + 10_000_000; + const tampered = reencode(decoded, tok); + const r = verifyConsentNonce(tampered, [k1]); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); + + it("sign produces distinct MACs for distinct payloads", () => { + // Sanity check: two payloads whose bytes already differ at index 1 + // (regardless of length-prefixing) produce different MACs. This + // does NOT prove the length-prefix property — see the dedicated + // length-prefix tests below for that. + const p1 = payload({ client_id: "ab", redirect_uri: "https://c.example/" }); + const p2 = payload({ client_id: "a", redirect_uri: "https://bc.example/" }); + const tok1 = signConsentNonce(p1, [k1]); + const tok2 = signConsentNonce(p2, [k1]); + const mac1 = tok1.split(".")[1]!; + const mac2 = tok2.split(".")[1]!; + expect(mac1).not.toBe(mac2); + }); + + it("canonicalBytes prepends 4-byte big-endian length per field", () => { + // Pin the exact wire layout. A regression that dropped the length + // prefix, switched to little-endian, or reordered fields would + // produce a different byte string and fail this assertion + // directly — independent of MAC inequality on chosen inputs. + const fixed: ConsentNoncePayload = { + client_id: "cid", + redirect_uri: "https://example.com/cb", + state: "s", + code_challenge: "cc", + code_challenge_method: "S256", + response_type: "code", + scope: "mcp", + resource: "", + exp: 1_700_000_000_000, + }; + const FIELDS_ORDER: Array<[string, string]> = [ + ["client_id", fixed.client_id], + ["redirect_uri", fixed.redirect_uri], + ["state", fixed.state], + ["code_challenge", fixed.code_challenge], + ["code_challenge_method", fixed.code_challenge_method], + ["response_type", fixed.response_type], + ["scope", fixed.scope], + ["resource", fixed.resource], + ["exp", String(fixed.exp)], + ]; + const parts: Buffer[] = []; + for (const [, v] of FIELDS_ORDER) { + const valBuf = Buffer.from(v, "utf8"); + const lenBuf = Buffer.alloc(4); + lenBuf.writeUInt32BE(valBuf.length, 0); + parts.push(lenBuf, valBuf); + } + const expected = Buffer.concat(parts); + expect(__testOnly_canonicalBytes(fixed)).toEqual(expected); + }); + + it("length-prefix prevents field-boundary ambiguity", () => { + // Construct two payloads whose NAIVE (no-prefix) byte concatenation + // would be identical: shifting bytes across the client_id / + // redirect_uri boundary yields the same raw stream. Without length + // prefixes, the MACs would collide. With them, the prefix bytes + // for client_id (2 vs 3) differ, so the canonical bytes — and + // thus the MACs — MUST differ. + const empty = { + state: "", + code_challenge: "", + code_challenge_method: "", + response_type: "", + scope: "", + resource: "", + }; + const exp = 1_700_000_000_000; + const a: ConsentNoncePayload = { + client_id: "AB", + redirect_uri: "CD", + ...empty, + exp, + }; + const b: ConsentNoncePayload = { + client_id: "ABC", + redirect_uri: "D", + ...empty, + exp, + }; + + // Sanity: the naive (prefix-stripped) concatenation of all fields + // IS identical between a and b — so this pair specifically + // requires the length prefix to distinguish them. + const naive = (p: ConsentNoncePayload): string => + p.client_id + + p.redirect_uri + + p.state + + p.code_challenge + + p.code_challenge_method + + p.response_type + + p.scope + + p.resource + + String(p.exp); + expect(naive(a)).toBe(naive(b)); + + // Canonical (length-prefixed) bytes MUST differ. + expect(__testOnly_canonicalBytes(a)).not.toEqual( + __testOnly_canonicalBytes(b), + ); + + // MACs MUST differ. + const tokA = signConsentNonce(a, [k1]); + const tokB = signConsentNonce(b, [k1]); + expect(tokA.split(".")[1]).not.toBe(tokB.split(".")[1]); + }); + + it("injectable now() enforces expiry against fixed clock", () => { + const tok = signConsentNonce(payload({ exp: 1_000 }), [k1]); + const r1 = verifyConsentNonce(tok, [k1], () => 500); + expect(r1.ok).toBe(true); + const r2 = verifyConsentNonce(tok, [k1], () => 2_000); + expect(r2.ok).toBe(false); + if (!r2.ok) expect(r2.reason).toBe("expired"); + }); + + it("signConsentNonce throws on empty key set", () => { + expect(() => signConsentNonce(payload(), [])).toThrow(); + }); + + it("verifyConsentNonce rejects with empty key set (no MAC match)", () => { + const tok = signConsentNonce(payload(), [k1]); + const r = verifyConsentNonce(tok, []); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("hmac"); + }); +}); diff --git a/src/__tests__/oauth-consent-template.test.ts b/src/__tests__/oauth-consent-template.test.ts new file mode 100644 index 0000000..7c0db44 --- /dev/null +++ b/src/__tests__/oauth-consent-template.test.ts @@ -0,0 +1,130 @@ +import { describe, it, expect } from "vitest"; +import { renderConsentHtml } from "../oauth/consent-template.js"; + +const base = { + clientName: "Demo App", + clientId: "cid", + redirectUri: "https://app.example.com/cb", + redirectUriHostname: "app.example.com", + scope: "mcp", + state: "abc", + codeChallenge: "cc", + codeChallengeMethod: "S256", + responseType: "code", + resource: "", + nonce: "nonce.token", +}; + +describe("renderConsentHtml", () => { + it("escapes client_name HTML", () => { + const html = renderConsentHtml({ + ...base, + clientName: "", + }); + expect(html).not.toContain(""); + expect(html).toContain("<script>"); + }); + + it("escapes redirect_uri attribute context", () => { + const html = renderConsentHtml({ + ...base, + redirectUri: 'https://a"b.example/', + }); + expect(html).toContain('value="https://a"b.example/"'); + }); + + it("renders the hostname in bold, full uri in code", () => { + const html = renderConsentHtml(base); + expect(html).toMatch(/]*>app\.example\.com<\/strong>/); + expect(html).toMatch(/]*>https:\/\/app\.example\.com\/cb<\/code>/); + }); + + it("posts to /authorize/consent with the nonce + every bound field", () => { + const html = renderConsentHtml(base); + expect(html).toContain('action="/authorize/consent"'); + expect(html).toContain('name="nonce" value="nonce.token"'); + expect(html).toContain('name="client_id" value="cid"'); + expect(html).toContain( + 'name="redirect_uri" value="https://app.example.com/cb"', + ); + expect(html).toContain('name="state" value="abc"'); + expect(html).toContain('name="code_challenge" value="cc"'); + expect(html).toContain('name="code_challenge_method" value="S256"'); + expect(html).toContain('name="response_type" value="code"'); + expect(html).toContain('name="scope" value="mcp"'); + }); + + it("includes resource hidden input even when empty", () => { + const html = renderConsentHtml(base); + expect(html).toContain('name="resource"'); + }); + + it("propagates a non-empty resource value (escaped)", () => { + const html = renderConsentHtml({ + ...base, + resource: 'https://api.example.com/">', + }); + expect(html).not.toContain('">'); + expect(html).toContain( + 'name="nonce" value=""><script>alert(1)</script>"', + ); + }); + + it("escapes hostname display element", () => { + const html = renderConsentHtml({ + ...base, + redirectUriHostname: "", + }); + expect(html).toMatch(/]*><img src=x><\/strong>/); + expect(html).not.toContain(""); + }); + + it("returns a self-contained HTML document with doctype", () => { + const html = renderConsentHtml(base); + expect(html.toLowerCase()).toContain(""); + expect(html).toContain(""); + }); + + it("uses POST method on the form", () => { + const html = renderConsentHtml(base); + expect(html).toMatch(/]*method="POST"/i); + }); + + it("does not include any script tags", () => { + const html = renderConsentHtml(base); + expect(html).not.toMatch(/]/i); + }); +}); diff --git a/src/__tests__/oauth-e2e.test.ts b/src/__tests__/oauth-e2e.test.ts index f28adc0..3331147 100644 --- a/src/__tests__/oauth-e2e.test.ts +++ b/src/__tests__/oauth-e2e.test.ts @@ -1,10 +1,22 @@ -import { describe, it, expect, beforeAll, afterAll, vi } from "vitest"; +import { + describe, + it, + expect, + beforeAll, + beforeEach, + afterAll, + vi, +} from "vitest"; import express from "express"; import type { Server } from "node:http"; import type { AddressInfo } from "node:net"; import { createHash, randomBytes } from "node:crypto"; // Use a stable secret so the handlers and our verifier both agree. +// `oauthConsentHmacKeys` MUST be present — the consent screen rendered by +// GET /authorize signs an HMAC nonce over the bound set, and POST +// /authorize/consent re-verifies it on the way back. Without this key, +// signConsentNonce throws and /authorize 500s. vi.mock("../config.js", () => ({ getConfig: vi.fn().mockReturnValue({ port: 0, @@ -21,6 +33,7 @@ vi.mock("../config.js", () => ({ discordPublicKey: "", notionToken: "", mcpJwtSecret: "e".repeat(64), + oauthConsentHmacKeys: ["c".repeat(64)], p2pTelemetryUrl: undefined, p2pTelemetryDisabled: false, packageVersion: "test", @@ -43,6 +56,15 @@ import { bearerMiddleware, type AuthContext, } from "../oauth/handlers.js"; +import { consentHandler } from "../oauth/consent-handler.js"; +import { clientStore } from "../oauth/store.js"; +import { setTrustingProxy } from "../oauth/trusted-client-ip.js"; +import { + registerLimiter, + authorizeLimiter, + tokenLimiter, + consentLimiter, +} from "../oauth/rate-limiter.js"; function base64url(buf: Buffer | string): string { const b = typeof buf === "string" ? Buffer.from(buf) : buf; @@ -53,6 +75,28 @@ function base64url(buf: Buffer | string): string { .replace(/=+$/g, ""); } +/** + * Pull every `` out of the consent + * screen HTML. Values are HTML-attribute encoded by `escHtml` in the + * template, so we reverse the same five entities here. This is precisely + * the shape a real browser submission would round-trip (sans the visible + * `decision` button name, which the test supplies). + */ +function extractFormFields(html: string): Record { + const out: Record = {}; + const re = //g; + let m: RegExpExecArray | null; + while ((m = re.exec(html)) !== null) { + out[m[1]!] = m[2]! + .replace(/"/g, '"') + .replace(/'/g, "'") + .replace(/</g, "<") + .replace(/>/g, ">") + .replace(/&/g, "&"); + } + return out; +} + let server: Server; let baseUrl: string; @@ -68,6 +112,8 @@ beforeAll(async () => { ); app.post("/register", registerHandler); app.get("/authorize", authorizeHandler); + // The consent screen POSTs here; the e2e suite exercises both decisions. + app.post("/authorize/consent", consentHandler); app.post("/token", tokenHandler); app.post("/revoke", revocationHandler); @@ -80,6 +126,13 @@ beforeAll(async () => { }, ); + // Force the deterministic fail-safe so XFF-spoof scenarios reach their + // intended assertion. The e2e harness inherits process-global state + // from other test files; without this call `oauthClientIp` could fall + // back to a trusting accessor and the cap-bypass test would mis-route + // its IPs through XFF. + setTrustingProxy(() => false); + await new Promise((resolve) => { server = app.listen(0, () => resolve()); }); @@ -87,6 +140,39 @@ beforeAll(async () => { baseUrl = `http://127.0.0.1:${addr.port}`; }); +beforeEach(() => { + // Reset rate-limiter buckets — registerLimiter is 10/min from the same + // socket peer, and the e2e tests below hammer /register from 127.0.0.1 + // for the cap scenarios. Without a wipe the 11th call gets a generic + // 429 from the rate limiter before our per-IP cap (or total cap) is + // ever exercised. + ( + registerLimiter as unknown as { buckets: Map } + ).buckets.clear(); + ( + authorizeLimiter as unknown as { buckets: Map } + ).buckets.clear(); + ( + tokenLimiter as unknown as { buckets: Map } + ).buckets.clear(); + ( + consentLimiter as unknown as { buckets: Map } + ).buckets.clear(); + // Reset the client store between tests so per-IP / total caps are + // measured against a clean baseline. + const cs = clientStore as unknown as { + clients: Map; + byIp: Map>; + clientIpOf: Map; + }; + cs.clients.clear(); + cs.byIp.clear(); + cs.clientIpOf.clear(); + // Reset the deterministic trust-proxy fail-safe in case a previous test + // (or another file under the same fork) flipped it. + setTrustingProxy(() => false); +}); + afterAll(async () => { await new Promise((resolve, reject) => { server.close((err) => (err ? reject(err) : resolve())); @@ -94,7 +180,7 @@ afterAll(async () => { }); describe("OAuth 2.1 end-to-end ceremonial flow", () => { - it("completes register → authorize → token → /mcp with Bearer", async () => { + it("completes register → authorize → consent (approve) → token → /mcp with Bearer", async () => { // 1. POST /register const registerRes = await fetch(`${baseUrl}/register`, { method: "POST", @@ -111,7 +197,7 @@ describe("OAuth 2.1 end-to-end ceremonial flow", () => { const verifier = base64url(randomBytes(32)); const challenge = base64url(createHash("sha256").update(verifier).digest()); - // 3. GET /authorize + // 3. GET /authorize — now renders the consent HTML (200), not a 302. const authorizeUrl = new URL(`${baseUrl}/authorize`); authorizeUrl.searchParams.set("response_type", "code"); authorizeUrl.searchParams.set("client_id", client_id); @@ -122,13 +208,46 @@ describe("OAuth 2.1 end-to-end ceremonial flow", () => { const authRes = await fetch(authorizeUrl.toString(), { redirect: "manual", }); - expect(authRes.status).toBe(302); - const location = authRes.headers.get("location"); + expect(authRes.status).toBe(200); + expect(authRes.headers.get("content-type")).toMatch(/text\/html/); + const html = await authRes.text(); + // Sanity-check shape — the consent form must include the hidden bound + // set and the POST action our handler expects. + expect(html).toMatch( + /