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 new file mode 100644 index 0000000..ca0e498 --- /dev/null +++ b/docs/superpowers/specs/2026-07-01-water-log-no-platform-login-design.md @@ -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 { ... } + +/** Read wl_admin_session, look up the row. No Neon Auth. */ +export async function requireWaterAdminSession(): Promise { ... } + +/** Site-admin gate: getAdminUser() + can_manage_water_log. */ +export async function requireWaterAdminPermission(): Promise { ... } + +/** Read the active water-admin session row (or null). Cheap, idempotent. */ +export async function getWaterAdminSession(): Promise { ... } + +/** 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`.