fix(auth): stop hasPermission crashing for non-admin ("user") roles
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.
This commit is contained in:
parent
f490c16a40
commit
00f196c115
2 changed files with 28 additions and 3 deletions
25
lib/permissions.test.ts
Normal file
25
lib/permissions.test.ts
Normal file
|
|
@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -74,17 +74,17 @@ export const userRole = ac.newRole({
|
||||||
|
|
||||||
// Helper function to check if a user has a specific permission
|
// Helper function to check if a user has a specific permission
|
||||||
export function hasPermission(
|
export function hasPermission(
|
||||||
userRole: string,
|
roleName: string,
|
||||||
resource: keyof typeof statement,
|
resource: keyof typeof statement,
|
||||||
action: string
|
action: string
|
||||||
): boolean {
|
): boolean {
|
||||||
const roles: Record<string, ReturnType<typeof ac.newRole>> = {
|
const roles: Record<string, ReturnType<typeof ac.newRole>> = {
|
||||||
"super-admin": superAdminRole,
|
"super-admin": superAdminRole,
|
||||||
admin: adminRole,
|
admin: adminRole,
|
||||||
user: userRole as unknown as ReturnType<typeof ac.newRole>,
|
user: userRole,
|
||||||
};
|
};
|
||||||
|
|
||||||
const role = roles[userRole];
|
const role = roles[roleName];
|
||||||
if (!role) return false;
|
if (!role) return false;
|
||||||
|
|
||||||
// Check if the role has the permission
|
// Check if the role has the permission
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue