From bb877e7a5fdfd2d54708fd1c3766afe6f86dcc71 Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Mon, 25 May 2026 11:49:54 -0400 Subject: [PATCH] fix(auth): centralize rbac role management order --- README.md | 6 +++ docs/rbac-matrix.md | 61 +++++++++++++++++++++++++++ src/core/permissions/index.ts | 27 +++--------- src/index.ts | 2 + src/types/auth.ts | 17 +++++++- src/types/index.ts | 2 + tests/permissions.test.ts | 79 ++++++++++++++++++++++++++++++++++- 7 files changed, 170 insertions(+), 24 deletions(-) create mode 100644 docs/rbac-matrix.md diff --git a/README.md b/README.md index 469ee29..eaed5b8 100644 --- a/README.md +++ b/README.md @@ -42,3 +42,9 @@ See the [Tinyland databaseless auth MVP](https://github.com/tinyland-inc/tinyland-auth/blob/main/docs/tinyland-databaseless-auth-mvp.md) and the [executable example](https://github.com/tinyland-inc/tinyland-auth/blob/main/examples/tinyland-databaseless-auth-mvp.ts). + +## RBAC + +Role management order and permission checks are intentionally separate. See +[the RBAC matrix](https://github.com/tinyland-inc/tinyland-auth/blob/main/docs/rbac-matrix.md) +for the package-owned matrix and downstream test guidance. diff --git a/docs/rbac-matrix.md b/docs/rbac-matrix.md new file mode 100644 index 0000000..20b2fd0 --- /dev/null +++ b/docs/rbac-matrix.md @@ -0,0 +1,61 @@ +# RBAC Matrix + +`@tummycrypt/tinyland-auth` has two related but different RBAC concepts: + +1. Role management order decides whether one role may grant, invite, update, or + revoke another role. +2. Permission matrix decides which product capabilities a role has. + +The permission matrix is not a strict superset hierarchy. For example, +`event_manager` owns event operations while `contributor` owns content +contribution. Neither role should be inferred to contain every permission from +the other. + +## Role Management Order + +Role management uses `ROLE_HIERARCHY` as the source of truth: + +| Rank | Role | Meaning | +| --- | --- | --- | +| 100 | `super_admin` | System owner; can manage every lower role. | +| 90 | `admin` | General admin; can manage non-owner roles. | +| 70 | `moderator` | Moderation lead; can manage editorial and community roles. | +| 60 | `editor` | Editorial lead; can manage publishing contributors. | +| 50 | `event_manager` | Event lead; can manage contributor/member/viewer roles. | +| 40 | `contributor` | Content contributor; can manage member/viewer roles. | +| 30 | `member` | Authenticated member; can manage viewer under this package order. | +| 10 | `viewer` | Read-only admin surface access; cannot manage roles. | + +`canManageRole(actor, target)` returns true only when the actor rank is +strictly greater than the target rank. Equal-rank management is always false. +Hyphenated role names are normalized to underscore names before lookup. + +## Capability Matrix + +Capability checks use `ROLE_PERMISSIONS`, `hasPermission()`, and the +domain-specific helpers such as `canCreateEvents()` or `canEditPosts()`. + +Important rule: do not assert that every higher management role has a strict +permission superset of every lower role. Product roles are allowed to be +capability-specific. + +Examples: + +- `event_manager` has `admin.events.manage`. +- `event_manager` does not get content-view permission through + `ROLE_PERMISSIONS`. +- `contributor` has `admin.content.view`. +- `contributor` does not get event-management permission. +- `super_admin` is the exception: `hasPermission()` grants it every permission. + +## Downstream Test Guidance + +Consumer repos such as `tinyland.dev` should test: + +- role-management policy against `ROLE_HIERARCHY` +- permission checks against `ROLE_PERMISSIONS` +- product-specific helper behavior where route policy uses helpers +- explicit capability gaps between peer/specialized roles + +Do not write a property test that assumes `ROLE_PERMISSIONS[higher]` is a +superset of `ROLE_PERMISSIONS[lower]`. That property is false by design. diff --git a/src/core/permissions/index.ts b/src/core/permissions/index.ts index 713dbfd..1467a9c 100644 --- a/src/core/permissions/index.ts +++ b/src/core/permissions/index.ts @@ -7,7 +7,7 @@ -import type { AdminRole, AdminUser } from '../../types/auth.js'; +import { ROLE_HIERARCHY, isValidAdminRole, type AdminRole, type AdminUser } from '../../types/auth.js'; import { PERMISSIONS, ROLE_PERMISSIONS, type ContentVisibility } from '../../types/permissions.js'; @@ -93,32 +93,17 @@ export function getUserPermissions(user: AdminUser): string[] { export function canManageRole(actorRole: AdminRole | string, targetRole: AdminRole | string): boolean { - const normalizeRole = (role: AdminRole | string): string => { - return String(role).toLowerCase().replace(/-/g, '_'); - }; - const normalizedActor = normalizeRole(actorRole); const normalizedTarget = normalizeRole(targetRole); - const roleHierarchy = [ - 'super_admin', - 'admin', - 'editor', - 'event_manager', - 'moderator', - 'contributor', - 'member', - 'viewer', - ]; - - const actorIndex = roleHierarchy.indexOf(normalizedActor); - const targetIndex = roleHierarchy.indexOf(normalizedTarget); - - if (actorIndex === -1 || targetIndex === -1) { + if ( + !isValidAdminRole(normalizedActor) || + !isValidAdminRole(normalizedTarget) + ) { return false; } - return actorIndex < targetIndex; + return ROLE_HIERARCHY[normalizedActor] > ROLE_HIERARCHY[normalizedTarget]; } diff --git a/src/index.ts b/src/index.ts index 541362d..d86f2ec 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,8 +32,10 @@ export { + ADMIN_ROLES, AdminRole, ROLE_HIERARCHY, + ROLE_MANAGEMENT_ORDER, AuditEventType, AuthErrorCode, isAdminUser, diff --git a/src/types/auth.ts b/src/types/auth.ts index ad5fdee..7ae2e9b 100644 --- a/src/types/auth.ts +++ b/src/types/auth.ts @@ -19,6 +19,17 @@ export type AdminRole = | 'member' | 'viewer'; +export const ADMIN_ROLES = [ + 'super_admin', + 'admin', + 'moderator', + 'editor', + 'event_manager', + 'contributor', + 'member', + 'viewer', +] as const satisfies readonly AdminRole[]; + @@ -48,6 +59,10 @@ export const ROLE_HIERARCHY: Record = { viewer: 10, }; +export const ROLE_MANAGEMENT_ORDER: readonly AdminRole[] = [ + ...ADMIN_ROLES, +].sort((left, right) => ROLE_HIERARCHY[right] - ROLE_HIERARCHY[left]); + @@ -387,7 +402,7 @@ export function isAdminUser(obj: unknown): obj is AdminUser { } export function isValidAdminRole(role: string): role is AdminRole { - return Object.keys(ROLE_HIERARCHY).includes(role); + return ADMIN_ROLES.includes(role as AdminRole); } export function hasHigherRole(userRole: AdminRole, targetRole: AdminRole): boolean { diff --git a/src/types/index.ts b/src/types/index.ts index 8ae3628..219ce54 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -6,8 +6,10 @@ export { + ADMIN_ROLES, AdminRole, ROLE_HIERARCHY, + ROLE_MANAGEMENT_ORDER, AuditEventType, AuthErrorCode, isAdminUser, diff --git a/tests/permissions.test.ts b/tests/permissions.test.ts index 874972c..dda6757 100644 --- a/tests/permissions.test.ts +++ b/tests/permissions.test.ts @@ -5,6 +5,16 @@ import { describe, it, expect } from 'vitest'; +import { + ADMIN_ROLES, + ROLE_HIERARCHY, + ROLE_MANAGEMENT_ORDER, + hasEqualOrHigherRole, + hasHigherRole, + isValidAdminRole, + type AdminRole, + type AdminUser, +} from '../src/types/auth.js'; import { hasPermission, hasAnyPermission, @@ -16,8 +26,7 @@ import { getAllowedVisibilityOptions, isMemberRole, } from '../src/core/permissions/index.js'; -import { PERMISSIONS } from '../src/types/permissions.js'; -import type { AdminUser } from '../src/types/auth.js'; +import { PERMISSIONS, ROLE_PERMISSIONS } from '../src/types/permissions.js'; const createTestUser = (role: string, id = 'user-1'): AdminUser => ({ @@ -44,6 +53,72 @@ const member = createTestUser('member'); const viewer = createTestUser('viewer'); describe('Permission Functions', () => { + describe('role authority', () => { + const expectedManagementOrder: AdminRole[] = [ + 'super_admin', + 'admin', + 'moderator', + 'editor', + 'event_manager', + 'contributor', + 'member', + 'viewer', + ]; + + it('exports the complete supported role set', () => { + expect(ADMIN_ROLES).toEqual(expectedManagementOrder); + for (const role of expectedManagementOrder) { + expect(isValidAdminRole(role)).toBe(true); + } + expect(isValidAdminRole('owner')).toBe(false); + }); + + it('derives role-management order from ROLE_HIERARCHY', () => { + expect(ROLE_MANAGEMENT_ORDER).toEqual(expectedManagementOrder); + + for (const [index, role] of ROLE_MANAGEMENT_ORDER.entries()) { + const lowerRole = ROLE_MANAGEMENT_ORDER[index + 1]; + if (!lowerRole) continue; + + expect(ROLE_HIERARCHY[role]).toBeGreaterThan( + ROLE_HIERARCHY[lowerRole], + ); + expect(hasHigherRole(role, lowerRole)).toBe(true); + expect(hasEqualOrHigherRole(role, lowerRole)).toBe(true); + } + }); + + it('uses ROLE_HIERARCHY as the canManageRole authority', () => { + for (const actorRole of ADMIN_ROLES) { + for (const targetRole of ADMIN_ROLES) { + expect(canManageRole(actorRole, targetRole)).toBe( + ROLE_HIERARCHY[actorRole] > ROLE_HIERARCHY[targetRole], + ); + } + } + + expect(canManageRole('super-admin', 'event-manager')).toBe(true); + expect(canManageRole('editor', 'moderator')).toBe(false); + expect(canManageRole('moderator', 'editor')).toBe(true); + expect(canManageRole('admin', 'owner')).toBe(false); + }); + + it('documents permissions as a capability matrix, not a strict superset hierarchy', () => { + expect(ROLE_PERMISSIONS.event_manager).toContain( + PERMISSIONS.ADMIN_EVENTS_MANAGE, + ); + expect(ROLE_PERMISSIONS.event_manager).not.toContain( + PERMISSIONS.ADMIN_CONTENT_VIEW, + ); + expect(ROLE_PERMISSIONS.contributor).toContain( + PERMISSIONS.ADMIN_CONTENT_VIEW, + ); + expect(ROLE_PERMISSIONS.contributor).not.toContain( + PERMISSIONS.ADMIN_EVENTS_MANAGE, + ); + }); + }); + describe('hasPermission', () => { it('should return true for super_admin with any permission', () => { expect(hasPermission(superAdmin, PERMISSIONS.ADMIN_ACCESS)).toBe(true);