From 3385163fd69416ddafd2114d476bddaeb18718f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:27:03 +0100 Subject: [PATCH] fix(demo): stop refusing a visitor's own sign-out, language and timings On the demo every page logged its own failures. The sign-out answered 403, so a visitor could not sign out at all; the language write answered 403 on every page load, and because its once-per-tab marker is only set on success it was sent again on the next one; the page-timing beacon was refused the same way. Five password sign-ins per 15 minutes per address locked out a group trying the published demo account together. Sign-out, the language write and the timing beacon now pass the demo block: each touches only the visitor. The language route still answers and sets the visitor's cookie, but under DEMO_MODE it no longer writes the shared account's row, so one visitor's choice is not the next one's starting state. A demo allows 30 password sign-ins per 15 minutes per address; other instances keep 5. --- .../baselines/version-stamp-comments.json | 6 +- .../proxy-demo-mode-allowlist.test.ts | 19 +++++ .../api/auth/login/__tests__/route.test.ts | 27 +++++++ src/app/api/auth/login/route.ts | 8 +- .../auth/me/locale/__tests__/route.test.ts | 73 +++++++++++++++++++ src/app/api/auth/me/locale/route.ts | 6 +- src/proxy.ts | 10 ++- 7 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 src/app/api/auth/me/locale/__tests__/route.test.ts diff --git a/src/__tests__/baselines/version-stamp-comments.json b/src/__tests__/baselines/version-stamp-comments.json index 9a3ee2c39..61643f8f6 100644 --- a/src/__tests__/baselines/version-stamp-comments.json +++ b/src/__tests__/baselines/version-stamp-comments.json @@ -1,7 +1,7 @@ { "$comment": "Comment lines that open with a release version, per file under src/. Maintained by src/__tests__/version-stamp-ratchet.test.ts.", "count": 2892, - "total": 9215, + "total": 9213, "entries": { "__tests__/account-payload-consumer-guard.test.ts": 2, "__tests__/acting-account-boundary-guard.test.ts": 7, @@ -137,7 +137,7 @@ "app/api/auth/codex/device-start/route.ts": 1, "app/api/auth/login/__tests__/native-token.test.ts": 2, "app/api/auth/login/__tests__/route.test.ts": 1, - "app/api/auth/login/route.ts": 5, + "app/api/auth/login/route.ts": 4, "app/api/auth/me/coach-prefs/route.ts": 7, "app/api/auth/me/cycle-prefs/route.ts": 1, "app/api/auth/me/devices/[id]/route.ts": 3, @@ -2892,7 +2892,7 @@ "lib/withings/sync.ts": 6, "lib/withings/webhook-handler.ts": 3, "lib/workouts/list-read.ts": 1, - "proxy.ts": 26, + "proxy.ts": 25, "types/analytics.ts": 8, "types/backups.ts": 1 } diff --git a/src/__tests__/proxy-demo-mode-allowlist.test.ts b/src/__tests__/proxy-demo-mode-allowlist.test.ts index d460c623d..aa6f89497 100644 --- a/src/__tests__/proxy-demo-mode-allowlist.test.ts +++ b/src/__tests__/proxy-demo-mode-allowlist.test.ts @@ -161,6 +161,25 @@ describe("proxy.ts DEMO_MODE mutation allowlist", () => { expect(proxy(makeRequest("/api/search", "DELETE")).status).toBe(403); }); + it("lets a visitor sign out, report page timings and pick a language", () => { + // Each touches only this visitor: their own session, a log line, their + // own language cookie (the locale route leaves the shared account's row + // alone under DEMO_MODE). Refused, sign-out stayed signed in and every + // page load sent the refused locale write and the timings again. + for (const [path, method] of [ + ["/api/auth/logout", "POST"], + ["/api/internal/web-vitals", "POST"], + ["/api/auth/me/locale", "PUT"], + ] as const) { + expect( + proxy(makeRequest(path, method)).status, + `${method} ${path} is blocked in the demo`, + ).not.toBe(403); + } + // The rest of the account's settings stay closed. + expect(proxy(makeRequest("/api/auth/me/timezone", "PUT")).status).toBe(403); + }); + it("still blocks a health-data mutation (POST /api/measurements)", () => { const res = proxy(makeRequest("/api/measurements", "POST")); expect(res.status).toBe(403); diff --git a/src/app/api/auth/login/__tests__/route.test.ts b/src/app/api/auth/login/__tests__/route.test.ts index 5de021855..0a5462a02 100644 --- a/src/app/api/auth/login/__tests__/route.test.ts +++ b/src/app/api/auth/login/__tests__/route.test.ts @@ -184,3 +184,30 @@ describe("POST /api/auth/login — OIDC_ONLY server-side enforcement", () => { expect(res.status).toBe(401); }); }); + +describe("POST /api/auth/login — per-address limit", () => { + const original = process.env.DEMO_MODE; + afterEach(() => { + if (original === undefined) delete process.env.DEMO_MODE; + else process.env.DEMO_MODE = original; + }); + + function limitAsked(): unknown { + const call = vi + .mocked(checkAuthSurfaceRateLimit) + .mock.calls.find((c) => c[1] === "auth:login"); + return call?.[2]; + } + + it("allows five attempts per address on an ordinary instance", async () => { + delete process.env.DEMO_MODE; + await POST(makeRequest()); + expect(limitAsked()).toBe(5); + }); + + it("allows thirty on a demo, whose one account everybody shares", async () => { + process.env.DEMO_MODE = "true"; + await POST(makeRequest()); + expect(limitAsked()).toBe(30); + }); +}); diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index 0f11f9daf..61baff702 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -38,13 +38,13 @@ export const POST = apiHandler(async (request: NextRequest) => { }); } - // v1.4.43 — `checkAuthSurfaceRateLimit` swaps to a tighter - // global bucket when the trust chain is misconfigured; otherwise it - // is byte-equivalent to the previous per-IP `auth:login:{ip}` key. + // Per IP, or one tighter shared bucket when the trust chain is + // misconfigured. A demo publishes its one account, and a class or an + // office behind one address tries it together: 5 locked them out. const rl = await checkAuthSurfaceRateLimit( request, "auth:login", - 5, + process.env.DEMO_MODE === "true" ? 30 : 5, 15 * 60 * 1000, ); const ip = rl.ip ?? "unknown"; diff --git a/src/app/api/auth/me/locale/__tests__/route.test.ts b/src/app/api/auth/me/locale/__tests__/route.test.ts new file mode 100644 index 000000000..a58f5669d --- /dev/null +++ b/src/app/api/auth/me/locale/__tests__/route.test.ts @@ -0,0 +1,73 @@ +/** + * The language write on a demo. + * + * A demo is one published account every visitor signs into, so a visitor's + * language must not become the account's: the route answers as usual and + * sets the visitor's own cookie, but leaves the shared row alone. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/api-handler", async () => { + const actual = + await vi.importActual( + "@/lib/api-handler", + ); + return { ...actual, requireActorAuth: vi.fn() }; +}); +vi.mock("@/lib/db", () => ({ prisma: { user: { update: vi.fn() } } })); +vi.mock("next/headers", () => ({ cookies: vi.fn(async () => ({})) })); +vi.mock("@/lib/i18n/locale-cookie", () => ({ setLocaleCookie: vi.fn() })); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn().mockResolvedValue({ allowed: true }), + rateLimitHeaders: vi.fn(() => ({})), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); + +import { PUT } from "../route"; +import { requireActorAuth } from "@/lib/api-handler"; +import { prisma } from "@/lib/db"; +import { setLocaleCookie } from "@/lib/i18n/locale-cookie"; + +const ORIGINAL_DEMO_MODE = process.env.DEMO_MODE; + +function put(locale: string) { + return PUT( + new Request("http://localhost/api/auth/me/locale", { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ locale }), + }) as never, + ); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(requireActorAuth).mockResolvedValue({ + user: { id: "demo-account", locale: "en" }, + } as never); +}); + +afterEach(() => { + if (ORIGINAL_DEMO_MODE === undefined) delete process.env.DEMO_MODE; + else process.env.DEMO_MODE = ORIGINAL_DEMO_MODE; +}); + +describe("PUT /api/auth/me/locale", () => { + it("stores the language on the account", async () => { + delete process.env.DEMO_MODE; + const res = await put("de"); + expect(res.status).toBe(200); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: "demo-account" }, + data: { locale: "de" }, + }); + }); + + it("keeps a demo visitor's language in their own cookie only", async () => { + process.env.DEMO_MODE = "true"; + const res = await put("de"); + expect(res.status).toBe(200); + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(setLocaleCookie).toHaveBeenCalledWith(expect.anything(), "de"); + }); +}); diff --git a/src/app/api/auth/me/locale/route.ts b/src/app/api/auth/me/locale/route.ts index 032b924a2..8893e84b8 100644 --- a/src/app/api/auth/me/locale/route.ts +++ b/src/app/api/auth/me/locale/route.ts @@ -66,8 +66,10 @@ export const PUT = apiHandler(async (request: NextRequest) => { } // Idempotent: skip the write (and the annotation noise) when the column - // already matches — the mount-time backfill fires on every page load. - if (user.locale !== locale) { + // already matches — the mount-time backfill fires on every page load. A + // demo is one account every visitor shares, so there the choice stays in + // this visitor's cookie and never reaches the row the next visitor reads. + if (user.locale !== locale && process.env.DEMO_MODE !== "true") { await prisma.user.update({ where: { id: user.id }, data: { locale }, diff --git a/src/proxy.ts b/src/proxy.ts index 1d337dc3e..4ecf2045c 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -183,6 +183,11 @@ const DEMO_MUTATION_ALLOWLIST: ReadonlyArray<{ path: string; method: string }> = { path: "/api/auth/login", method: "POST" }, { path: "/api/auth/passkey/login-options", method: "POST" }, { path: "/api/auth/passkey/login-verify", method: "POST" }, + // Each touches only this visitor: the session, the language cookie (the + // route leaves the shared row alone under DEMO_MODE), a metrics log line. + { path: "/api/auth/logout", method: "POST" }, + { path: "/api/auth/me/locale", method: "PUT" }, + { path: "/api/internal/web-vitals", method: "POST" }, // Chart display-pref toggles above the charts (dashboard + insights): { path: "/api/dashboard/chart-overlay-prefs", method: "PUT" }, { path: "/api/dashboard/widgets", method: "PUT" }, @@ -231,8 +236,7 @@ const DEMO_MUTATION_ALLOWLIST: ReadonlyArray<{ path: string; method: string }> = // The per-profile routes (`/api/managed-profiles/{id}`, its guardians) // are different paths and stay closed too. { path: "/api/auth/profile", method: "PUT" }, - // v1.42.2 — the record search is a read that travels as a POST so the - // query stays out of URLs and access logs. It writes nothing. + // The record search: a read sent as a POST to keep the query out of logs. { path: "/api/search", method: "POST" }, ]; @@ -473,7 +477,7 @@ export function proxy(request: NextRequest) { ); } - // Demo mode: block all mutations except login + // Demo mode: block every mutation outside the allowlist. if (process.env.DEMO_MODE === "true") { const method = request.method.toUpperCase(); const isMutation =