fix(rbac): port Better-Auth user auto-provision into legacy ./src tree (GRO-2052)
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 #139a2b09bais 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 <noreply@paperclip.ing>
This commit is contained in:
+214
-25
@@ -43,42 +43,103 @@ const GROOMER: StaffRow = {
|
|||||||
|
|
||||||
// ─── Mock DB ──────────────────────────────────────────────────────────────────
|
// ─── 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;
|
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;
|
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", () => {
|
vi.mock("@groombook/db", () => {
|
||||||
const staff = new Proxy(
|
function tableMarker(name: string) {
|
||||||
{ _name: "staff" },
|
return new Proxy(
|
||||||
{
|
{ _name: name },
|
||||||
get(target, prop) {
|
{
|
||||||
if (prop === "_name") return "staff";
|
get(_target, prop) {
|
||||||
if (prop === "$inferSelect") return {};
|
if (prop === "_name") return name;
|
||||||
return { table: "staff", column: prop };
|
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 {
|
return {
|
||||||
getDb: () => ({
|
getDb: () => ({
|
||||||
select: () => ({
|
select: (_columns?: unknown) => ({
|
||||||
from: () => ({
|
from: (table: { _name?: string }) => {
|
||||||
where: () => ({
|
const name = table?._name ?? "staff";
|
||||||
limit: () => {
|
return {
|
||||||
// dev mode fallback to first manager
|
where: () => ({
|
||||||
return managerFallbackResult ? [managerFallbackResult] : [];
|
limit: () => {
|
||||||
},
|
// The user / account auto-provision branches always call
|
||||||
[Symbol.iterator]: function* () {
|
// `.limit(1)`; route to the per-table lookup state.
|
||||||
if (staffLookupResult) yield staffLookupResult;
|
if (name === "user")
|
||||||
},
|
return userLookupResult ? [userLookupResult] : [];
|
||||||
0: staffLookupResult,
|
if (name === "account")
|
||||||
length: staffLookupResult ? 1 : 0,
|
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,
|
staff,
|
||||||
|
user,
|
||||||
|
account,
|
||||||
eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })),
|
eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })),
|
||||||
and: vi.fn((..._clauses: unknown[]) => ({})),
|
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() {
|
function resetMocks() {
|
||||||
staffLookupResult = null;
|
staffLookupResult = null;
|
||||||
managerFallbackResult = MANAGER;
|
managerFallbackResult = MANAGER;
|
||||||
|
userLookupResult = null;
|
||||||
|
accountLookupResult = null;
|
||||||
|
insertReturningResult = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Build a minimal Hono app with jwtPayload pre-set, then apply a middleware. */
|
/** Build a minimal Hono app with jwtPayload pre-set, then apply a middleware. */
|
||||||
function buildApp(
|
function buildApp(
|
||||||
middleware: MiddlewareHandler<AppEnv>,
|
middleware: MiddlewareHandler<AppEnv>,
|
||||||
handler?: (c: Context<AppEnv>) => Response | Promise<Response>
|
handler?: (c: Context<AppEnv>) => Response | Promise<Response>,
|
||||||
|
jwtOverride?: Partial<{ sub: string; email: string; name: string }>
|
||||||
) {
|
) {
|
||||||
const app = new Hono<AppEnv>();
|
const app = new Hono<AppEnv>();
|
||||||
app.use("*", async (c, next) => {
|
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();
|
await next();
|
||||||
});
|
});
|
||||||
app.use("*", middleware);
|
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 ────────────────────────────────────────────────────────
|
// ─── requireRole tests ────────────────────────────────────────────────────────
|
||||||
|
|
||||||
describe("requireRole", () => {
|
describe("requireRole", () => {
|
||||||
|
|||||||
+43
-2
@@ -1,5 +1,5 @@
|
|||||||
import type { MiddlewareHandler } from "hono";
|
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 StaffRole = "groomer" | "receptionist" | "manager";
|
||||||
export type StaffRow = typeof staff.$inferSelect;
|
export type StaffRow = typeof staff.$inferSelect;
|
||||||
@@ -111,8 +111,49 @@ export const resolveStaffMiddleware: MiddlewareHandler<AppEnv> = 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<typeof db.insert>[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
|
// 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) {
|
if (jwt.email) {
|
||||||
const [oidcAccount] = await db
|
const [oidcAccount] = await db
|
||||||
.select({ id: account.id })
|
.select({ id: account.id })
|
||||||
|
|||||||
Reference in New Issue
Block a user