feat : audit log (table + writer + listener + API + admin UI + timeline) #9
Reference in New Issue
Block a user
Delete Branch "feat/audit-log"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Résumé
Implémente le journal d'audit append-only couvrant les 5 tickets de
doc/audit-log.mdet embarque au passage plusieurs corrections périphériques (sidebar Admin/Mon compte, drawer RBAC, Swagger, schema_filter Doctrine) ainsi que l'initialisation de la suite e2e Playwright. Toutes les mutations Doctrine sur les entités portant#[Auditable]sont tracées dans une table PostgreSQL dédiée, exposée en lecture seule via API Platform et consultable par les admins dans une page dédiée.Ce qui change
Audit log — cœur de la PR
Backend
audit_log(UUID v7 natif Postgres en PK,jsonb changes, 3 index pour tri chrono, par entité et par utilisateur).AuditLogWriter: service bas-niveau, écrit via une connexion DBAL dédiéeaudit(même DSN quedefault, service séparé) pour sortir de la transaction ORM en batch. Blacklist defense-in-depthpassword/plainPassword/token/secret.RequestIdProvider: UUID v4 généré aukernel.requestprincipal, injecté dans chaque ligne d'audit de la requête.#[Auditable]/#[AuditIgnore]danssrc/Shared/Domain/Attribute/(accessibles par tous les modules).AuditListener: captureonFlush/ écriturepostFlushavec pattern swap-and-clear contre les flushes ré-entrants. Erreurs loguées, jamais propagées. EntitéUserannotée (password / plainPassword ignorés)./api/audit-logs(permission RBACcore.audit_log.view) :GETcollection paginée +GETitem, pas de POST/PUT/PATCH/DELETE. Filtresentity_type,entity_id,action,performed_by,performed_at[after]/[before].DbalPaginatorimplémentantPaginatorInterface:hydra:viewgénéré automatiquement par API Platform, pas de construction manuelle.AuditLogEntityTypesResource+ provider dédié pour peupler le filtre par type d'entité côté UI (réponse cachée, pas de requête à chaque ouverture du drawer).core.audit_log.viewdéclarée dansCoreModule::permissions().audit_logexclu duschema_filterDoctrine : plus de faux diff surmake migration-diff.Frontend
/admin/audit-log: tableau paginé, filtres locaux (état dans le composant, non persistés dans l'URL — conforme règle CLAUDE.md « Tableaux : pas de persistance URL »), drawer de détail (diff + timeline complète de l'entité), badges colorés par action.useAuditLogavecresetAuditLog()auto-enregistré suronAuthSessionCleared(règle CLAUDE.md composables singletons).<AuditTimeline :entity-type :entity-id>: garde permission (pas d'appel API sans le droit), lazy loading (10 items + bouton « Voir plus »), dates relatives FR viaIntl.RelativeTimeFormat, skeleton loader.core.audit_log.view+ clés i18n imbriquées dansfr.json.Fixes embarqués
37eafd2,1505e84,99c77eb) : précision des timestamps,ESCAPEsur lesLIKE, plafond pagination, diverses remarques du 1er tour de review.701a480,e2fbf51) : nouvelle section « Administration » + groupe « Mon compte », gate de section sur permissions, « Tableau de bord » déplacé dans « Mon compte ». Convention admin documentée.617ee31,5f5afcc) : corrige l'affichage des sites et l'écrasement via merge-patch (garde anti-écrasement + specGET /users/{id}/rbacdocumentée).6db955f) : réactivé en ajoutantsymfony/twig-bundleaux deps (régression depuis l'arrivée d'API Platform 4.2).phpunit.dist.xml:<env APP_ENV=dev>forçait la suite à tourner sousframework.test=false(→test.service_containerintrouvable) ;JWT_PASSPHRASEne matchait pas les clés de dev. Corrigés pour débloquer la suite.E2E Playwright (nouveau, commit
4603ab2)playwright.config.ts+ structurefrontend/tests/e2e/(personas, helpersloginAs, page objectsLoginPage+SidebarComponent).auth/login.spec.ts+permissions/sidebar-visibility.spec.ts(vérifie la visibilité de la sidebar par rôle RBAC).SeedE2ECommandpour préparer un jeu de données déterministe côté backend.make e2eajouté au Makefile.Décisions techniques
performed_at~40 % plus petit sur une table append-only à croissance infinie.entity_typeformatmodule.Entity(ex:core.User) : évite les collisions si deux modules ont des entités de même nom.performed_bydénormalisé (string, pas FK) : le nom persiste même après suppression de l'utilisateur.audit: évite l'entanglement transactionnel entre audit et ORM en batch.ManyToManynon audité : limitation connue (getEntityChangeSet()ne couvre pas les collections) ; extension future viagetScheduledCollectionUpdates()si besoin.Test plan
make test: 218 tests passent (writer unitaires + listener intégration + API fonctionnels + UserRbacProcessor).npm run lint+npm run test+npm run build(frontend).audit_logignoré parschema_filter.app:sync-permissions)./api/docsaccessible de nouveau.make e2evert en local (login + sidebar-visibility)./admin/audit-log.core.audit_log.view→ 403 sur l'endpoint + item absent de la sidebar.PATCH.Points d'attention pour le review
AuditListener: pattern swap-and-clear surpostFlush— relire la gestion des flushes ré-entrants.DbalPaginator: vérifier que l'absence d'Iteratorcustom ne casse pas la normalisation API Platform sur collections vides.UserRbacProcessor: logique merge-patch + garde anti-écrasement des sites (régression corrigée dans617ee31).make e2ene fait pas partie du pipeline CI par défaut (à brancher explicitement).Code review
Found 5 issues:
AuditListenerbypasse silencieusement l'audit sur les entites chargees en lazy (proxies Doctrine) —$class = $entity::class(ligne 155) renvoieProxies\__CG__\App\Module\Core\Domain\Entity\Usersur une entite lazy-loaded.isAuditable()(ligne 258) construitnew ReflectionClass($class)->getAttributes(Auditable::class)sans remonter la classe parente, donc#[Auditable]n'est pas detecte et la mutation n'est pas auditee. Cas tres frequent en pratique (toute modification via navigation de relation). Viole directement la regle du CLAUDE.md : "Audit obligatoire : toute entite doit porter#[Auditable]". Fix :Doctrine\Common\Util\ClassUtils::getRealClass($class)ou$em->getClassMetadata($entity)->getName().AuditTimelinesaute les items 11 a 30 de la premiere page — L'API retourne 30 items/page (paginationItemsPerPage: 30surAuditLogResource), le composant slice aINITIAL_LIMIT = 10au premier chargement (ligne 121), puisloadMore()incrementepage.value + 1(ligne 134) pour fetch la page 2 (items 31-60). Les items 11-30 ne sont jamais affiches — l'historique d'une entite avec plus de 10 evenements sera incoherent. Fix : soit charger avecitemsPerPage=10cote backend, soit augmenter le slice quand l'utilisateur clique "Voir plus" sur la meme page.audit-log.vuepersiste l'etat du tableau dans l'URL — Les filtres et le numero de page sont initialises depuisroute.queryviareadQuery()(lignes 212-217) et resynchronises viarouter.replace({ query })danssyncQuery()(ligne 340). Contraire a la convention projet "Tableaux : pas de persistance URL — aucun etat de tableau (filtres, pagination, tri...) ne doit etre persiste dans la query string ou reinjecte depuisroute.queryau montage". Seuls les deep links metier (ex:/users/42) sont dans l'URL.<table>brut avec pagination custom — La liste/admin/audit-logutilise un<table>hand-rolled avec<thead>/<tbody>(ligne 83) et une pagination custom viagoPrevious/goNextau lieu deMalioDataTable. Contraire a la convention "Tableaux de donnees : utiliser MalioDataTable — tout affichage LISTE tabulaire (donnees metier paginees, CRUD admin) doit passer parMalioDataTable". L'exception "presentationnel non-paginable" ne s'applique pas ici.<input>/<button>bruts sans composantsMalio*— Les filtres "Entite" (<input type="text">ligne 36), "Utilisateur" (ligne 47), les checkboxes d'actions (ligne 59), le bouton Reset (ligne 71) et les deux<input type="datetime-local">(lignes 16, 26) ne passent pas parMalioInputText/MalioSelectCheckbox/MalioCheckbox/MalioButton. Convention projet "Composants formulaires : utiliser @malio/layer-ui — tout champ de formulaire / filtre doit utiliser les composants Malio*". Les exceptions legitimes (datetime-local) doivent porter un commentaire TODO explicite.🤖 Generated with Claude Code
- If this code review was useful, please react with 👍. Otherwise, react with 👎.
Fixes appliquees
Commits a95bb6c + 1505e84 — 228/228 tests verts.
$em->getClassMetadata($entity::class)->getName()appliquee aux deux sites (capturePendingLog+captureCollectionChange). Regression testtestLogsUpdateOnProxyEntityviagetReference().PAGE_SIZE = 10cote composant,itemsPerPage=10passe au backend viafetchEntityLogs(entityType, entityId, page, itemsPerPage). Token anti-race en bonus.URL persistence →
readQuery/syncQuery/route.queryentierement supprimes deaudit-log.vue.MalioDataTable → la liste admin passe par
<MalioDataTable>avec slots#header-*et#cell-*. Plus de pagination hand-rolled.Composants Malio* →
MalioInputTextpour les filtres texte,MalioSelectCheckboxpour le multi-selectentity_type(alimente par nouveau endpoint/api/audit-log-entity-types),MalioButton/MalioButtonIconpour les actions.datetime-localconserve avec TODO pointant l'exception CLAUDE.md.Bonus (hors review) : couverture ManyToMany (
getScheduledCollectionUpdates/Deletions) avec 3 tests de regression, filtreentity_type[]multi-valeurs cote API,resetAuditLog()cable danslogout.vue.Conventions concernees par les findings 3-5 ajoutees a CLAUDE.md dans le commit a95bb6c.
Le drawer RBAC de /admin/users initialisait l'etat des sites a partir du payload /api/users (groupe user:list) qui n'expose pas la collection sites. Consequence : la section "Sites autorises" affichait toujours 0 case cochee, et la sauvegarde ecrasait silencieusement les sites existants en BDD. - Ajout d'une operation GET /users/{id}/rbac (groupe user:rbac:read) dediee au chargement du detail pour l'edition : payload list reste leger, detail riche sur une URI symetrique au PATCH existant. - Drawer charge desormais GET /users/{id}/rbac pour initialiser sites, roles et directPermissions ; UserListItem ne contient plus sites (inutilise). - Colonne "Sites" retiree de la table /admin/users : l'info est consultee via le drawer, pas la liste (evite aussi la fuite cross-site pour les users avec core.users.view mais sans sites.bypass_scope). - Garde anti-ecrasement dans UserRbacProcessor : respect de la semantique merge-patch+json (cle absente = preservee, cle = [] = vidage explicite). Restaure les collections ManyToMany absentes du payload a partir du snapshot Doctrine. Couvre roles, directPermissions et sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>- Dashboard ("/") sort de la section Administration et rejoint "Mon compte" (accessible a tout user authentifie, comme Deconnexion). - Cle i18n migree : sidebar.general.dashboard -> sidebar.account.dashboard. Le namespace "general" devenu vide est supprime. - Documentation inline dans config/sidebar.php : formalise la convention "etre admin = detenir au moins une permission admin-scoped (core.* ou equivalent sites.view)". Le gate implicite via filtrage d'items rend inutile un gate section-level tant que chaque item porte sa permission. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Backend : - AuditLogWriter::stripSensitive rendu reellement recursif (matche doc). - Tests GET /api/permissions/{id} non-admin pour chaque branche OR (gap Codex). - Gardes non-regression UserRbacProcessor : PATCH /rbac sans clef sites ne doit ni auto-selectionner currentSite ni exiger sites.manage. Frontend : - useAuditLog : renomme export trompeur fetchLogs -> fetchLogsCached, le nom reflete desormais le comportement (cache pollue sinon). - RoleDrawer / UserRbacDrawer : catch explicite + message d'erreur + bouton save disabled si le chargement des referentiels a echoue (evite un ecrasement silencieux des droits). - AuditTimeline / AuditLogDetail : `oui`/`non` passent par common.yes/no. - AuditTimeline : Intl.RelativeTimeFormat et toLocaleString suivent la locale i18n courante (plus de hardcode 'fr'). E2E : - sidebar-visibility.spec : remplace waitForLoadState('networkidle') fragile par attente semantique sur accountDashboardLink (stable en CI). Tests : 237/237 green, eslint clean, php-cs-fixer clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>restoreAbsentCollections utilisait `array_key_exists('sites', $payload)` qui retourne true pour une valeur null, sautant la restauration. API Platform rejette deja `sites: null` au denormalize (400 type mismatch), donc le bypass n'est pas reellement exploitable aujourd'hui via l'API HTTP. Mais le test `&& is_array(...)` reste une defense-in-depth si la config denormalizer change un jour, et rend l'intention explicite. Test de regression : PATCH {sites: null} -> 400 + collection sites intacte en DB (aucune trace de l'echec). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>AuditListener::captureCollectionChange utilise getScheduledCollectionUpdates et getScheduledCollectionDeletions pour produire {fieldName: {added, removed}}. La spec (lignes 197 et 418) annoncait l'inverse - alignement sur le code reel et sur .claude/rules/backend.md.- testItemEndpointWithoutPermissionGets403 : symetrique de testAuthenticatedUserWithoutPermissionGets403 sur /api/audit-logs/{id}, prouve que la security expression `is_granted('core.audit_log.view')` est appliquee aussi sur l'operation Get item (couvre M-4 du backlog). - testPageZeroDoesNotProduceServerError : verrouille l'invariant fonctionnel "?page=0 ne produit jamais 500 PG", quel que soit le mecanisme protecteur (clamp provider ou validation API Platform amont). Couvre M-7 du backlog.