From e2dc230b7ffc5def52939d403b5a7ff3cf850071 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Tue, 2 Jun 2026 01:43:54 +0000 Subject: [PATCH] fix(rbac): port Better-Auth user auto-provision into legacy ./src tree (GRO-2052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #139 (a2b09ba) ported the GRO-2013 owner-bypass into the deployed ./src/routes/pets.ts but did NOT port the rbac auto-provision change. As a result, on UAT (api:2026.06.01-4e9c4c5) the owner-bypass code is unreachable for any Better-Auth email/password customer: ./src/middleware/rbac.ts (the deployed tree) only auto-provisions staff rows when account.providerId IN ('authentik','google','github') for jwt.sub. The UAT customer uat-customer@groombook.dev has a row in the Better-Auth `user` table but no row in `account` for those providers, so resolveStaffMiddleware falls through to: 403 "Forbidden: no staff record found for authenticated user" before pets.ts ever runs. The canonical apps/api/src/middleware/rbac.ts already has a Better-Auth user-table fallback. This commit mirrors that branch into the deployed ./src/middleware/rbac.ts. Behaviour ========= - New: when no staff row exists for jwt.sub but the Better-Auth `user` table has a row whose id matches jwt.sub, INSERT a minimal role='groomer', isSuperUser=false, active=true staff row, set it on the request context, and continue. - The legacy OIDC `account` branch is kept as a fallback for backwards compatibility with any pre-Better-Auth OIDC sessions whose user row may not yet exist in `user`. - Lookup order: staff (userId) -> staff (oidcSub) -> staff (email, user_id IS NULL) -> Better-Auth user -> OIDC account -> 403. - Name derivation: userRow.name -> jwt.name -> email prefix -> "Unknown". Tests ===== src/__tests__/rbac.test.ts: - Mock @groombook/db rewritten to be table-aware so SELECTs from `user`/`account`/`staff` route to distinct lookup queues, and insert(staff).values(...).returning() is supported. - buildApp() helper gains an optional jwtOverride param so tests can set jwt.email/name explicitly. - 5 new cases under "resolveStaffMiddleware — auto-provision": 1. Better-Auth user found -> staff row provisioned with role=groomer 2. INSERT returns no row -> 500 "auto-provision failed" 3. Better-Auth branch runs without jwt.email (regression of the pre-fix gate) 4. OIDC fallback still works when user row is missing but account row exists 5. Neither user nor account row -> 403 with "no staff record" message Existing rbac.test.ts cases all keep passing (15 prior cases retained). Full pnpm test on apps/api: 572/572 pass. pnpm typecheck: clean. Scope ===== - ./src/middleware/rbac.ts only — apps/api/src/middleware/rbac.ts already has this branch and is unchanged. - No schema/migration changes; staff and user tables are unchanged. - pre-existing lint error in src/__tests__/petProfileSummary.test.ts:167 (`servicesTable` declared/assigned but never read) introduced by PR #139 a2b09ba is NOT addressed here — it is out of this PR's scope. Resolves: GRO-2052 Refs: GRO-2013, GRO-2050, GRO-2035 Co-Authored-By: Paperclip --- src/__tests__/rbac.test.ts | 239 +++++++++++++++++++++++++++++++++---- src/middleware/rbac.ts | 45 ++++++- 2 files changed, 257 insertions(+), 27 deletions(-) diff --git a/src/__tests__/rbac.test.ts b/src/__tests__/rbac.test.ts index f975316..642ed65 100644 --- a/src/__tests__/rbac.test.ts +++ b/src/__tests__/rbac.test.ts @@ -43,42 +43,103 @@ const GROOMER: StaffRow = { // ─── Mock DB ────────────────────────────────────────────────────────────────── +// staffLookupResult drives every `from(staff)` query that doesn't go through +// the dev-mode `.limit()` shortcut. Tests that want to simulate "no staff row" +// leave it null. let staffLookupResult: StaffRow | null = null; + +// managerFallbackResult is only consumed by the dev-mode `from(staff).limit(1)` +// path (looking up the first manager when AUTH_DISABLED=true and no header). let managerFallbackResult: StaffRow | null = MANAGER; +// userLookupResult drives `from(user).limit(1)` for the Better-Auth user +// auto-provision branch (GRO-2052). Tests that simulate "no Better-Auth user" +// leave it null. +type UserRow = { id: string; name: string | null; email: string | null }; +let userLookupResult: UserRow | null = null; + +// accountLookupResult drives `from(account).limit(1)` for the legacy OIDC +// auto-provision branch. Null means "no OIDC account row". +let accountLookupResult: { id: string } | null = null; + +// insertReturningResult drives `insert(staff).values(...).returning()` for +// any auto-provision branch that actually creates a staff record. Null means +// the INSERT returned no rows (simulating a DB failure). +let insertReturningResult: StaffRow | null = null; + vi.mock("@groombook/db", () => { - const staff = new Proxy( - { _name: "staff" }, - { - get(target, prop) { - if (prop === "_name") return "staff"; - if (prop === "$inferSelect") return {}; - return { table: "staff", column: prop }; - }, - } - ); + function tableMarker(name: string) { + return new Proxy( + { _name: name }, + { + get(_target, prop) { + if (prop === "_name") return name; + if (prop === "$inferSelect") return {}; + return { table: name, column: prop }; + }, + } + ); + } + + const staff = tableMarker("staff"); + const user = tableMarker("user"); + const account = tableMarker("account"); + + function lookupFor(tableName: string) { + if (tableName === "user") return userLookupResult; + if (tableName === "account") return accountLookupResult; + return staffLookupResult; + } return { getDb: () => ({ - select: () => ({ - from: () => ({ - where: () => ({ - limit: () => { - // dev mode fallback to first manager - return managerFallbackResult ? [managerFallbackResult] : []; - }, - [Symbol.iterator]: function* () { - if (staffLookupResult) yield staffLookupResult; - }, - 0: staffLookupResult, - length: staffLookupResult ? 1 : 0, - }), + select: (_columns?: unknown) => ({ + from: (table: { _name?: string }) => { + const name = table?._name ?? "staff"; + return { + where: () => ({ + limit: () => { + // The user / account auto-provision branches always call + // `.limit(1)`; route to the per-table lookup state. + if (name === "user") + return userLookupResult ? [userLookupResult] : []; + if (name === "account") + return accountLookupResult ? [accountLookupResult] : []; + // dev-mode `from(staff).limit(1)` falls back to the first + // manager when AUTH_DISABLED is set with no header. + return managerFallbackResult ? [managerFallbackResult] : []; + }, + [Symbol.iterator]: function* () { + const row = lookupFor(name); + if (row) yield row; + }, + 0: lookupFor(name), + length: lookupFor(name) ? 1 : 0, + }), + }; + }, + }), + insert: (_table: unknown) => ({ + values: (_v: unknown) => ({ + returning: () => + insertReturningResult ? [insertReturningResult] : [], + }), + }), + update: (_table: unknown) => ({ + set: (_v: unknown) => ({ + where: () => Promise.resolve(undefined), }), }), }), staff, + user, + account, eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })), and: vi.fn((..._clauses: unknown[]) => ({})), + sql: Object.assign( + vi.fn((..._tpl: unknown[]) => ({})), + { raw: vi.fn(() => ({})) } + ), }; }); @@ -87,16 +148,25 @@ vi.mock("@groombook/db", () => { function resetMocks() { staffLookupResult = null; managerFallbackResult = MANAGER; + userLookupResult = null; + accountLookupResult = null; + insertReturningResult = null; } /** Build a minimal Hono app with jwtPayload pre-set, then apply a middleware. */ function buildApp( middleware: MiddlewareHandler, - handler?: (c: Context) => Response | Promise + handler?: (c: Context) => Response | Promise, + jwtOverride?: Partial<{ sub: string; email: string; name: string }> ) { const app = new Hono(); app.use("*", async (c, next) => { - c.set("jwtPayload", { sub: staffLookupResult?.userId ?? "unknown-sub" }); + const defaultSub = staffLookupResult?.userId ?? "unknown-sub"; + c.set("jwtPayload", { + sub: jwtOverride?.sub ?? defaultSub, + ...(jwtOverride?.email !== undefined ? { email: jwtOverride.email } : {}), + ...(jwtOverride?.name !== undefined ? { name: jwtOverride.name } : {}), + }); await next(); }); app.use("*", middleware); @@ -204,6 +274,125 @@ describe("resolveStaffMiddleware", () => { }); }); +// ─── Auto-provision branches (GRO-2052) ─────────────────────────────────────── +// +// Each branch creates a staff row on first authenticated request when no row +// exists yet. The Better-Auth branch (user table) is the primary path for +// email/password customers; the OIDC branch (account table) is a fallback for +// legacy authentik/google/github sessions. + +describe("resolveStaffMiddleware — auto-provision", () => { + const PROVISIONED: StaffRow = { + ...MANAGER, + id: "staff-provisioned-id", + oidcSub: null, + userId: "ba-user-customer", + role: "groomer", + isSuperUser: false, + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + + it("Better-Auth: creates a groomer staff row when user exists but no staff record (GRO-2052)", async () => { + // No existing staff row, no OIDC account row, but a Better-Auth user row. + staffLookupResult = null; + userLookupResult = { + id: "ba-user-customer", + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + accountLookupResult = null; + insertReturningResult = PROVISIONED; + + let capturedStaff: StaffRow | null = null; + const app = buildApp( + resolveStaffMiddleware, + (c) => { + capturedStaff = c.get("staff"); + return c.json({ ok: true }); + }, + { sub: "ba-user-customer", email: "uat-customer@groombook.dev" } + ); + + const res = await app.request("/test"); + expect(res.status).toBe(200); + expect(capturedStaff).not.toBeNull(); + expect(capturedStaff!.role).toBe("groomer"); + expect(capturedStaff!.userId).toBe("ba-user-customer"); + }); + + it("Better-Auth: returns 500 if INSERT yields no row", async () => { + staffLookupResult = null; + userLookupResult = { + id: "ba-user-customer", + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + insertReturningResult = null; // simulate INSERT … RETURNING returning [] + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "ba-user-customer", + email: "uat-customer@groombook.dev", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(500); + const body = await res.json(); + expect(body.error).toMatch(/auto-provision failed/i); + }); + + it("Better-Auth branch runs before OIDC branch (does not require jwt.email)", async () => { + // A Better-Auth user row alone is sufficient: jwt.email is intentionally + // absent. The pre-GRO-2052 code only auto-provisioned inside `if (jwt.email)`. + staffLookupResult = null; + userLookupResult = { + id: "ba-user-customer", + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + insertReturningResult = PROVISIONED; + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "ba-user-customer", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(200); + }); + + it("OIDC fallback: still provisions when user row is missing but account row exists", async () => { + // No staff row, no Better-Auth user, but an OIDC account row. + staffLookupResult = null; + userLookupResult = null; + accountLookupResult = { id: "oidc-account-id" }; + insertReturningResult = { ...PROVISIONED, userId: "oidc-sub" }; + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "oidc-sub", + email: "oidc-user@example.com", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(200); + }); + + it("falls through to 403 when neither Better-Auth user nor OIDC account row exists", async () => { + staffLookupResult = null; + userLookupResult = null; + accountLookupResult = null; + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "ghost-sub", + email: "ghost@example.com", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(403); + const body = await res.json(); + expect(body.error).toMatch(/no staff record/i); + }); +}); + // ─── requireRole tests ──────────────────────────────────────────────────────── describe("requireRole", () => { diff --git a/src/middleware/rbac.ts b/src/middleware/rbac.ts index de1fdec..e6fd055 100644 --- a/src/middleware/rbac.ts +++ b/src/middleware/rbac.ts @@ -1,5 +1,5 @@ import type { MiddlewareHandler } from "hono"; -import { and, eq, getDb, sql, staff, account } from "@groombook/db"; +import { and, eq, getDb, sql, staff, account, user } from "@groombook/db"; export type StaffRole = "groomer" | "receptionist" | "manager"; export type StaffRow = typeof staff.$inferSelect; @@ -111,8 +111,49 @@ export const resolveStaffMiddleware: MiddlewareHandler = async ( } } + // Auto-provision for Better-Auth users (GRO-2052): the user signed in via + // Better-Auth (email/password, magic link, etc.), so a row exists in `user` + // for jwt.sub but no `account` provider row is required. Create a minimal + // groomer staff record on first login. This is the primary auto-provision + // path; the OIDC branch below remains as a fallback for legacy accounts + // that exist in `account` but not in `user`. + const [userRow] = await db + .select({ id: user.id, name: user.name, email: user.email }) + .from(user) + .where(eq(user.id, jwt.sub)) + .limit(1); + if (userRow) { + const emailPrefix = userRow.email ? userRow.email.split("@")[0] : "Unknown"; + const name = userRow.name?.trim() || jwt.name?.trim() || emailPrefix; + + const [newStaff] = await db + .insert(staff) + .values({ + userId: jwt.sub, + email: userRow.email ?? jwt.email ?? "", + name, + role: "groomer", + isSuperUser: false, + active: true, + } as Parameters[0] extends { values: infer V } ? V : never) + .returning()!; + + if (!newStaff) { + return c.json({ error: "Forbidden: auto-provision failed" }, 500); + } + + console.log( + `[rbac] auto-provisioned staff record for Better-Auth user: ${jwt.sub} -> staff:${newStaff.id} (${name})` + ); + c.set("staff", newStaff); + await next(); + return; + } + // Auto-provision for OIDC users: check if jwt.sub has an OAuth/OIDC account - // (e.g. authentik). If so, create a groomer staff record on the fly. + // (e.g. authentik). If so, create a groomer staff record on the fly. This + // is kept for backward compatibility with legacy OIDC sessions whose user + // row may not yet exist in the Better-Auth `user` table. if (jwt.email) { const [oidcAccount] = await db .select({ id: account.id })