From 00f196c115a4eaf4425aa2a1cf5ca7e7d1a6f479 Mon Sep 17 00:00:00 2001 From: lorentz Date: Sun, 12 Jul 2026 18:20:36 -0400 Subject: [PATCH] fix(auth): stop hasPermission crashing for non-admin ("user") roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasPermission()'s parameter was named userRole: string, shadowing the module-level userRole role object exported earlier in the same file. The internal roles map's `user: userRole` entry therefore bound to the shadowed string parameter (e.g. "user") instead of the actual role object — so any permission check for a "user"-role session (the only non-admin role in the app) hit `"user".statements[resource]`, which is undefined, and threw instead of returning false. Net effect: every requirePermission()-gated route in the app returned a 500 instead of a 403 for non-admin users. This predates phase 14 — surfaced now because phase 14's PAX8 resolve route is admin-gated and got exercised by a non-admin account during verification. Renamed the parameter to roleName to remove the collision. Added lib/permissions.test.ts (previously zero coverage on this file) to lock in the "user"/admin/super-admin behavior and prevent regression. --- lib/permissions.test.ts | 25 +++++++++++++++++++++++++ lib/permissions.ts | 6 +++--- 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 lib/permissions.test.ts diff --git a/lib/permissions.test.ts b/lib/permissions.test.ts new file mode 100644 index 0000000..3b45233 --- /dev/null +++ b/lib/permissions.test.ts @@ -0,0 +1,25 @@ +import { describe, it, expect } from 'vitest'; +import { hasPermission } from './permissions'; + +describe('hasPermission', () => { + it('returns false (not a crash) for a "user" role on an admin-only resource', () => { + expect(() => hasPermission('user', 'admin', 'access')).not.toThrow(); + expect(hasPermission('user', 'admin', 'access')).toBe(false); + }); + + it('returns true for "admin" role on an admin-gated resource', () => { + expect(hasPermission('admin', 'admin', 'access')).toBe(true); + }); + + it('returns true for "super-admin" role on an admin-gated resource', () => { + expect(hasPermission('super-admin', 'admin', 'access')).toBe(true); + }); + + it('returns true for "user" role on a permitted resource', () => { + expect(hasPermission('user', 'tickets', 'read')).toBe(true); + }); + + it('returns false for an unknown role', () => { + expect(hasPermission('bogus-role', 'admin', 'access')).toBe(false); + }); +}); diff --git a/lib/permissions.ts b/lib/permissions.ts index 008b7ef..c11d963 100644 --- a/lib/permissions.ts +++ b/lib/permissions.ts @@ -74,17 +74,17 @@ export const userRole = ac.newRole({ // Helper function to check if a user has a specific permission export function hasPermission( - userRole: string, + roleName: string, resource: keyof typeof statement, action: string ): boolean { const roles: Record> = { "super-admin": superAdminRole, admin: adminRole, - user: userRole as unknown as ReturnType, + user: userRole, }; - const role = roles[userRole]; + const role = roles[roleName]; if (!role) return false; // Check if the role has the permission