Skip to content

Commit ac9ec28

Browse files
lane711claude
andcommitted
chore: merge origin/main, renumber demo-login-stats spec to 100
Main added E2E specs 97-99 (rbac, user-edit, public-role). Renamed 98-demo-login-stats → 100-demo-login-stats to avoid collision. Using main's lock file (already has better-sqlite3 resolved). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 parents cfe63d9 + 9e9df42 commit ac9ec28

13 files changed

Lines changed: 714 additions & 107 deletions

package-lock.json

Lines changed: 124 additions & 67 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,25 +112,27 @@ describe('RBAC enforcement - requireRole middleware', () => {
112112
})
113113

114114
describe('role validation on user creation', () => {
115-
it('accepts valid roles', () => {
116-
const validRoles = ['admin', 'editor', 'author', 'viewer']
117-
for (const role of validRoles) {
118-
expect(validRoles.includes(role)).toBe(true)
119-
}
115+
// admin-users.ts validates against dynamic DB roles (RbacService.getRoles()), not a
116+
// static list. Roles are slugs like 'role-admin', not the old 'admin'/'viewer' strings.
117+
it('accepts a role that exists in the dynamic valid set', () => {
118+
const validRoles = ['role-admin', 'role-editor', 'role-public']
119+
const roleInput = 'role-admin'
120+
const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'role-admin')
121+
expect(role).toBe('role-admin')
120122
})
121123

122-
it('rejects invalid role and defaults to viewer', () => {
123-
const validRoles = ['admin', 'editor', 'author', 'viewer']
124+
it('rejects unknown role and falls back to first valid role', () => {
125+
const validRoles = ['role-admin', 'role-editor']
124126
const roleInput = 'superadmin'
125-
const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
126-
expect(role).toBe('viewer')
127+
const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'role-admin')
128+
expect(role).toBe('role-admin')
127129
})
128130

129-
it('rejects empty string role and defaults to viewer', () => {
130-
const validRoles = ['admin', 'editor', 'author', 'viewer']
131+
it('rejects empty string role and falls back to first valid role', () => {
132+
const validRoles = ['role-admin', 'role-editor']
131133
const roleInput = ''
132-
const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
133-
expect(role).toBe('viewer')
134+
const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'role-admin')
135+
expect(role).toBe('role-admin')
134136
})
135137
})
136138

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,28 @@ describe('Document ACL — isAllowed against real document_permissions (D5/D11)'
301301
const repoB = new DocumentRepository(db, 'tenantB')
302302
expect(await repoB.isAllowed([{ type: 'public', id: '*' }], 'root1', 'read', PUBLIC_READ)).toBe(true)
303303
})
304+
305+
// Security: an authenticated user whose auth_user.role == 'public' must NOT inherit the
306+
// anonymous baseGrants['public'] bucket. If it did, a logged-in "public-role" user would
307+
// bypass ACL and see everything an anonymous visitor sees — defeating per-role isolation.
308+
it('authenticated user with role=public does NOT inherit baseGrants["public"] (collision guard)', async () => {
309+
const repo = new DocumentRepository(db, 'default')
310+
// Principal set for an authed user whose role name is 'public':
311+
// { type: 'user', id: 'u-pub' }, { type: 'role', id: 'public' }
312+
const authedPublicRolePrincipal = [
313+
{ type: 'user', id: 'u-pub' },
314+
{ type: 'role', id: 'public' },
315+
]
316+
// PUBLIC_READ has baseGrants.public = ['read']. An anonymous visitor { type:'public', id:'*' }
317+
// can read. An authed user with role='public' must NOT — their role principal must not match
318+
// the 'public' key in baseGrants.
319+
expect(await repo.isAllowed(authedPublicRolePrincipal, 'root1', 'read', PUBLIC_READ)).toBe(false)
320+
})
321+
322+
it('anonymous { type:public } still inherits baseGrants["public"] normally', async () => {
323+
const repo = new DocumentRepository(db, 'default')
324+
expect(await repo.isAllowed([{ type: 'public', id: '*' }], 'root1', 'read', PUBLIC_READ)).toBe(true)
325+
})
304326
})
305327

306328
describe('versioning off', () => {

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,53 @@ describe('RbacService — document-backed', () => {
160160
await rbac.deleteRole('role-admin')
161161
expect((await rbac.getRoles()).some((r) => r.id === 'role-admin')).toBe(true)
162162
})
163+
164+
// ─── isGrantedForRole ─────────────────────────────────────────────────────────
165+
// Covers the two-system ACL fix: RBAC dynamic grants must be checked alongside
166+
// code-time baseGrants so admin UI changes take effect immediately.
167+
168+
it('isGrantedForRole returns false for unknown role', async () => {
169+
const rbac = new RbacService(db)
170+
expect(await rbac.isGrantedForRole('role-nonexistent', 'document_type:example', 'read')).toBe(false)
171+
})
172+
173+
it('isGrantedForRole returns false for role with no matching grant', async () => {
174+
const rbac = new RbacService(db)
175+
// Public role is seeded with zero grants by default.
176+
expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(false)
177+
expect(await rbac.isGrantedForRole('public', 'document_type:example', 'read')).toBe(false)
178+
})
179+
180+
it('isGrantedForRole matches by slug (role-public) after setRoleGrants', async () => {
181+
const rbac = new RbacService(db)
182+
await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
183+
expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(true)
184+
})
185+
186+
it('isGrantedForRole matches by name (public) after setRoleGrants', async () => {
187+
const rbac = new RbacService(db)
188+
await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
189+
expect(await rbac.isGrantedForRole('public', 'document_type:example', 'read')).toBe(true)
190+
})
191+
192+
it('isGrantedForRole respects wildcard resource (document_type:*)', async () => {
193+
const rbac = new RbacService(db)
194+
await rbac.setRoleGrants('role-public', [{ resource: 'document_type:*', verb: 'read' }])
195+
expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(true)
196+
expect(await rbac.isGrantedForRole('role-public', 'document_type:blog_post', 'read')).toBe(true)
197+
})
198+
199+
it('isGrantedForRole is false after grants are cleared', async () => {
200+
const rbac = new RbacService(db)
201+
await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
202+
expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(true)
203+
await rbac.setRoleGrants('role-public', [])
204+
expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'read')).toBe(false)
205+
})
206+
207+
it('isGrantedForRole verb mismatch returns false', async () => {
208+
const rbac = new RbacService(db)
209+
await rbac.setRoleGrants('role-public', [{ resource: 'document_type:example', verb: 'read' }])
210+
expect(await rbac.isGrantedForRole('role-public', 'document_type:example', 'create')).toBe(false)
211+
})
163212
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* AUTO-GENERATED FILE - DO NOT EDIT
33
* Generated by: scripts/generate-migrations.ts
4-
* Generated at: 2026-07-03T00:01:49.480Z
4+
* Generated at: 2026-07-06T20:53:50.482Z
55
*
66
* This file contains all migration SQL bundled for use in Cloudflare Workers
77
* where filesystem access is not available at runtime.

packages/core/src/routes/admin-users.ts

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { renderUsersListPage, type UsersListPageData, type User } from '../templ
1010
import type { Bindings, Variables } from '../app'
1111
import { getUserProfileConfig, getRegistrationFields, renderCustomProfileSection, getCustomData, saveCustomData, extractCustomFieldsFromForm, sanitizeCustomData, validateCustomData, readProfileData, writeProfileData } from '../plugins/core-plugins/user-profiles'
1212
import { TenantService, VALID_MEMBER_ROLES } from '../plugins/core-plugins/multi-tenant-plugin/services/tenant-service'
13+
import { RbacService } from '../services/rbac'
1314

1415
const userRoutes = new Hono<{ Bindings: Bindings; Variables: Variables }>()
1516

@@ -58,13 +59,10 @@ const LANGUAGES = [
5859
{ value: 'zh', label: 'Chinese' }
5960
]
6061

61-
// Role options for user form
62-
const ROLES = [
63-
{ value: 'admin', label: 'Administrator' },
64-
{ value: 'editor', label: 'Editor' },
65-
{ value: 'author', label: 'Author' },
66-
{ value: 'viewer', label: 'Viewer' }
67-
]
62+
async function fetchRoleOptions(db: any): Promise<Array<{ value: string; label: string }>> {
63+
const roles = await new RbacService(db).getRoles()
64+
return roles.map((r) => ({ value: r.id, label: r.display_name }))
65+
}
6866

6967
/**
7068
* GET /admin/profile - Show user profile page
@@ -630,8 +628,10 @@ userRoutes.get('/users/new', async (c) => {
630628
? renderCustomProfileSection({ fields: regFields }, {})
631629
: undefined
632630

631+
const roles = await fetchRoleOptions(db)
632+
633633
const pageData: UserNewPageData = {
634-
roles: ROLES,
634+
roles,
635635
registrationFieldsHtml,
636636
user: {
637637
name: user!.email.split('@')[0] || user!.email,
@@ -668,9 +668,9 @@ userRoutes.post('/users/new', async (c) => {
668668
const email = formData.get('email')?.toString()?.trim().toLowerCase() || ''
669669
const phone = sanitizeInput(formData.get('phone')?.toString()) || null
670670
const bio = sanitizeInput(formData.get('bio')?.toString()) || null
671-
const roleInput = formData.get('role')?.toString() || 'viewer'
672-
const validRoles = ['admin', 'editor', 'author', 'viewer']
673-
const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
671+
const roleInput = formData.get('role')?.toString() || ''
672+
const validRoles = (await new RbacService(db).getRoles()).map((r) => r.id)
673+
const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'admin')
674674
const password = formData.get('password')?.toString() || ''
675675
const confirmPassword = formData.get('confirm_password')?.toString() || ''
676676
const isActive = formData.get('is_active') === '1'
@@ -942,9 +942,11 @@ userRoutes.get('/users/:id/edit', async (c) => {
942942
const queryMessage = c.req.query('message')
943943
const queryType = c.req.query('type') === 'error' ? 'error' : 'success'
944944

945+
const roles = await fetchRoleOptions(db)
946+
945947
const pageData: UserEditPageData = {
946948
userToEdit: editData,
947-
roles: ROLES,
949+
roles,
948950
hasProfilePlugin: profileConfig !== null && upRow !== null,
949951
customProfileFieldsHtml,
950952
tenantMemberships,
@@ -985,9 +987,9 @@ userRoutes.put('/users/:id', async (c) => {
985987
const lastName = sanitizeInput(formData.get('last_name')?.toString())
986988
const email = formData.get('email')?.toString()?.trim().toLowerCase() || ''
987989
const phone = sanitizeInput(formData.get('phone')?.toString()) || null
988-
const roleInput = formData.get('role')?.toString() || 'viewer'
989-
const validRoles = ['admin', 'editor', 'author', 'viewer']
990-
const role = validRoles.includes(roleInput) ? roleInput : 'viewer'
990+
const roleInput = formData.get('role')?.toString() || ''
991+
const validRoles = (await new RbacService(db).getRoles()).map((r) => r.id)
992+
const role = validRoles.includes(roleInput) ? roleInput : (validRoles[0] ?? 'admin')
991993
const isActive = formData.get('is_active') === '1'
992994
const emailVerified = formData.get('email_verified') === '1'
993995

0 commit comments

Comments
 (0)