Skip to content

feat(admin): mimoquage (impersonation) d'une entreprise#3188

Merged
LucasCharrier merged 21 commits intoalphafrom
feat/issue-3179-admin-impersonate
Apr 13, 2026
Merged

feat(admin): mimoquage (impersonation) d'une entreprise#3188
LucasCharrier merged 21 commits intoalphafrom
feat/issue-3179-admin-impersonate

Conversation

@LucasCharrier
Copy link
Copy Markdown
Contributor

Summary

  • Admin peut mimoquer une entreprise par SIREN depuis /admin/impersonate (recherche → prévisualisation → validation).
  • État stocké dans le JWT NextAuth ; audit trail en DB (app_admin_impersonation_event) écrit atomiquement dans le callback jwt pour rester cohérent avec le token effectif.
  • Bandeau global ImpersonateBanner dans le layout pour sortir du mimoquage depuis n'importe quelle page ; companyProcedure et findUserCompany accordent l'accès à la SIREN mimoquée uniquement quand isAdmin && impersonation.siren === siren (bypass strict, pas de propagation).

Closes #3179

Note base: ciblé sur feat/issue-3178-admin-role (PR #3187) car cette branche en dépend. Quand #3187 sera mergée sur alpha, la base de cette PR se mettra à jour automatiquement.

Quality gates

  • Typecheck / Tests (1104 passing) / Lint / Format
  • Structural / RGAA / Security audit (findings adressés dans le commit de refactor)

Generated with Claude Code

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.
- 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.
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.
Addresses security and RGAA findings from the validation agents:

- The audit trail is now written inside the NextAuth jwt callback, so
  every row in app_admin_impersonation_event corresponds to an actual
  live JWT state (previously the client could start the mutation and
  never update the session, or vice-versa).
- Company upsert in startImpersonate switches to onConflictDoNothing()
  so an admin search cannot silently overwrite metadata visible to real
  referents of that company.
- DSFR fr-alert and fr-notice markup fixed (heading for title, proper
  container nesting).
- Redundant role=alert + aria-live on the form removed; aria-invalid
  and aria-required added on the SIREN input.
- ImpersonateForm preview state derived from mutation data.
- modules/admin barrel re-exports schemas.
- E2E: non-admin redirect covered for /admin/impersonate.
@LucasCharrier LucasCharrier requested a review from a team as a code owner April 8, 2026 15:43
@revu-bot revu-bot Bot requested a review from revu-bot April 8, 2026 15:43
@LucasCharrier LucasCharrier temporarily deployed to build-review-auto April 8, 2026 15:44 — with GitHub Actions Inactive
Copy link
Copy Markdown
Collaborator

@revu-bot revu-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admin Impersonation Feature Review

This PR implements an admin impersonation ("mimoquage") feature with a solid overall architecture: JWT-based state, atomic audit trail writes, strict SIREN validation, and centralized access control via isImpersonatingSiren. The test coverage for the core logic is good.

However, there are several issues ranging from a security gap to silent failure modes and a write-mutation concern that need attention before merging.

File Lines Severity Issue
server/auth/config.ts 218–232 CRITICAL Silent no-op on invalid impersonation payload — no error feedback to client
server/auth/config.ts 60–80 IMPORTANT closeOpenImpersonationEvents runs outside the transaction in recordImpersonationStart
server/api/routers/company.ts 198–204 IMPORTANT updateHasCse mutation allowed for impersonating admin — write bypass too broad
modules/layout/ImpersonateBanner.tsx 24–27 IMPORTANT onStop has no error handling — silent failure leaves stale banner state
server/api/routers/admin.ts 26–27 MINOR searchCompany uses .mutation for a pure read — should be .query

When an admin is impersonating a company, the server-side page was still
passing the admin's own SIRET (from ProConnect) to MonEspacePage. Now it
passes the impersonated SIREN, so /mon-espace correctly shows the
impersonated company's declarations.
@LucasCharrier LucasCharrier temporarily deployed to build-review-auto April 9, 2026 08:16 — with GitHub Actions Inactive
@LucasCharrier LucasCharrier linked an issue Apr 9, 2026 that may be closed by this pull request
7 tasks
Base automatically changed from feat/issue-3178-admin-role to alpha April 13, 2026 07:35
@LucasCharrier LucasCharrier requested a review from a team as a code owner April 13, 2026 07:35
…n-impersonate

# Conflicts:
#	.kontinuous/values.yaml
#	packages/app/drizzle/meta/_journal.json
#	packages/app/src/e2e/admin.e2e.ts
#	packages/app/src/env.js
#	packages/app/src/modules/admin/index.ts
#	packages/app/src/server/auth/config.ts
In CI, test@fia1.fr is admin (ADMIN_EMAILS), so the non-admin redirect
test always fails. Replace with a positive test that verifies the admin
can access the impersonate page.
@tokenbureau
Copy link
Copy Markdown

tokenbureau Bot commented Apr 13, 2026

@LucasCharrier LucasCharrier merged commit badc16a into alpha Apr 13, 2026
18 checks passed
@LucasCharrier LucasCharrier deleted the feat/issue-3179-admin-impersonate branch April 13, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backoffice - Mimoquage d'un utilisateur

3 participants