From 652061f55d7b1e15650d6de96fb389a6218c992b Mon Sep 17 00:00:00 2001
From: "groombook-engineer[bot]"
 <3141748+groombook-engineer[bot]@users.noreply.github.com>
Date: Fri, 3 Apr 2026 00:49:01 +0000
Subject: [PATCH] fix(api): requireRoleOrSuperUser for /admin/* routes
 (GRO-412)

Fix bug where super users granted via Staff UI were blocked from
admin routes because requireRole("manager") checked role before
isSuperUser. Changed to requireRoleOrSuperUser("manager") so
super users bypass the manager-role check.

Also adds 7 unit tests for requireRoleOrSuperUser middleware
covering: manager access, super user bypass, non-super-user
blocking, and multi-role scenarios.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 apps/api/src/__tests__/rbac.test.ts | 65 ++++++++++++++++++++++++++++-
 apps/api/src/index.ts               |  2 +-
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/apps/api/src/__tests__/rbac.test.ts b/apps/api/src/__tests__/rbac.test.ts
index 43a1061..7351c6a 100644
--- a/apps/api/src/__tests__/rbac.test.ts
+++ b/apps/api/src/__tests__/rbac.test.ts
@@ -124,7 +124,7 @@ function buildWithStaff(
 
 // ─── Import middleware ────────────────────────────────────────────────────────
 
-const { resolveStaffMiddleware, requireRole, requireSuperUser } = await import(
+const { resolveStaffMiddleware, requireRole, requireSuperUser, requireRoleOrSuperUser } = await import(
   "../middleware/rbac.js"
 );
 
@@ -326,3 +326,66 @@ describe("requireSuperUser", () => {
     expect(contentType).toContain("application/json");
   });
 });
+
+// ─── requireRoleOrSuperUser tests ─────────────────────────────────────────────
+
+describe("requireRoleOrSuperUser", () => {
+  it("allows a manager to access manager-only routes", async () => {
+    const app = buildWithStaff(MANAGER, requireRoleOrSuperUser("manager"));
+    const res = await app.request("/test");
+    expect(res.status).toBe(200);
+  });
+
+  it("allows a super user with receptionist role to access manager-only routes (GRO-412 bug fix)", async () => {
+    // GRO-412: a receptionist granted super user via Staff UI should access admin routes
+    const superReceptionist: StaffRow = {
+      ...RECEPTIONIST,
+      isSuperUser: true,
+    };
+    const app = buildWithStaff(superReceptionist, requireRoleOrSuperUser("manager"));
+    const res = await app.request("/test");
+    expect(res.status).toBe(200);
+  });
+
+  it("allows a super user with groomer role to access manager-only routes", async () => {
+    const superGroomer: StaffRow = {
+      ...GROOMER,
+      isSuperUser: true,
+    };
+    const app = buildWithStaff(superGroomer, requireRoleOrSuperUser("manager"));
+    const res = await app.request("/test");
+    expect(res.status).toBe(200);
+  });
+
+  it("blocks a non-super-user receptionist from manager-only routes", async () => {
+    const app = buildWithStaff(RECEPTIONIST, requireRoleOrSuperUser("manager"));
+    const res = await app.request("/test");
+    expect(res.status).toBe(403);
+    const body = await res.json();
+    expect(body.error).toMatch(/super user privileges required/i);
+  });
+
+  it("blocks a non-super-user groomer from manager-only routes", async () => {
+    const app = buildWithStaff(GROOMER, requireRoleOrSuperUser("manager"));
+    const res = await app.request("/test");
+    expect(res.status).toBe(403);
+    const body = await res.json();
+    expect(body.error).toMatch(/super user privileges required/i);
+  });
+
+  it("allows a manager with multiple allowed roles", async () => {
+    const app = buildWithStaff(MANAGER, requireRoleOrSuperUser("manager", "receptionist"));
+    const res = await app.request("/test");
+    expect(res.status).toBe(200);
+  });
+
+  it("allows a super user with disallowed role to access route with multiple allowed roles", async () => {
+    const superGroomer: StaffRow = {
+      ...GROOMER,
+      isSuperUser: true,
+    };
+    const app = buildWithStaff(superGroomer, requireRoleOrSuperUser("manager", "receptionist"));
+    const res = await app.request("/test");
+    expect(res.status).toBe(200);
+  });
+});
diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts
index ea10d9b..fbea4da 100644
--- a/apps/api/src/index.ts
+++ b/apps/api/src/index.ts
@@ -110,7 +110,7 @@ api.route("/auth", authRouter);
 api.on(["GET"], "/staff/*", requireRole("manager", "receptionist", "groomer"));
 // Staff write routes: manager OR super-user (combined guard — avoids AND stacking)
 api.on(["POST", "PATCH", "DELETE"], "/staff/*", requireRoleOrSuperUser("manager"));
-api.use("/admin/*", requireRole("manager"));
+api.use("/admin/*", requireRoleOrSuperUser("manager"));
 api.use("/admin/settings/*", requireSuperUser());
 api.use("/reports/*", requireRole("manager"));
 api.use("/invoices/*", requireRole("manager"));