docs(spec): water log — remove platform-login dependency

Bug: every server action in src/actions/water-log/{field,admin,settings}.ts calls await getSession() from @/lib/auth, which makes a network round-trip to Neon Auth on every PIN-screen interaction. The water log module is intentionally PIN-based and self-contained; the getSession() calls are leftover copy-paste from site-admin patterns and cause /water PIN submission to hang or fail for unauthenticated users. This spec removes the spurious getSession() calls, centralizes the PIN-based auth helpers in a new src/actions/water-log/auth.ts, and adds a regression-guard E2E test.
2026-07-01 17:02:34 -06:00
parent 602adb4588
commit 015eb33291
1 changed files with 390 additions and 0 deletions
@@ -0,0 +1,390 @@
+# Water Log — remove platform-login dependency — 2026-07-01
+
+> Type: Bug fix + refactor (auth layer cleanup)
+> Branch: `fix/water-log-no-platform-login`
+> Status: Draft — awaiting user review
+
+## 1. Context
+
+The Water Log module is **PIN-based and self-contained**: `/water` (irrigator)
+and `/water/admin/login` (brand water admin) both authenticate via a
+4-digit PIN that mints a `wl_session` or `wl_admin_session` cookie. Neither
+surface is meant to require a platform admin (Neon Auth) login.
+
+The middleware already agrees — `src/proxy.ts` lists `/water` and
+`/api/water-photo-upload` among the public routes. The `/water` page
+itself loads with no cookie and shows the language → role → PIN flow.
+
+**But every server action in `src/actions/water-log/{field,admin,settings}.ts`
+opens with `await getSession()` from `@/lib/auth`.** That call hits Neon
+Auth on every PIN-screen interaction. In production:
+
+- When `NEON_AUTH_BASE_URL` is misconfigured, `getSession()` short-circuits
+  to `{ data: null, error: null }` and the rest of the action runs.
+- When it is configured, the wrapper falls through to the real Neon Auth
+  `getSession()`, which (a) checks the request for a session cookie (none
+  is present for `/water` users), (b) makes a network round-trip to
+  `NEON_AUTH_BASE_URL` anyway, and (c) may hang or return an error if the
+  Neon Auth service is slow / unhealthy / cross-region.
+
+User-visible result: **PIN submission hangs or fails on `/water` even
+though the user is doing exactly what the design asks them to do — enter
+a 4-digit PIN — and is correctly authenticated by the cookie they
+receive in response.**
+
+The root cause is leftover copy-paste from the site-admin actions, which
+*do* legitimately call `getSession()` because they sit behind
+`/admin/water-log/*` and are gated by `admin_users`. The field and
+admin-PIN actions have no such dependency; the `getSession()` lines are
+spurious and should be removed.
+
+## 2. Goals
+
+- **`/water` PIN entry works with no platform login, no Neon Auth call,
+  no `dev_session` cookie.** A ditch rider in the field can complete the
+  full language → role → PIN → entry flow with no cookies set on the
+  request other than the `wl_session` minted on success.
+- **`/water/admin/login` works the same way** for the brand water admin
+  portal.
+- **The site-admin `/admin/water-log/*` surface is unchanged** —
+  it continues to require a platform admin (Neon Auth + `admin_users`).
+- **No DB migration, no schema change, no env-var change.** Deploy is a
+  pure code change.
+- **Regression guard:** an E2E test runs a fresh, cookie-free Playwright
+  context against `/water` and proves the PIN flow works.
+
+## 3. Non-goals
+
+- No change to admin-permissions, Neon Auth, or the PIN hashing library.
+- No change to middleware, page components, or the existing API routes
+  (all of which are already clean — see §6).
+- No refactor of the irrigation-season / reporting / CSV layers.
+- No change to the photo-upload or QR-code APIs.
+- No attempt to enforce brand_id on the field PIN flow beyond the existing
+  data model (irrigators carry a `brand_id`, and `wl_session` is keyed by
+  `irrigator_id`, so brand isolation is preserved by the join).
+
+## 4. Architecture
+
+### 4.1 Three auth gates, three helpers
+
+Every water-log server action is gated by exactly one helper, matched
+to its consumer:
+
+| Surface | Cookie | Helper | Backed by |
+|---|---|---|---|
+| `/water` (irrigator) | `wl_session` | `requireFieldSession()` | `cookies()` + `water_sessions` row |
+| `/water/admin/*` (brand water admin) | `wl_admin_session` | `requireWaterAdminSession()` | `cookies()` + `water_admin_sessions` row |
+| `/admin/water-log/*` (site admin) | Neon Auth | `requireWaterAdminPermission()` | `getAdminUser()` + `admin_users.can_manage_water_log` |
+
+The three `require*` helpers in `auth.ts` **never call `getSession()`**.
+The two PIN-only helpers (`requireFieldSession`, `requireWaterAdminSession`)
+read the cookie directly, join the cookie value to the appropriate
+session row in Postgres, and return. No network round-trip, no Neon
+Auth dependency, no platform-admin requirement.
+
+The site-admin helper (`requireWaterAdminPermission`) keeps using
+`getAdminUser()` because that is the correct auth check for
+`/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.
+
+### 4.2 New file: `src/actions/water-log/auth.ts`
+
+Centralized auth layer for the water-log module. Holds the three helpers
+plus small typed return shapes.
+
+```ts
+// src/actions/water-log/auth.ts (sketch)
+
+import "server-only";
+import { cookies } from "next/headers";
+import { and, eq } from "drizzle-orm";
+import { withPlatformAdmin } from "@/db/client";
+import {
+  waterSessions, waterIrrigators,
+  waterAdminSessions, waterAdminSettings,
+} from "@/db/schema/water-log";
+import { getAdminUser } from "@/lib/admin-permissions";
+
+type FieldSession =
+  | { ok: true; userId: string; brandId: string; role: "irrigator" | "water_admin" }
+  | { ok: false; error: string };
+
+type AdminPinSession =
+  | { ok: true; sessionId: string; brandId: string; adminUserId: string }
+  | { ok: false; error: string };
+
+type SiteAdminPermission =
+  | { ok: true; adminUser: AdminUser }
+  | { ok: false; error: string };
+
+/** Read wl_session, look up the row + irrigator. No Neon Auth. */
+export async function requireFieldSession(): Promise<FieldSession> { ... }
+
+/** Read wl_admin_session, look up the row. No Neon Auth. */
+export async function requireWaterAdminSession(): Promise<AdminPinSession> { ... }
+
+/** Site-admin gate: getAdminUser() + can_manage_water_log. */
+export async function requireWaterAdminPermission(): Promise<SiteAdminPermission> { ... }
+
+/** Read the active water-admin session row (or null). Cheap, idempotent. */
+export async function getWaterAdminSession(): Promise<AdminPinSession | null> { ... }
+
+/** Read the active field session row (or null). Cheap, idempotent. */
+export async function getFieldSessionUser(): Promise<{ userId; name; brandId; role } | null> { ... }
+```
+
+The helpers are exact ports of the existing local helpers in
+`field.ts` and `admin.ts` — behavior is unchanged, only the import path
+moves.
+
+### 4.3 File-by-file changes
+
+**`src/actions/water-log/field.ts`**
+- Drop `import { getSession } from "@/lib/auth"`.
+- Drop every `await getSession();` line (10 occurrences).
+- Move `requireFieldSession`, `getFieldSessionUser`, `getWaterAdminSession`,
+  `getWaterSession` into `auth.ts`. Import from there.
+- `setWaterLang()` keeps `cookies()` only.
+- `verifyWaterPin`, `getWaterHeadgates`, `submitWaterEntry`, `logoutWater`,
+  `logoutWaterAdmin` all remain — no behavior change other than
+  dropping the spurious `getSession()` calls.
+
+**`src/actions/water-log/admin.ts`**
+- Drop `import { getSession } from "@/lib/auth"`.
+- Drop every `await getSession();` line (18 occurrences).
+- Move `requireWaterAdminSession` and `requireWaterAdminPermission` into
+  `auth.ts`. Import from there.
+- Action-by-action gate stays as today: mutations/reads in `admin.ts`
+  use `requireWaterAdminPermission()` because every action that lives
+  in this file is invoked from `/admin/water-log/*` (site-admin path).
+
+**`src/actions/water-log/settings.ts`**
+- Drop `import { getSession } from "@/lib/auth"`.
+- Drop every `await getSession();` line (4 occurrences, including the
+  one in `verifyWaterAdminPin`).
+- `getWaterAdminSettings`, `updateWaterAdminSettings`, `regenerateAdminPin`
+  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.
+
+### 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
+   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.
+
+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
+the brand-admin session to a platform-admin user is convenient for
+audit, not required for the flow to work.
+
+## 5. Data flow
+
+### 5.1 `verifyWaterPin` (irrigator)
+
+```
+client POST via server action: verifyWaterPin(brandId, pin)
+  └─ validatePin(pin)                              ── format check
+  └─ withPlatformAdmin(db => ...)                  ── look up irrigator
+       WHERE active = true                         ── across all brands
+  └─ match = irrigators.find(i => verifyPin(...))  ── scrypt compare
+  └─ if no match: 200ms delay, return error
+  └─ withBrand(brandId, db => ...)
+       INSERT water_sessions (irrigator_id, expires_at = now+8h)
+       UPDATE water_irrigators SET last_used_at
+  └─ cookies().set("wl_session", sessionId, ...)
+  └─ return { success: true, role, name, ... }
+```
+
+No `getSession()`. No `getAdminUser()`. No network call beyond the local
+DB round-trips.
+
+### 5.2 `verifyWaterAdminPin` (brand water admin)
+
+```
+client POST via server action: verifyWaterAdminPin(brandId, pin)
+  └─ validatePin(pin)
+  └─ withBrand(brandId, db => ...)
+       SELECT water_admin_settings WHERE brand_id
+       if !settings or !settings.enabled or !settings.pinHash → return error
+       if !verifyPin(pin, settings.pinHash) → 200ms delay, return error
+       try { adminUser = await getAdminUser() } catch { adminUser = null }
+       INSERT water_admin_sessions (brand_id, admin_user_id, expires_at)
+  └─ cookies().set("wl_admin_session", sessionId, ...)
+  └─ return { success: true, session_id }
+```
+
+### 5.3 `submitWaterEntry` (irrigator mutation)
+
+```
+client POST via server action: submitWaterEntry(...)
+  └─ requireFieldSession()  →  { ok: false, error }  or  { ok: true, userId, brandId }
+  └─ validate measurement / notes / lat / lng
+  └─ withBrand(brandId, db => ...)
+       SELECT headgate (verify it belongs to brand)
+       INSERT water_log_entries
+       UPDATE water_headgates SET last_used_at
+       (optional) logAlert() if threshold breach
+  └─ return { success: true, entry_id }
+```
+
+If `requireFieldSession()` returns `ok: false`, the action returns
+`{ success: false, error: "Session expired or invalid" }`. The client
+(`WaterFieldClient`) already handles this: it resets to the language
+step and shows the "Session expired" message. **No change to that
+client behavior is needed.**
+
+## 6. Error handling
+
+| Failure | Helper return | Caller return | Client behavior |
+|---|---|---|---|
+| No `wl_session` cookie | `{ ok: false, error: "Not logged in" }` | `{ success: false, error: "Session expired or invalid" }` | Reset to language step |
+| `wl_session` cookie but no matching row | same as above | same | same |
+| Session row past `expires_at` | same as above (best-effort delete) | same | same |
+| Irrigator row marked `active = false` | `{ ok: false, error: "User is inactive" }` | `{ success: false, error: "User is inactive" }` | Reset to language step |
+| `wl_admin_session` missing / expired | `null` (from `getWaterAdminSession`) | `{ success: false, error: "Not signed in" }` | Bounce to `/water/admin/login` |
+| Site admin not signed in | `{ ok: false, error: "Not signed in" }` | `{ success: false, error: ... }` | Toast + stay |
+| Site admin signed in but `can_manage_water_log` false | `{ ok: false, error: "Not permitted" }` | `{ success: false, error: "Forbidden" }` | Toast + redirect |
+
+## 7. Testing
+
+### 7.1 Unit (`tests/unit/water-log-auth.test.ts`, new)
+
+Each helper, no network:
+
+- `requireFieldSession()`:
+  - Returns `{ ok: false, error: "Not logged in" }` when no cookie.
+  - Returns `{ ok: false, error: "Session expired" }` when cookie points
+    at a row with `expires_at` in the past.
+  - Returns `{ ok: false, error: "Session not found" }` when cookie has
+    no matching row.
+  - Returns `{ ok: false, error: "User is inactive" }` when irrigator
+    `active = false`.
+  - Returns `{ ok: true, userId, brandId, role }` on happy path.
+  - **Asserts no `getSession` import is reachable from this module**
+    (literal `import.meta.glob` or a vitest module-mock trick).
+
+- `requireWaterAdminSession()`:
+  - Returns `{ ok: false, error: "Not signed in" }` when no cookie.
+  - Returns `{ ok: false, error: "Session expired" }` when past expiry.
+  - Returns `{ ok: true, ... }` on happy path.
+  - No `getSession` import reachable.
+
+- `requireWaterAdminPermission()`:
+  - Mock `getAdminUser` to return `null` → `{ ok: false, error: "Not signed in" }`.
+  - Mock `getAdminUser` to return user without `can_manage_water_log`
+    → `{ ok: false, error: "Not permitted" }`.
+  - Mock to return platform_admin → `{ ok: true, ... }`.
+  - This helper is allowed to call `getAdminUser()` — that is its
+    defined behavior. No "no getSession" assertion here.
+
+### 7.2 E2E (`tests/water-log.spec.ts`, new block)
+
+New `test.describe("Water Log — no platform login required")`:
+
+```ts
+test("GET /water renders the PIN flow with no cookies set", async ({ browser }) => {
+  // Fresh context: no storageState, no cookies, no dev_session.
+  const ctx = await browser.newContext();
+  const page = await ctx.newPage();
+  const res = await page.goto(`${BASE}/water`);
+  expect(res?.status()).toBe(200);
+  // The page goes through language → role → pin before showing the input.
+  // We assert it never 302s to /login.
+  const allResponses: string[] = [];
+  page.on("response", (r) => allResponses.push(r.url()));
+  // Click through the language step
+  await page.getByRole("button", { name: /english/i }).click();
+  // Click the "Irrigator" button (the lighter one)
+  await page.getByRole("button", { name: /irrigator/i }).click();
+  // PIN input is now visible
+  const pin = page.locator('input[type="password"][inputmode="numeric"]').first();
+  await expect(pin).toBeVisible({ timeout: 10_000 });
+  // Confirm we never hit /login
+  expect(allResponses.filter((u) => u.includes("/login"))).toHaveLength(0);
+  await ctx.close();
+});
+
+test("setWaterLang server action does not require auth", async ({ request }) => {
+  // Empty storage state, no cookies.
+  const ctx = await request.newContext({ baseURL: BASE, storageState: { cookies: [], origins: [] } });
+  // We don't have a direct POST endpoint for setWaterLang; instead hit
+  // the page and check the language cookie gets set without a redirect.
+  const res = await ctx.get("/water");
+  expect(res.status()).toBe(200);
+  await ctx.dispose();
+});
+```
+
+The existing `tests/water-log.spec.ts` blocks for the `/water/admin/login`
+and `/admin/water-log` redirects remain — they continue to pass because
+we only relax the field and admin-PIN paths, not the site-admin path.
+
+### 7.3 Manual smoke (post-deploy)
+
+1. Open `/water` in a private window. Confirm language → role → PIN.
+2. Enter a wrong PIN — confirm error toast, no redirect to `/login`.
+3. Enter a correct PIN — confirm the entry form loads with headgates.
+4. Submit an entry — confirm the success banner.
+5. Open `/water/admin/login` in another private window. Same flow.
+6. Sign in as a site admin in a third window, navigate to
+   `/admin/water-log` — confirm it still requires the platform login.
+
+## 8. Out of scope (explicit)
+
+- No DB migration. (`water_sessions`, `water_admin_sessions`,
+  `water_admin_settings` already have all columns the auth helpers
+  read.)
+- No new RPC. The auth helpers run direct Drizzle queries against the
+  pool.
+- No env-var changes.
+- No middleware change. `/water` and the water API routes are already
+  in `PUBLIC_ROUTES`.
+- No change to PIN hashing (`src/lib/water-log-pin.ts` is correct).
+- No change to the photo-upload route — it already uses the cookie.
+- No change to QR-code routes — they're truly public.
+- No change to `/api/water-logs/export` or `/api/v1/water-logs` —
+  both are correctly site-admin-gated via `getAdminUser()` and
+  belong on the platform login.
+
+## 9. Rollout
+
+- Single branch `fix/water-log-no-platform-login`, single PR.
+- Merge → push to `origin/main` → `.gitea/workflows/deploy.yml` runs
+  the normal build + migration gate (no migration in this change).
+- Existing `wl_session` / `wl_admin_session` cookies continue to work.
+- No data backfill.
+- Rollback = revert the commit. The `getSession()` calls being removed
+  were inert in practice (just slow / occasionally failing), so any
+  caller that worked before this change will continue to work after.
+
+## 10. Open questions
+
+None at design time. Decisions taken:
+
+- Helpers move to `auth.ts`, not split across files.
+- `verifyWaterAdminPin` keeps the best-effort `getAdminUser()` call for
+  audit, wrapped in try/catch.
+- `getWaterAdminSettings` / `updateWaterAdminSettings` / `regenerateAdminPin`
+  stay site-admin-gated (these mutate brand-level settings, which is a
+  site-admin action — the brand water admin does not manage them).
+- Field session type union is preserved verbatim from current `field.ts`.