From e3c5ebb337474dc2578b39228de8a4a98c448f72 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Sun, 29 Mar 2026 07:29:35 +0000 Subject: [PATCH 1/7] feat(staff): super user grant/revoke UI with last-user guardrail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - GET /api/staff/me — returns current staff record (includes isSuperUser) - PATCH /api/staff/:id — accepts optional isSuperUser boolean; only super users can change this field; last-super-user guardrail prevents revoking the only active super user - PATCH /api/staff/:id (active=false) — guardrail prevents deactivating the last active super user - DELETE /api/staff/:id — guardrail prevents deleting the last active super user Frontend (Staff.tsx): - Fetches current user via GET /api/staff/me to determine isSuperUser - Shows ★ Super User badge for super user staff rows - Super user sees "+ Grant" button on non-super-user rows - Super user sees "Revoke" toggle on super-user rows (disabled if last one) - Deactivate button disabled for the last active super user - Error message displayed when backend guardrail triggers GRO-206 Co-Authored-By: Paperclip --- apps/api/src/routes/staff.ts | 81 +++++++++++++++++++++++++++++++++++- apps/web/src/pages/Staff.tsx | 68 ++++++++++++++++++++++++++++-- 2 files changed, 144 insertions(+), 5 deletions(-) diff --git a/apps/api/src/routes/staff.ts b/apps/api/src/routes/staff.ts index 3316c45..05b334d 100644 --- a/apps/api/src/routes/staff.ts +++ b/apps/api/src/routes/staff.ts @@ -15,7 +15,18 @@ const createStaffSchema = z.object({ active: z.boolean().default(true), }); -const updateStaffSchema = createStaffSchema.partial().omit({ email: true }); +const updateStaffSchema = z.object({ + name: z.string().min(1).max(200).optional(), + role: z.enum(["groomer", "receptionist", "manager"]).optional(), + active: z.boolean().optional(), + isSuperUser: z.boolean().optional(), + oidcSub: z.string().optional(), +}); + +staffRouter.get("/me", async (c) => { + const staffRow = c.get("staff"); + return c.json(staffRow); +}); staffRouter.get("/", async (c) => { const db = getDb(); @@ -45,11 +56,57 @@ staffRouter.post("/", zValidator("json", createStaffSchema), async (c) => { staffRouter.patch("/:id", zValidator("json", updateStaffSchema), async (c) => { const db = getDb(); + const currentStaff = c.get("staff"); const body = c.req.valid("json"); + const targetId = c.req.param("id"); + + // Only super users can change isSuperUser + if (body.isSuperUser !== undefined && !currentStaff.isSuperUser) { + return c.json({ error: "Forbidden: super user privileges required to modify super user status" }, 403); + } + + // Before revoking super user, check if this is the last one + if (body.isSuperUser === false) { + const superUserCount = await db + .select({ id: staff.id }) + .from(staff) + .where(and(eq(staff.isSuperUser, true), eq(staff.active, true))) + .limit(2); + // If only 1 or 0 active super users and the target is one of them, block revoke + if (superUserCount.length <= 1) { + return c.json( + { error: "Cannot revoke the last super user. Assign another super user first." }, + 400 + ); + } + } + + // When deactivating a super user, also check last-super-user guardrail + if (body.active === false) { + const [target] = await db + .select({ isSuperUser: staff.isSuperUser }) + .from(staff) + .where(eq(staff.id, targetId)) + .limit(1); + if (target?.isSuperUser) { + const superUserCount = await db + .select({ id: staff.id }) + .from(staff) + .where(and(eq(staff.isSuperUser, true), eq(staff.active, true))) + .limit(2); + if (superUserCount.length <= 1) { + return c.json( + { error: "Cannot deactivate the last super user. Assign another super user first." }, + 400 + ); + } + } + } + const [row] = await db .update(staff) .set({ ...body, updatedAt: new Date() }) - .where(eq(staff.id, c.req.param("id"))) + .where(eq(staff.id, targetId)) .returning(); if (!row) return c.json({ error: "Not found" }, 404); return c.json(row); @@ -81,6 +138,26 @@ staffRouter.delete("/:id", async (c) => { ); } + // Prevent deleting the last super user + const [targetStaff] = await db + .select({ isSuperUser: staff.isSuperUser }) + .from(staff) + .where(eq(staff.id, id)) + .limit(1); + if (targetStaff?.isSuperUser) { + const superUserCount = await db + .select({ id: staff.id }) + .from(staff) + .where(and(eq(staff.isSuperUser, true), eq(staff.active, true))) + .limit(2); + if (superUserCount.length <= 1) { + return c.json( + { error: "Cannot delete the last super user. Assign another super user first." }, + 400 + ); + } + } + const [row] = await db .delete(staff) .where(eq(staff.id, id)) diff --git a/apps/web/src/pages/Staff.tsx b/apps/web/src/pages/Staff.tsx index aac23a7..db75145 100644 --- a/apps/web/src/pages/Staff.tsx +++ b/apps/web/src/pages/Staff.tsx @@ -11,6 +11,7 @@ const EMPTY_FORM: StaffForm = { name: "", email: "", role: "groomer" }; export function StaffPage() { const [staff, setStaff] = useState([]); + const [me, setMe] = useState(null); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [editing, setEditing] = useState(null); @@ -18,6 +19,11 @@ export function StaffPage() { const [form, setForm] = useState(EMPTY_FORM); const [formError, setFormError] = useState(null); const [saving, setSaving] = useState(false); + const [togglingSuperUser, setTogglingSuperUser] = useState(null); + const [toggleError, setToggleError] = useState(null); + + const isCurrentUserSuperUser = me?.isSuperUser ?? false; + const activeSuperUserCount = staff.filter((s) => s.active && s.isSuperUser).length; async function load() { const r = await fetch("/api/staff?includeInactive=true"); @@ -25,8 +31,15 @@ export function StaffPage() { setStaff((await r.json()) as Staff[]); } + async function loadMe() { + const r = await fetch("/api/staff/me"); + if (r.ok) { + setMe((await r.json()) as Staff); + } + } + useEffect(() => { - load() + Promise.all([load(), loadMe()]) .catch((e: unknown) => setError(e instanceof Error ? e.message : "Unknown error")) .finally(() => setLoading(false)); }, []); @@ -71,6 +84,27 @@ export function StaffPage() { await load(); } + async function toggleSuperUser(s: Staff) { + setTogglingSuperUser(s.id); + setToggleError(null); + try { + const newValue = !s.isSuperUser; + const res = await fetch(`/api/staff/${s.id}`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ isSuperUser: newValue }), + }); + if (!res.ok) { + const err = (await res.json()) as { error?: string }; + setToggleError(err.error ?? `HTTP ${res.status}`); + return; + } + await load(); + } finally { + setTogglingSuperUser(null); + } + } + if (loading) return

Loading…

; if (error) return

Error: {error}

; @@ -83,6 +117,12 @@ export function StaffPage() { + {toggleError && ( +

+ {toggleError} +

+ )} + {staff.length === 0 ? (

No staff members yet.

) : ( @@ -90,7 +130,7 @@ export function StaffPage() { - {["Name", "Email", "Role", "Status", ""].map((h) => ( + {["Name", "Email", "Role", "Super User", "Status", ""].map((h) => ( ))} @@ -101,6 +141,21 @@ export function StaffPage() { + ))} -- 2.52.0 From c76a37b15cc41581338c7a7769db105bf25fc898 Mon Sep 17 00:00:00 2001 From: "groombook-ci[bot]" Date: Sun, 29 Mar 2026 12:11:53 +0000 Subject: [PATCH 2/7] fix(staff): add revoke button to super user rows + serialize guardrail in transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frontend: - Super users now see a "Revoke" button (disabled when last super user) alongside the ★ badge on super-user rows in the Staff table. Non-super-user rows show the existing "+ Grant" button. Backend (race condition fix): - PATCH /api/staff/:id (isSuperUser=false or active=false): count check + update now wrapped in a db.transaction() with FOR UPDATE lock on the target row, preventing a race where two concurrent revokes could both pass the guard and leave zero super users. - DELETE /api/staff/:id: same transaction + FOR UPDATE guard applied. GRO-206 CTO review feedback Co-Authored-By: Paperclip --- apps/api/src/routes/staff.ts | 128 ++++++++++++++++++++--------------- apps/web/src/pages/Staff.tsx | 18 ++++- 2 files changed, 90 insertions(+), 56 deletions(-) diff --git a/apps/api/src/routes/staff.ts b/apps/api/src/routes/staff.ts index 05b334d..4f5498c 100644 --- a/apps/api/src/routes/staff.ts +++ b/apps/api/src/routes/staff.ts @@ -65,42 +65,61 @@ staffRouter.patch("/:id", zValidator("json", updateStaffSchema), async (c) => { return c.json({ error: "Forbidden: super user privileges required to modify super user status" }, 403); } - // Before revoking super user, check if this is the last one - if (body.isSuperUser === false) { - const superUserCount = await db - .select({ id: staff.id }) - .from(staff) - .where(and(eq(staff.isSuperUser, true), eq(staff.active, true))) - .limit(2); - // If only 1 or 0 active super users and the target is one of them, block revoke - if (superUserCount.length <= 1) { - return c.json( - { error: "Cannot revoke the last super user. Assign another super user first." }, - 400 - ); - } - } + // Before revoking or deactivating the last super user, serialize access with a + // transaction + FOR UPDATE to prevent a race where two concurrent requests both + // pass the count check and leave zero super users. + const needsSuperUserGuard = body.isSuperUser === false || body.active === false; + if (needsSuperUserGuard) { + const [guardError, row] = await db.transaction(async (tx) => { + // Lock the target row so no other request can modify it concurrently + const [target] = await tx + .select({ isSuperUser: staff.isSuperUser }) + .from(staff) + .where(eq(staff.id, targetId)) + .limit(1) + .for("update"); - // When deactivating a super user, also check last-super-user guardrail - if (body.active === false) { - const [target] = await db - .select({ isSuperUser: staff.isSuperUser }) - .from(staff) - .where(eq(staff.id, targetId)) - .limit(1); - if (target?.isSuperUser) { - const superUserCount = await db + if (!target) return ["Not found", null as (typeof staff.$inferSelect | null)]; + + // Only enforce guard if the target is actually a super user + const isRevokingSuperUser = body.isSuperUser === false && target.isSuperUser; + const isDeactivatingSuperUser = body.active === false && target.isSuperUser; + if (!isRevokingSuperUser && !isDeactivatingSuperUser) { + const [updated] = await tx + .update(staff) + .set({ ...body, updatedAt: new Date() }) + .where(eq(staff.id, targetId)) + .returning(); + return [null, updated]; + } + + // Count active super users (excluding target — it will be changed) + const superUserCount = await tx .select({ id: staff.id }) .from(staff) - .where(and(eq(staff.isSuperUser, true), eq(staff.active, true))) + .where(and(eq(staff.isSuperUser, true), eq(staff.active, true), ne(staff.id, targetId))) .limit(2); + if (superUserCount.length <= 1) { - return c.json( - { error: "Cannot deactivate the last super user. Assign another super user first." }, - 400 - ); + return [ + body.isSuperUser === false + ? "Cannot revoke the last super user. Assign another super user first." + : "Cannot deactivate the last super user. Assign another super user first.", + null, + ]; } - } + + const [updated] = await tx + .update(staff) + .set({ ...body, updatedAt: new Date() }) + .where(eq(staff.id, targetId)) + .returning(); + return [null, updated]; + }); + + if (guardError) return c.json({ error: guardError }, guardError === "Not found" ? 404 : 400); + if (!row) return c.json({ error: "Not found" }, 404); + return c.json(row); } const [row] = await db @@ -138,31 +157,34 @@ staffRouter.delete("/:id", async (c) => { ); } - // Prevent deleting the last super user - const [targetStaff] = await db - .select({ isSuperUser: staff.isSuperUser }) - .from(staff) - .where(eq(staff.id, id)) - .limit(1); - if (targetStaff?.isSuperUser) { - const superUserCount = await db - .select({ id: staff.id }) + // Prevent deleting the last super user — use transaction to avoid race + const [guardError, deleted] = await db.transaction(async (tx) => { + const [targetStaff] = await tx + .select({ isSuperUser: staff.isSuperUser }) .from(staff) - .where(and(eq(staff.isSuperUser, true), eq(staff.active, true))) - .limit(2); - if (superUserCount.length <= 1) { - return c.json( - { error: "Cannot delete the last super user. Assign another super user first." }, - 400 - ); - } - } + .where(eq(staff.id, id)) + .limit(1) + .for("update"); - const [row] = await db - .delete(staff) - .where(eq(staff.id, id)) - .returning(); - if (!row) return c.json({ error: "Not found" }, 404); + if (!targetStaff) return ["Not found", null]; + + if (targetStaff.isSuperUser) { + const superUserCount = await tx + .select({ id: staff.id }) + .from(staff) + .where(and(eq(staff.isSuperUser, true), eq(staff.active, true), ne(staff.id, id))) + .limit(2); + if (superUserCount.length <= 1) { + return ["Cannot delete the last super user. Assign another super user first.", null]; + } + } + + const [row] = await tx.delete(staff).where(eq(staff.id, id)).returning(); + return [null, row]; + }); + + if (guardError === "Not found") return c.json({ error: "Not found" }, 404); + if (guardError) return c.json({ error: guardError }, 400); return c.json({ ok: true }); }); diff --git a/apps/web/src/pages/Staff.tsx b/apps/web/src/pages/Staff.tsx index db75145..1b12a85 100644 --- a/apps/web/src/pages/Staff.tsx +++ b/apps/web/src/pages/Staff.tsx @@ -143,9 +143,21 @@ export function StaffPage() {
{h}
{s.name} {s.email} {s.role} + {s.isSuperUser ? ( + + ★ Super User + + ) : isCurrentUserSuperUser ? ( + + ) : null} + {s.active ? "Active" : "Inactive"} @@ -108,7 +163,14 @@ export function StaffPage() { - +
{s.role} {s.isSuperUser ? ( - - ★ Super User - + <> + + ★ Super User + + {isCurrentUserSuperUser && s.id !== me?.id && ( + + )} + ) : isCurrentUserSuperUser ? (