geocrop-platform./apps/nextgen/.harness/changelogs/2026-07-17-p1-10-require-role.md

2026-07-17 — requireRole.js middleware + RBAC matrix tests (Sprint A1 / P1-10)

Branch: fix/p1-10-require-role Worktree: .worktrees/fix-p1-10-require-role

Why

Inline if (!['school_admin', 'systems_admin'].includes(req.user.role)) checks are scattered across 23+ controllers. Some are subtly inconsistent (one controller blocks teachers, another doesn't; one allows principal, another doesn't). P1-10 from the master readiness audit called for a single middleware helper + an executable form of the SCREENS.md permission matrix so we can catch a widening access regression in CI.

Changes

server/src/middleware/requireRole.js (new)

• requireRole(...allowed) — factory. Returns 401 if no req.user, 403 if role not in the allowed set, otherwise calls next().
• requireAdmin — systems_admin, school_admin, principal. Maps to the rows where SCREENS.md grants principal View access (e.g. /users, /departments).
• requireStrictAdmin — systems_admin, school_admin only. Maps to routes that intentionally exclude principal (audit logs, settings, academic rollover execution).
• requireStaff — admin/principal + hr/bursar/accountant/nurse/dining/ teacher/librarian/clubs_head. Excludes student and parent.
• requireOwnerOrAdmin — admin OR req.user.id === req.params.[userId|id|studentId], with explicit parent pass-through.
• ROLE — frozen object exposing every role string as a stable constant so callers don't repeat string literals.

server/tests/require-role.test.js (new, 15 specs)

Unit tests with no DB or Express:

• Allow / 403 / 401 / missing-role cells for requireRole.
• requireAdmin accepts admin+principal, rejects teacher/student/parent.
• requireStaff admits every staff + teacher + librarian + clubs_head, rejects student/parent.
• requireOwnerOrAdmin admits admins, the URL-param owner, parents; rejects unrelated students.
• ROLE is frozen.

server/tests/rbac-matrix.test.js (new, 152 specs)

Builds a tiny Express app that mounts the requireRole middleware with fake handlers, mocks req.user from an X-Test-Role header, then walks every (role, route) cell of the SCREENS.md matrix and asserts the expected HTTP status (200 / 403 / 401 for anonymous). 10 matrix rows × 13 roles + an extra negative-cell regression block = 152 specs. All green.

This is the executable form of SCREENS.md §Role Permissions Matrix.

Controller refactors (4 files)

• server/src/controllers/audit.controller.js — local adminOnly is now requireStrictAdmin re-aliased. Same predicate, one source.
• server/src/controllers/academic-rollover.controller.js — same.
• server/src/controllers/users.controller.js — local adminOnly is now requireAdmin (admin + principal, matches SCREENS.md matrix).
• server/src/controllers/payments.controller.js — the inline check on GET /api/payments/student/:studentId is replaced with requireOwnerOrAdmin mounted between auth and the handler. The parent pass-through behavior is preserved exactly as before (controller-level scope by parent-student mapping is unchanged).

These are demonstration refactors; the remaining 19 controllers with inline RBAC predicates are out of scope for this PR but should follow the same pattern over the next sprints. conventions.md will list the canonical middleware aliases so the sweep converges.

Verification

• npx vitest run tests/require-role.test.js → 15 passed (15 ms)
• npx vitest run tests/rbac-matrix.test.js → 152 passed (411 ms)
• npx vitest run (full suite) → 185 passed, 1 pre-existing failure in paynow-webhook.test.js ("paid_amount" undefined) caused by the dev DB lacking a student_fees row at id=1 in this worktree. The failure is independent of these changes — git diff against dev shows only controller refactors that do not affect that test's query.
• node --check clean on every modified file.

Out of scope (deferred)

• 19 remaining controllers with inline RBAC; sweep to be scheduled.
• conventions.md update with the alias table.
• Express-rate-limit on auth (left in place; was already covered by existing rate-limit middleware where enabled).