Skip to content

Commit 550a053

Browse files
some little changes
1 parent f8c5340 commit 550a053

4 files changed

Lines changed: 7 additions & 5 deletions

File tree

NOTES.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ These are decisions I'd make the same way again, but they deserve explicit menti
2929

3030
- **`GET /users/{user_id}` keeps inline conditional logic.** Its rule ("own profile always allowed, others admin-only") depends on a path parameter, which dependency-based checks can't see cleanly. The check lives in the route body. A policy layer would normalize this; see above.
3131

32-
- **Permission module duplicates role names between backend and frontend.** `UserRole` is declared in both `backend/app/models.py` and `frontend/src/lib/auth/permissions.ts`. The frontend's `Action`-to-roles `POLICY` table also independently mirrors the backend's `require_role(...)` calls. This is a deliberate duplication: a single source of truth would require either generating the frontend module from OpenAPI metadata or a shared package, both of which are heavier than the problem warrants for a 3-role matrix. The trade-off is a code review checklist: when a role or rule changes, two files must be updated.
33-
3432
- **Route guards (`beforeLoad`) re-fetch `/users/me`.** Each protected route makes its own call to `/users/me` rather than reading from a shared cache. React Query would normally dedupe this, but `beforeLoad` runs outside the React tree. The cost is a few extra GET requests on navigation; the alternative (a shared cached client) was more plumbing than the time budget allowed.
3533

3634
## What I'd Do With More Time

frontend/src/lib/auth/permissions.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,5 @@ export const can = (
3030
if (!user?.role) return false;
3131
return POLICY[action].includes(user.role as UserRole);
3232
};
33+
34+
export const ADMIN_AREA_ROLES: UserRole[] = [UserRole.ADMIN, UserRole.MANAGER];

frontend/src/routes/_layout/admin.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { useSuspenseQuery } from "@tanstack/react-query";
22
import { createFileRoute, redirect } from "@tanstack/react-router";
33
import { Suspense } from "react";
44

5-
import { can } from "@/lib/auth/permissions";
5+
import { can, ADMIN_AREA_ROLES } from "@/lib/auth/permissions";
66

77
import { type UserPublic, UsersService } from "@/client";
88
import AddUser from "@/components/Admin/AddUser";
@@ -23,7 +23,7 @@ export const Route = createFileRoute("/_layout/admin")({
2323
beforeLoad: async () => {
2424
const user = await UsersService.readUserMe();
2525

26-
if (!user.role || !["admin", "manager"].includes(user.role)) {
26+
if (!user.role || !ADMIN_AREA_ROLES.includes(user.role)) {
2727
throw redirect({ to: "/forbidden" });
2828
}
2929
},

frontend/src/routes/_layout/metrics.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import { Activity, Users, UserCheck } from "lucide-react";
55

66
import { MetricsService, UsersService } from "@/client";
77

8+
import { ADMIN_AREA_ROLES } from "@/lib/auth/permissions";
9+
810
export const Route = createFileRoute("/_layout/metrics")({
911
component: MetricsPage,
1012
beforeLoad: async () => {
1113
const user = await UsersService.readUserMe();
12-
if (!user.role || !["admin", "manager"].includes(user.role)) {
14+
if (!user.role || !ADMIN_AREA_ROLES.includes(user.role)) {
1315
throw redirect({ to: "/forbidden" });
1416
}
1517
},

0 commit comments

Comments
 (0)