feat(admin): backoffice access control (isAdmin role)#3187
Merged
LucasCharrier merged 14 commits intoalphafrom Apr 13, 2026
Merged
feat(admin): backoffice access control (isAdmin role)#3187LucasCharrier merged 14 commits intoalphafrom
LucasCharrier merged 14 commits intoalphafrom
Conversation
Adds an is_admin boolean flag on app_user to support the backoffice access control (issue #3178).
Introduce a server-side ADMIN_EMAILS env var (comma-separated). On each login, the NextAuth jwt callback syncs app_user.is_admin with that list (promotion and demotion), and propagates the flag into the JWT and session. Refactor the dbUser block to remove non-null assertions.
Restricts procedures to users with session.user.isAdmin === true and throws FORBIDDEN otherwise.
Edge middleware checks the JWT isAdmin flag on /admin/*. The admin layout re-checks the session on the Node runtime (defense in depth) and redirects non-admin users to /mon-espace. A minimal AdminHomePage is rendered for admin users.
revu-bot
reviewed
Apr 8, 2026
Collaborator
revu-bot
left a comment
There was a problem hiding this comment.
Admin Access Control — Review Summary
The PR implements a solid, defense-in-depth admin access control system: DB column + env-var promotion, JWT caching, edge middleware, layout guard, tRPC procedure, and tests. The overall architecture is sound. A few issues need attention before merging.
| File | Lines | Severity | Issue |
|---|---|---|---|
src/server/auth/config.ts |
216–238 | CRITICAL | Admin flag synced only on login — new users get isAdmin: false in JWT even if in ADMIN_EMAILS |
src/middleware.ts |
26–28 | CRITICAL | Unauthenticated non-admin redirected to /mon-espace instead of /login |
src/server/auth/config.ts |
36–42 | IMPORTANT | getAdminEmails() re-parses env var on every login — should be memoized |
src/e2e/admin.e2e.ts |
1–9 | IMPORTANT | E2E test doesn't set up an authenticated session — will always hit the unauthenticated redirect path |
src/app/admin/page.tsx |
11–13 | MINOR | Unauthenticated user redirected to /mon-espace instead of /login |
revu-bot
reviewed
Apr 8, 2026
revu-bot
reviewed
Apr 8, 2026
revu-bot
reviewed
Apr 8, 2026
revu-bot
reviewed
Apr 8, 2026
- middleware: treat a missing token.isAdmin as 'old token' and force re-login so the flag gets populated (otherwise existing users with a pre-PR JWT would be bounced to /mon-espace for up to 30 days). - auth/config: memoize the ADMIN_EMAILS set at module load instead of reparsing the env var on every sign-in. - admin/page: split the defense-in-depth check to redirect to /login when there is no session (avoids a redirect chain).
Adds unit tests for src/middleware.ts (4 cases: no token, missing isAdmin, non-admin, admin) and src/app/admin/page.tsx (4 cases: no session, non-admin, admin, fallback to email when name is null) to lift coverage on the new code.
Viczei
reviewed
Apr 8, 2026
Viczei
reviewed
Apr 8, 2026
maxgfr
reviewed
Apr 8, 2026
The DB column was redundant: ADMIN_EMAILS is already the sole source of truth, re-evaluated on every sign-in. Remove the migration, the schema column, and the DB sync in the jwt callback. token.isAdmin is now computed directly from the memoized env var set. Promotion/demotion still takes effect at next login (same as before) but with zero persistent state to manage.
…edback - Extract the ADMIN_EMAILS parsing into a pure parseAdminEmails helper with 9 unit tests (edge cases: null/undefined, empty, whitespace, casing, trailing commas, dedup). Addresses maxgfr's review. - Remove the redundant defense-in-depth check in app/admin/page.tsx: the edge middleware + layout already guard the route. Addresses Viczei's review. - Wire ADMIN_EMAILS via a new optional 'admin' configmap on dev and preprod environments with test@fia1.fr preset, so QA and the E2E auth.setup user get backoffice access on non-prod. Prod has no configmap and must set ADMIN_EMAILS explicitly. Addresses Viczei's review.
2 tasks
2 tasks
Viczei
reviewed
Apr 9, 2026
| * Comma-separated list of emails that should be granted the admin role | ||
| * on login. The flag is then persisted in the `app_user.is_admin` column. | ||
| */ | ||
| ADMIN_EMAILS: z.string().optional().default(""), |
Contributor
There was a problem hiding this comment.
je pense pas que l'on devrait laisser cette valeur en optional. Il vaut mieux que l'on ait toujours au moins un compte admin
Viczei
approved these changes
Apr 9, 2026
Address Viczei's review: ADMIN_EMAILS should never be empty — there must always be at least one admin account. Changed from optional with empty default to z.string().min(1). Added prod SealedSecret and wired both configMapRef + secretRef in values.yaml.
The E2E workflow runs the app without SKIP_ENV_VALIDATION, so it needs the now-required ADMIN_EMAILS variable.
The CI test user is in ADMIN_EMAILS, so the test now verifies that an admin can access /admin and sees the backoffice page. The non-admin redirect path is covered by unit tests.
|
🎉 Deployment for commit 19bfe27 : Docker images
|
LucasCharrier
added a commit
that referenced
this pull request
Apr 13, 2026
- middleware: treat a missing token.isAdmin as 'old token' and force re-login so the flag gets populated (otherwise existing users with a pre-PR JWT would be bounced to /mon-espace for up to 30 days). - auth/config: memoize the ADMIN_EMAILS set at module load instead of reparsing the env var on every sign-in. - admin/page: split the defense-in-depth check to redirect to /login when there is no session (avoids a redirect chain).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
users.isAdmincolumn (migration0023_add_user_is_admin.sql).ADMIN_EMAILSenv var in the NextAuthjwtcallback (bidirectional: promotes and demotes on login).adminProcedurein tRPC, edge middleware + layout guard on/admin/*(redirects non-admin to/mon-espace), placeholderAdminHomePage.Closes #3178
Quality gates
Follow-ups (not blocking)
Generated with Claude Code