diff --git a/app/api/auth/[...all]/route.ts b/app/api/auth/[...all]/route.ts index 4050d40c..a68103e4 100644 --- a/app/api/auth/[...all]/route.ts +++ b/app/api/auth/[...all]/route.ts @@ -63,7 +63,16 @@ async function handler(request: Request): Promise { if (!isAllowed(pathname)) { return new Response("Not Found", { status: 404 }); } - return auth.handler(request); + const response = await auth.handler(request); + // Stop shared caches from storing session-bearing responses. Set + // unconditionally across the allowlist (all non-cacheable surfaces), + // but guard with `has` so BA-owned directives pass through unchanged — + // notably the well-known discovery docs, which BA tags + // `public, max-age=15` before this wrapper runs. + if (!response.headers.has("cache-control")) { + response.headers.set("cache-control", "no-store"); + } + return response; } export const GET = handler; diff --git a/app/api/auth/oauth2/token/route.ts b/app/api/auth/oauth2/token/route.ts index f64c5afc..b6948b0d 100644 --- a/app/api/auth/oauth2/token/route.ts +++ b/app/api/auth/oauth2/token/route.ts @@ -6,6 +6,23 @@ const origin = new URL(baseUrl).origin; const mcpResource = `${origin}/api/mcp`; const grantsNeedingResource = new Set(["authorization_code", "refresh_token"]); +/** + * Backfill `Cache-Control: no-store` when the response does not already + * carry the header. Better Auth's oauth-provider sets `no-store` on the + * token response today (`@better-auth/oauth-provider/dist/index.mjs:600`), + * so this is a no-op-on-write; the guard makes the contract project-owned + * so a future BA upgrade that drops the header cannot silently regress it. + * + * @param response - Response returned by `auth.handler`. + * @returns The same response, with `Cache-Control: no-store` ensured. + */ +function withNoStore(response: Response): Response { + if (!response.headers.has("cache-control")) { + response.headers.set("cache-control", "no-store"); + } + return response; +} + /** * OAuth 2.0 token endpoint wrapper that defaults the `resource` parameter * for MCP clients that omit it. @@ -29,7 +46,7 @@ const grantsNeedingResource = new Set(["authorization_code", "refresh_token"]); export async function POST(request: Request): Promise { const contentType = request.headers.get("content-type") ?? ""; if (!contentType.includes("application/x-www-form-urlencoded")) { - return auth.handler(request); + return withNoStore(await auth.handler(request)); } const body = new URLSearchParams(await request.text()); @@ -49,5 +66,7 @@ export async function POST(request: Request): Promise { }); const response = await auth.handler(forwarded); - return logTokenGrant(response, grantType, body.get("scope") ?? ""); + return withNoStore( + await logTokenGrant(response, grantType, body.get("scope") ?? ""), + ); } diff --git a/lib/security/headers.ts b/lib/security/headers.ts index bcdbf644..c684c090 100644 --- a/lib/security/headers.ts +++ b/lib/security/headers.ts @@ -115,5 +115,25 @@ export function headerRules(isProd: boolean): HeaderRule[] { }); } + // Pin `Cache-Control: no-store` on the rendered auth pages so a shared + // cache (CDN, corporate proxy, browser bfcache) cannot store and replay + // session-bearing HTML to a different user. Exact-source patterns: each + // route is a single Next page. Independent of `isProd` — caching dev + // sign-in HTML is the same fixation risk in a different deployment. + rules.push( + { + source: "/sign-in", + headers: [{ key: "Cache-Control", value: "no-store" }], + }, + { + source: "/sign-up", + headers: [{ key: "Cache-Control", value: "no-store" }], + }, + { + source: "/consent", + headers: [{ key: "Cache-Control", value: "no-store" }], + }, + ); + return rules; } diff --git a/tests/auth/cache-control.test.ts b/tests/auth/cache-control.test.ts new file mode 100644 index 00000000..91c3e252 --- /dev/null +++ b/tests/auth/cache-control.test.ts @@ -0,0 +1,216 @@ +import { test, expect, afterEach } from "bun:test"; +import { + GET as authCatchAllGET, + POST as authCatchAllPOST, +} from "@/app/api/auth/[...all]/route"; +import { POST as oauthTokenPOST } from "@/app/api/auth/oauth2/token/route"; +import { headerRules } from "@/lib/security/headers"; +import { truncateAll } from "@/tests/setup/schema"; + +/** + * PYZ-198: pin `Cache-Control: no-store` at every Piyaz-owned auth surface + * so a shared cache (CDN edge, corporate proxy, browser bfcache) cannot + * store and replay a session-bearing response to a different user. + * + * Two test styles bracket the change, matching `cookie-attributes.test.ts`: + * - static `config pin`: assert the `headerRules()` shape for the auth + * pages (`/sign-in`, `/sign-up`, `/consent`). Bun cannot exercise the + * Next.js header-rules pipeline (`next.config.ts` `headers()` callback), + * so the pin is on the config the pipeline consumes — the same approach + * `cookie-attributes.test.ts` uses for `revokeSessionsOnPasswordReset`. + * - dynamic: drive the catch-all route handler (`app/api/auth/[...all]`) + * and the dedicated token route (`app/api/auth/oauth2/token`) with + * synthetic `Request`s so the project-owned `no-store` wrapper code path + * runs. Calling `auth.handler` directly would skip the wrapper; the + * point is to exercise it. Even a 4xx/401 response must carry + * `Cache-Control: no-store` because the wrapper applies uniformly. + * + * The wrapper deliberately also covers `/jwks` (BA emits no Cache-Control; + * keys rotate and clients fetch on demand, so eliminating intermediate + * caching is defense-in-depth). The well-known discovery docs are NOT + * downgraded: BA tags them `public, max-age=15` before the wrapper runs and + * the `headers.has('cache-control')` guard lets that pass through. Browser + * bfcache disqualification is the intended UX of `no-store` on auth pages. + * + * `tests/setup/preload.ts` forces `NODE_ENV=production` plus a test-only + * `BETTER_AUTH_SECRET` / `BETTER_AUTH_URL` before any test file loads, so + * the static imports boot BA correctly regardless of import order. + * + * Loopback IP range: this file owns `127.0.0.40+` via `cf-connecting-ip` + * (`lib/auth.ts:117` `ipAddressHeaders`) to keep BA's in-memory rate-limit + * bucket isolated. `cookie-attributes.test.ts` owns `127.0.0.10-15`; + * `rate-limit.test.ts` owns `127.0.1.x`. Do not reuse these. + */ + +const BASE = "https://example.test/api/auth"; + +afterEach(async () => { + await truncateAll(); +}); + +test("config pin: headerRules includes Cache-Control: no-store for /sign-in, /sign-up, /consent", () => { + // AC #1. Pin the static config the Next.js `headers()` pipeline consumes, + // for both prod and dev — auth pages must not be cached in either. + for (const isProd of [false, true]) { + const rules = headerRules(isProd); + for (const source of ["/sign-in", "/sign-up", "/consent"]) { + const rule = rules.find( + (r) => + r.source === source && + r.headers.some( + (h) => h.key === "Cache-Control" && h.value === "no-store", + ), + ); + expect(rule, `${source} (isProd=${isProd})`).toBeDefined(); + } + } +}); + +test("BA core /sign-in/email response carries Cache-Control: no-store via the catch-all wrapper", async () => { + // AC #2. Reaching auth.handler is the point — a bad-credentials 4xx still + // flows through the wrapper, so the header must be present regardless. + const response = await authCatchAllPOST( + new Request(`${BASE}/sign-in/email`, { + method: "POST", + headers: { + "content-type": "application/json", + "cf-connecting-ip": "127.0.0.40", + }, + body: JSON.stringify({ + email: "no-such-user@test.local", + password: "x", + }), + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("BA core /sign-up/email response carries Cache-Control: no-store", async () => { + // AC #2. + const response = await authCatchAllPOST( + new Request(`${BASE}/sign-up/email`, { + method: "POST", + headers: { + "content-type": "application/json", + "cf-connecting-ip": "127.0.0.41", + }, + body: JSON.stringify({ + email: "cache-signup@test.local", + name: "Cache Signup", + password: "test-password-12345", + }), + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("BA core /sign-out response carries Cache-Control: no-store", async () => { + // AC #2. No session cookie attached; BA returns without a Set-Cookie but + // the wrapper applies unconditionally. + const response = await authCatchAllPOST( + new Request(`${BASE}/sign-out`, { + method: "POST", + headers: { + "content-type": "application/json", + "cf-connecting-ip": "127.0.0.42", + }, + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("BA core /get-session response carries Cache-Control: no-store", async () => { + // AC #2. Unauthenticated get-session returns 200 with a null body and no + // Cache-Control from BA; the wrapper supplies no-store. + const response = await authCatchAllGET( + new Request(`${BASE}/get-session`, { + method: "GET", + headers: { "cf-connecting-ip": "127.0.0.43" }, + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("/oauth2/token response carries Cache-Control: no-store (BA + wrapper guard)", async () => { + // AC #3. Drive the dedicated route POST so the withNoStore wrapper runs. + // An invalid grant yields a 4xx; BA sets no-store on it (index.mjs:600) + // and the wrapper is a no-op-on-write, but either way the header is set. + const body = new URLSearchParams({ grant_type: "client_credentials" }); + const response = await oauthTokenPOST( + new Request(`${BASE}/oauth2/token`, { + method: "POST", + headers: { + "content-type": "application/x-www-form-urlencoded", + "cf-connecting-ip": "127.0.0.44", + }, + body: body.toString(), + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("/oauth2/userinfo response carries Cache-Control: no-store (wrapper-supplied)", async () => { + // AC #3. BA emits no Cache-Control on userinfo; the catch-all wrapper + // supplies it. A missing/invalid bearer token yields 401 — still wrapped. + const response = await authCatchAllGET( + new Request(`${BASE}/oauth2/userinfo`, { + method: "GET", + headers: { + authorization: "Bearer invalid-token", + "cf-connecting-ip": "127.0.0.45", + }, + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("/oauth2/introspect response carries Cache-Control: no-store (wrapper-supplied)", async () => { + // AC #3. introspect takes form-urlencoded client_id + token. + const response = await authCatchAllPOST( + new Request(`${BASE}/oauth2/introspect`, { + method: "POST", + headers: { + "content-type": "application/x-www-form-urlencoded", + "cf-connecting-ip": "127.0.0.46", + }, + body: new URLSearchParams({ + client_id: "no-such-client", + token: "no-such-token", + }).toString(), + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("/oauth2/revoke response carries Cache-Control: no-store (wrapper-supplied)", async () => { + // AC #3. revoke takes form-urlencoded client_id + token. + const response = await authCatchAllPOST( + new Request(`${BASE}/oauth2/revoke`, { + method: "POST", + headers: { + "content-type": "application/x-www-form-urlencoded", + "cf-connecting-ip": "127.0.0.47", + }, + body: new URLSearchParams({ + client_id: "no-such-client", + token: "no-such-token", + }).toString(), + }), + ); + expect(response.headers.get("cache-control")).toBe("no-store"); +}); + +test("well-known discovery doc keeps BA's public cache hint (wrapper does not downgrade)", async () => { + // AC #3 negative: the well-known route is allowlisted, but BA tags it + // `public, max-age=15, ...` before the wrapper runs and the + // headers.has('cache-control') guard must leave that untouched. + const response = await authCatchAllGET( + new Request(`${BASE}/.well-known/oauth-authorization-server`, { + method: "GET", + headers: { "cf-connecting-ip": "127.0.0.48" }, + }), + ); + const cacheControl = response.headers.get("cache-control"); + expect(cacheControl).toContain("public"); + expect(cacheControl).not.toContain("no-store"); +}); diff --git a/tests/security/headers.test.ts b/tests/security/headers.test.ts index 758a0364..1ea453b1 100644 --- a/tests/security/headers.test.ts +++ b/tests/security/headers.test.ts @@ -104,18 +104,28 @@ test("dev CSP omits upgrade-insecure-requests (HMR on http://localhost)", () => expect(csp).not.toContain("upgrade-insecure-requests"); }); -test("headerRules(false) returns only the always-on rule with no HSTS", () => { +test("headerRules(false) has the always-on security rule and no HSTS", () => { const rules = headerRules(false); - expect(rules).toHaveLength(1); - expect(rules[0]!.source).toBe("/:path*"); - expect(rules[0]!.missing).toBeUndefined(); - const keys = rules[0]!.headers.map((h) => h.key); - expect(keys).not.toContain("Strict-Transport-Security"); + // Always-on security headers on every path, never host-scoped. + const securityRule = rules.find((r) => r.source === "/:path*")!; + expect(securityRule).toBeTruthy(); + expect(securityRule.missing).toBeUndefined(); + // No rule carries HSTS in dev. + const hasHsts = rules.some((r) => + r.headers.some((h) => h.key === "Strict-Transport-Security"), + ); + expect(hasHsts).toBe(false); }); test("headerRules(true) adds a host-scoped HSTS rule", () => { const rules = headerRules(true); - expect(rules).toHaveLength(2); + // One HSTS rule present in prod (auth-page Cache-Control rules are + // asserted separately in tests/auth/cache-control.test.ts). + expect( + rules.filter((r) => + r.headers.some((h) => h.key === "Strict-Transport-Security"), + ), + ).toHaveLength(1); const hstsRule = rules.find((r) => r.headers.some((h) => h.key === "Strict-Transport-Security"), )!;