feat(auth/frontend): migrate UI from roles to group-membership authorization#914
feat(auth/frontend): migrate UI from roles to group-membership authorization#914cristim wants to merge 2 commits into
Conversation
…ization Pairs with #912 (backend). The API no longer returns user.role; all authorization derives from group membership. Administrators group UUID (00000000-0000-5000-8000-000000000001) replaces the admin-role check. Key changes: - api/types.ts: User/APIUser drop role; CreateUserRequest groups required (>=1) - permissions.ts: isAdmin() checks Administrators group membership instead of role; canAccess() returns true only for Administrators-group members until the /me/permissions endpoint lands (deferred follow-up) - auth.ts, history.ts: replace user.role === 'admin' with isAdmin() - index.html: remove role selector from user modal; groups multi-select is now required (HTML required attr + JS validation: at least 1 group required) - users/userModals.ts: saveUser rejects zero-group submissions with a clear validation message mirroring the backend DB CHECK constraint - users/userList.ts: admin count uses ADMINISTRATORS_GROUP_ID membership; effectivePermissions derives from group permissions only (no role defaults) - users/handlers.ts, userActions.ts: remove bulkChangeRole (role concept gone) - users/filters.ts: role filter maps to Administrators group membership - All test fixtures updated from role strings to groups arrays - Permissions tests rewritten for group-based model with new isAdmin tests Frontend half of #907 (backend: #912). Both PRs must land together.
cristim
added
triaged
|
Warning Review limit reached
More reviews will be available in 19 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR removes user ChangesFrontend Authorization Migration: Role to Group-Based Access Control
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (10)
frontend/src/__tests__/plans-permissions.test.ts (1)
75-79: ⚡ Quick winImport the admin group constant instead of hardcoding the UUID.
The
mockUserhelper hardcodes the Administrators group UUID. ImportADMINISTRATORS_GROUP_IDfromfrontend/src/permissions.tsto maintain consistency with the authorization contract layer.♻️ Suggested refactor
+import { ADMINISTRATORS_GROUP_ID } from '../permissions'; + /** * Plans page permission-gating tests for issue `#365`.const mockUser = (role: string | null) => { (state.getCurrentUser as jest.Mock).mockReturnValue( - role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? ['00000000-0000-5000-8000-000000000001'] : [] }, + role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? [ADMINISTRATORS_GROUP_ID] : [] }, ); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/plans-permissions.test.ts` around lines 75 - 79, The test helper mockUser currently hardcodes the Administrators group UUID; update it to import and use ADMINISTRATORS_GROUP_ID from frontend/src/permissions.ts instead of the literal '00000000-0000-5000-8000-000000000001'. Modify the mockUser implementation that calls (state.getCurrentUser as jest.Mock).mockReturnValue(...) to set groups to role === 'admin' ? [ADMINISTRATORS_GROUP_ID] : [], and add the import for ADMINISTRATORS_GROUP_ID at the top of the test file.frontend/src/__tests__/recommendations-permissions.test.ts (1)
67-71: ⚡ Quick winImport the admin group constant instead of hardcoding the UUID.
The
mockUserhelper hardcodes the Administrators group UUID. ImportADMINISTRATORS_GROUP_IDfromfrontend/src/permissions.tsfor consistency.♻️ Suggested refactor
+import { ADMINISTRATORS_GROUP_ID } from '../permissions'; + /** * Recommendations permission gating tests.const mockUser = (role: string | null) => { (state.getCurrentUser as jest.Mock).mockReturnValue( - role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? ['00000000-0000-5000-8000-000000000001'] : [] }, + role === null ? null : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? [ADMINISTRATORS_GROUP_ID] : [] }, ); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/recommendations-permissions.test.ts` around lines 67 - 71, Replace the hardcoded Administrators UUID in the mockUser helper with the exported constant ADMINISTRATORS_GROUP_ID: import ADMINISTRATORS_GROUP_ID from the permissions module, then use ADMINISTRATORS_GROUP_ID in the groups array when role === 'admin' (the mockUser function that calls state.getCurrentUser should reference ADMINISTRATORS_GROUP_ID instead of the literal UUID).frontend/src/__tests__/state.test.ts (1)
37-37: ⚡ Quick winImport the admin group constant instead of hardcoding the UUID.
The Administrators group UUID
'00000000-0000-5000-8000-000000000001'is hardcoded here and appears in multiple test files. According to the layer context, this constant is defined asADMINISTRATORS_GROUP_IDinfrontend/src/permissions.ts. Import and use that constant to maintain a single source of truth and simplify future changes.♻️ Suggested refactor
+import { ADMINISTRATORS_GROUP_ID } from '../permissions'; + /** * State module tests */Then replace the hardcoded UUIDs:
- const user = { id: '123', email: 'test@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }; + const user = { id: '123', email: 'test@example.com', groups: [ADMINISTRATORS_GROUP_ID] }; setCurrentUser(user);- setCurrentUser({ id: '123', email: 'test@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }); + setCurrentUser({ id: '123', email: 'test@example.com', groups: [ADMINISTRATORS_GROUP_ID] }); setCurrentUser(null);Apply the same pattern to line 197.
Also applies to: 43-43, 197-197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/state.test.ts` at line 37, Replace hardcoded Administrators UUIDs with the shared constant: import ADMINISTRATORS_GROUP_ID from frontend/src/permissions.ts and use it in the test user/group arrays instead of the literal '00000000-0000-5000-8000-000000000001'; update every occurrence in this test file (the user constant and the other instance noted) so the tests reference ADMINISTRATORS_GROUP_ID rather than the hardcoded string.frontend/src/__tests__/plans.test.ts (1)
52-52: ⚡ Quick winImport the admin group constant instead of hardcoding the UUID.
The Administrators group UUID should be imported from
frontend/src/permissions.tsasADMINISTRATORS_GROUP_IDrather than hardcoded here. This maintains consistency with the authorization contract layer and simplifies future maintenance.♻️ Suggested refactor
Add the import at the top of the file:
+import { ADMINISTRATORS_GROUP_ID } from '../permissions'; + /** * Plans module tests */Then use the constant:
- getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }), + getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] }), }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/plans.test.ts` at line 52, Replace the hardcoded Administrators UUID in the getCurrentUser jest mock with the exported ADMINISTRATORS_GROUP_ID constant: add an import for ADMINISTRATORS_GROUP_ID from frontend/src/permissions.ts at the top of the test file and update the getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: [/* replace value */] }) to use ADMINISTRATORS_GROUP_ID in the groups array so the test references the shared authorization constant.frontend/src/__tests__/recommendations-enabled-providers.test.ts (1)
65-65: ⚡ Quick winImport the admin group constant instead of hardcoding the UUID.
Replace the hardcoded Administrators group UUID with
ADMINISTRATORS_GROUP_IDimported fromfrontend/src/permissions.tsto maintain consistency across the codebase.♻️ Suggested refactor
+import { ADMINISTRATORS_GROUP_ID } from '../permissions'; + /** * Issue `#463`: the Settings → General → Enabled Providers toggles must- getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }), + getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] }), }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/recommendations-enabled-providers.test.ts` at line 65, The test currently hardcodes the Administrators group UUID in the jest mock for getCurrentUser; replace the literal '00000000-0000-5000-8000-000000000001' with the exported ADMINISTRATORS_GROUP_ID imported from frontend/src/permissions.ts, and update the imports at the top of frontend/src/__tests__/recommendations-enabled-providers.test.ts to include ADMINISTRATORS_GROUP_ID so the getCurrentUser mock uses that constant.frontend/src/__tests__/history-approval-queue.test.ts (1)
71-71: ⚡ Quick winConsider importing ADMINISTRATORS_GROUP_ID constant instead of hardcoding the UUID.
The admin group UUID
'00000000-0000-5000-8000-000000000001'is hardcoded here and in multiple other test files. According to the review stack context,permissions.tsexports anADMINISTRATORS_GROUP_IDconstant. Importing and using that constant would improve maintainability.♻️ Proposed refactor
Add the import at the top of the file:
import { getCurrentUser } from '../state'; import { getAccountName } from '../recommendations'; +import { ADMINISTRATORS_GROUP_ID } from '../permissions';Then update the constant:
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }; +const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/history-approval-queue.test.ts` at line 71, Replace the hardcoded admin group UUID in the test's ADMIN_USER definition with the exported constant ADMINISTRATORS_GROUP_ID from permissions.ts: import ADMINISTRATORS_GROUP_ID at the top of the test file and use that symbol in the groups array for ADMIN_USER (referencing ADMIN_USER and ADMINISTRATORS_GROUP_ID to locate the change).frontend/src/__tests__/history-approve-button.test.ts (1)
67-68: ⚡ Quick winConsider importing ADMINISTRATORS_GROUP_ID constant instead of hardcoding the UUID.
The admin group UUID
'00000000-0000-5000-8000-000000000001'is hardcoded here and in multiple other test files. According to the review stack context,permissions.tsexports anADMINISTRATORS_GROUP_IDconstant. Importing and using that constant would improve maintainability.♻️ Proposed refactor
Add the import at the top of the file:
import { confirmDialog } from '../confirmDialog'; import { showToast } from '../toast'; import { getCurrentUser } from '../state'; +import { ADMINISTRATORS_GROUP_ID } from '../permissions';Then update the constant:
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }; +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: [] };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/history-approve-button.test.ts` around lines 67 - 68, Replace the hardcoded admin group UUID in the test by importing and using the ADMINISTRATORS_GROUP_ID constant from permissions.ts; update the ADMIN_USER definition (the symbol ADMIN_USER in this test) to include [ADMINISTRATORS_GROUP_ID] instead of the string literal, and ensure the import for ADMINISTRATORS_GROUP_ID is added at the top of the test file so other tests can reuse the same constant.frontend/src/__tests__/history-cancel-button.test.ts (1)
64-65: ⚡ Quick winConsider importing ADMINISTRATORS_GROUP_ID constant instead of hardcoding the UUID.
The admin group UUID
'00000000-0000-5000-8000-000000000001'is hardcoded here and in multiple other test files. According to the review stack context,permissions.tsexports anADMINISTRATORS_GROUP_IDconstant. Importing and using that constant would improve maintainability.♻️ Proposed refactor
Add the import at the top of the file:
import { confirmDialog } from '../confirmDialog'; import { showToast } from '../toast'; import { getCurrentUser } from '../state'; +import { ADMINISTRATORS_GROUP_ID } from '../permissions';Then update the constant:
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }; +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: [] };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/history-cancel-button.test.ts` around lines 64 - 65, Replace the hardcoded admin group UUID in the test user fixtures with the exported ADMINISTRATORS_GROUP_ID constant: add an import for ADMINISTRATORS_GROUP_ID from the permissions module at the top of the test file and update ADMIN_USER (and any similar occurrences in this file) to use ADMINISTRATORS_GROUP_ID in its groups array instead of the literal '00000000-0000-5000-8000-000000000001'; keep REG_USER unchanged.frontend/src/__tests__/history-retry-button.test.ts (1)
73-74: ⚡ Quick winConsider importing ADMINISTRATORS_GROUP_ID constant instead of hardcoding the UUID.
The admin group UUID
'00000000-0000-5000-8000-000000000001'is hardcoded here and in multiple other test files. According to the review stack context,permissions.tsexports anADMINISTRATORS_GROUP_IDconstant. Importing and using that constant would improve maintainability.♻️ Proposed refactor
Add the import at the top of the file:
import { confirmDialog } from '../confirmDialog'; import { showToast } from '../toast'; import { getCurrentUser } from '../state'; +import { ADMINISTRATORS_GROUP_ID } from '../permissions';Then update the constant:
-const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }; +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: [] };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/history-retry-button.test.ts` around lines 73 - 74, Replace the hardcoded admin group UUID in the test's ADMIN_USER definition with the exported ADMINISTRATORS_GROUP_ID constant from permissions.ts: add an import for ADMINISTRATORS_GROUP_ID at the top of the test file and use that symbol in the ADMIN_USER.groups array instead of the literal '00000000-0000-5000-8000-000000000001' to centralize the identifier and avoid duplication.frontend/src/__tests__/auth.test.ts (1)
404-408: ⚡ Quick winConsider importing ADMINISTRATORS_GROUP_ID constant instead of hardcoding the UUID.
The admin group UUID
'00000000-0000-5000-8000-000000000001'is hardcoded in multiple test files. According to the review stack context,permissions.tsexports anADMINISTRATORS_GROUP_IDconstant. Importing and using that constant would improve maintainability—if the UUID changes, tests will automatically stay in sync.♻️ Proposed refactor
At the top of the file, add the import:
import * as api from '../api'; import * as state from '../state'; +import { ADMINISTRATORS_GROUP_ID } from '../permissions';Then update the admin mock fixture:
(state.getCurrentUser as jest.Mock).mockReturnValue({ id: 'admin-1', email: 'admin@example.com', - groups: ['00000000-0000-5000-8000-000000000001'] + groups: [ADMINISTRATORS_GROUP_ID] });Apply the same pattern to all other admin fixture mocks in this file.
Also applies to: 417-421, 439-443, 454-458, 469-473, 483-487, 522-526
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/auth.test.ts` around lines 404 - 408, Replace hardcoded admin UUIDs in the test mocks with the exported ADMINISTRATORS_GROUP_ID constant: add an import for ADMINISTRATORS_GROUP_ID from the module that exports it (permissions.ts) at the top of the test file, then update the mocked user/group fixtures (the places where (state.getCurrentUser as jest.Mock).mockReturnValue or similar admin mock arrays are set) to use ADMINISTRATORS_GROUP_ID instead of the literal '00000000-0000-5000-8000-000000000001'; apply the same change to all other admin fixture mocks in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/__tests__/auth.test.ts`:
- Around line 404-408: Replace hardcoded admin UUIDs in the test mocks with the
exported ADMINISTRATORS_GROUP_ID constant: add an import for
ADMINISTRATORS_GROUP_ID from the module that exports it (permissions.ts) at the
top of the test file, then update the mocked user/group fixtures (the places
where (state.getCurrentUser as jest.Mock).mockReturnValue or similar admin mock
arrays are set) to use ADMINISTRATORS_GROUP_ID instead of the literal
'00000000-0000-5000-8000-000000000001'; apply the same change to all other admin
fixture mocks in this file.
In `@frontend/src/__tests__/history-approval-queue.test.ts`:
- Line 71: Replace the hardcoded admin group UUID in the test's ADMIN_USER
definition with the exported constant ADMINISTRATORS_GROUP_ID from
permissions.ts: import ADMINISTRATORS_GROUP_ID at the top of the test file and
use that symbol in the groups array for ADMIN_USER (referencing ADMIN_USER and
ADMINISTRATORS_GROUP_ID to locate the change).
In `@frontend/src/__tests__/history-approve-button.test.ts`:
- Around line 67-68: Replace the hardcoded admin group UUID in the test by
importing and using the ADMINISTRATORS_GROUP_ID constant from permissions.ts;
update the ADMIN_USER definition (the symbol ADMIN_USER in this test) to include
[ADMINISTRATORS_GROUP_ID] instead of the string literal, and ensure the import
for ADMINISTRATORS_GROUP_ID is added at the top of the test file so other tests
can reuse the same constant.
In `@frontend/src/__tests__/history-cancel-button.test.ts`:
- Around line 64-65: Replace the hardcoded admin group UUID in the test user
fixtures with the exported ADMINISTRATORS_GROUP_ID constant: add an import for
ADMINISTRATORS_GROUP_ID from the permissions module at the top of the test file
and update ADMIN_USER (and any similar occurrences in this file) to use
ADMINISTRATORS_GROUP_ID in its groups array instead of the literal
'00000000-0000-5000-8000-000000000001'; keep REG_USER unchanged.
In `@frontend/src/__tests__/history-retry-button.test.ts`:
- Around line 73-74: Replace the hardcoded admin group UUID in the test's
ADMIN_USER definition with the exported ADMINISTRATORS_GROUP_ID constant from
permissions.ts: add an import for ADMINISTRATORS_GROUP_ID at the top of the test
file and use that symbol in the ADMIN_USER.groups array instead of the literal
'00000000-0000-5000-8000-000000000001' to centralize the identifier and avoid
duplication.
In `@frontend/src/__tests__/plans-permissions.test.ts`:
- Around line 75-79: The test helper mockUser currently hardcodes the
Administrators group UUID; update it to import and use ADMINISTRATORS_GROUP_ID
from frontend/src/permissions.ts instead of the literal
'00000000-0000-5000-8000-000000000001'. Modify the mockUser implementation that
calls (state.getCurrentUser as jest.Mock).mockReturnValue(...) to set groups to
role === 'admin' ? [ADMINISTRATORS_GROUP_ID] : [], and add the import for
ADMINISTRATORS_GROUP_ID at the top of the test file.
In `@frontend/src/__tests__/plans.test.ts`:
- Line 52: Replace the hardcoded Administrators UUID in the getCurrentUser jest
mock with the exported ADMINISTRATORS_GROUP_ID constant: add an import for
ADMINISTRATORS_GROUP_ID from frontend/src/permissions.ts at the top of the test
file and update the getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin',
email: 'admin@example.com', groups: [/* replace value */] }) to use
ADMINISTRATORS_GROUP_ID in the groups array so the test references the shared
authorization constant.
In `@frontend/src/__tests__/recommendations-enabled-providers.test.ts`:
- Line 65: The test currently hardcodes the Administrators group UUID in the
jest mock for getCurrentUser; replace the literal
'00000000-0000-5000-8000-000000000001' with the exported ADMINISTRATORS_GROUP_ID
imported from frontend/src/permissions.ts, and update the imports at the top of
frontend/src/__tests__/recommendations-enabled-providers.test.ts to include
ADMINISTRATORS_GROUP_ID so the getCurrentUser mock uses that constant.
In `@frontend/src/__tests__/recommendations-permissions.test.ts`:
- Around line 67-71: Replace the hardcoded Administrators UUID in the mockUser
helper with the exported constant ADMINISTRATORS_GROUP_ID: import
ADMINISTRATORS_GROUP_ID from the permissions module, then use
ADMINISTRATORS_GROUP_ID in the groups array when role === 'admin' (the mockUser
function that calls state.getCurrentUser should reference
ADMINISTRATORS_GROUP_ID instead of the literal UUID).
In `@frontend/src/__tests__/state.test.ts`:
- Line 37: Replace hardcoded Administrators UUIDs with the shared constant:
import ADMINISTRATORS_GROUP_ID from frontend/src/permissions.ts and use it in
the test user/group arrays instead of the literal
'00000000-0000-5000-8000-000000000001'; update every occurrence in this test
file (the user constant and the other instance noted) so the tests reference
ADMINISTRATORS_GROUP_ID rather than the hardcoded string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5621bff8-99b7-41a4-a7c0-e781ac47e6dd
📒 Files selected for processing (36)
frontend/src/__tests__/a11y.test.tsfrontend/src/__tests__/allowed-accounts.test.tsfrontend/src/__tests__/api.test.tsfrontend/src/__tests__/auth-mfa-enroll.test.tsfrontend/src/__tests__/auth.test.tsfrontend/src/__tests__/groups.test.tsfrontend/src/__tests__/history-approval-queue.test.tsfrontend/src/__tests__/history-approve-button.test.tsfrontend/src/__tests__/history-cancel-button.test.tsfrontend/src/__tests__/history-retry-button.test.tsfrontend/src/__tests__/html.test.tsfrontend/src/__tests__/permissions.test.tsfrontend/src/__tests__/plans-permissions.test.tsfrontend/src/__tests__/plans-range-validation.test.tsfrontend/src/__tests__/plans.test.tsfrontend/src/__tests__/recommendations-enabled-providers.test.tsfrontend/src/__tests__/recommendations-permissions.test.tsfrontend/src/__tests__/recommendations.test.tsfrontend/src/__tests__/riexchange-permissions.test.tsfrontend/src/__tests__/riexchange.test.tsfrontend/src/__tests__/settings-permissions.test.tsfrontend/src/__tests__/state.test.tsfrontend/src/__tests__/users.test.tsfrontend/src/__tests__/xss-purchase-status.test.tsfrontend/src/api/types.tsfrontend/src/auth.tsfrontend/src/history.tsfrontend/src/index.htmlfrontend/src/permissions.tsfrontend/src/users.tsfrontend/src/users/filters.tsfrontend/src/users/handlers.tsfrontend/src/users/index.tsfrontend/src/users/userActions.tsfrontend/src/users/userList.tsfrontend/src/users/userModals.ts
…S_GROUP_ID constant Replace literal '00000000-0000-5000-8000-000000000001' with imported ADMINISTRATORS_GROUP_ID from permissions.ts in 8 test files where top-level imports can reach the factory; leave the literal (with an explanatory comment) in the 2 jest.mock() factories where permissions.ts cannot be required due to a circular import through the mocked state module (permissions.test.ts pins the UUID value as a regression guard). Fixes CodeRabbit nitpicks on PR #914.
|
Addressed all 10 CodeRabbit nitpicks from the 2026-06-01T20:19Z review (commit e2e9e8d). Findings fixed (8/10): Added
Findings kept as literal (2/10): All 65 test suites pass (2196 passed, 1 skipped). Build clean. @coderabbitai review |
|
Triggering the review now. ✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
Frontend half of #907 (backend: #912). Both PRs must land together -- #912 drops
user.rolefrom the API response and the current UI gates on it.user.role === 'admin'checks withisAdmin()(group-membership-based)isAdmin()now returns true when the current user belongs to the Administrators group (UUID00000000-0000-5000-8000-000000000001, seeded by migration 000057 in feat(auth): group-membership-only authorization, remove roles, require >=1 group (closes #907) #912)requiredattr, mirroring the backend DB CHECK constraint)bulkChangeRole(role concept no longer exists)api/types.ts:User,APIUserlose therolefield;CreateUserRequestmakesgroupsrequiredcanAccess()for non-admin users returnsfalsepending a future/me/permissionsendpoint (deferred); backend still enforces all permissionsWhat #912 changed on the backend
users.roleandsessions.rolecolumns dropped; authorization is purely group-derived viaHasPermissionAPIGET /api/auth/menow returnsgroups: string[]instead ofrole: stringPOST /api/usersandPUT /api/users/:idrequiregroupslength >= 1 (400 otherwise)Test plan
npm test(65 suites, 2196+ tests): all greennpm run build: compiles without errorsFrontend half of #907 (backend: #912)
🤖 Generated with claude-flow
Summary by CodeRabbit
Release Notes
New Features
Changes