From 98fc1d3bdda1108ae6d85abc51f27d2cf8ee4aea Mon Sep 17 00:00:00 2001 From: Tyler Date: Wed, 1 Jul 2026 17:45:30 -0600 Subject: [PATCH] fix(water-log): make verifyWaterAdminPin fully PIN-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous PR wrapped the getAdminUser() call in try/catch as a best-effort audit hook. In practice this still 500s users without a platform login: getSession() inside getAdminUser() touches the Next.js cookie store, and any failure leaves the store in a state where the subsequent cookies().set('wl_admin_session', ...) throws. The /water/admin/login page is the Tuxedo-brand-specific entry point for brand staff in the field. It must work PIN-only, with no Neon Auth dependency. Fix: - Drop the getAdminUser() call inside verifyWaterAdminPin entirely. - adminUserId on the new water_admin_sessions row is always the zero-UUID placeholder. Audit attribution is intentionally deferred. - Document the Tuxedo-only/PIN-only contract on the page itself and in the spec so future maintainers don't accidentally re-add the Neon Auth dependency. Guards (TDD, red → green): - tests/unit/water-log-auth.test.ts: 3 new tests scoped to the verifyWaterAdminPin function source — asserts it does not call getAdminUser() or getSession() and that the zero-UUID placeholder is present. These fail if a future refactor re-introduces the dependency. --- ...7-01-water-log-no-platform-login-design.md | 37 ++++++----- src/actions/water-log/settings.ts | 20 +++--- src/app/water/admin/login/page.tsx | 13 ++++ tests/unit/water-log-auth.test.ts | 66 +++++++++++++++++++ 4 files changed, 107 insertions(+), 29 deletions(-) diff --git a/docs/superpowers/specs/2026-07-01-water-log-no-platform-login-design.md b/docs/superpowers/specs/2026-07-01-water-log-no-platform-login-design.md index ca0e498..fe892d6 100644 --- a/docs/superpowers/specs/2026-07-01-water-log-no-platform-login-design.md +++ b/docs/superpowers/specs/2026-07-01-water-log-no-platform-login-design.md @@ -88,12 +88,13 @@ The site-admin helper (`requireWaterAdminPermission`) keeps using `/admin/water-log/*` (which sits behind Neon Auth in the middleware and on the server). -> Note: the action `verifyWaterAdminPin` (in `settings.ts`) does make a -> best-effort `getAdminUser()` call *after* a successful PIN check, but -> only to attach a platform-admin `adminUserId` to the new -> `water_admin_sessions` row for audit. It is wrapped in try/catch and -> falls back to a zero-UUID placeholder. The `wl_admin_session` is -> valid either way — that helper call is not an auth gate. +> Note: the action `verifyWaterAdminPin` (in `settings.ts`) is now a +> fully PIN-only flow — it does NOT call `getAdminUser()` or +> `getSession()` at all. The `adminUserId` on the new +> `water_admin_sessions` row is always the zero-UUID placeholder. This +> is because `/water/admin/login` is the Tuxedo-brand-specific entry +> point and must work for users who are not signed into the platform. +> Audit attribution is intentionally deferred to a future ticket. ### 4.2 New file: `src/actions/water-log/auth.ts` @@ -174,23 +175,25 @@ moves. keep their existing `getAdminUser()` gate (site-admin path). - `verifyWaterAdminPin` becomes a pure PIN-only flow: validate format, look up `water_admin_settings` for the brand, verify PIN, mint - `wl_admin_session` cookie. **No `getSession()` call.** `getAdminUser()` - is still called inside the success path to attach an `adminUserId` - to the new session row for audit — wrapped in try/catch, falls back - to a zero-UUID placeholder if no admin user is signed in. The - session itself is valid either way. + `wl_admin_session` cookie. **No `getSession()` call. No + `getAdminUser()` call.** `adminUserId` is always the zero-UUID + placeholder. The page is the Tuxedo-brand-specific entry point and + must work without a platform login. ### 4.4 Why `verifyWaterAdminPin` is a special case `verifyWaterAdminPin` is the most-affected function in this PR. Today: -1. It calls `await getSession()` (Neon Auth) — drop. -2. It calls `getAdminUser()` inside the success path to attach the +1. It called `await getSession()` (Neon Auth) — dropped. +2. It called `getAdminUser()` inside the success path to attach the caller's platform-admin user to the new `wl_admin_session` row for - audit. This is correct and stays, but the call is now **wrapped in - a try/catch that swallows any error and falls back to a zero-UUID - `adminUserId`**. The `wl_admin_session` is still valid in either - case — the cookie holds the brand + session id, not the admin user. + audit. That call was wrapped in a try/catch that fell back to a + zero-UUID `adminUserId`, but the underlying `getSession()` call + inside `getAdminUser()` could still corrupt the Next.js cookie + store and cause the subsequent `cookies().set("wl_admin_session", + ...)` to throw — surfacing as a 500 to users without a platform + login. The call is now **fully removed**; `adminUserId` is always + the zero-UUID placeholder. Audit attribution is a follow-up. This is the right shape because the water-admin PIN flow is for ditch riders / brand water admins, who often aren't platform admins. Tying diff --git a/src/actions/water-log/settings.ts b/src/actions/water-log/settings.ts index 9f903ee..707f5ad 100644 --- a/src/actions/water-log/settings.ts +++ b/src/actions/water-log/settings.ts @@ -226,17 +226,13 @@ const formatError = validatePin(pin); return { success: false, error: "Invalid PIN" }; } - // Tie the session to the calling site admin (best-effort). - // The `wl_admin_session` is valid whether or not a platform admin - // is signed in — this call only attaches an `adminUserId` for - // audit. If `getAdminUser()` fails or returns null, we fall back - // to a zero-UUID placeholder. - let adminUser: Awaited> = null; - try { - adminUser = await getAdminUser(); - } catch { - // ignore — the session is valid regardless - } + // The `/water/admin/login` page is Tuxedo-brand-specific and PIN-only. + // There is no platform-admin requirement here — `wl_admin_session` is + // valid whether or not a Neon Auth session exists. We deliberately do + // NOT call `getAdminUser()` (it would touch cookies via Neon Auth and + // 500 for users without a platform login). `adminUserId` is therefore + // always the zero-UUID placeholder; audit attribution happens elsewhere. + const PLACEHOLDER_ADMIN_USER_ID = "00000000-0000-0000-0000-000000000000"; const expiresAt = new Date( Date.now() + settings.sessionDurationHours * 60 * 60 * 1000, @@ -245,7 +241,7 @@ const formatError = validatePin(pin); .insert(waterAdminSessions) .values({ brandId, - adminUserId: adminUser?.user_id ?? adminUser?.id ?? "00000000-0000-0000-0000-000000000000", + adminUserId: PLACEHOLDER_ADMIN_USER_ID, pinHashUsed: settings.pinHash, expiresAt, }) diff --git a/src/app/water/admin/login/page.tsx b/src/app/water/admin/login/page.tsx index a28551f..56a5579 100644 --- a/src/app/water/admin/login/page.tsx +++ b/src/app/water/admin/login/page.tsx @@ -3,6 +3,19 @@ import WaterAdminPinClient from "./WaterAdminPinClient"; export const dynamic = "force-dynamic"; +/** + * Tuxedo-brand-specific water admin login. + * + * The 4-digit admin PIN gates `/water/admin/*` for the Tuxedo brand. + * Users must be able to enter the PIN WITHOUT being signed into the + * platform — this is a PIN-only entry point for brand staff in the + * field. The corresponding `verifyWaterAdminPin()` action deliberately + * does not call Neon Auth (no `getSession()` / no `getAdminUser()`). + * + * If you need a similar portal for another brand, duplicate this + * page under a different slug (e.g. `/water/admin-tuxedo/login`) and + * swap in that brand's UUID. + */ const TUXEDO_BRAND_ID = "64294306-5f42-463d-a5e8-2ad6c81a96de"; export default async function WaterAdminLoginPage() { diff --git a/tests/unit/water-log-auth.test.ts b/tests/unit/water-log-auth.test.ts index 345b3c7..6510820 100644 --- a/tests/unit/water-log-auth.test.ts +++ b/tests/unit/water-log-auth.test.ts @@ -80,6 +80,72 @@ describe("water-log/auth — module surface", () => { }); }); +// ── Module guard for the Tuxedo admin-PIN entry flow ─────────────────── +// +// The `/water/admin/login` page is Tuxedo-brand-specific and PIN-only. +// Users must be able to enter the admin PIN WITHOUT being signed into +// the platform. If a future refactor re-adds a Neon Auth dependency +// (getSession / getAdminUser) inside `verifyWaterAdminPin`, the PIN +// submission 500s for unauthenticated field staff. +// +// Note: other functions in settings.ts (getWaterAdminSettings, +// saveWaterAdminSettings, regenerateAdminPin) are called from the +// platform-admin-gated `/admin/water-log/settings/*` UI and correctly +// DO depend on `getAdminUser()`. Only `verifyWaterAdminPin` is the +// PIN-only entry point. + +describe("water-log/settings — verifyWaterAdminPin surface", () => { + /** + * Extract just the body of `verifyWaterAdminPin` from settings.ts so + * the regression check is scoped to that function (and not falsely + * tripped by sibling functions that legitimately use getAdminUser). + * + * The function is the LAST export in the file, so we read from + * `export async function verifyWaterAdminPin` to EOF. + */ + async function getVerifyWaterAdminPinSource(): Promise { + const fs = await import("node:fs/promises"); + const path = await import("node:path"); + const raw = await fs.readFile( + path.resolve( + process.cwd(), + "src/actions/water-log/settings.ts", + ), + "utf8", + ); + const idx = raw.indexOf("export async function verifyWaterAdminPin"); + if (idx === -1) throw new Error("verifyWaterAdminPin not found"); + return raw.slice(idx); + } + + function stripComments(src: string): string { + return src + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/^\s*\/\/.*$/gm, "") + .replace(/\s+/g, " "); + } + + it("does not call getAdminUser inside verifyWaterAdminPin", async () => { + const stripped = stripComments(await getVerifyWaterAdminPinSource()); + expect(stripped).not.toMatch(/\bgetAdminUser\s*\(/); + }); + + it("does not call getSession inside verifyWaterAdminPin", async () => { + const stripped = stripComments(await getVerifyWaterAdminPinSource()); + expect(stripped).not.toMatch(/\bgetSession\s*\(/); + }); + + it("always inserts the zero-UUID fallback adminUserId", async () => { + const stripped = stripComments(await getVerifyWaterAdminPinSource()); + // Confirms the session row is inserted with the placeholder UUID + // — i.e. the function no longer conditions adminUserId on a + // successful platform-admin lookup. + expect(stripped).toMatch( + /00000000-0000-0000-0000-000000000000/, + ); + }); +}); + // ── requireFieldSession ───────────────────────────────────────────────── describe("requireFieldSession()", () => {