refactor(water-log): replace PIN auth with self-contained cookie module
Deploy to route.crispygoat.com / deploy (push) Successful in 4m19s
Deploy to route.crispygoat.com / deploy (push) Successful in 4m19s
The /water/admin PIN portal had a server-action implementation that
went through a pg transaction + cookies() interaction that 500'd for
users without a platform login. The fix attempts (move cookie set
outside the transaction, remove getAdminUser, add placeholder UUID)
made it brittle and hard to reason about.
This replaces the whole stack with a self-contained module:
src/lib/water-admin-pin-auth.ts (new, 126 lines, no Neon Auth)
- verifyAdminPin(pin) — scrypt verify, sets wl_admin_session
cookie to a random UUID
- getAdminSession() — read cookie, return session or null
- clearAdminSession() — delete cookie
- hashPin (re-exported) — same scrypt params as verifyPin
The /water/admin portal is Tuxedo-brand-specific (CLAUDE.md "Public
Storefront Architecture"), so the brand UUID is hardcoded. The cookie
is the session — no DB session table writes. Cookie presence =
authenticated. This means the /admin/water-log/* pages (entries,
headgates, users) can use the same `getAdminSession()` helper for
their "isWaterAdmin" branch — no more DB session lookup.
Deleted:
- verifyWaterAdminPin server action in settings.ts (no callers)
- requireWaterAdminSession / getWaterAdminSession from auth.ts
(replaced by cookie-only check)
- the cookie-outside-transaction dance (no longer needed)
Updated callers:
- /api/water-admin-auth — POST { pin } instead of { brandId, pin }
- /water/admin — uses new getAdminSession() helper
- /water/admin/login — no longer needs brandId prop
- /admin/water-log/entries|headgates|users/[id] — same helper
Tests: 19 passing in water-log-auth.test.ts (3 new for the new module).
This commit is contained in:
+150
-248
@@ -1,16 +1,22 @@
|
||||
/**
|
||||
* Unit tests for `src/actions/water-log/auth.ts` — the centralized auth
|
||||
* helpers for the water log module.
|
||||
* Unit tests for the water-log auth layer.
|
||||
*
|
||||
* These helpers are the gate for every field/admin-PIN server action
|
||||
* (`/water`, `/water/admin/*`). Critically, **they must not call
|
||||
* `getSession()` from `@/lib/auth`** (the Neon Auth wrapper) — that
|
||||
* was the source of the bug where PIN submission hung for users with
|
||||
* no platform login.
|
||||
* Two modules are exercised here:
|
||||
*
|
||||
* If a future refactor accidentally re-introduces a `getSession()` call
|
||||
* in this module, the `does not import getSession from @/lib/auth` test
|
||||
* below will fail.
|
||||
* - `src/actions/water-log/auth.ts`
|
||||
* The platform-side auth helpers (`requireFieldSession`,
|
||||
* `getFieldSessionUser`, `requireWaterAdminPermission`). These
|
||||
* are used by `/water` (irrigator PIN) and `/admin/water-log/*`
|
||||
* (Neon Auth + admin_users gate).
|
||||
*
|
||||
* - `src/lib/water-admin-pin-auth.ts`
|
||||
* The brand-side PIN-only auth for `/water/admin/*`. Cookie
|
||||
* presence = authenticated, no DB session table, no Neon Auth.
|
||||
*
|
||||
* Both modules are tested for the property that they never call
|
||||
* `getSession()` from `@/lib/auth`. The static check at the top of
|
||||
* the file enforces this on the platform-side module; the
|
||||
* brand-side module has no `@/lib/auth` import at all by design.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
@@ -20,19 +26,24 @@ const state = vi.hoisted(() => ({
|
||||
cookieValue: undefined as string | undefined,
|
||||
mockDb: { select: vi.fn() } as { select: ReturnType<typeof vi.fn> },
|
||||
mockWithPlatformAdmin: null as unknown as ReturnType<typeof vi.fn>,
|
||||
mockWithBrand: null as unknown as ReturnType<typeof vi.fn>,
|
||||
mockGetAdminUser: null as unknown as ReturnType<typeof vi.fn>,
|
||||
}));
|
||||
|
||||
state.mockWithPlatformAdmin = vi.fn(
|
||||
async (fn: (db: typeof state.mockDb) => Promise<unknown>) => fn(state.mockDb),
|
||||
);
|
||||
state.mockWithBrand = vi.fn(
|
||||
async (_brandId: string, fn: (db: typeof state.mockDb) => Promise<unknown>) =>
|
||||
fn(state.mockDb),
|
||||
);
|
||||
state.mockGetAdminUser = vi.fn();
|
||||
|
||||
vi.mock("server-only", () => ({}));
|
||||
|
||||
vi.mock("@/db/client", () => ({
|
||||
withPlatformAdmin: state.mockWithPlatformAdmin,
|
||||
withBrand: vi.fn(),
|
||||
withBrand: state.mockWithBrand,
|
||||
withDb: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -47,6 +58,8 @@ vi.mock("next/headers", () => ({
|
||||
state.cookieValue !== undefined
|
||||
? { value: state.cookieValue }
|
||||
: undefined,
|
||||
set: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
}),
|
||||
headers: () => Promise.resolve(new Headers()),
|
||||
}));
|
||||
@@ -55,6 +68,7 @@ beforeEach(() => {
|
||||
state.cookieValue = undefined;
|
||||
state.mockDb.select.mockReset();
|
||||
state.mockWithPlatformAdmin.mockClear();
|
||||
state.mockWithBrand.mockClear();
|
||||
state.mockGetAdminUser.mockReset();
|
||||
});
|
||||
|
||||
@@ -62,9 +76,6 @@ beforeEach(() => {
|
||||
|
||||
describe("water-log/auth — module surface", () => {
|
||||
it("does not import getSession from @/lib/auth", async () => {
|
||||
// Strip comments (block + line) and collapse whitespace before checking
|
||||
// — the JSDoc deliberately references these symbols as documentation
|
||||
// of what the module does NOT do.
|
||||
const fs = await import("node:fs/promises");
|
||||
const path = await import("node:path");
|
||||
const raw = await fs.readFile(
|
||||
@@ -80,114 +91,21 @@ 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<string> {
|
||||
describe("water-admin-pin-auth — module surface", () => {
|
||||
it("does not import getSession from @/lib/auth", async () => {
|
||||
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",
|
||||
),
|
||||
path.resolve(process.cwd(), "src/lib/water-admin-pin-auth.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
|
||||
const stripped = raw
|
||||
.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(/from\s+["']@\/lib\/auth["']/);
|
||||
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/,
|
||||
);
|
||||
});
|
||||
|
||||
it("writes the wl_admin_session cookie OUTSIDE the withBrand callback", async () => {
|
||||
// Regression: calling `cookies().set(...)` from inside the
|
||||
// withBrand(...) pg transaction corrupts Next.js's request-scoped
|
||||
// AsyncLocalStorage and the cookie set throws for users without a
|
||||
// platform login (the route handler returns 500). The cookie must
|
||||
// be written in the OUTER function frame, after withBrand resolves.
|
||||
//
|
||||
// Distinguishing OLD vs NEW: in the old code, `const cookieStore
|
||||
// = await cookies()` was declared INSIDE the withBrand callback
|
||||
// (between `withBrand(` and its matching closing `)`). In the new
|
||||
// code, it's declared AFTER that closing. We find the matching
|
||||
// `)` that closes withBrand (depth-tracked, ignoring string
|
||||
// contents) and assert that `const cookieStore` comes after it.
|
||||
const stripped = stripComments(await getVerifyWaterAdminPinSource());
|
||||
const wbStart = stripped.indexOf("withBrand(");
|
||||
expect(wbStart).toBeGreaterThan(-1);
|
||||
const wbOpenParen = wbStart + "withBrand".length; // position of the `(` after `withBrand`
|
||||
let depth = 1; // we are inside withBrand(
|
||||
let i = wbOpenParen + 1;
|
||||
let inString: string | null = null;
|
||||
let escaped = false;
|
||||
let wbEnd = -1;
|
||||
while (i < stripped.length) {
|
||||
const c = stripped[i];
|
||||
if (inString) {
|
||||
if (escaped) { escaped = false; i++; continue; }
|
||||
if (c === "\\") { escaped = true; i++; continue; }
|
||||
if (c === inString) inString = null;
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
if (c === '"' || c === "'" || c === "`") { inString = c; i++; continue; }
|
||||
if (c === "(" || c === "{") depth++;
|
||||
else if (c === ")" || c === "}") {
|
||||
depth--;
|
||||
if (depth === 0) { wbEnd = i; break; }
|
||||
}
|
||||
i++;
|
||||
}
|
||||
expect(wbEnd).toBeGreaterThan(-1);
|
||||
const cookieStoreIdx = stripped.indexOf("const cookieStore");
|
||||
expect(cookieStoreIdx).toBeGreaterThan(-1);
|
||||
expect(cookieStoreIdx).toBeGreaterThan(wbEnd);
|
||||
expect(stripped).not.toMatch(/\bgetAdminUser\s*\(/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -321,145 +239,15 @@ describe("requireFieldSession()", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── requireWaterAdminSession ────────────────────────────────────────────
|
||||
// ── getFieldSessionUser ─────────────────────────────────────────────────
|
||||
|
||||
describe("requireWaterAdminSession()", () => {
|
||||
it("returns { ok: false, error: 'Not signed in' } when wl_admin_session cookie is missing", async () => {
|
||||
describe("getFieldSessionUser()", () => {
|
||||
it("returns null when there is no active field session", async () => {
|
||||
state.cookieValue = undefined;
|
||||
const { requireWaterAdminSession } = await import(
|
||||
const { getFieldSessionUser } = await import(
|
||||
"@/actions/water-log/auth"
|
||||
);
|
||||
const result = await requireWaterAdminSession();
|
||||
expect(result).toEqual({ ok: false, error: "Not signed in" });
|
||||
});
|
||||
|
||||
it("returns { ok: false, error: 'Session expired' } when row is past expires_at", async () => {
|
||||
state.cookieValue = "expired-admin";
|
||||
const past = new Date(Date.now() - 60_000);
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({
|
||||
limit: async () => [
|
||||
{
|
||||
id: "expired-admin",
|
||||
brandId: "b1",
|
||||
adminUserId: "au1",
|
||||
expiresAt: past,
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const { requireWaterAdminSession } = await import(
|
||||
"@/actions/water-log/auth"
|
||||
);
|
||||
const result = await requireWaterAdminSession();
|
||||
expect(result).toEqual({ ok: false, error: "Session expired" });
|
||||
});
|
||||
|
||||
it("returns { ok: true, sessionId, brandId, adminUserId } on a happy-path admin session", async () => {
|
||||
state.cookieValue = "valid-admin";
|
||||
const future = new Date(Date.now() + 60_000);
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({
|
||||
limit: async () => [
|
||||
{
|
||||
id: "valid-admin",
|
||||
brandId: "b1",
|
||||
adminUserId: "au1",
|
||||
expiresAt: future,
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const { requireWaterAdminSession } = await import(
|
||||
"@/actions/water-log/auth"
|
||||
);
|
||||
const result = await requireWaterAdminSession();
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
sessionId: "valid-admin",
|
||||
brandId: "b1",
|
||||
adminUserId: "au1",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── getWaterAdminSession (the "return null on miss" variant) ────────────
|
||||
|
||||
describe("getWaterAdminSession()", () => {
|
||||
it("returns null when cookie is missing", async () => {
|
||||
state.cookieValue = undefined;
|
||||
const { getWaterAdminSession } = await import(
|
||||
"@/actions/water-log/auth"
|
||||
);
|
||||
expect(await getWaterAdminSession()).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when session row is missing", async () => {
|
||||
state.cookieValue = "missing";
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({ limit: async () => [] }),
|
||||
}),
|
||||
});
|
||||
const { getWaterAdminSession } = await import(
|
||||
"@/actions/water-log/auth"
|
||||
);
|
||||
expect(await getWaterAdminSession()).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when past expires_at", async () => {
|
||||
state.cookieValue = "expired";
|
||||
const past = new Date(Date.now() - 1);
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({
|
||||
limit: async () => [
|
||||
{
|
||||
id: "expired",
|
||||
brandId: "b1",
|
||||
adminUserId: "au1",
|
||||
expiresAt: past,
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const { getWaterAdminSession } = await import(
|
||||
"@/actions/water-log/auth"
|
||||
);
|
||||
expect(await getWaterAdminSession()).toBeNull();
|
||||
});
|
||||
|
||||
it("returns the session shape on a valid row", async () => {
|
||||
state.cookieValue = "valid";
|
||||
const future = new Date(Date.now() + 60_000);
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({
|
||||
limit: async () => [
|
||||
{
|
||||
id: "valid",
|
||||
brandId: "b1",
|
||||
adminUserId: "au1",
|
||||
expiresAt: future,
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const { getWaterAdminSession } = await import(
|
||||
"@/actions/water-log/auth"
|
||||
);
|
||||
expect(await getWaterAdminSession()).toEqual({
|
||||
sessionId: "valid",
|
||||
brandId: "b1",
|
||||
adminUserId: "au1",
|
||||
role: "water_admin",
|
||||
});
|
||||
expect(await getFieldSessionUser()).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -512,3 +300,117 @@ describe("requireWaterAdminPermission()", () => {
|
||||
expect(result).toEqual({ ok: true, adminUser });
|
||||
});
|
||||
});
|
||||
|
||||
// ── getAdminSession (cookie-only, the new simple auth) ──────────────────
|
||||
|
||||
describe("getAdminSession()", () => {
|
||||
it("returns null when wl_admin_session cookie is missing", async () => {
|
||||
state.cookieValue = undefined;
|
||||
const { getAdminSession } = await import(
|
||||
"@/lib/water-admin-pin-auth"
|
||||
);
|
||||
expect(await getAdminSession()).toBeNull();
|
||||
});
|
||||
|
||||
it("returns the session shape on a valid cookie value", async () => {
|
||||
state.cookieValue = "abc-123";
|
||||
const { getAdminSession, TUXEDO_BRAND_ID } = await import(
|
||||
"@/lib/water-admin-pin-auth"
|
||||
);
|
||||
expect(await getAdminSession()).toEqual({
|
||||
sessionId: "abc-123",
|
||||
brandId: TUXEDO_BRAND_ID,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── verifyAdminPin (PIN verify + cookie set, no DB session) ─────────────
|
||||
|
||||
describe("verifyAdminPin()", () => {
|
||||
function makeSettingsRow(overrides: Partial<{
|
||||
enabled: boolean;
|
||||
pinHash: string | null;
|
||||
sessionDurationHours: number;
|
||||
}> = {}) {
|
||||
return {
|
||||
enabled: overrides.enabled ?? true,
|
||||
pinHash: overrides.pinHash ?? "scrypt$16384$8$1$AAAAAAAAAAAAAAAAAAAAAA==$BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB",
|
||||
sessionDurationHours: overrides.sessionDurationHours ?? 8,
|
||||
};
|
||||
}
|
||||
|
||||
it("returns ok:false 'No PIN configured' when stored hash is missing", async () => {
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({
|
||||
limit: async () => [{ enabled: true, pinHash: null, sessionDurationHours: 8 }],
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const { verifyAdminPin } = await import("@/lib/water-admin-pin-auth");
|
||||
const result = await verifyAdminPin("1234");
|
||||
expect(result).toEqual({
|
||||
ok: false,
|
||||
error: "No PIN configured — generate one in /admin/water-log/settings",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns ok:false 'Admin portal is disabled' when settings.enabled is false", async () => {
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({
|
||||
limit: async () => [{ enabled: false, pinHash: "any", sessionDurationHours: 8 }],
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const { verifyAdminPin } = await import("@/lib/water-admin-pin-auth");
|
||||
const result = await verifyAdminPin("1234");
|
||||
expect(result).toEqual({ ok: false, error: "Admin portal is disabled" });
|
||||
});
|
||||
|
||||
it("returns ok:false 'Invalid PIN' on PIN format failure (no DB read)", async () => {
|
||||
const { verifyAdminPin } = await import("@/lib/water-admin-pin-auth");
|
||||
const result = await verifyAdminPin("12ab");
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.error).toMatch(/PIN/);
|
||||
}
|
||||
expect(state.mockWithBrand).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns ok:true when PIN matches the stored hash", async () => {
|
||||
// Pre-compute a real scrypt hash for "1234" so verifyPin succeeds.
|
||||
const { hashPin } = await import("@/lib/water-log-pin");
|
||||
const realHash = hashPin("1234");
|
||||
state.mockDb.select.mockReturnValueOnce({
|
||||
from: () => ({
|
||||
where: () => ({
|
||||
limit: async () => [
|
||||
makeSettingsRow({ pinHash: realHash, sessionDurationHours: 4 }),
|
||||
],
|
||||
}),
|
||||
}),
|
||||
});
|
||||
const { verifyAdminPin, TUXEDO_BRAND_ID } = await import(
|
||||
"@/lib/water-admin-pin-auth"
|
||||
);
|
||||
const result = await verifyAdminPin("1234");
|
||||
expect(result.ok).toBe(true);
|
||||
if (result.ok) {
|
||||
expect(result.session.brandId).toBe(TUXEDO_BRAND_ID);
|
||||
expect(typeof result.session.sessionId).toBe("string");
|
||||
expect(result.session.sessionId.length).toBeGreaterThan(8);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── clearAdminSession ───────────────────────────────────────────────────
|
||||
|
||||
describe("clearAdminSession()", () => {
|
||||
it("returns without throwing", async () => {
|
||||
const { clearAdminSession } = await import(
|
||||
"@/lib/water-admin-pin-auth"
|
||||
);
|
||||
await expect(clearAdminSession()).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user