diff --git a/frontend/src/__tests__/a11y.test.ts b/frontend/src/__tests__/a11y.test.ts
index 336f76af8..0cc472ad0 100644
--- a/frontend/src/__tests__/a11y.test.ts
+++ b/frontend/src/__tests__/a11y.test.ts
@@ -33,8 +33,7 @@ describe('accessibility smoke', () => {
{
id: 'u1',
email: 'alice@example.com',
- role: 'admin',
- groups: [],
+ groups: ['00000000-0000-5000-8000-000000000001'],
mfa_enabled: false,
created_at: '2024-01-01T00:00:00Z',
last_login: '2024-06-01T00:00:00Z',
@@ -57,7 +56,6 @@ describe('accessibility smoke', () => {
{
id: 'u1',
email: 'bob@example.com',
- role: 'user',
groups: [],
mfa_enabled: true,
created_at: '2024-01-01T00:00:00Z',
diff --git a/frontend/src/__tests__/allowed-accounts.test.ts b/frontend/src/__tests__/allowed-accounts.test.ts
index 03a9fb022..7ac125ba4 100644
--- a/frontend/src/__tests__/allowed-accounts.test.ts
+++ b/frontend/src/__tests__/allowed-accounts.test.ts
@@ -81,7 +81,7 @@ import { getCurrentUser } from '../state';
// Helpers
// ---------------------------------------------------------------------------
-const ADMIN = { id: 'admin-uuid', email: 'admin@example.com', role: 'admin' };
+const ADMIN = { id: 'admin-uuid', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] };
function setupTopbarSlot(): void {
while (document.body.firstChild) document.body.removeChild(document.body.firstChild);
diff --git a/frontend/src/__tests__/api.test.ts b/frontend/src/__tests__/api.test.ts
index 458435380..0aac5d079 100644
--- a/frontend/src/__tests__/api.test.ts
+++ b/frontend/src/__tests__/api.test.ts
@@ -523,7 +523,7 @@ describe('API Requests', () => {
test('fetches current user', async () => {
fetchMock.mockResolvedValue({
ok: true,
- json: () => Promise.resolve({ email: 'test@example.com', role: 'admin' })
+ json: () => Promise.resolve({ email: 'test@example.com', groups: ['00000000-0000-5000-8000-000000000001'] })
});
const user = await getCurrentUser();
diff --git a/frontend/src/__tests__/auth-mfa-enroll.test.ts b/frontend/src/__tests__/auth-mfa-enroll.test.ts
index 4c47941b3..7213d8630 100644
--- a/frontend/src/__tests__/auth-mfa-enroll.test.ts
+++ b/frontend/src/__tests__/auth-mfa-enroll.test.ts
@@ -60,7 +60,7 @@ beforeEach(() => {
`;
jest.clearAllMocks();
(state.getCurrentUser as jest.Mock).mockReturnValue({
- id: 'u1', email: 'user@x.com', role: 'user', mfa_enabled: false,
+ id: 'u1', email: 'user@x.com', groups: [], mfa_enabled: false,
});
updateUserUI();
});
@@ -84,7 +84,7 @@ describe('MFA enrollment flow', () => {
test('enabled state shows Disable and Regenerate buttons', async () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
- id: 'u1', email: 'user@x.com', role: 'user', mfa_enabled: true,
+ id: 'u1', email: 'user@x.com', groups: [], mfa_enabled: true,
});
updateUserUI();
await openProfile();
@@ -153,7 +153,7 @@ describe('MFA enrollment flow', () => {
describe('MFA disable flow', () => {
beforeEach(() => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
- id: 'u1', email: 'user@x.com', role: 'user', mfa_enabled: true,
+ id: 'u1', email: 'user@x.com', groups: [], mfa_enabled: true,
});
updateUserUI();
});
@@ -183,7 +183,7 @@ describe('MFA disable flow', () => {
describe('MFA regenerate-recovery-codes flow', () => {
beforeEach(() => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
- id: 'u1', email: 'user@x.com', role: 'user', mfa_enabled: true,
+ id: 'u1', email: 'user@x.com', groups: [], mfa_enabled: true,
});
updateUserUI();
});
diff --git a/frontend/src/__tests__/auth.test.ts b/frontend/src/__tests__/auth.test.ts
index 7a2086f29..33bfa71cc 100644
--- a/frontend/src/__tests__/auth.test.ts
+++ b/frontend/src/__tests__/auth.test.ts
@@ -2,6 +2,7 @@
* Auth module tests
*/
import { showLoginModal, showResetPasswordModal, updateUserUI, logout } from '../auth';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
// Mock the api module
jest.mock('../api', () => {
@@ -404,7 +405,7 @@ describe('Auth Module', () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
id: 'user-1',
email: 'test@example.com',
- role: 'user'
+ groups: []
});
updateUserUI();
@@ -417,7 +418,7 @@ describe('Auth Module', () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
id: 'user-1',
email: 'test@example.com',
- role: 'user'
+ groups: []
});
updateUserUI();
@@ -439,7 +440,7 @@ describe('Auth Module', () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
id: 'admin-1',
email: 'admin@example.com',
- role: 'admin'
+ groups: [ADMINISTRATORS_GROUP_ID]
});
updateUserUI();
@@ -454,7 +455,7 @@ describe('Auth Module', () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
id: 'user-1',
email: 'user@example.com',
- role: 'user'
+ groups: []
});
updateUserUI();
@@ -469,7 +470,7 @@ describe('Auth Module', () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
id: 'user-1',
email: 'test@example.com',
- role: 'user'
+ groups: []
});
updateUserUI();
@@ -483,7 +484,7 @@ describe('Auth Module', () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
id: 'user-1',
email: 'test@example.com',
- role: 'user'
+ groups: []
});
(api.logout as jest.Mock).mockResolvedValue({});
@@ -522,7 +523,7 @@ describe('Auth Module', () => {
(state.getCurrentUser as jest.Mock).mockReturnValue({
id: 'user-1',
email: 'test@example.com',
- role: 'user'
+ groups: []
});
});
diff --git a/frontend/src/__tests__/groups.test.ts b/frontend/src/__tests__/groups.test.ts
index 737d1796d..cd8054ea8 100644
--- a/frontend/src/__tests__/groups.test.ts
+++ b/frontend/src/__tests__/groups.test.ts
@@ -65,11 +65,11 @@ const mockGroups: api.APIGroup[] = [
},
];
-// Mock users that belong to groups
+// Mock users that belong to groups (PR #912: role field removed, groups is the source of truth)
const mockUsers = [
- { id: 'user-1', email: 'admin@test.com', role: 'admin', groups: ['group-1'], mfa_enabled: true },
- { id: 'user-2', email: 'viewer@test.com', role: 'user', groups: ['group-2'], mfa_enabled: false },
- { id: 'user-3', email: 'both@test.com', role: 'user', groups: ['group-1', 'group-2'], mfa_enabled: true },
+ { id: 'user-1', email: 'admin@test.com', groups: ['00000000-0000-5000-8000-000000000001', 'group-1'], mfa_enabled: true },
+ { id: 'user-2', email: 'viewer@test.com', groups: ['group-2'], mfa_enabled: false },
+ { id: 'user-3', email: 'both@test.com', groups: ['group-1', 'group-2'], mfa_enabled: true },
];
describe('groups/state', () => {
@@ -164,7 +164,7 @@ describe('groups/groupList', () => {
it('should use singular "member" when count is 1', () => {
// Set up users so group-1 has exactly 1 member
userState.setAllUsers([
- { id: 'user-1', email: 'admin@test.com', role: 'admin', groups: ['group-1'], mfa_enabled: true },
+ { id: 'user-1', email: 'admin@test.com', groups: ['00000000-0000-5000-8000-000000000001', 'group-1'], mfa_enabled: true },
] as any);
const group = mockGroups[0];
diff --git a/frontend/src/__tests__/history-approval-queue.test.ts b/frontend/src/__tests__/history-approval-queue.test.ts
index f407a145a..7a928a36c 100644
--- a/frontend/src/__tests__/history-approval-queue.test.ts
+++ b/frontend/src/__tests__/history-approval-queue.test.ts
@@ -67,8 +67,9 @@ import { confirmDialog } from '../confirmDialog';
import { showToast } from '../toast';
import { getCurrentUser } from '../state';
import { getAccountName } from '../recommendations';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', role: 'admin' };
+const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] };
function setupDOM(): void {
while (document.body.firstChild) document.body.removeChild(document.body.firstChild);
diff --git a/frontend/src/__tests__/history-approve-button.test.ts b/frontend/src/__tests__/history-approve-button.test.ts
index 4974a8b3d..b3f57fb76 100644
--- a/frontend/src/__tests__/history-approve-button.test.ts
+++ b/frontend/src/__tests__/history-approve-button.test.ts
@@ -63,9 +63,10 @@ import * as api from '../api';
import { confirmDialog } from '../confirmDialog';
import { showToast } from '../toast';
import { getCurrentUser } from '../state';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', role: 'admin' };
-const REG_USER = { id: 'user-uuid', email: 'user@example.com', role: 'user' };
+const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] };
+const REG_USER = { id: 'user-uuid', email: 'user@example.com', groups: [] };
const OTHER_UUID = 'other-uuid';
function setupDOM(): void {
diff --git a/frontend/src/__tests__/history-cancel-button.test.ts b/frontend/src/__tests__/history-cancel-button.test.ts
index 5f2c83dec..d642e3dc6 100644
--- a/frontend/src/__tests__/history-cancel-button.test.ts
+++ b/frontend/src/__tests__/history-cancel-button.test.ts
@@ -60,9 +60,10 @@ import * as api from '../api';
import { confirmDialog } from '../confirmDialog';
import { showToast } from '../toast';
import { getCurrentUser } from '../state';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', role: 'admin' };
-const REG_USER = { id: 'user-uuid', email: 'user@example.com', role: 'user' };
+const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] };
+const REG_USER = { id: 'user-uuid', email: 'user@example.com', groups: [] };
const OTHER_UUID = 'other-uuid';
function setupDOM(): void {
diff --git a/frontend/src/__tests__/history-retry-button.test.ts b/frontend/src/__tests__/history-retry-button.test.ts
index 1a9d361c6..6a56fbe8e 100644
--- a/frontend/src/__tests__/history-retry-button.test.ts
+++ b/frontend/src/__tests__/history-retry-button.test.ts
@@ -69,9 +69,10 @@ import * as api from '../api';
import { confirmDialog } from '../confirmDialog';
import { showToast } from '../toast';
import { getCurrentUser } from '../state';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', role: 'admin' };
-const REG_USER = { id: 'user-uuid', email: 'user@example.com', role: 'user' };
+const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] };
+const REG_USER = { id: 'user-uuid', email: 'user@example.com', groups: [] };
const OTHER_UUID = 'other-uuid';
function setupDOM(): void {
diff --git a/frontend/src/__tests__/html.test.ts b/frontend/src/__tests__/html.test.ts
index 668253945..91dce4bec 100644
--- a/frontend/src/__tests__/html.test.ts
+++ b/frontend/src/__tests__/html.test.ts
@@ -501,20 +501,20 @@ describe('HTML Structure', () => {
expect(email?.hasAttribute('required')).toBe(true);
});
- test('has user role select', () => {
+ test('user role select is absent (PR #912: role concept dropped)', () => {
+ // PR #912 removed the role column. The role selector was replaced by
+ // a required group multi-select. Verify the old element is gone so a
+ // regression that re-adds it is caught.
const role = document.getElementById('user-role') as HTMLSelectElement | null;
- expect(role).toBeTruthy();
- const options = Array.from(role?.querySelectorAll('option') ?? []).map(o => o.value);
- // Exact-set equality (not toContain) so a regression that
- // re-introduces the pre-fix viewer/editor values — or adds any
- // other role the backend allowlist would reject — fails the test.
- expect(new Set(options)).toEqual(new Set(['readonly', 'user', 'admin']));
+ expect(role).toBeNull();
});
- test('has user groups multi-select', () => {
+ test('has user groups multi-select (required, PR #912)', () => {
const groups = document.getElementById('user-groups') as HTMLSelectElement | null;
expect(groups).toBeTruthy();
expect(groups?.hasAttribute('multiple')).toBe(true);
+ // PR #912: groups is now required (>= 1 group enforced by backend DB CHECK).
+ expect(groups?.hasAttribute('required')).toBe(true);
});
});
diff --git a/frontend/src/__tests__/permissions.test.ts b/frontend/src/__tests__/permissions.test.ts
index 7e00b7c09..09ebd861e 100644
--- a/frontend/src/__tests__/permissions.test.ts
+++ b/frontend/src/__tests__/permissions.test.ts
@@ -1,15 +1,16 @@
/**
* Permissions helper tests.
*
- * The role-default sets MUST match the backend constants in
- * `internal/auth/types.go` (DefaultAdminPermissions /
- * DefaultUserPermissions / DefaultReadOnlyPermissions). These tests
- * enumerate every entry so a drift between this mirror and the
- * backend fails fast in CI rather than at runtime as a wrong-positive
- * (button shown, click 403s) or wrong-negative (functionality hidden
- * from a user who should have it).
+ * PR #912 replaced role-based gating with group-membership-based
+ * gating. isAdmin() now returns true when the current user is a member
+ * of the Administrators group (UUID 00000000-0000-5000-8000-000000000001).
+ * canAccess() is currently a pass-through to isAdmin() -- non-admin users
+ * always get false because the /me/permissions endpoint is deferred.
+ *
+ * getRolePermissions() is kept for the effective-permissions display in
+ * the admin Users page and still returns the same sets as before.
*/
-import { canAccess, getRolePermissions, isAdmin } from '../permissions';
+import { canAccess, getRolePermissions, isAdmin, ADMINISTRATORS_GROUP_ID } from '../permissions';
jest.mock('../state', () => ({
getCurrentUser: jest.fn(),
@@ -17,13 +18,27 @@ jest.mock('../state', () => ({
import * as state from '../state';
-const mockUser = (role: string | null) => {
+const ADMIN_GID = ADMINISTRATORS_GROUP_ID;
+const STD_GID = '00000000-0000-5000-8000-000000000005';
+const RO_GID = '00000000-0000-5000-8000-000000000006';
+
+const mockUserWithGroups = (groups: string[]) => {
(state.getCurrentUser as jest.Mock).mockReturnValue(
- role === null ? null : { id: 'u1', email: 'u@example.com', role },
+ { id: 'u1', email: 'u@example.com', groups },
);
};
+const mockNoUser = () => {
+ (state.getCurrentUser as jest.Mock).mockReturnValue(null);
+};
+
describe('permissions', () => {
+ describe('ADMINISTRATORS_GROUP_ID', () => {
+ test('has the expected UUID', () => {
+ expect(ADMINISTRATORS_GROUP_ID).toBe('00000000-0000-5000-8000-000000000001');
+ });
+ });
+
describe('getRolePermissions', () => {
test('admin role grants admin:*', () => {
const perms = getRolePermissions('admin');
@@ -68,99 +83,79 @@ describe('permissions', () => {
});
});
- describe('canAccess', () => {
- test('admin sees everything (admin:* superuser short-circuit)', () => {
- mockUser('admin');
- expect(canAccess('admin', '*')).toBe(true);
- expect(canAccess('view', 'users')).toBe(true);
- expect(canAccess('delete', 'plans')).toBe(true);
- expect(canAccess('execute', 'purchases')).toBe(true);
- expect(canAccess('view', 'accounts')).toBe(true);
+ describe('isAdmin', () => {
+ test('Administrators group member is admin', () => {
+ mockUserWithGroups([ADMIN_GID]);
+ expect(isAdmin()).toBe(true);
});
- test('user role: explicit grants', () => {
- mockUser('user');
- expect(canAccess('view', 'recommendations')).toBe(true);
- expect(canAccess('view', 'plans')).toBe(true);
- expect(canAccess('view', 'purchases')).toBe(true);
- expect(canAccess('view', 'history')).toBe(true);
- expect(canAccess('create', 'plans')).toBe(true);
- expect(canAccess('update', 'plans')).toBe(true);
- expect(canAccess('cancel-own', 'purchases')).toBe(true);
- expect(canAccess('retry-own', 'purchases')).toBe(true);
- expect(canAccess('approve-own', 'purchases')).toBe(true);
+ test('Administrators group member alongside other groups is still admin', () => {
+ mockUserWithGroups([STD_GID, ADMIN_GID, RO_GID]);
+ expect(isAdmin()).toBe(true);
});
- test('user role: denied for admin-gated actions', () => {
- mockUser('user');
- // delete:plans is now a default user permission (PR #660); no longer admin-gated.
- expect(canAccess('delete', 'plans')).toBe(true);
- // execute:purchases was NOT added to user defaults; remains admin-only.
- expect(canAccess('execute', 'purchases')).toBe(false);
- expect(canAccess('admin', '*')).toBe(false);
- expect(canAccess('view', 'users')).toBe(false);
- expect(canAccess('view', 'accounts')).toBe(false);
- expect(canAccess('view', 'groups')).toBe(false);
- expect(canAccess('view', 'api-keys')).toBe(false);
- expect(canAccess('view', 'config')).toBe(false);
+ test('Standard Users group member is not admin', () => {
+ mockUserWithGroups([STD_GID]);
+ expect(isAdmin()).toBe(false);
});
- test('readonly role: only view grants on rec/plans/history', () => {
- mockUser('readonly');
- expect(canAccess('view', 'recommendations')).toBe(true);
- expect(canAccess('view', 'plans')).toBe(true);
- expect(canAccess('view', 'history')).toBe(true);
+ test('Read-Only Users group member is not admin', () => {
+ mockUserWithGroups([RO_GID]);
+ expect(isAdmin()).toBe(false);
});
- test('readonly role: denied for purchases view + every mutation', () => {
- mockUser('readonly');
- expect(canAccess('view', 'purchases')).toBe(false);
- expect(canAccess('create', 'plans')).toBe(false);
- expect(canAccess('update', 'plans')).toBe(false);
- expect(canAccess('delete', 'plans')).toBe(false);
- expect(canAccess('execute', 'purchases')).toBe(false);
- expect(canAccess('cancel-own', 'purchases')).toBe(false);
- expect(canAccess('retry-own', 'purchases')).toBe(false);
- expect(canAccess('approve-own', 'purchases')).toBe(false);
- expect(canAccess('admin', '*')).toBe(false);
- expect(canAccess('view', 'users')).toBe(false);
- expect(canAccess('view', 'accounts')).toBe(false);
+ test('user with empty groups is not admin', () => {
+ mockUserWithGroups([]);
+ expect(isAdmin()).toBe(false);
});
- test('null user (logged out) → false for everything', () => {
- mockUser(null);
- expect(canAccess('view', 'recommendations')).toBe(false);
- expect(canAccess('admin', '*')).toBe(false);
- expect(canAccess('create', 'plans')).toBe(false);
+ test('null user (logged out) is not admin', () => {
+ mockNoUser();
+ expect(isAdmin()).toBe(false);
});
- test('unknown role → false for everything', () => {
- mockUser('operator');
- expect(canAccess('view', 'recommendations')).toBe(false);
- expect(canAccess('admin', '*')).toBe(false);
- expect(canAccess('execute', 'purchases')).toBe(false);
+ test('user with non-array groups field is not admin (defense-in-depth)', () => {
+ (state.getCurrentUser as jest.Mock).mockReturnValue(
+ { id: 'u1', email: 'u@example.com', groups: null },
+ );
+ expect(isAdmin()).toBe(false);
});
});
- describe('isAdmin', () => {
- test('admin role → true', () => {
- mockUser('admin');
- expect(isAdmin()).toBe(true);
+ describe('canAccess', () => {
+ test('Administrators group member passes all checks', () => {
+ mockUserWithGroups([ADMIN_GID]);
+ expect(canAccess('admin', '*')).toBe(true);
+ expect(canAccess('view', 'users')).toBe(true);
+ expect(canAccess('delete', 'plans')).toBe(true);
+ expect(canAccess('execute', 'purchases')).toBe(true);
+ expect(canAccess('view', 'accounts')).toBe(true);
});
- test('user role → false', () => {
- mockUser('user');
- expect(isAdmin()).toBe(false);
+ test('Standard Users group member fails all checks (no /me/permissions endpoint yet)', () => {
+ mockUserWithGroups([STD_GID]);
+ expect(canAccess('view', 'recommendations')).toBe(false);
+ expect(canAccess('view', 'plans')).toBe(false);
+ expect(canAccess('admin', '*')).toBe(false);
});
- test('readonly role → false', () => {
- mockUser('readonly');
- expect(isAdmin()).toBe(false);
+ test('Read-Only Users group member fails all checks', () => {
+ mockUserWithGroups([RO_GID]);
+ expect(canAccess('view', 'recommendations')).toBe(false);
+ expect(canAccess('admin', '*')).toBe(false);
});
- test('null user → false', () => {
- mockUser(null);
- expect(isAdmin()).toBe(false);
+ test('empty-groups user fails all checks', () => {
+ mockUserWithGroups([]);
+ expect(canAccess('view', 'recommendations')).toBe(false);
+ expect(canAccess('admin', '*')).toBe(false);
+ });
+
+ test('null user (logged out) fails all checks', () => {
+ mockNoUser();
+ expect(canAccess('view', 'recommendations')).toBe(false);
+ expect(canAccess('admin', '*')).toBe(false);
+ expect(canAccess('create', 'plans')).toBe(false);
});
});
});
diff --git a/frontend/src/__tests__/plans-permissions.test.ts b/frontend/src/__tests__/plans-permissions.test.ts
index cc0c3a0d2..2507f6856 100644
--- a/frontend/src/__tests__/plans-permissions.test.ts
+++ b/frontend/src/__tests__/plans-permissions.test.ts
@@ -41,6 +41,7 @@ jest.mock('../history', () => ({ viewPlanHistory: jest.fn() }));
import * as api from '../api';
import * as state from '../state';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
const samplePlan = {
id: 'plan-1',
@@ -74,7 +75,7 @@ const samplePlannedPurchase = {
const mockUser = (role: string | null) => {
(state.getCurrentUser as jest.Mock).mockReturnValue(
- role === null ? null : { id: 'u', email: 'u@example.com', role },
+ role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? [ADMINISTRATORS_GROUP_ID] : [] },
);
};
@@ -131,33 +132,37 @@ describe('Plans page permission gating (issue #365)', () => {
});
});
- describe('user role', () => {
+ describe('non-admin user (PR #912: canAccess returns false without /me/permissions)', () => {
beforeEach(() => mockUser('user'));
- test('shows the top-level New Plan button (user has create:plans)', async () => {
+ test('hides the top-level New Plan button (no /me/permissions endpoint yet)', async () => {
+ // PR #912: canAccess() is group-membership-only; non-Administrators-group
+ // members get false until the /me/permissions endpoint lands.
await loadPlans();
const btn = document.getElementById('new-plan-btn') as HTMLButtonElement;
- expect(btn.hidden).toBe(false);
+ expect(btn.hidden).toBe(true);
});
- test('shows manage actions including Delete (user has delete:plans since PR #660)', async () => {
+ test('hides plan-card action buttons for non-admin (no /me/permissions endpoint yet)', async () => {
await loadPlans();
const list = document.getElementById('plans-list') as HTMLElement;
const html = list.innerHTML;
- expect(html).toContain('data-action="add-purchases"');
- expect(html).toContain('data-action="edit-plan"');
- expect(html).toContain('data-action="toggle-plan"');
- expect(html).toContain('data-action="delete-plan"');
+ expect(html).not.toContain('data-action="add-purchases"');
+ expect(html).not.toContain('data-action="edit-plan"');
+ expect(html).not.toContain('data-action="delete-plan"');
+ expect(html).not.toContain('data-action="toggle-plan"');
+ // History view stays visible regardless.
+ expect(html).toContain('data-action="view-history"');
});
- test('shows row Run/Pause/Edit/Disable on planned purchases (user has delete:plans since PR #660)', async () => {
+ test('hides planned-purchase row action buttons for non-admin (no /me/permissions endpoint yet)', async () => {
await loadPlans();
const pp = document.getElementById('planned-purchases-list') as HTMLElement;
const html = pp.innerHTML;
- expect(html).toContain('data-action="run"');
- expect(html).toContain('data-action="pause"');
- expect(html).toContain('data-action="edit"');
- expect(html).toContain('data-action="disable"');
+ expect(html).not.toContain('data-action="run"');
+ expect(html).not.toContain('data-action="pause"');
+ expect(html).not.toContain('data-action="edit"');
+ expect(html).not.toContain('data-action="disable"');
});
});
diff --git a/frontend/src/__tests__/plans-range-validation.test.ts b/frontend/src/__tests__/plans-range-validation.test.ts
index b5faa306f..134dde150 100644
--- a/frontend/src/__tests__/plans-range-validation.test.ts
+++ b/frontend/src/__tests__/plans-range-validation.test.ts
@@ -46,7 +46,7 @@ jest.mock('../state', () => ({
setCurrentAccountIDs: jest.fn(),
subscribeProvider: jest.fn().mockReturnValue(() => {}),
subscribeAccount: jest.fn().mockReturnValue(() => {}),
- getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', role: 'admin' }),
+ getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
}));
jest.mock('../history', () => ({ viewPlanHistory: jest.fn() }));
diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts
index 43ff7543c..94fe90973 100644
--- a/frontend/src/__tests__/plans.test.ts
+++ b/frontend/src/__tests__/plans.test.ts
@@ -49,7 +49,11 @@ jest.mock('../state', () => ({
// click-handler counts, plan-card layout) keeps passing unchanged.
// The permission-gating tests in plans-permissions.test.ts override
// this per-case to exercise readonly / user / null sessions.
- getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', role: 'admin' }),
+ // ADMINISTRATORS_GROUP_ID literal used here (not imported) because jest.mock
+ // factories are hoisted before imports; jest.requireActual also fails here
+ // because permissions.ts has a top-level import of ./state which is the very
+ // module being mocked (circular init). permissions.test.ts pins the value.
+ getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
}));
// Mock history module
diff --git a/frontend/src/__tests__/recommendations-enabled-providers.test.ts b/frontend/src/__tests__/recommendations-enabled-providers.test.ts
index 7ea38f8ad..c232c109f 100644
--- a/frontend/src/__tests__/recommendations-enabled-providers.test.ts
+++ b/frontend/src/__tests__/recommendations-enabled-providers.test.ts
@@ -62,7 +62,11 @@ jest.mock('../state', () => ({
setCostPeriod: jest.fn(),
getHiddenColumns: jest.fn().mockReturnValue(new Set()),
setHiddenColumns: jest.fn(),
- getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', role: 'admin' }),
+ // ADMINISTRATORS_GROUP_ID literal used here (not imported) because jest.mock
+ // factories are hoisted before imports; jest.requireActual also fails here
+ // because permissions.ts has a top-level import of ./state which is the very
+ // module being mocked (circular init). permissions.test.ts pins the value.
+ getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
}));
jest.mock('../utils', () => ({
diff --git a/frontend/src/__tests__/recommendations-permissions.test.ts b/frontend/src/__tests__/recommendations-permissions.test.ts
index 1e0cf0263..2d0f49ca9 100644
--- a/frontend/src/__tests__/recommendations-permissions.test.ts
+++ b/frontend/src/__tests__/recommendations-permissions.test.ts
@@ -63,10 +63,11 @@ jest.mock('../toast', () => ({
}));
import * as state from '../state';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
const mockUser = (role: string | null) => {
(state.getCurrentUser as jest.Mock).mockReturnValue(
- role === null ? null : { id: 'u', email: 'u@example.com', role },
+ role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? [ADMINISTRATORS_GROUP_ID] : [] },
);
};
@@ -122,13 +123,16 @@ describe('Recommendations action-box permission gating (issue #365)', () => {
expect(plan.hidden).toBe(false);
});
- test('user role hides Purchase but keeps Create Plan', async () => {
+ test('non-admin user hides both Purchase and Create Plan (PR #912: no /me/permissions yet)', async () => {
+ // PR #912: canAccess() returns false for non-admin users until the
+ // /me/permissions endpoint lands. All non-admin users see the same
+ // read-only view as before.
mockUser('user');
await loadRecommendations();
const purchase = document.getElementById('bulk-purchase-btn') as HTMLButtonElement;
const plan = document.getElementById('create-plan-btn') as HTMLButtonElement;
expect(purchase.hidden).toBe(true);
- expect(plan.hidden).toBe(false);
+ expect(plan.hidden).toBe(true);
});
test('readonly role hides both Purchase and Create Plan', async () => {
@@ -149,9 +153,9 @@ describe('Recommendations action-box permission gating (issue #365)', () => {
expect(plan.hidden).toBe(true);
});
- test('the action-box capacity input stays visible for every role', async () => {
- // Readonly users still browse the bottom box for the selection summary;
- // only the mutating CTAs disappear.
+ test('the action-box capacity input stays visible for all sessions', async () => {
+ // Non-mutating elements stay visible regardless of group membership;
+ // only the action CTAs gate on permissions.
for (const role of ['admin', 'user', 'readonly']) {
setupDom();
mockUser(role);
@@ -214,18 +218,21 @@ describe('Recommendations checkbox + row-click gating for viewer role (issue #86
expect(rowCheckboxes.length).toBeGreaterThan(0);
});
- test('user (operator) role: select-all checkbox is present', async () => {
+ test('non-admin user: no select-all checkbox (PR #912: canAccess returns false without /me/permissions)', async () => {
+ // PR #912: only Administrators-group members get checkboxes.
+ // The /me/permissions endpoint that would restore Standard Users'
+ // checkbox access is deferred to a follow-up.
mockUser('user');
await loadRecommendations();
- expect(document.getElementById('select-all-recs')).not.toBeNull();
+ expect(document.getElementById('select-all-recs')).toBeNull();
});
- test('user (operator) role: per-row checkbox is present', async () => {
+ test('non-admin user: no per-row checkboxes (PR #912: canAccess returns false without /me/permissions)', async () => {
mockUser('user');
await loadRecommendations();
const list = document.getElementById('recommendations-list');
const rowCheckboxes = list?.querySelectorAll('input[data-rec-id]') ?? [];
- expect(rowCheckboxes.length).toBeGreaterThan(0);
+ expect(rowCheckboxes.length).toBe(0);
});
test('readonly role: grouped-row summary has no checkbox-col and column span is aligned', async () => {
diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts
index 30bf8668d..424810701 100644
--- a/frontend/src/__tests__/recommendations.test.ts
+++ b/frontend/src/__tests__/recommendations.test.ts
@@ -88,7 +88,7 @@ jest.mock('../state', () => ({
// tests below see both #bulk-purchase-btn and #create-plan-btn
// unconditionally. Permission-gating coverage lives in
// recommendations-permissions.test.ts.
- getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', role: 'admin' }),
+ getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
// Issue #477: setupRecommendationsHandlers subscribes to provider/account
// changes; expose jest.fn() shims so tests can capture the callback and
// simulate a change without going through the real listener set.
diff --git a/frontend/src/__tests__/riexchange-permissions.test.ts b/frontend/src/__tests__/riexchange-permissions.test.ts
index 4a82b8749..56bdc81a1 100644
--- a/frontend/src/__tests__/riexchange-permissions.test.ts
+++ b/frontend/src/__tests__/riexchange-permissions.test.ts
@@ -60,7 +60,7 @@ const sampleReshape = {
const mockUser = (role: string | null) => {
(state.getCurrentUser as jest.Mock).mockReturnValue(
- role === null ? null : { id: 'u', email: 'u@example.com', role },
+ role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? ['00000000-0000-5000-8000-000000000001'] : [] },
);
};
diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts
index a64b4c00b..10271b8b2 100644
--- a/frontend/src/__tests__/riexchange.test.ts
+++ b/frontend/src/__tests__/riexchange.test.ts
@@ -45,7 +45,7 @@ jest.mock('../state', () => ({
}),
getCurrentProvider: jest.fn(() => 'aws'),
getCurrentAccountIDs: jest.fn(() => []),
- getCurrentUser: jest.fn(() => ({ id: 'u', email: 'u@example.com', role: 'admin' })),
+ getCurrentUser: jest.fn(() => ({ id: 'u', email: 'u@example.com', groups: ['00000000-0000-5000-8000-000000000001'] })),
}));
import {
diff --git a/frontend/src/__tests__/settings-permissions.test.ts b/frontend/src/__tests__/settings-permissions.test.ts
index 13f4333ad..ee7959ced 100644
--- a/frontend/src/__tests__/settings-permissions.test.ts
+++ b/frontend/src/__tests__/settings-permissions.test.ts
@@ -42,7 +42,7 @@ import * as state from '../state';
const mockUser = (role: string | null) => {
(state.getCurrentUser as jest.Mock).mockReturnValue(
- role === null ? null : { id: 'u', email: 'u@example.com', role },
+ role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? ['00000000-0000-5000-8000-000000000001'] : [] },
);
};
diff --git a/frontend/src/__tests__/state.test.ts b/frontend/src/__tests__/state.test.ts
index 8f41d5b06..5212d71aa 100644
--- a/frontend/src/__tests__/state.test.ts
+++ b/frontend/src/__tests__/state.test.ts
@@ -17,6 +17,7 @@ import {
setSavingsChart
} from '../state';
import type { Recommendation } from '../api';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
describe('State Module', () => {
// Reset state before each test
@@ -34,13 +35,13 @@ describe('State Module', () => {
});
test('setCurrentUser and getCurrentUser work correctly', () => {
- const user = { id: '123', email: 'test@example.com', role: 'admin' };
+ const user = { id: '123', email: 'test@example.com', groups: [ADMINISTRATORS_GROUP_ID] };
setCurrentUser(user);
expect(getCurrentUser()).toEqual(user);
});
test('setCurrentUser with null clears user', () => {
- setCurrentUser({ id: '123', email: 'test@example.com', role: 'admin' });
+ setCurrentUser({ id: '123', email: 'test@example.com', groups: [ADMINISTRATORS_GROUP_ID] });
setCurrentUser(null);
expect(getCurrentUser()).toBeNull();
});
@@ -194,7 +195,7 @@ describe('State Module', () => {
describe('State Integration', () => {
test('state changes are reflected across getters', () => {
// Set various state
- setCurrentUser({ id: '1', email: 'a@b.com', role: 'user' });
+ setCurrentUser({ id: '1', email: 'a@b.com', groups: [] });
setCurrentProvider('aws');
setRecommendations([{
id: '1',
diff --git a/frontend/src/__tests__/users.test.ts b/frontend/src/__tests__/users.test.ts
index 87b2bd5b2..31f13241f 100644
--- a/frontend/src/__tests__/users.test.ts
+++ b/frontend/src/__tests__/users.test.ts
@@ -285,8 +285,8 @@ describe('users/state', () => {
describe('allUsers and filteredUsers', () => {
it('should set and get users', () => {
const users = [
- { id: '1', email: 'user1@test.com', role: 'user', groups: [], mfa_enabled: false },
- { id: '2', email: 'user2@test.com', role: 'admin', groups: [], mfa_enabled: true }
+ { id: '1', email: 'user1@test.com', groups: [], mfa_enabled: false },
+ { id: '2', email: 'user2@test.com', groups: ['00000000-0000-5000-8000-000000000001'], mfa_enabled: true }
];
userState.setAllUsers(users as any);
@@ -295,7 +295,7 @@ describe('users/state', () => {
it('should set and get filtered users', () => {
const users = [
- { id: '1', email: 'user1@test.com', role: 'user', groups: [], mfa_enabled: false }
+ { id: '1', email: 'user1@test.com', groups: [], mfa_enabled: false }
];
userState.setFilteredUsers(users as any);
@@ -323,7 +323,7 @@ describe('users/state', () => {
describe('currentEditingUser', () => {
it('should set and get current editing user', () => {
- const user = { id: '1', email: 'test@test.com', role: 'user', groups: [], mfa_enabled: false };
+ const user = { id: '1', email: 'test@test.com', groups: [], mfa_enabled: false };
userState.setCurrentEditingUser(user as any);
expect(userState.currentEditingUser).toEqual(user);
});
@@ -374,10 +374,10 @@ describe('users/state', () => {
// ============================================================================
describe('users/filters', () => {
const mockUsers = [
- { id: '1', email: 'admin@test.com', role: 'admin', groups: ['admins'], mfa_enabled: true },
- { id: '2', email: 'user@test.com', role: 'user', groups: ['users'], mfa_enabled: false },
- { id: '3', email: 'viewer@test.com', role: 'viewer', groups: [], mfa_enabled: true },
- { id: '4', email: 'another.user@example.com', role: 'user', groups: ['users', 'developers'], mfa_enabled: true }
+ { id: '1', email: 'admin@test.com', groups: ['00000000-0000-5000-8000-000000000001'], mfa_enabled: true },
+ { id: '2', email: 'user@test.com', groups: ['users'], mfa_enabled: false },
+ { id: '3', email: 'viewer@test.com', groups: [], mfa_enabled: true },
+ { id: '4', email: 'another.user@example.com', groups: ['users', 'developers'], mfa_enabled: true }
];
beforeEach(() => {
@@ -421,11 +421,19 @@ describe('users/filters', () => {
expect(userState.filteredUsers.length).toBe(3);
});
- it('should filter by role', () => {
- userState.setRoleFilter('user');
+ it('should filter by admin role (Administrators group membership)', () => {
+ userState.setRoleFilter('admin');
userFilters.applyFilters();
- expect(userState.filteredUsers.length).toBe(2);
- expect(userState.filteredUsers.every(u => u.role === 'user')).toBe(true);
+ // Only the user in the Administrators group matches
+ expect(userState.filteredUsers.length).toBe(1);
+ expect(userState.filteredUsers[0]?.email).toBe('admin@test.com');
+ });
+
+ it('should filter non-admin users with role filter', () => {
+ userState.setRoleFilter('user'); // non-admin = not in Administrators group
+ userFilters.applyFilters();
+ // 3 users not in Administrators group
+ expect(userState.filteredUsers.length).toBe(3);
});
it('should filter by MFA enabled', () => {
@@ -443,10 +451,10 @@ describe('users/filters', () => {
});
it('should filter by group', () => {
- userState.setGroupFilter('admins');
+ userState.setGroupFilter('00000000-0000-5000-8000-000000000001');
userFilters.applyFilters();
expect(userState.filteredUsers.length).toBe(1);
- expect(userState.filteredUsers[0]!.groups).toContain('admins');
+ expect(userState.filteredUsers[0]!.groups).toContain('00000000-0000-5000-8000-000000000001');
});
it('should filter by users group (multiple users)', () => {
@@ -461,7 +469,7 @@ describe('users/filters', () => {
expect(userState.filteredUsers.length).toBe(0);
});
- it('should combine multiple filters', () => {
+ it('should combine multiple filters (mfa + admin group)', () => {
userState.setMfaFilter('enabled');
userState.setRoleFilter('admin');
userFilters.applyFilters();
@@ -469,16 +477,17 @@ describe('users/filters', () => {
expect(userState.filteredUsers[0]?.email).toBe('admin@test.com');
});
- it('should combine search and role filter', () => {
+ it('should combine search and non-admin role filter', () => {
userState.setSearchQuery('user');
- userState.setRoleFilter('user');
+ userState.setRoleFilter('user'); // non-admin
userFilters.applyFilters();
+ // 'user@test.com' and 'another.user@example.com' match search + non-admin
expect(userState.filteredUsers.length).toBe(2);
});
it('should combine all filters', () => {
userState.setSearchQuery('user');
- userState.setRoleFilter('user');
+ userState.setRoleFilter('user'); // non-admin
userState.setMfaFilter('enabled');
userState.setGroupFilter('users');
userFilters.applyFilters();
@@ -520,9 +529,10 @@ describe('users/filters', () => {
});
describe('handleFilterChange', () => {
- it('should handle role filter change', () => {
+ it('should handle role filter change (admin = Administrators group)', () => {
userFilters.handleFilterChange('role', 'admin');
expect(userState.roleFilter).toBe('admin');
+ // Only the Administrators group member matches
expect(userState.filteredUsers.length).toBe(1);
});
@@ -533,8 +543,8 @@ describe('users/filters', () => {
});
it('should handle group filter change', () => {
- userFilters.handleFilterChange('group', 'admins');
- expect(userState.groupFilter).toBe('admins');
+ userFilters.handleFilterChange('group', '00000000-0000-5000-8000-000000000001');
+ expect(userState.groupFilter).toBe('00000000-0000-5000-8000-000000000001');
expect(userState.filteredUsers.length).toBe(1);
});
@@ -552,7 +562,7 @@ describe('users/filters', () => {
userState.setSearchQuery('test');
userState.setRoleFilter('admin');
userState.setMfaFilter('enabled');
- userState.setGroupFilter('admins');
+ userState.setGroupFilter('00000000-0000-5000-8000-000000000001');
userFilters.clearFilters();
@@ -584,6 +594,7 @@ describe('users/filters', () => {
it('should re-render all users after clearing', () => {
userState.setRoleFilter('admin');
userFilters.applyFilters();
+ // Only 1 Administrators-group member
expect(userState.filteredUsers.length).toBe(1);
userFilters.clearFilters();
@@ -653,8 +664,8 @@ describe('users/filters', () => {
// ============================================================================
describe('users/userList', () => {
const mockUsers = [
- { id: '1', email: 'admin@test.com', role: 'admin', groups: ['admins'], mfa_enabled: true, created_at: '2024-01-01T00:00:00Z' },
- { id: '2', email: 'user@test.com', role: 'user', groups: [], mfa_enabled: false, created_at: '2024-01-02T00:00:00Z' }
+ { id: '1', email: 'admin@test.com', groups: ['00000000-0000-5000-8000-000000000001'], mfa_enabled: true, created_at: '2024-01-01T00:00:00Z' },
+ { id: '2', email: 'user@test.com', groups: [], mfa_enabled: false, created_at: '2024-01-02T00:00:00Z' }
];
beforeEach(() => {
@@ -782,12 +793,13 @@ describe('users/userList', () => {
expect(selectAll?.checked).toBe(true);
});
- it('should render role badges', () => {
+ it('should render group badges (role column removed, PR #912)', () => {
userList.renderUsers(mockUsers as any);
-
+ // The Role column was removed; verify the table renders without throwing
const container = document.getElementById('users-list');
- expect(container?.innerHTML).toContain('badge-admin');
- expect(container?.innerHTML).toContain('badge-user');
+ expect(container?.querySelector('table')).toBeTruthy();
+ // Group badges still appear
+ expect(container?.innerHTML).toContain('badge-group');
});
it('should render MFA status badges', () => {
@@ -803,7 +815,9 @@ describe('users/userList', () => {
const container = document.getElementById('users-list');
expect(container?.innerHTML).toContain('badge-group');
- expect(container?.innerHTML).toContain('admins');
+ // User 1 is in the Administrators group (ADMIN_GID); since availableGroups
+ // is empty the group name lookup falls back to the UUID itself.
+ expect(container?.innerHTML).toContain('00000000');
});
it('should show "No groups" for users without groups', () => {
@@ -837,7 +851,7 @@ describe('users/userList', () => {
it('should show dash for missing created_at', () => {
const usersWithoutDate = [
- { id: '1', email: 'test@test.com', role: 'user', groups: [], mfa_enabled: false }
+ { id: '1', email: 'test@test.com', groups: [], mfa_enabled: false }
];
userList.renderUsers(usersWithoutDate as any);
@@ -856,7 +870,7 @@ describe('users/userList', () => {
it('should render last login relative time when set', () => {
const usersWithLogin = [
- { id: '1', email: 'test@test.com', role: 'user', groups: [], mfa_enabled: false, last_login: new Date().toISOString() }
+ { id: '1', email: 'test@test.com', groups: [], mfa_enabled: false, last_login: new Date().toISOString() }
];
userList.renderUsers(usersWithLogin as any);
@@ -866,7 +880,7 @@ describe('users/userList', () => {
it('should mark current user with "You" badge', () => {
const usersWithCurrent = [
- { id: 'current', email: 'me@test.com', role: 'user', groups: [], mfa_enabled: false }
+ { id: 'current', email: 'me@test.com', groups: [], mfa_enabled: false }
];
userList.renderUsers(usersWithCurrent as any);
@@ -877,7 +891,7 @@ describe('users/userList', () => {
it('should escape HTML in email', () => {
const usersWithXss = [
- { id: '1', email: '', role: 'user', groups: [], mfa_enabled: false }
+ { id: '1', email: '', groups: [], mfa_enabled: false }
];
userList.renderUsers(usersWithXss as any);
@@ -981,8 +995,8 @@ describe('users/userList', () => {
// ============================================================================
describe('users/userActions', () => {
const mockUsers = [
- { id: '1', email: 'user1@test.com', role: 'user', groups: [], mfa_enabled: false },
- { id: '2', email: 'user2@test.com', role: 'admin', groups: ['admins'], mfa_enabled: true }
+ { id: '1', email: 'user1@test.com', groups: [], mfa_enabled: false },
+ { id: '2', email: 'user2@test.com', groups: ['admins'], mfa_enabled: true }
];
// `allowed_accounts: []` matches the backend Group shape after the
@@ -1196,72 +1210,12 @@ describe('users/userActions', () => {
});
});
- describe('bulkChangeRole', () => {
- beforeEach(() => {
- userState.setAllUsers(mockUsers as any);
- (global.confirm as jest.Mock).mockReturnValue(true);
- });
-
- it('should update role for selected users', async () => {
- userState.addSelectedUserId('1');
- userState.addSelectedUserId('2');
-
- await userActions.bulkChangeRole('admin');
-
- expect(api.updateUser).toHaveBeenCalledWith('1', { role: 'admin' });
- expect(api.updateUser).toHaveBeenCalledWith('2', { role: 'admin' });
- });
-
- it('should not update when no users selected', async () => {
- await userActions.bulkChangeRole('admin');
-
- expect(api.updateUser).not.toHaveBeenCalled();
- });
-
- it('should show confirmation with count and role', async () => {
- userState.addSelectedUserId('1');
-
- await userActions.bulkChangeRole('admin');
-
- expect(global.confirm).toHaveBeenCalledWith(
- expect.stringContaining('admin')
- );
- });
-
- it('should not update when cancelled', async () => {
- (global.confirm as jest.Mock).mockReturnValue(false);
- userState.addSelectedUserId('1');
-
- await userActions.bulkChangeRole('admin');
-
- expect(api.updateUser).not.toHaveBeenCalled();
- });
-
- it('should clear selection after update', async () => {
- userState.addSelectedUserId('1');
-
- await userActions.bulkChangeRole('user');
-
- expect(userState.selectedUserIds.size).toBe(0);
- });
-
- it('should show success message', async () => {
- jest.useFakeTimers();
- userState.addSelectedUserId('1');
-
- await userActions.bulkChangeRole('admin');
-
- expect(document.querySelector('.toast-success')).toBeTruthy();
- jest.useRealTimers();
- });
-
- it('should handle update error', async () => {
- userState.addSelectedUserId('1');
- (api.updateUser as jest.Mock).mockRejectedValue(new Error('Update failed'));
-
- await userActions.bulkChangeRole('admin');
-
- expect(document.querySelector('.toast-error')).toBeTruthy();
+ describe('bulkChangeRole (removed in PR #912)', () => {
+ it('bulkChangeRole no longer exists on userActions', () => {
+ // PR #912 removed the role concept. bulkChangeRole was removed from
+ // userActions. Verify the export is absent so callers that relied
+ // on it are caught at compile/test time.
+ expect((userActions as any).bulkChangeRole).toBeUndefined();
});
});
@@ -1376,7 +1330,7 @@ describe('users/userModals', () => {
const mockUser = {
id: '1',
email: 'test@test.com',
- role: 'user',
+ // role removed: PR #912 -- authorization is group-membership based.
groups: ['users'],
mfa_enabled: false
};
@@ -1396,10 +1350,7 @@ describe('users/userModals', () => {
-
+
@@ -1520,7 +1471,7 @@ describe('users/userModals', () => {
expect((document.getElementById('user-id') as HTMLInputElement).value).toBe('1');
expect((document.getElementById('user-email') as HTMLInputElement).value).toBe('test@test.com');
- expect((document.getElementById('user-role') as HTMLSelectElement).value).toBe('user');
+ // role field removed in PR #912
});
it('should hide password field when editing', async () => {
@@ -1597,19 +1548,22 @@ describe('users/userModals', () => {
(document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com';
(document.getElementById('user-password') as HTMLInputElement).value = 'SecurePass123!';
- (document.getElementById('user-role') as HTMLSelectElement).value = 'user';
+
+ // PR #912: >= 1 group required. Select the first option populated by openCreateUserModal.
+ const gs7 = document.getElementById('user-groups') as HTMLSelectElement;
+ if (gs7.options.length > 0) gs7.options[0]!.selected = true;
const event = new Event('submit');
event.preventDefault = jest.fn();
await userModals.saveUser(event);
- expect(api.createUser).toHaveBeenCalledWith({
- email: 'new@test.com',
- password: 'SecurePass123!',
- role: 'user',
- groups: []
- });
+ expect(api.createUser).toHaveBeenCalledWith(
+ expect.objectContaining({
+ email: 'new@test.com',
+ password: 'SecurePass123!',
+ })
+ );
jest.useRealTimers();
});
@@ -1618,8 +1572,6 @@ describe('users/userModals', () => {
await userModals.openEditUserModal('1');
(document.getElementById('user-email') as HTMLInputElement).value = 'updated@test.com';
- (document.getElementById('user-role') as HTMLSelectElement).value = 'admin';
-
const event = new Event('submit');
event.preventDefault = jest.fn();
@@ -1627,7 +1579,7 @@ describe('users/userModals', () => {
expect(api.updateUser).toHaveBeenCalledWith('1', {
email: 'updated@test.com',
- role: 'admin',
+ // role removed: PR #912
groups: ['users']
});
jest.useRealTimers();
@@ -1663,13 +1615,17 @@ describe('users/userModals', () => {
it('should allow empty password for new user (invite flow, issue #348)', async () => {
// Empty password used to be a validation error; after issue #348
- // it triggers the invite flow — createUser is called with an
+ // it triggers the invite flow -- createUser is called with an
// empty password and the backend emails a set-password link.
userModals.openCreateUserModal();
(document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com';
(document.getElementById('user-password') as HTMLInputElement).value = '';
+ // PR #912: >= 1 group required. Select first option so validation passes.
+ const gs5 = document.getElementById('user-groups') as HTMLSelectElement;
+ if (gs5.options.length > 0) gs5.options[0]!.selected = true;
+
const event = new Event('submit');
event.preventDefault = jest.fn();
@@ -1683,6 +1639,26 @@ describe('users/userModals', () => {
expect(document.querySelector('.toast-error')).toBeNull();
});
+
+ it('should reject save when zero groups selected (PR #912 required-group validation)', async () => {
+ userModals.openCreateUserModal();
+
+ (document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com';
+ (document.getElementById('user-password') as HTMLInputElement).value = 'SecurePass123!';
+
+ // Ensure no groups are selected
+ const groupsSelect = document.getElementById('user-groups') as HTMLSelectElement;
+ Array.from(groupsSelect.options).forEach(o => { o.selected = false; });
+
+ const event = new Event('submit');
+ event.preventDefault = jest.fn();
+
+ await userModals.saveUser(event);
+
+ // Backend would reject with 400; frontend should show a clear validation message.
+ expect(api.createUser).not.toHaveBeenCalled();
+ expect(document.querySelector('.toast-error')).toBeTruthy();
+ });
it('should close modal after save', async () => {
jest.useFakeTimers();
userModals.openCreateUserModal();
@@ -1690,6 +1666,9 @@ describe('users/userModals', () => {
(document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com';
(document.getElementById('user-password') as HTMLInputElement).value = 'SecurePass123!';
+ const gs2 = document.getElementById('user-groups') as HTMLSelectElement;
+ if (gs2.options.length > 0) gs2.options[0]!.selected = true;
+
const event = new Event('submit');
event.preventDefault = jest.fn();
@@ -1707,6 +1686,9 @@ describe('users/userModals', () => {
(document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com';
(document.getElementById('user-password') as HTMLInputElement).value = 'SecurePass123!';
+ const gs3 = document.getElementById('user-groups') as HTMLSelectElement;
+ if (gs3.options.length > 0) gs3.options[0]!.selected = true;
+
const event = new Event('submit');
event.preventDefault = jest.fn();
@@ -1723,6 +1705,9 @@ describe('users/userModals', () => {
(document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com';
(document.getElementById('user-password') as HTMLInputElement).value = 'SecurePass123!';
+ const gs4 = document.getElementById('user-groups') as HTMLSelectElement;
+ if (gs4.options.length > 0) gs4.options[0]!.selected = true;
+
const event = new Event('submit');
event.preventDefault = jest.fn();
@@ -1753,6 +1738,11 @@ describe('users/userModals', () => {
(document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com';
(document.getElementById('user-password') as HTMLInputElement).value = 'SecurePass123!';
+ // PR #912: select a group so the required-group check passes and
+ // the error comes from the API call, not the validation.
+ const gs6 = document.getElementById('user-groups') as HTMLSelectElement;
+ if (gs6.options.length > 0) gs6.options[0]!.selected = true;
+
const event = new Event('submit');
event.preventDefault = jest.fn();
@@ -1943,51 +1933,19 @@ describe('users/handlers', () => {
expect(api.deleteUser).toHaveBeenCalled();
});
- it('should set up bulk role change handler', async () => {
+ it('bulk-role-btn is intentionally not wired (PR #912: role concept dropped)', async () => {
+ // bulkChangeRole was removed. The handler does not wire bulk-role-btn
+ // anymore. Clicking it must be a no-op rather than throwing.
userState.addSelectedUserId('1');
- (global.prompt as jest.Mock).mockReturnValue('admin');
-
userHandlers.setupUserHandlers();
-
const bulkRoleBtn = document.getElementById('bulk-role-btn');
- bulkRoleBtn?.click();
-
- // Wait for async operation
+ if (bulkRoleBtn) bulkRoleBtn.click();
await new Promise(resolve => setTimeout(resolve, 0));
-
- expect(api.updateUser).toHaveBeenCalledWith('1', { role: 'admin' });
- });
-
- it('should validate role input', async () => {
- userState.addSelectedUserId('1');
- (global.prompt as jest.Mock).mockReturnValue('invalid');
-
- userHandlers.setupUserHandlers();
-
- const bulkRoleBtn = document.getElementById('bulk-role-btn');
- bulkRoleBtn?.click();
-
- await new Promise(resolve => setTimeout(resolve, 0));
-
- expect(api.updateUser).not.toHaveBeenCalled();
- });
-
- it('should handle cancelled role prompt', async () => {
- userState.addSelectedUserId('1');
- (global.prompt as jest.Mock).mockReturnValue(null);
-
- userHandlers.setupUserHandlers();
-
- const bulkRoleBtn = document.getElementById('bulk-role-btn');
- bulkRoleBtn?.click();
-
- await new Promise(resolve => setTimeout(resolve, 0));
-
expect(api.updateUser).not.toHaveBeenCalled();
});
it('should set up bulk group handler', async () => {
- const mockUsers = [{ id: '1', email: 'test@test.com', role: 'user', groups: [], mfa_enabled: false }];
+ const mockUsers = [{ id: '1', email: 'test@test.com', groups: [], mfa_enabled: false }];
userState.setAllUsers(mockUsers as any);
userState.addSelectedUserId('1');
(global.prompt as jest.Mock).mockReturnValue('admins');
@@ -2023,8 +1981,8 @@ describe('users/handlers', () => {
// ============================================================================
describe('users module integration', () => {
const mockUsers = [
- { id: '1', email: 'admin@test.com', role: 'admin', groups: ['admins'], mfa_enabled: true, created_at: '2024-01-01' },
- { id: '2', email: 'user@test.com', role: 'user', groups: [], mfa_enabled: false, created_at: '2024-01-02' }
+ { id: '1', email: 'admin@test.com', groups: ['00000000-0000-5000-8000-000000000001'], mfa_enabled: true, created_at: '2024-01-01' },
+ { id: '2', email: 'user@test.com', groups: [], mfa_enabled: false, created_at: '2024-01-02' }
];
const mockGroups = [
diff --git a/frontend/src/__tests__/xss-purchase-status.test.ts b/frontend/src/__tests__/xss-purchase-status.test.ts
index f8bf92772..6b9c388b4 100644
--- a/frontend/src/__tests__/xss-purchase-status.test.ts
+++ b/frontend/src/__tests__/xss-purchase-status.test.ts
@@ -55,7 +55,7 @@ jest.mock('../state', () => ({
setCurrentAccountIDs: jest.fn(),
subscribeProvider: jest.fn().mockReturnValue(() => {}),
subscribeAccount: jest.fn().mockReturnValue(() => {}),
- getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', role: 'admin' }),
+ getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
}));
jest.mock('../history', () => ({
diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts
index 900627ba2..2b26c3d79 100644
--- a/frontend/src/api/types.ts
+++ b/frontend/src/api/types.ts
@@ -11,9 +11,13 @@ export type RampSchedule = 'immediate' | 'weekly-25pct' | 'monthly-10pct' | 'cus
export interface User {
id: string;
email: string;
- role: string;
+ // groups holds the UUIDs of the groups this user belongs to.
+ // Authorization is now derived entirely from group membership
+ // (PR #912 drops the role column). Admin status = member of
+ // Administrators group (00000000-0000-5000-8000-000000000001).
+ groups: string[];
// Whether two-factor authentication is enabled on this user
- // account. Optional for backward compatibility — older login
+ // account. Optional for backward compatibility -- older login
// responses may not include it. The profile/MFA section in
// auth.ts treats `mfa_enabled === true` (strict) as "enabled".
mfa_enabled?: boolean;
@@ -341,7 +345,9 @@ export interface PlannedPurchase {
export interface APIUser {
id: string;
email: string;
- role: string;
+ // role is intentionally absent: authorization is now purely
+ // group-membership based (PR #912). The groups array is the
+ // single source of truth for what a user can do.
groups: string[];
mfa_enabled: boolean;
created_at?: string;
@@ -352,8 +358,9 @@ export interface APIUser {
export interface CreateUserRequest {
email: string;
password: string;
- role: string;
- groups?: string[];
+ // groups is required (>= 1) by the backend (PR #912). Sending an
+ // empty array is rejected with 400.
+ groups: string[];
}
// CreateUserResponse extends APIUser with optional invite-delivery
@@ -370,7 +377,7 @@ export interface CreateUserResponse extends APIUser {
export interface UpdateUserRequest {
email?: string;
- role?: string;
+ // role is removed; update group membership to change authorization.
groups?: string[];
}
diff --git a/frontend/src/auth.ts b/frontend/src/auth.ts
index 1eb19428b..7b67d3d9a 100644
--- a/frontend/src/auth.ts
+++ b/frontend/src/auth.ts
@@ -913,12 +913,11 @@ export function updateUserUI(): void {
userEmailEl.parentNode?.replaceChild(freshEmailEl, userEmailEl);
freshEmailEl.addEventListener('click', () => void openProfileModal());
}
- // Show role badge for admin users. The parenthesised form was
- // visually indistinct from the email address (read as a domain
- // suffix); drop the parens and rely on the pill styling for
- // separation.
+ // Show admin badge when the user is a member of the Administrators
+ // group. PR #912 removed user.role from the API response; use the
+ // group-membership-based isAdmin() predicate instead.
if (roleEl) {
- if (currentUser.role === 'admin') {
+ if (permissionsIsAdmin()) {
roleEl.textContent = 'admin';
roleEl.classList.remove('hidden');
} else {
@@ -931,7 +930,7 @@ export function updateUserUI(): void {
userInfoEl.classList.remove('hidden');
}
- const adminOnly = currentUser.role === 'admin';
+ const adminOnly = permissionsIsAdmin();
document.querySelectorAll('.admin-only').forEach(el => {
el.classList.toggle('visible', adminOnly);
});
diff --git a/frontend/src/history.ts b/frontend/src/history.ts
index bc2d09d13..1d25f6bac 100644
--- a/frontend/src/history.ts
+++ b/frontend/src/history.ts
@@ -11,6 +11,7 @@ import { confirmDialog } from './confirmDialog';
import { buildApprovalDetailsBody } from './approval-details';
import { showToast } from './toast';
import { getCurrentUser } from './state';
+import { isAdmin } from './permissions';
import { showSkeletonRows, teardownSkeleton } from './lib/skeleton';
import { getAccountName } from './recommendations';
@@ -397,7 +398,7 @@ function canCancelPendingRow(p: HistoryPurchase): boolean {
if (status !== 'pending' && status !== 'notified') return false;
const user = getCurrentUser();
if (!user) return false;
- if (user.role === 'admin') return true;
+ if (isAdmin()) return true;
// Non-admin: only the original creator. Legacy rows with no
// created_by_user_id can't be cancelled via this UI; the email-token
// path remains the escape hatch.
@@ -427,7 +428,7 @@ function canApprovePendingRow(p: HistoryPurchase): boolean {
if (status !== 'pending' && status !== 'notified') return false;
const user = getCurrentUser();
if (!user) return false;
- if (user.role === 'admin') return true;
+ if (isAdmin()) return true;
if (!p.created_by_user_id) return false;
return p.created_by_user_id === user.id;
}
@@ -453,7 +454,7 @@ function canRetryFailedRow(p: HistoryPurchase): boolean {
if (p.retry_execution_id) return false; // already retried — user should act on the descendant
const user = getCurrentUser();
if (!user) return false;
- if (user.role === 'admin') return true;
+ if (isAdmin()) return true;
if (!p.created_by_user_id) return false;
return p.created_by_user_id === user.id;
}
diff --git a/frontend/src/index.html b/frontend/src/index.html
index 3e4f178a5..979905186 100644
--- a/frontend/src/index.html
+++ b/frontend/src/index.html
@@ -984,18 +984,10 @@
Create User
Leave blank to send the user an invitation email; they will set their own password on first login. If you enter a password it must be at least 12 characters and include upper / lower / number / special.
-
-
-
-
-
- Hold Ctrl/Cmd to select multiple groups
+ At least one group is required. Hold Ctrl/Cmd to select multiple groups. Authorization is determined entirely by group membership.
diff --git a/frontend/src/permissions.ts b/frontend/src/permissions.ts
index 61164cebb..d742a5794 100644
--- a/frontend/src/permissions.ts
+++ b/frontend/src/permissions.ts
@@ -1,39 +1,42 @@
/**
* Permission helper for CUDly frontend.
*
- * Issue #365: drives UI gating so a non-admin session never sees a
- * button whose only outcome is a backend 403 + "admin access required"
- * toast. The backend remains the security boundary (handlers still call
- * `requirePermission`); this helper is a UX-only gate.
+ * PR #912 removed the `role` column from users and sessions.
+ * Authorization is now purely group-membership based. The server
+ * derives every permission from the union of the groups a user
+ * belongs to via HasPermissionAPI; the frontend mirrors that by
+ * checking group membership for the UI-gating predicates below.
*
- * The role-to-default-permissions map lives in ./permissions.generated.ts,
- * which is regenerated from the backend constants in
- * internal/auth/types.go (DefaultAdminPermissions /
- * DefaultUserPermissions / DefaultReadOnlyPermissions) by
- * `go run ./cmd/gen-permissions`. A pre-commit hook + CI step re-runs the
- * generator and `git diff --exit-code` so the committed file never drifts
- * from the Go source of truth.
+ * Admin status = member of the Administrators group
+ * (UUID 00000000-0000-5000-8000-000000000001). That group carries
+ * the `admin:*` capability on the backend, which grants every action
+ * on every resource. The three built-in groups seeded by migration
+ * 000057 are:
*
- * The closed-union `Action` and `Resource` types below are hand-written
- * and mirror the `Action*` / `Resource*` constants in
- * internal/auth/types.go. They drift less often than the default sets
- * (most schema changes add a permission to a role, not a new top-level
- * constant), so we keep the compile-time safety they give callers and
- * accept the small bookkeeping cost when a new constant is added on the
- * Go side. The permissions.test.ts suite exercises the helpers; the
- * codegen check catches the data drift.
+ * Administrators 00000000-0000-5000-8000-000000000001 (admin:*)
+ * Standard Users 00000000-0000-5000-8000-000000000005
+ * Read-Only Users 00000000-0000-5000-8000-000000000006
*
- * Group-grant permissions are not folded in here. The current
- * `state.currentUser` carries only role; group memberships live in
- * `availableGroups` which is loaded only on the admin Users page (a
- * path readonly users can't reach). When a future enhancement adds a
- * `/me/permissions` round-trip, replace `getRolePermissions(role)`
- * with the server-provided permission set and keep the API the same.
+ * The closed-union Action and Resource types below are hand-written
+ * and mirror the Action* / Resource* constants in
+ * internal/auth/types.go.
+ *
+ * The frontend is a UX-only gate. The backend still enforces
+ * permissions on every request; a wrong-positive here surfaces as a
+ * 403 on click, and a wrong-negative just hides a button.
+ *
+ * The ADMIN_PERMS / USER_PERMS / READONLY_PERMS sets from
+ * permissions.generated.ts remain exported for the effective-
+ * permissions display in the admin Users page expand panel.
+ * They are not used for session gating anymore.
*/
import * as state from './state';
import { ADMIN_PERMS, USER_PERMS, READONLY_PERMS } from './permissions.generated';
+// Re-export so the user expand panel can still display them.
+export { ADMIN_PERMS, USER_PERMS, READONLY_PERMS };
+
// Action verbs. Closed enum so typos at call sites become compile
// errors. Mirrors the constants in internal/auth/types.go.
export type Action =
@@ -65,50 +68,63 @@ export type Resource =
| '*';
/**
- * Return the default permission set for the given role as a readonly
- * set of `${action}:${resource}` strings. Unknown roles return the
- * empty set (no permissions) so a typo in role assignment fails closed
- * rather than open.
+ * Well-known group UUID for the Administrators group seeded by
+ * migration 000057. Being a member of this group is the frontend
+ * equivalent of the backend's HasPermissionAPI(admin, *) == true.
*/
-export function getRolePermissions(role: string | undefined | null): ReadonlySet {
- switch (role) {
- case 'admin':
- return ADMIN_PERMS;
- case 'user':
- return USER_PERMS;
- case 'readonly':
- return READONLY_PERMS;
- default:
- return new Set();
- }
+export const ADMINISTRATORS_GROUP_ID = '00000000-0000-5000-8000-000000000001';
+
+/**
+ * Return true when the current session user is a member of the
+ * Administrators group. This replaces the former `user.role === "admin"`
+ * check that PR #912 removed from both the backend and the API response.
+ *
+ * A null user (logged out, pre-init race) returns false.
+ */
+export function isAdmin(): boolean {
+ const user = state.getCurrentUser();
+ if (!user) return false;
+ return Array.isArray(user.groups) && user.groups.includes(ADMINISTRATORS_GROUP_ID);
}
/**
- * Returns true when the current session's role grants the
- * `${action}:${resource}` permission.
+ * Returns true when the current session's group membership grants the
+ * specified permission.
*
- * Admin role short-circuits to true for every check (admin:* covers
- * every action/resource pair). A null user (logged out, or pre-init
- * race) returns false for everything.
+ * Administrators-group members pass every check (admin:* covers every
+ * action/resource pair). For all other groups a full /me/permissions
+ * round-trip is needed to resolve fine-grained permissions; that
+ * endpoint is not yet available, so non-admins return false here and
+ * the backend remains the authoritative gate.
*
- * UX-only gate. The backend still enforces; a wrong-positive here
- * surfaces as a 403 on click, and a wrong-negative just hides a
- * button.
+ * UX-only gate. A wrong-positive surfaces as a 403 on click; a
+ * wrong-negative just hides a button.
*/
export function canAccess(action: Action, resource: Resource): boolean {
+ // Suppress unused-variable warning -- action/resource are kept in
+ // the signature for forward-compatibility with the /me/permissions
+ // endpoint that will replace this stub.
+ void action; void resource;
const user = state.getCurrentUser();
if (!user) return false;
- const perms = getRolePermissions(user.role);
- if (perms.has('admin:*')) return true;
- return perms.has(`${action}:${resource}`);
+ return isAdmin();
}
/**
- * Convenience predicate for the legacy `.admin-only` toggle in
- * auth.ts:updateUserUI. Strictly equivalent to
- * `canAccess('admin', '*')` but spelled for readability at call sites
- * that pre-date the `canAccess` helper.
+ * Return the well-known permission set for a legacy role name. Kept
+ * for the effective-permissions display on the admin Users page, which
+ * shows what permissions the built-in role-mirror groups carry.
+ * No longer used for session gating.
*/
-export function isAdmin(): boolean {
- return canAccess('admin', '*');
+export function getRolePermissions(role: string | undefined | null): ReadonlySet {
+ switch (role) {
+ case 'admin':
+ return ADMIN_PERMS;
+ case 'user':
+ return USER_PERMS;
+ case 'readonly':
+ return READONLY_PERMS;
+ default:
+ return new Set();
+ }
}
diff --git a/frontend/src/users.ts b/frontend/src/users.ts
index a10512366..ec62628f5 100644
--- a/frontend/src/users.ts
+++ b/frontend/src/users.ts
@@ -20,7 +20,7 @@ export {
saveUser,
deleteUser,
bulkDeleteUsers,
- bulkChangeRole,
+ // bulkChangeRole removed in #912 (role concept dropped).
bulkAddToGroup,
setupUserHandlers
} from './users/index';
diff --git a/frontend/src/users/filters.ts b/frontend/src/users/filters.ts
index fbdb356de..08156a81e 100644
--- a/frontend/src/users/filters.ts
+++ b/frontend/src/users/filters.ts
@@ -2,6 +2,7 @@
* User filtering functionality
*/
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
import {
allUsers,
filteredUsers,
@@ -29,9 +30,13 @@ export function applyFilters(): void {
return false;
}
- // Role filter
- if (roleFilter && user.role !== roleFilter) {
- return false;
+ // Role filter: map legacy role values to group membership checks
+ // (PR #912 dropped user.role; admin = Administrators group member).
+ if (roleFilter) {
+ const isAdminUser = Array.isArray(user.groups) &&
+ user.groups.includes(ADMINISTRATORS_GROUP_ID);
+ if (roleFilter === 'admin' && !isAdminUser) return false;
+ if (roleFilter !== 'admin' && isAdminUser) return false;
}
// MFA filter
diff --git a/frontend/src/users/handlers.ts b/frontend/src/users/handlers.ts
index 421a1fb99..5a852f8b4 100644
--- a/frontend/src/users/handlers.ts
+++ b/frontend/src/users/handlers.ts
@@ -1,11 +1,16 @@
/**
* Event handlers setup for user management
+ *
+ * PR #912: bulkChangeRole removed (role concept dropped). The role
+ * filter UI element is kept wired for backward compatibility with
+ * any deployed HTML that may still have the element, but new builds
+ * will not render it.
*/
import { availableGroups } from './state';
import { handleUserSearch, handleFilterChange, clearFilters, updateGroupFilterDropdown } from './filters';
import { openCreateUserModal, closeUserModal, saveUser } from './userModals';
-import { bulkDeleteUsers, bulkChangeRole, bulkAddToGroup } from './userActions';
+import { bulkDeleteUsers, bulkAddToGroup } from './userActions';
/**
* Setup event handlers for user management
@@ -63,15 +68,9 @@ export function setupUserHandlers(): void {
bulkDeleteBtn.addEventListener('click', () => void bulkDeleteUsers());
}
- const bulkRoleBtn = document.getElementById('bulk-role-btn');
- if (bulkRoleBtn) {
- bulkRoleBtn.addEventListener('click', () => {
- const role = prompt('Enter new role (admin or user):');
- if (role && (role === 'admin' || role === 'user')) {
- void bulkChangeRole(role);
- }
- });
- }
+ // bulk-role-btn removed: bulkChangeRole no longer exists (PR #912).
+ // Any existing UI element for it is intentionally left unwired so
+ // it becomes a no-op rather than crashing.
const bulkGroupBtn = document.getElementById('bulk-group-btn');
if (bulkGroupBtn) {
diff --git a/frontend/src/users/index.ts b/frontend/src/users/index.ts
index 060c516ac..52d952af1 100644
--- a/frontend/src/users/index.ts
+++ b/frontend/src/users/index.ts
@@ -24,7 +24,8 @@ export { renderUsers, renderUserStats, updateBulkActionsBar } from './userList';
export { openCreateUserModal, openEditUserModal, closeUserModal, saveUser } from './userModals';
// Re-export user actions
-export { loadUsers, deleteUser, bulkDeleteUsers, bulkChangeRole, bulkAddToGroup } from './userActions';
+// bulkChangeRole removed in #912 (no role concept).
+export { loadUsers, deleteUser, bulkDeleteUsers, bulkAddToGroup } from './userActions';
// Re-export permission matrix
export { renderPermissionMatrix } from './permissionMatrix';
diff --git a/frontend/src/users/userActions.ts b/frontend/src/users/userActions.ts
index 8dad7a91c..aaacdadcf 100644
--- a/frontend/src/users/userActions.ts
+++ b/frontend/src/users/userActions.ts
@@ -57,8 +57,7 @@ function withCurrentUser(users: api.APIUser[]): api.APIUser[] {
const synthetic: api.APIUser = {
id: 'current',
email: current.email,
- role: current.role,
- groups: [],
+ groups: Array.isArray(current.groups) ? current.groups : [],
mfa_enabled: false,
};
return [synthetic, ...users];
@@ -156,33 +155,8 @@ export async function bulkDeleteUsers(): Promise {
}
}
-/**
- * Bulk change role
- */
-export async function bulkChangeRole(newRole: string): Promise {
- if (selectedUserIds.size === 0) return;
-
- const count = selectedUserIds.size;
- if (!confirm(`Change role to "${newRole}" for ${count} user(s)?`)) {
- return;
- }
-
- try {
- // Update users in parallel
- await Promise.all(
- Array.from(selectedUserIds).map(userId =>
- api.updateUser(userId, { role: newRole })
- )
- );
-
- clearSelectedUserIds();
- await loadUsers();
- showSuccess(`Successfully updated ${count} user(s)`);
- } catch (error) {
- console.error('Failed to update users:', error);
- showError('Failed to update some users');
- }
-}
+// bulkChangeRole removed in PR #912 frontend: role no longer exists.
+// Use the group multi-select in the Edit User modal to manage access.
/**
* Bulk add to group
diff --git a/frontend/src/users/userList.ts b/frontend/src/users/userList.ts
index 9c5c388aa..2ee3e1656 100644
--- a/frontend/src/users/userList.ts
+++ b/frontend/src/users/userList.ts
@@ -15,7 +15,7 @@ import {
} from './state';
import { escapeHtml, formatRelativeTime, formatDate, showSuccess, showError } from './utils';
import { openEditUserModal, deleteUser, loadUsers } from './userActions';
-import { getRolePermissions } from '../permissions';
+import { ADMINISTRATORS_GROUP_ID } from '../permissions';
/**
* Render user statistics
@@ -25,7 +25,10 @@ export function renderUserStats(): void {
if (!statsContainer) return;
const totalUsers = allUsers.length;
- const adminUsers = allUsers.filter(u => u.role === 'admin').length;
+ // Count administrators by group membership (PR #912: role dropped).
+ const adminUsers = allUsers.filter(u =>
+ Array.isArray(u.groups) && u.groups.includes(ADMINISTRATORS_GROUP_ID)
+ ).length;
const mfaEnabled = allUsers.filter(u => u.mfa_enabled).length;
const showing = filteredUsers.length;
@@ -72,7 +75,6 @@ export function renderUsers(users: APIUser[]): void {
@@ -141,18 +142,16 @@ function groupName(groupId: string): string {
}
/**
- * Compute the effective permissions for a user: union of role defaults and
- * all group permissions. Returns permissions as {action}:{resource} strings.
+ * Compute the effective permissions for a user from their group
+ * memberships. Returns permissions as {action}:{resource} strings.
*
- * Role defaults come from the shared permissions module so the badge here
- * and the global UI gates stay in lockstep (issue #365). Group-grant
- * permissions are layered on top here because this admin-only page is the
- * one place `availableGroups` is loaded.
+ * PR #912 removed user.role; permissions now derive purely from the
+ * union of the groups the user belongs to. availableGroups is loaded
+ * on this admin-only page so we can resolve group permissions here.
*/
function effectivePermissions(user: APIUser): string[] {
- const perms = new Set(getRolePermissions(user.role));
+ const perms = new Set();
- // Group permissions
user.groups.forEach(gid => {
const g = availableGroups.find(gr => gr.id === gid);
g?.permissions?.forEach(p => {
diff --git a/frontend/src/users/userModals.ts b/frontend/src/users/userModals.ts
index 400938aac..65b00aa8b 100644
--- a/frontend/src/users/userModals.ts
+++ b/frontend/src/users/userModals.ts
@@ -1,5 +1,10 @@
/**
* User modal functionality
+ *
+ * PR #912: role removed from the user API contract. Group membership
+ * is now required (>= 1 group) on create and update. The role selector
+ * has been replaced by a required group multi-select that mirrors the
+ * backend's cardinality >= 1 constraint.
*/
import * as api from '../api';
@@ -37,7 +42,7 @@ export function openCreateUserModal(): void {
(document.getElementById('user-password') as HTMLInputElement).required = false;
}
- // Populate groups dropdown
+ // Populate groups dropdown (no pre-selection for new user)
populateGroupsDropdown();
openModal(modal);
@@ -60,7 +65,7 @@ export async function openEditUserModal(userId: string): Promise {
title.textContent = 'Edit User';
(document.getElementById('user-id') as HTMLInputElement).value = user.id;
(document.getElementById('user-email') as HTMLInputElement).value = user.email;
- (document.getElementById('user-role') as HTMLSelectElement).value = user.role;
+ // role field no longer exists in the API response (PR #912).
// Hide password field for editing
const passwordFields = document.getElementById('password-fields');
@@ -69,7 +74,7 @@ export async function openEditUserModal(userId: string): Promise {
(document.getElementById('user-password') as HTMLInputElement).required = false;
}
- // Populate and select groups
+ // Populate and pre-select groups
populateGroupsDropdown(user.groups);
openModal(modal);
@@ -91,23 +96,34 @@ export function closeUserModal(): void {
}
/**
- * Save user (create or update)
+ * Save user (create or update).
+ *
+ * Groups are required (>= 1). The backend enforces this with a DB
+ * CHECK constraint (migration 000057); the frontend mirrors it with
+ * a pre-submit validation so the user gets a clear message rather
+ * than a generic 400 from the server.
*/
export async function saveUser(e: Event): Promise {
e.preventDefault();
const email = (document.getElementById('user-email') as HTMLInputElement).value;
const password = (document.getElementById('user-password') as HTMLInputElement).value;
- const role = (document.getElementById('user-role') as HTMLSelectElement).value;
+ // role selector removed: PR #912 drops the role column.
const groupsSelect = document.getElementById('user-groups') as HTMLSelectElement;
const selectedGroups = Array.from(groupsSelect.selectedOptions).map(opt => opt.value);
+ // Enforce >= 1 group on the client side so the user gets a clear
+ // validation message rather than a 400 from the backend.
+ if (selectedGroups.length === 0) {
+ showError('At least one group is required. Select one or more groups for this user.');
+ return;
+ }
+
try {
if (currentEditingUser) {
// Update existing user
await api.updateUser(currentEditingUser.id, {
email,
- role,
groups: selectedGroups
});
showSuccess('User updated successfully');
@@ -126,13 +142,12 @@ export async function saveUser(e: Event): Promise {
const result = await api.createUser({
email,
password,
- role,
groups: selectedGroups
});
// Three outcomes: password-up-front (no invite),
// password-omitted + invite delivered, password-omitted + invite
// send failed (user row exists but the recipient is unreachable
- // — surface a warning so the admin knows to re-mail the link via
+ // -- surface a warning so the admin knows to re-mail the link via
// Forgot Password).
if (password) {
showSuccess('User created successfully');
@@ -144,7 +159,7 @@ export async function saveUser(e: Event): Promise {
);
} else {
showSuccess(
- `Invitation email sent to ${email} — they will set their password on first login`
+ `Invitation email sent to ${email} -- they will set their password on first login`
);
}
}
@@ -159,15 +174,17 @@ export async function saveUser(e: Event): Promise {
}
/**
- * Populate groups dropdown
+ * Populate groups multi-select. Options are generated from the
+ * availableGroups list loaded in loadUsers(); preSelectedGroups holds
+ * the user's current group UUIDs (empty for new users).
*/
-function populateGroupsDropdown(selectedGroups: string[] = []): void {
+function populateGroupsDropdown(preSelectedGroups: string[] = []): void {
const groupsSelect = document.getElementById('user-groups') as HTMLSelectElement;
if (!groupsSelect) return;
groupsSelect.innerHTML = availableGroups
.map(group => `
-
`)