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>
This commit is contained in:
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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"));
|
||||
|
||||
Reference in New Issue
Block a user