fix(water-log): make verifyWaterAdminPin fully PIN-only
Deploy to route.crispygoat.com / deploy (push) Successful in 4m3s
Deploy to route.crispygoat.com / deploy (push) Successful in 4m3s
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.
This commit is contained in:
@@ -88,12 +88,13 @@ The site-admin helper (`requireWaterAdminPermission`) keeps using
|
|||||||
`/admin/water-log/*` (which sits behind Neon Auth in the middleware
|
`/admin/water-log/*` (which sits behind Neon Auth in the middleware
|
||||||
and on the server).
|
and on the server).
|
||||||
|
|
||||||
> Note: the action `verifyWaterAdminPin` (in `settings.ts`) does make a
|
> Note: the action `verifyWaterAdminPin` (in `settings.ts`) is now a
|
||||||
> best-effort `getAdminUser()` call *after* a successful PIN check, but
|
> fully PIN-only flow — it does NOT call `getAdminUser()` or
|
||||||
> only to attach a platform-admin `adminUserId` to the new
|
> `getSession()` at all. The `adminUserId` on the new
|
||||||
> `water_admin_sessions` row for audit. It is wrapped in try/catch and
|
> `water_admin_sessions` row is always the zero-UUID placeholder. This
|
||||||
> falls back to a zero-UUID placeholder. The `wl_admin_session` is
|
> is because `/water/admin/login` is the Tuxedo-brand-specific entry
|
||||||
> valid either way — that helper call is not an auth gate.
|
> 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`
|
### 4.2 New file: `src/actions/water-log/auth.ts`
|
||||||
|
|
||||||
@@ -174,23 +175,25 @@ moves.
|
|||||||
keep their existing `getAdminUser()` gate (site-admin path).
|
keep their existing `getAdminUser()` gate (site-admin path).
|
||||||
- `verifyWaterAdminPin` becomes a pure PIN-only flow: validate format,
|
- `verifyWaterAdminPin` becomes a pure PIN-only flow: validate format,
|
||||||
look up `water_admin_settings` for the brand, verify PIN, mint
|
look up `water_admin_settings` for the brand, verify PIN, mint
|
||||||
`wl_admin_session` cookie. **No `getSession()` call.** `getAdminUser()`
|
`wl_admin_session` cookie. **No `getSession()` call. No
|
||||||
is still called inside the success path to attach an `adminUserId`
|
`getAdminUser()` call.** `adminUserId` is always the zero-UUID
|
||||||
to the new session row for audit — wrapped in try/catch, falls back
|
placeholder. The page is the Tuxedo-brand-specific entry point and
|
||||||
to a zero-UUID placeholder if no admin user is signed in. The
|
must work without a platform login.
|
||||||
session itself is valid either way.
|
|
||||||
|
|
||||||
### 4.4 Why `verifyWaterAdminPin` is a special case
|
### 4.4 Why `verifyWaterAdminPin` is a special case
|
||||||
|
|
||||||
`verifyWaterAdminPin` is the most-affected function in this PR. Today:
|
`verifyWaterAdminPin` is the most-affected function in this PR. Today:
|
||||||
|
|
||||||
1. It calls `await getSession()` (Neon Auth) — drop.
|
1. It called `await getSession()` (Neon Auth) — dropped.
|
||||||
2. It calls `getAdminUser()` inside the success path to attach the
|
2. It called `getAdminUser()` inside the success path to attach the
|
||||||
caller's platform-admin user to the new `wl_admin_session` row for
|
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
|
audit. That call was wrapped in a try/catch that fell back to a
|
||||||
a try/catch that swallows any error and falls back to a zero-UUID
|
zero-UUID `adminUserId`, but the underlying `getSession()` call
|
||||||
`adminUserId`**. The `wl_admin_session` is still valid in either
|
inside `getAdminUser()` could still corrupt the Next.js cookie
|
||||||
case — the cookie holds the brand + session id, not the admin user.
|
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
|
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
|
riders / brand water admins, who often aren't platform admins. Tying
|
||||||
|
|||||||
@@ -226,17 +226,13 @@ const formatError = validatePin(pin);
|
|||||||
return { success: false, error: "Invalid PIN" };
|
return { success: false, error: "Invalid PIN" };
|
||||||
}
|
}
|
||||||
|
|
||||||
// Tie the session to the calling site admin (best-effort).
|
// The `/water/admin/login` page is Tuxedo-brand-specific and PIN-only.
|
||||||
// The `wl_admin_session` is valid whether or not a platform admin
|
// There is no platform-admin requirement here — `wl_admin_session` is
|
||||||
// is signed in — this call only attaches an `adminUserId` for
|
// valid whether or not a Neon Auth session exists. We deliberately do
|
||||||
// audit. If `getAdminUser()` fails or returns null, we fall back
|
// NOT call `getAdminUser()` (it would touch cookies via Neon Auth and
|
||||||
// to a zero-UUID placeholder.
|
// 500 for users without a platform login). `adminUserId` is therefore
|
||||||
let adminUser: Awaited<ReturnType<typeof getAdminUser>> = null;
|
// always the zero-UUID placeholder; audit attribution happens elsewhere.
|
||||||
try {
|
const PLACEHOLDER_ADMIN_USER_ID = "00000000-0000-0000-0000-000000000000";
|
||||||
adminUser = await getAdminUser();
|
|
||||||
} catch {
|
|
||||||
// ignore — the session is valid regardless
|
|
||||||
}
|
|
||||||
|
|
||||||
const expiresAt = new Date(
|
const expiresAt = new Date(
|
||||||
Date.now() + settings.sessionDurationHours * 60 * 60 * 1000,
|
Date.now() + settings.sessionDurationHours * 60 * 60 * 1000,
|
||||||
@@ -245,7 +241,7 @@ const formatError = validatePin(pin);
|
|||||||
.insert(waterAdminSessions)
|
.insert(waterAdminSessions)
|
||||||
.values({
|
.values({
|
||||||
brandId,
|
brandId,
|
||||||
adminUserId: adminUser?.user_id ?? adminUser?.id ?? "00000000-0000-0000-0000-000000000000",
|
adminUserId: PLACEHOLDER_ADMIN_USER_ID,
|
||||||
pinHashUsed: settings.pinHash,
|
pinHashUsed: settings.pinHash,
|
||||||
expiresAt,
|
expiresAt,
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -3,6 +3,19 @@ import WaterAdminPinClient from "./WaterAdminPinClient";
|
|||||||
|
|
||||||
export const dynamic = "force-dynamic";
|
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";
|
const TUXEDO_BRAND_ID = "64294306-5f42-463d-a5e8-2ad6c81a96de";
|
||||||
|
|
||||||
export default async function WaterAdminLoginPage() {
|
export default async function WaterAdminLoginPage() {
|
||||||
|
|||||||
@@ -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<string> {
|
||||||
|
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 ─────────────────────────────────────────────────
|
// ── requireFieldSession ─────────────────────────────────────────────────
|
||||||
|
|
||||||
describe("requireFieldSession()", () => {
|
describe("requireFieldSession()", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user