From 57e9670410b252b582cc23ca2e2ada7070db1107 Mon Sep 17 00:00:00 2001
From: "groombook-engineer[bot]" <groombook-engineer@users.noreply.github.com>
Date: Sat, 28 Mar 2026 01:48:25 +0000
Subject: [PATCH] fix(rbac): fallback lookup for staff records predating
 Better-Auth userId

GRO-153: /api/staff returned 403 for all staff because resolveStaffMiddleware
looked up by staff.userId (Better-Auth ID) but dev login sent staff.id (PK),
and existing staff records had userId=NULL.

Changes:
- resolveStaffMiddleware: try userId first, fall back to staff.id (dev mode)
- resolveStaffMiddleware: try userId first, fall back to oidcSub (production)
- GET /api/dev/users: include userId field for DevLoginSelector
- DevLoginSelector: send userId (not staff.id) as X-Dev-User-Id
- Migration 0018: backfill userId for known demo staff

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 apps/api/src/middleware/rbac.ts               | 31 ++++++++++++++++---
 apps/api/src/routes/dev.ts                    |  1 +
 apps/web/src/pages/DevLoginSelector.tsx       |  3 +-
 .../0018_backfill_staff_user_id.sql           | 14 +++++++++
 packages/db/migrations/meta/_journal.json     |  7 +++++
 5 files changed, 50 insertions(+), 6 deletions(-)
 create mode 100644 packages/db/migrations/0018_backfill_staff_user_id.sql

diff --git a/apps/api/src/middleware/rbac.ts b/apps/api/src/middleware/rbac.ts
index 8dcd93f..1bc2228 100644
--- a/apps/api/src/middleware/rbac.ts
+++ b/apps/api/src/middleware/rbac.ts
@@ -40,18 +40,29 @@ export const resolveStaffMiddleware: MiddlewareHandler<AppEnv> = async (
       await next();
       return;
     }
-    // Treat X-Dev-User-Id as the Better-Auth user ID
+    // Treat X-Dev-User-Id as the Better-Auth user ID first
     const [row] = await db
       .select()
       .from(staff)
       .where(eq(staff.userId, devUserId));
-    if (!row) {
+    if (row) {
+      c.set("staff", row);
+      await next();
+      return;
+    }
+    // Fallback: if userId is null, treat X-Dev-User-Id as staff.id (dev login
+    // may send the primary key for staff records that predate the userId field)
+    const [fallbackRow] = await db
+      .select()
+      .from(staff)
+      .where(eq(staff.id, devUserId));
+    if (!fallbackRow) {
       return c.json(
         { error: "Forbidden: no staff record found for X-Dev-User-Id" },
         403
       );
     }
-    c.set("staff", row);
+    c.set("staff", fallbackRow);
     await next();
     return;
   }
@@ -61,13 +72,23 @@ export const resolveStaffMiddleware: MiddlewareHandler<AppEnv> = async (
     .select()
     .from(staff)
     .where(eq(staff.userId, jwt.sub));
-  if (!row) {
+  if (row) {
+    c.set("staff", row);
+    await next();
+    return;
+  }
+  // Fallback: staff records that predate the userId field may still have oidcSub
+  const [fallbackRow] = await db
+    .select()
+    .from(staff)
+    .where(eq(staff.oidcSub, jwt.sub));
+  if (!fallbackRow) {
     return c.json(
       { error: "Forbidden: no staff record found for authenticated user" },
       403
     );
   }
-  c.set("staff", row);
+  c.set("staff", fallbackRow);
   await next();
 };
 
diff --git a/apps/api/src/routes/dev.ts b/apps/api/src/routes/dev.ts
index dfc5708..363da85 100644
--- a/apps/api/src/routes/dev.ts
+++ b/apps/api/src/routes/dev.ts
@@ -20,6 +20,7 @@ devRouter.get("/users", async (c) => {
   const staffList = await db
     .select({
       id: staff.id,
+      userId: staff.userId,
       name: staff.name,
       email: staff.email,
       role: staff.role,
diff --git a/apps/web/src/pages/DevLoginSelector.tsx b/apps/web/src/pages/DevLoginSelector.tsx
index e171613..6de753b 100644
--- a/apps/web/src/pages/DevLoginSelector.tsx
+++ b/apps/web/src/pages/DevLoginSelector.tsx
@@ -3,6 +3,7 @@ import { useNavigate } from "react-router-dom";
 
 interface StaffUser {
   id: string;
+  userId: string | null;
   name: string;
   email: string;
   role: string;
@@ -66,7 +67,7 @@ export function DevLoginSelector() {
           {staff.map((s) => (
             <button
               key={s.id}
-              onClick={() => selectUser("staff", s.id, s.name)}
+              onClick={() => selectUser("staff", s.userId ?? s.id, s.name)}
               style={userButtonStyle}
             >
               <div style={{ fontWeight: 600, fontSize: 14 }}>{s.name}</div>
diff --git a/packages/db/migrations/0018_backfill_staff_user_id.sql b/packages/db/migrations/0018_backfill_staff_user_id.sql
new file mode 100644
index 0000000..9da9f54
--- /dev/null
+++ b/packages/db/migrations/0018_backfill_staff_user_id.sql
@@ -0,0 +1,14 @@
+-- Backfill staff.user_id for staff records created before Better-Auth integration.
+-- Staff records that predate this migration have user_id = NULL; the resolveStaffMiddleware
+-- now falls back to staff.id (dev mode) and oidcSub (production) so these records still work.
+-- This migration populates user_id for the known demo/dev staff seeded by seed.ts.
+
+-- Create demo Better-Auth users for seeded staff (these match the ba-user-* IDs used in tests)
+INSERT INTO "user" (id, name, email, email_verified, created_at, updated_at)
+VALUES ('ba-user-manager', 'Demo Manager', 'demo-manager@groombook.dev', true, NOW(), NOW())
+ON CONFLICT (id) DO NOTHING;
+
+-- Link the demo manager staff record to the Better-Auth user
+UPDATE staff
+SET user_id = 'ba-user-manager', updated_at = NOW()
+WHERE oidc_sub = 'demo-manager-001' AND user_id IS NULL;
diff --git a/packages/db/migrations/meta/_journal.json b/packages/db/migrations/meta/_journal.json
index affedd1..9bc272a 100644
--- a/packages/db/migrations/meta/_journal.json
+++ b/packages/db/migrations/meta/_journal.json
@@ -127,6 +127,13 @@
       "when": 1774512000000,
       "tag": "0017_better_auth_tables",
       "breakpoints": true
+    },
+    {
+      "idx": 18,
+      "version": "7",
+      "when": 1774598400000,
+      "tag": "0018_backfill_staff_user_id",
+      "breakpoints": true
     }
   ]
 }
\ No newline at end of file