Update permission system and permission wall to the new system #289
Update permission system and permission wall to the new system #289georgelgeback wants to merge 1 commit into
Conversation
…t JWT embedded permissions
There was a problem hiding this comment.
Pull request overview
Updates the frontend permission model to use backend-fetched permissions (instead of decoding them from the JWT), and adapts permission-gated UI (permission wall + admin navigation) to handle loading/error states accordingly.
Changes:
- Add a React Query–backed
usePermissionsState/usePermissionsAPI to fetch/build aPermissionMapfrom the backend. - Update
PermissionWallto wait for permissions to load and show an error state on fetch failure. - Update admin UI (sidebar + member pages) to consume the new permissions hooks; reformat locale JSON and adjust permission-wall contact strings.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/auth.ts | Introduces permission-fetching hooks and builds PermissionMap from backend data; removes JWT-derived permissions. |
| src/components/PermissionWall.tsx | Uses backend permissions hook, adds loading/error handling, and adjusts permission-denied markup. |
| src/app/admin/AdminSidebar.tsx | Filters admin nav entries using backend permissions; adds loading skeleton and fails closed on error. |
| src/app/admin/members/page.tsx | Switches to new usePermissions() API for permission checks. |
| src/app/admin/members/MemberEditForm.tsx | Switches to new usePermissions() API for permission checks. |
| src/locales/en/main.json | Reformat + adjust permission-wall contact string punctuation. |
| src/locales/sv/main.json | Reformat + adjust permission-wall contact string punctuation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function buildPermissionMap( | ||
| permissions: [string, string][] | null | undefined, | ||
| ): PermissionMap { | ||
| const map = new PermissionMap(); | ||
| if (!permissions) return map; | ||
|
|
||
| function buildPermissionMap(token: BearerResponse | null): PermissionMap { | ||
| const map = new PermissionMap(); | ||
| if (!token) return map; | ||
| try { | ||
| const payload = JSON.parse( | ||
| Buffer.from(token.access_token.split(".")[1], "base64").toString(), | ||
| ) as { permissions: string[] }; | ||
| for (const [actionStr, targetStr] of permissions) { | ||
| const actionEnum = actionStr as ActionEnum; | ||
| const targetEnum = targetStr as TargetEnum; | ||
|
|
||
| for (const entry of payload.permissions) { | ||
| const parts = entry.split(":"); | ||
| if (parts.length !== 2) continue; | ||
| if (!actionEnum || !targetEnum) continue; | ||
|
|
There was a problem hiding this comment.
In buildPermissionMap, the as ActionEnum / as TargetEnum casts don’t validate the values coming from the backend, so the if (!actionEnum || !targetEnum) guard won’t actually filter out unexpected strings. Consider validating against Object.values(ActionEnum) / Object.values(TargetEnum) (or typing permissions as [ActionEnum, TargetEnum][] if that’s what the API returns) so invalid server data can’t silently populate the map with unknown targets/actions.
| export type UsePermissionsState = { | ||
| permissions: PermissionMap; | ||
| isLoading: boolean; | ||
| isError: boolean; | ||
| error: unknown; | ||
| }; |
There was a problem hiding this comment.
UsePermissionsState.error is typed as unknown, which loses the (default) Error type from TanStack Query and forces extra runtime checks downstream. Consider typing this as Error | null (or DefaultError | null) to keep type-safety and simplify consumers.
| <div className="flex items-center justify-center min-h-screen bg-background text-foreground"> | ||
| <section className="text-center"> | ||
| <h1 className="mb-4 text-7xl font-extrabold tracking-tight lg:text-9xl text-destructive"> | ||
| <div className="mb-4 text-7xl font-extrabold tracking-tight lg:text-9xl text-destructive"> | ||
| {t("permission-wall.stop")} | ||
| </h1> | ||
| <p className="mb-4 text-3xl font-bold md:text-4xl"> | ||
| </div> | ||
| <div className="mb-4 text-3xl font-bold md:text-4xl"> | ||
| {t("permission-wall.subtitle")} | ||
| </p> | ||
| <p className="mb-4 text-lg font-light text-muted-foreground"> | ||
| </div> | ||
| <div className="mb-4 text-lg font-light text-muted-foreground"> |
There was a problem hiding this comment.
The permission denied UI replaced semantic heading/paragraph elements (h1/p) with plain divs. This is an accessibility regression (screen readers and document outline lose structure). Consider keeping appropriate heading levels (e.g. h1 for the main “Stop” heading, p for body text).
| <div className="mb-4 text-lg font-light text-muted-foreground"> | ||
| {t("permission-wall.message")} | ||
| <Obfuscate email={"spindelman@fsektionen.se"}> | ||
| <p className="inline-flex text-forange hover:bg-primary hover:text-white"> | ||
| <div className="inline-flex text-forange hover:bg-primary hover:text-white"> | ||
| {t("permission-wall.contact")} | ||
| </p> | ||
| </div> | ||
| </Obfuscate> |
There was a problem hiding this comment.
Inside the sentence text, the email link is wrapped in a div (inline-flex). Using a block element in the middle of a sentence can create layout/whitespace issues and is semantically odd; a span (or letting Obfuscate render an anchor) would fit better here.
| if (isError) { | ||
| const errorMessage = | ||
| error instanceof Error ? error : "Failed to load permissions"; | ||
| return <LoadingErrorCard error={errorMessage} isLoading={false} />; |
There was a problem hiding this comment.
The fallback error string "Failed to load permissions" is hard-coded in English. Since this UI uses i18n elsewhere, consider using a translated string (or passing through the original error so LoadingErrorCard can render a localized fallback).
Fetches the current user's (or non-user's) permissions from the backend instead of using the JWT. The PR compiles without errors when the backend changes are used.
This PR needs to be pushed to prod right after the corresponding backend fixes: