[fix] Align frontend RBAC with the backend permission model - #406
Conversation
New Event is now visible only to Creator and above, Reviews only to Operator and above, Creators can edit only their own pending events, and the Extract button is limited to incident types.
|
@ecosquad-autoreview review |
There was a problem hiding this comment.
Review of PR #406
The role-gating changes look correct overall: CanCreate (Creator/Operator/Admin) is applied consistently to the New Event nav entries (desktop + mobile) and the NewEvent page, the Reviews nav entry is now gated on CanApprove in both navs, the extract button is restricted to incident types, and the edit button now requires ownership for plain creators. CI checks are green (CodeQL, GitGuardian, check). A few things to verify/clean up:
Warning
-
Ownership check may silently fail —
src/Components/Event/EventCard.tsx:61-62roles.has(Roles.Creators) && currentUserId === Event.Creator
currentUserIdis the JWTsub(src/Components/Auth/With.tsx:96-98), andEvent.Creatorcomes fromevent.creator(src/Services/Status.Trans.V2.ts:191, optionalstring). If the backend stores anything other than the exact JWTsub(e.g. an email address or a different Zitadel ID), the comparison is always false and creators silently lose the Edit button on their own pending events — with no visible error. Please confirm thecreatorfield is populated with thesubclaim on create/update; if not, map it at the source rather than comparing here. -
Client-side only enforcement —
src/Components/Event/EventCard.tsx:59-65,src/Pages/NewEvent.tsx:13
Hiding the Edit/New Event UI restricts only the UI. If the underlying create/update API does not itself enforce creator ownership and role, a user can still call it directly. This PR description doesn't mention a server-side counterpart — please confirm the backend already enforces these same rules.
Suggestion
-
Redundant nested
Authorized—src/Components/Layout/MobileMenu.tsx:67-72
The newAuthorized rules={CanCreate}is nested inside a plain<Authorized>, and the parent block already requires authentication. Collapse to a single<Authorized rules={CanCreate}>around the New Event item (keep the outer one only for Reviews/Logout) for consistency withTopNavBar.tsx:43-45. -
Extract button behavior change —
src/Components/Event/EventCard.tsx:53-56
Switching fromEvent.Type !== EventType.MaintenancetoIsIncident(Event.Type)also removes the Extract button forOperationalandInformationevents, not just Maintenance. The PR description says "limited to incident types instead of all non-maintenance types," so this looks intentional, but double-check thatOperational/Informationevents with >1 region service never needed extraction.
Notes (no action needed)
NewForm.tsx:35already limits creators toMaintenanceonly, which matches the newCanCreategating — consistent.Reviewspage (src/Pages/Reviews.tsx:101) was already gated onCanApprove; the nav change just aligns the entry point with it.- Dependency bumps (
vite,typescript-eslint,@types/node) are patch/minor and lockfile is consistent.
Changes
New helpers in With.tsx
Verification