280 lines
12 KiB
Markdown
280 lines
12 KiB
Markdown
|
|
# Implementation Plan: Group-Based Access Control
|
||
|
|
|
||
|
|
## Overview
|
||
|
|
|
||
|
|
Replace the existing role-based access control (admin/editor/viewer) with a four-group model (Admin, Standard_User, Leadership, Read_Only). This touches the database schema, backend middleware, all route authorization, frontend permission helpers, and the admin panel UI. Tasks build incrementally: migration first, then middleware, then routes, then frontend.
|
||
|
|
|
||
|
|
## Tasks
|
||
|
|
|
||
|
|
- [ ] 1. Create database migration for user groups
|
||
|
|
- [x] 1.1 Create `backend/migrations/add_user_groups.js` migration script
|
||
|
|
- Add `user_group` column (VARCHAR(20), NOT NULL, DEFAULT 'Read_Only') to users table
|
||
|
|
- Map existing role values: admin to Admin, editor to Standard_User, viewer to Read_Only
|
||
|
|
- Map NULL or unrecognized role values to Read_Only
|
||
|
|
- Add CHECK constraint: user_group IN ('Admin', 'Standard_User', 'Leadership', 'Read_Only')
|
||
|
|
- Add index `idx_users_user_group` on user_group column
|
||
|
|
- Use idempotent checks so migration is safe to run multiple times
|
||
|
|
- Follow existing migration pattern: open db, db.serialize(), log progress, close db
|
||
|
|
- _Requirements: 1.1, 1.2, 1.3, 1.4, 2.1, 2.2, 2.3, 2.4, 2.5_
|
||
|
|
|
||
|
|
- [ ]* 1.2 Write property test for migration role mapping
|
||
|
|
- **Property 8: Migration maps all role values correctly**
|
||
|
|
- Generate users with random roles from {admin, editor, viewer, NULL, arbitrary}, run migration against in-memory SQLite, verify mapping
|
||
|
|
- **Validates: Requirements 2.1, 2.2, 2.3, 2.5**
|
||
|
|
|
||
|
|
- [ ]* 1.3 Write property test for migration idempotency
|
||
|
|
- **Property 9: Migration is idempotent**
|
||
|
|
- Run migration N times (N in 1-5) against in-memory SQLite, verify schema and data identical each time
|
||
|
|
- **Validates: Requirements 2.4**
|
||
|
|
|
||
|
|
- [ ] 1.4 Write unit tests for migration
|
||
|
|
- Test column creation with correct CHECK constraint
|
||
|
|
- Test role mapping: admin to Admin, editor to Standard_User, viewer to Read_Only
|
||
|
|
- Test NULL and unrecognized role handling defaults to Read_Only
|
||
|
|
- Test new user defaults to Read_Only group
|
||
|
|
- _Requirements: 1.3, 1.4, 2.1, 2.2, 2.3, 2.5_
|
||
|
|
|
||
|
|
- [ ] 2. Update auth middleware to use groups
|
||
|
|
- [x] 2.1 Update `requireAuth` in `backend/middleware/auth.js`
|
||
|
|
- Modify session join query to SELECT user_group and attach as req.user.group
|
||
|
|
- _Requirements: 3.4_
|
||
|
|
|
||
|
|
- [x] 2.2 Add `requireGroup` middleware function
|
||
|
|
- Accept spread of allowed group names
|
||
|
|
- Return 401 if req.user is missing
|
||
|
|
- Return 403 with error details if user group not in allowed set
|
||
|
|
- Call next() if group is allowed
|
||
|
|
- _Requirements: 3.1, 3.2, 3.3_
|
||
|
|
|
||
|
|
- [x] 2.3 Remove `requireRole` and export `requireGroup`
|
||
|
|
- Remove requireRole function and its export
|
||
|
|
- Export requireGroup in its place
|
||
|
|
- _Requirements: 3.1_
|
||
|
|
|
||
|
|
- [ ]* 2.4 Write property test for group constraint
|
||
|
|
- **Property 1: Group constraint rejects invalid values**
|
||
|
|
- Generate random strings not in valid group set, attempt DB insert, verify constraint error
|
||
|
|
- **Validates: Requirements 1.1, 1.4**
|
||
|
|
|
||
|
|
- [ ]* 2.5 Write property test for requireGroup
|
||
|
|
- **Property 3: requireGroup rejects unauthorized groups**
|
||
|
|
- Generate random group and allowedGroups pairs where group is not in allowed set, verify 403
|
||
|
|
- **Validates: Requirements 3.3**
|
||
|
|
|
||
|
|
- [ ] 2.6 Write unit tests for requireGroup middleware
|
||
|
|
- Test 401 for unauthenticated requests
|
||
|
|
- Test 403 for wrong group
|
||
|
|
- Test group attached to req.user
|
||
|
|
- Test next() called for allowed group
|
||
|
|
- _Requirements: 3.2, 3.3, 3.4_
|
||
|
|
|
||
|
|
- [x] 3. Checkpoint: Verify migration and middleware
|
||
|
|
- Ensure all tests pass, ask the user if questions arise.
|
||
|
|
|
||
|
|
- [ ] 4. Update auth routes to return group
|
||
|
|
- [x] 4.1 Update login endpoint in `backend/routes/auth.js`
|
||
|
|
- Return group (from user_group) instead of role in user response object
|
||
|
|
- Update audit log details to log group instead of role
|
||
|
|
- _Requirements: 3.4, 9.2_
|
||
|
|
|
||
|
|
- [x] 4.2 Update me endpoint in `backend/routes/auth.js`
|
||
|
|
- Return group instead of role in user response object
|
||
|
|
- _Requirements: 3.4_
|
||
|
|
|
||
|
|
- [ ] 5. Update user management routes
|
||
|
|
- [x] 5.1 Switch `backend/routes/users.js` to use requireGroup
|
||
|
|
- Replace requireRole('admin') with requireGroup('Admin')
|
||
|
|
- _Requirements: 4.2, 4.3_
|
||
|
|
|
||
|
|
- [x] 5.2 Update GET endpoints to return user_group
|
||
|
|
- Return user_group instead of role in user records
|
||
|
|
- _Requirements: 8.1_
|
||
|
|
|
||
|
|
- [x] 5.3 Update POST create user to accept group param
|
||
|
|
- Validate group against valid values
|
||
|
|
- Default to Read_Only if not provided
|
||
|
|
- Return 400 for invalid group values
|
||
|
|
- _Requirements: 1.3, 8.2_
|
||
|
|
|
||
|
|
- [x] 5.4 Update PATCH update user to accept group param
|
||
|
|
- Validate group against valid values
|
||
|
|
- Prevent admin self-demotion (return 400)
|
||
|
|
- _Requirements: 8.2, 8.5_
|
||
|
|
|
||
|
|
- [x] 5.5 Add audit logging for group changes
|
||
|
|
- Log acting user ID, target user ID, previous group, new group, IP address, timestamp
|
||
|
|
- _Requirements: 9.1, 9.3_
|
||
|
|
|
||
|
|
- [ ]* 5.6 Write property test for user group validity
|
||
|
|
- **Property 2: Every user has exactly one valid group**
|
||
|
|
- Generate random user sets, query all users, verify each has exactly one valid group
|
||
|
|
- **Validates: Requirements 1.2**
|
||
|
|
|
||
|
|
- [ ] 5.7 Write unit tests for user management group logic
|
||
|
|
- Test group validation rejects invalid values
|
||
|
|
- Test self-demotion prevention
|
||
|
|
- Test audit logging includes all required fields
|
||
|
|
- _Requirements: 8.2, 8.5, 9.1, 9.3_
|
||
|
|
|
||
|
|
- [ ] 6. Update backend route authorization across all routes
|
||
|
|
- [x] 6.1 Update `backend/routes/auditLog.js`
|
||
|
|
- Replace requireRole('admin') with requireGroup('Admin')
|
||
|
|
- _Requirements: 4.2_
|
||
|
|
|
||
|
|
- [x] 6.2 Update `backend/routes/archerTickets.js`
|
||
|
|
- Use requireGroup('Admin', 'Standard_User') for create, update, delete
|
||
|
|
- _Requirements: 5.2_
|
||
|
|
|
||
|
|
- [x] 6.3 Update `backend/routes/knowledgeBase.js`
|
||
|
|
- Use requireGroup('Admin', 'Standard_User') for upload and delete
|
||
|
|
- _Requirements: 5.2_
|
||
|
|
|
||
|
|
- [x] 6.4 Update `backend/routes/ivantiFindings.js`
|
||
|
|
- Use requireGroup('Admin', 'Standard_User') for override endpoint
|
||
|
|
- _Requirements: 5.2_
|
||
|
|
|
||
|
|
- [x] 6.5 Update `backend/routes/compliance.js`
|
||
|
|
- Use requireGroup('Admin', 'Standard_User') for preview and commit
|
||
|
|
- _Requirements: 5.2_
|
||
|
|
|
||
|
|
- [x] 6.6 Update `backend/server.js` inline CVE routes
|
||
|
|
- Use requireGroup('Admin', 'Standard_User') for POST, PUT, PATCH, DELETE
|
||
|
|
- _Requirements: 5.2_
|
||
|
|
|
||
|
|
- [x] 6.7 Update `backend/server.js` route mounting
|
||
|
|
- Pass requireGroup instead of requireRole to route factories
|
||
|
|
- _Requirements: 3.1_
|
||
|
|
|
||
|
|
- [ ]* 6.8 Write property test for Leadership restrictions
|
||
|
|
- **Property 5: Leadership cannot mutate any resource**
|
||
|
|
- Generate random mutation requests as Leadership, verify 403
|
||
|
|
- **Validates: Requirements 6.3**
|
||
|
|
|
||
|
|
- [ ]* 6.9 Write property test for Read_Only restrictions
|
||
|
|
- **Property 6: Read_Only cannot mutate or export**
|
||
|
|
- Generate random mutation and export requests as Read_Only, verify 403
|
||
|
|
- **Validates: Requirements 7.2, 7.3**
|
||
|
|
|
||
|
|
- [x] 7. Checkpoint: Verify backend route authorization
|
||
|
|
- Ensure all tests pass, ask the user if questions arise.
|
||
|
|
|
||
|
|
- [ ] 8. Implement Standard User conditional delete logic
|
||
|
|
- [x] 8.1 Add created_by column tracking
|
||
|
|
- Add created_by to CVE, finding, and ticket creation endpoints storing req.user.id on insert
|
||
|
|
- _Requirements: 3.5_
|
||
|
|
|
||
|
|
- [x] 8.2 Implement ownership check for CVE delete
|
||
|
|
- Standard_User can only delete CVEs they created
|
||
|
|
- Return 403 if not owner
|
||
|
|
- _Requirements: 3.5_
|
||
|
|
|
||
|
|
- [x] 8.3 Implement cascade impact check for CVE delete
|
||
|
|
- Query associated Archer tickets and documents
|
||
|
|
- Check compliance linkage on cascaded tickets
|
||
|
|
- Return cascade_impact response schema
|
||
|
|
- Block deletion if any cascaded ticket is compliance-linked
|
||
|
|
- _Requirements: 3.8, 3.9_
|
||
|
|
|
||
|
|
- [x] 8.4 Implement state check for finding delete
|
||
|
|
- Standard_User cannot delete resolved or closed findings
|
||
|
|
- Return 403 with appropriate error message
|
||
|
|
- _Requirements: 3.6_
|
||
|
|
|
||
|
|
- [x] 8.5 Implement compliance linkage check for ticket delete
|
||
|
|
- Standard_User cannot delete tickets linked to compliance reports
|
||
|
|
- Return 403 with appropriate error message
|
||
|
|
- _Requirements: 3.7_
|
||
|
|
|
||
|
|
- [x] 8.6 Ensure Admin bypasses all delete restrictions
|
||
|
|
- Admin group skips ownership, state, and compliance checks
|
||
|
|
- _Requirements: 3.10, 4.5_
|
||
|
|
|
||
|
|
- [ ]* 8.7 Write property test for Admin delete bypass
|
||
|
|
- **Property 4: Admin bypasses all delete restrictions**
|
||
|
|
- Generate resources with random ownership, state, compliance linkage, delete as Admin, verify success
|
||
|
|
- **Validates: Requirements 3.10, 4.1, 4.5**
|
||
|
|
|
||
|
|
- [ ] 8.8 Write unit tests for conditional delete logic
|
||
|
|
- Test ownership rejection for non-owner
|
||
|
|
- Test state rejection for resolved/closed findings
|
||
|
|
- Test compliance linkage rejection
|
||
|
|
- Test cascade impact response format
|
||
|
|
- Test Admin bypass of all restrictions
|
||
|
|
- _Requirements: 3.5, 3.6, 3.7, 3.8, 3.9, 3.10_
|
||
|
|
|
||
|
|
- [x] 9. Checkpoint: Verify conditional delete logic
|
||
|
|
- Ensure all tests pass, ask the user if questions arise.
|
||
|
|
|
||
|
|
- [ ] 10. Update frontend AuthContext with group helpers
|
||
|
|
- [x] 10.1 Update `frontend/src/contexts/AuthContext.js`
|
||
|
|
- Read group from user object instead of role
|
||
|
|
- Replace hasRole with isInGroup(...groups) helper
|
||
|
|
- Update canWrite to check isInGroup('Admin', 'Standard_User')
|
||
|
|
- Add canDelete(resource) helper: Admin always true, Standard_User only if owns resource, others false
|
||
|
|
- Add canExport() helper: true for Admin, Standard_User, Leadership
|
||
|
|
- Update isAdmin() to check isInGroup('Admin')
|
||
|
|
- _Requirements: 10.1, 10.2, 10.3, 10.4, 10.5_
|
||
|
|
|
||
|
|
- [ ]* 10.2 Write property test for permission helpers
|
||
|
|
- **Property 7: Group permission helpers are consistent with group matrix**
|
||
|
|
- Generate all valid group values, call each helper, verify against permission matrix
|
||
|
|
- **Validates: Requirements 10.5**
|
||
|
|
|
||
|
|
- [ ] 11. Update frontend UI for group-based rendering
|
||
|
|
- [x] 11.1 Update `App.js` conditional rendering
|
||
|
|
- Use canWrite, canDelete, canExport, isAdmin for button and link visibility
|
||
|
|
- _Requirements: 10.1, 10.2, 10.3_
|
||
|
|
|
||
|
|
- [x] 11.2 Update `NavDrawer.js`
|
||
|
|
- Show admin panel link only when isAdmin() is true
|
||
|
|
- _Requirements: 10.3_
|
||
|
|
|
||
|
|
- [x] 11.3 Update `UserMenu.js`
|
||
|
|
- Display user group instead of role
|
||
|
|
- _Requirements: 10.1_
|
||
|
|
|
||
|
|
- [x] 11.4 Update all components using hasRole or canWrite
|
||
|
|
- Replace with new group-based helpers throughout components
|
||
|
|
- _Requirements: 10.5_
|
||
|
|
|
||
|
|
- [x] 11.5 Hide delete buttons for non-owned resources
|
||
|
|
- Standard_User sees delete only on resources they created
|
||
|
|
- _Requirements: 10.4_
|
||
|
|
|
||
|
|
- [ ] 12. Update User Management UI
|
||
|
|
- [x] 12.1 Replace role dropdown with group dropdown in `UserManagement.js`
|
||
|
|
- Options: Admin, Standard_User, Leadership, Read_Only
|
||
|
|
- _Requirements: 8.1, 8.2_
|
||
|
|
|
||
|
|
- [x] 12.2 Update form data and API calls to use group field
|
||
|
|
- Send group instead of role in create and update requests
|
||
|
|
- _Requirements: 8.2_
|
||
|
|
|
||
|
|
- [x] 12.3 Add confirmation dialog for group changes
|
||
|
|
- Show confirmation before applying any group change
|
||
|
|
- _Requirements: 8.3_
|
||
|
|
|
||
|
|
- [x] 12.4 Add extra warning when downgrading Admin
|
||
|
|
- Show additional warning in confirmation dialog
|
||
|
|
- _Requirements: 8.4_
|
||
|
|
|
||
|
|
- [x] 12.5 Prevent admin self-demotion in UI
|
||
|
|
- Disable group change dropdown for current user if Admin
|
||
|
|
- _Requirements: 8.5_
|
||
|
|
|
||
|
|
- [x] 12.6 Update user table to show group badges
|
||
|
|
- Display group badge with appropriate colors instead of role badge
|
||
|
|
- _Requirements: 8.1_
|
||
|
|
|
||
|
|
- [x] 13. Final checkpoint: Verify full integration
|
||
|
|
- Ensure all tests pass, ask the user if questions arise.
|
||
|
|
|
||
|
|
## Notes
|
||
|
|
|
||
|
|
- Tasks marked with `*` are optional and can be skipped for faster MVP
|
||
|
|
- Each task references specific requirements for traceability
|
||
|
|
- Checkpoints ensure incremental validation
|
||
|
|
- Property tests use `fast-check` library with minimum 100 iterations per test
|
||
|
|
- All backend code uses callback-based SQLite API wrapped in promises (matching existing patterns)
|
||
|
|
- All frontend code uses plain JavaScript (no TypeScript)
|