Merge pull request #998 from SonicJs-Org/lane711/rbac-testing-summary

fix(rbac): RBAC dynamic grants + API ACL enforcement

Author: lane711

package-lock.json

@@ -112,25 +112,27 @@ describe('RBAC enforcement - requireRole middleware', () => {
   })
 
   describe('role validation on user creation', () => {
-    it('accepts valid roles', () => {
-      const validRoles = ['admin', 'editor', 'author', 'viewer']
-      for (const role of validRoles) {
-        expect(validRoles.includes(role)).toBe(true)
-      }
+    // admin-users.ts validates against dynamic DB roles (RbacService.getRoles()), not a
+    // static list. Roles are slugs like 'role-admin', not the old 'admin'/'viewer' strings.
+    it('accepts a role that exists in the dynamic valid set', () => {
+      const validRoles = ['role-admin', 'role-editor', 'role-public']
+      const roleInput = 'role-admin'
+      const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'role-admin')
+      expect(role).toBe('role-admin')
     })
 
-    it('rejects invalid role and defaults to viewer', () => {
-      const validRoles = ['admin', 'editor', 'author', 'viewer']
+    it('rejects unknown role and falls back to first valid role', () => {
+      const validRoles = ['role-admin', 'role-editor']
       const roleInput = 'superadmin'
-      const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
-      expect(role).toBe('viewer')
+      const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'role-admin')
+      expect(role).toBe('role-admin')
     })
 
-    it('rejects empty string role and defaults to viewer', () => {
-      const validRoles = ['admin', 'editor', 'author', 'viewer']
+    it('rejects empty string role and falls back to first valid role', () => {
+      const validRoles = ['role-admin', 'role-editor']
       const roleInput = ''
-      const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
-      expect(role).toBe('viewer')
+      const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'role-admin')
+      expect(role).toBe('role-admin')
     })
   })
 

packages/core/src/__tests__/middleware/rbac-enforcement.test.ts

@@ -301,6 +301,28 @@ describe('Document ACL — isAllowed against real document_permissions (D5/D11)'
     const repoB = new DocumentRepository(db, 'tenantB')
     expect(await repoB.isAllowed([{ type: 'public', id: '*' }], 'root1', 'read', PUBLIC_READ)).toBe(true)
   })
+
+  // Security: an authenticated user whose auth_user.role == 'public' must NOT inherit the
+  // anonymous baseGrants['public'] bucket. If it did, a logged-in "public-role" user would
+  // bypass ACL and see everything an anonymous visitor sees — defeating per-role isolation.
+  it('authenticated user with role=public does NOT inherit baseGrants["public"] (collision guard)', async () => {
+    const repo = new DocumentRepository(db, 'default')
+    // Principal set for an authed user whose role name is 'public':
+    //   { type: 'user', id: 'u-pub' }, { type: 'role', id: 'public' }
+    const authedPublicRolePrincipal = [
+      { type: 'user', id: 'u-pub' },
+      { type: 'role', id: 'public' },
+    ]
+    // PUBLIC_READ has baseGrants.public = ['read']. An anonymous visitor { type:'public', id:'*' }
+    // can read. An authed user with role='public' must NOT — their role principal must not match
+    // the 'public' key in baseGrants.
+    expect(await repo.isAllowed(authedPublicRolePrincipal, 'root1', 'read', PUBLIC_READ)).toBe(false)
+  })
+
+  it('anonymous { type:public } still inherits baseGrants["public"] normally', async () => {
+    const repo = new DocumentRepository(db, 'default')
+    expect(await repo.isAllowed([{ type: 'public', id: '*' }], 'root1', 'read', PUBLIC_READ)).toBe(true)
+  })
 })
 
 describe('versioning off', () => {

packages/core/src/__tests__/services/documents.sqlite.test.ts

@@ -160,4 +160,53 @@ describe('RbacService — document-backed', () => {
     await rbac.deleteRole('role-admin')
     expect((await rbac.getRoles()).some((r) => r.id === 'role-admin')).toBe(true)
   })
+
+  // ─── isGrantedForRole ─────────────────────────────────────────────────────────
+  // Covers the two-system ACL fix: RBAC dynamic grants must be checked alongside
+  // code-time baseGrants so admin UI changes take effect immediately.
+
+  it('isGrantedForRole returns false for unknown role', async () => {
+    const rbac = new RbacService(db)
+    expect(await rbac.isGrantedForRole('role-nonexistent', 'document_type:example', 'read')).toBe(false)
+  })
+
+  it('isGrantedForRole returns false for role with no matching grant', async () => {
+    const rbac = new RbacService(db)
+    // Public role is seeded with zero grants by default.
+    expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(false)
+    expect(await rbac.isGrantedForRole('public', 'document_type:example', 'read')).toBe(false)
+  })
+
+  it('isGrantedForRole matches by slug (role-public) after setRoleGrants', async () => {
+    const rbac = new RbacService(db)
+    await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
+    expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(true)
+  })
+
+  it('isGrantedForRole matches by name (public) after setRoleGrants', async () => {
+    const rbac = new RbacService(db)
+    await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
+    expect(await rbac.isGrantedForRole('public', 'document_type:example', 'read')).toBe(true)
+  })
+
+  it('isGrantedForRole respects wildcard resource (document_type:*)', async () => {
+    const rbac = new RbacService(db)
+    await rbac.setRoleGrants('role-public', [{ resource: 'document_type:*', verb: 'read' }])
+    expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(true)
+    expect(await rbac.isGrantedForRole('role-public', 'document_type:blog_post', 'read')).toBe(true)
+  })
+
+  it('isGrantedForRole is false after grants are cleared', async () => {
+    const rbac = new RbacService(db)
+    await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
+    expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(true)
+    await rbac.setRoleGrants('role-public', [])
+    expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(false)
+  })
+
+  it('isGrantedForRole verb mismatch returns false', async () => {
+    const rbac = new RbacService(db)
+    await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
+    expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'create')).toBe(false)
+  })
 })

packages/core/src/__tests__/services/rbac-documents.test.ts

@@ -1,7 +1,7 @@
 /**
  * AUTO-GENERATED FILE - DO NOT EDIT
  * Generated by: scripts/generate-migrations.ts
- * Generated at: 2026-07-03T00:01:49.480Z
+ * Generated at: 2026-07-06T20:53:50.482Z
  *
  * This file contains all migration SQL bundled for use in Cloudflare Workers
  * where filesystem access is not available at runtime.

packages/core/src/db/migrations-bundle.ts

@@ -10,6 +10,7 @@ import { renderUsersListPage, type UsersListPageData, type User } from '../templ
 import type { Bindings, Variables } from '../app'
 import { getUserProfileConfig, getRegistrationFields, renderCustomProfileSection, getCustomData, saveCustomData, extractCustomFieldsFromForm, sanitizeCustomData, validateCustomData, readProfileData, writeProfileData } from '../plugins/core-plugins/user-profiles'
 import { TenantService, VALID_MEMBER_ROLES } from '../plugins/core-plugins/multi-tenant-plugin/services/tenant-service'
+import { RbacService } from '../services/rbac'
 
 const userRoutes = new Hono<{ Bindings: Bindings; Variables: Variables }>()
 
@@ -58,13 +59,10 @@ const LANGUAGES = [
   { value: 'zh', label: 'Chinese' }
 ]
 
-// Role options for user form
-const ROLES = [
-  { value: 'admin', label: 'Administrator' },
-  { value: 'editor', label: 'Editor' },
-  { value: 'author', label: 'Author' },
-  { value: 'viewer', label: 'Viewer' }
-]
+async function fetchRoleOptions(db: any): Promise<Array<{ value: string; label: string }>> {
+  const roles = await new RbacService(db).getRoles()
+  return roles.map((r) => ({ value: r.id, label: r.display_name }))
+}
 
 /**
  * GET /admin/profile - Show user profile page
@@ -630,8 +628,10 @@ userRoutes.get('/users/new', async (c) => {
       ? renderCustomProfileSection({ fields: regFields }, {})
       : undefined
 
+    const roles = await fetchRoleOptions(db)
+
     const pageData: UserNewPageData = {
-      roles: ROLES,
+      roles,
       registrationFieldsHtml,
       user: {
         name: user!.email.split('@')[0] || user!.email,
@@ -668,9 +668,9 @@ userRoutes.post('/users/new', async (c) => {
     const email = formData.get('email')?.toString()?.trim().toLowerCase() || ''
     const phone = sanitizeInput(formData.get('phone')?.toString()) || null
     const bio = sanitizeInput(formData.get('bio')?.toString()) || null
-    const roleInput = formData.get('role')?.toString() || 'viewer'
-    const validRoles = ['admin', 'editor', 'author', 'viewer']
-    const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
+    const roleInput = formData.get('role')?.toString() || ''
+    const validRoles = (await new RbacService(db).getRoles()).map((r) => r.id)
+    const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'admin')
     const password = formData.get('password')?.toString() || ''
     const confirmPassword = formData.get('confirm_password')?.toString() || ''
     const isActive = formData.get('is_active') === '1'
@@ -942,9 +942,11 @@ userRoutes.get('/users/:id/edit', async (c) => {
     const queryMessage = c.req.query('message')
     const queryType = c.req.query('type') === 'error' ? 'error' : 'success'
 
+    const roles = await fetchRoleOptions(db)
+
     const pageData: UserEditPageData = {
       userToEdit: editData,
-      roles: ROLES,
+      roles,
       hasProfilePlugin: profileConfig !== null && upRow !== null,
       customProfileFieldsHtml,
       tenantMemberships,
@@ -985,9 +987,9 @@ userRoutes.put('/users/:id', async (c) => {
     const lastName = sanitizeInput(formData.get('last_name')?.toString())
     const email = formData.get('email')?.toString()?.trim().toLowerCase() || ''
     const phone = sanitizeInput(formData.get('phone')?.toString()) || null
-    const roleInput = formData.get('role')?.toString() || 'viewer'
-    const validRoles = ['admin', 'editor', 'author', 'viewer']
-    const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
+    const roleInput = formData.get('role')?.toString() || ''
+    const validRoles = (await new RbacService(db).getRoles()).map((r) => r.id)
+    const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'admin')
     const isActive = formData.get('is_active') === '1'
     const emailVerified = formData.get('email_verified') === '1'