From bb6a4c387b0c11ea42a406fab2274dfac96810d7 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Thu, 23 Apr 2026 09:29:34 +0200 Subject: [PATCH] fix(review) : applique fixes blockers review PR #9 (permission guard, sites LAZY, audit UI stale) - Permission entity : remplace le guard `ROLE_USER` par `core.permissions.view` sur GetCollection/Get. Le catalogue complet des permissions RBAC etait accessible a tout utilisateur authentifie. Ajoute la permission manquante dans CoreModule::permissions() et inverse les tests standardUser* (attendent maintenant un 403 pour un user sans la permission). - UserRbacProcessor::restoreAbsentCollections() : force PersistentCollection::initialize() avant de lire le snapshot. Pour une association fetch=LAZY (ex: User::$sites), le snapshot est vide tant que la collection n'est pas materialisee, ce qui faisait vider silencieusement tous les sites d'un user sur un PATCH ne contenant pas la cle `sites`. - admin/audit-log.vue : ajoute un catch sur loadEntries() qui reset entries/totalItems pour ne pas afficher de donnees stale si le fetch echoue (reseau coupe, 403 inopinee...). Le toast d'erreur reste gere par useApi. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/modules/core/pages/admin/audit-log.vue | 8 ++++++++ src/Module/Core/CoreModule.php | 1 + src/Module/Core/Domain/Entity/Permission.php | 4 ++-- .../State/Processor/UserRbacProcessor.php | 11 +++++++++++ tests/Module/Core/Api/PermissionApiTest.php | 13 ++++++++----- 5 files changed, 30 insertions(+), 7 deletions(-) diff --git a/frontend/modules/core/pages/admin/audit-log.vue b/frontend/modules/core/pages/admin/audit-log.vue index 1f84216..4a8a7f9 100644 --- a/frontend/modules/core/pages/admin/audit-log.vue +++ b/frontend/modules/core/pages/admin/audit-log.vue @@ -286,6 +286,14 @@ async function loadEntries(): Promise { if (token !== requestToken) return entries.value = data.member ?? [] totalItems.value = data.totalItems ?? 0 + } catch { + // En cas d'echec (reseau, 403, 500...), on reset l'etat pour ne pas + // laisser l'utilisateur croire que les donnees affichees sont a jour. + // Le toast d'erreur est deja emis par `useApi()` via useAuditLog. + if (token === requestToken) { + entries.value = [] + totalItems.value = 0 + } } finally { if (token === requestToken) { loading.value = false diff --git a/src/Module/Core/CoreModule.php b/src/Module/Core/CoreModule.php index 8b3be4f..997745e 100644 --- a/src/Module/Core/CoreModule.php +++ b/src/Module/Core/CoreModule.php @@ -34,6 +34,7 @@ final class CoreModule ['code' => 'core.users.manage', 'label' => 'Gerer les utilisateurs (creer, editer, supprimer)'], ['code' => 'core.roles.view', 'label' => 'Voir les roles RBAC'], ['code' => 'core.roles.manage', 'label' => 'Gerer les roles et permissions'], + ['code' => 'core.permissions.view', 'label' => 'Consulter le catalogue des permissions RBAC'], ['code' => 'core.audit_log.view', 'label' => 'Consulter le journal d\'audit'], ]; } diff --git a/src/Module/Core/Domain/Entity/Permission.php b/src/Module/Core/Domain/Entity/Permission.php index 9825a05..284652b 100644 --- a/src/Module/Core/Domain/Entity/Permission.php +++ b/src/Module/Core/Domain/Entity/Permission.php @@ -20,11 +20,11 @@ use Symfony\Component\Serializer\Attribute\Groups; operations: [ new GetCollection( normalizationContext: ['groups' => ['permission:read']], - security: "is_granted('ROLE_USER')", + security: "is_granted('core.permissions.view')", ), new Get( normalizationContext: ['groups' => ['permission:read']], - security: "is_granted('ROLE_USER')", + security: "is_granted('core.permissions.view')", ), ], )] diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php index b3d349c..1528e71 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -260,6 +260,17 @@ final class UserRbacProcessor implements ProcessorInterface continue; } + // Force l'initialisation LAZY avant de lire le snapshot : pour une + // association fetch=LAZY (ex: User::$sites), la PersistentCollection + // existe mais son snapshot est vide tant que la collection n'a pas + // ete materialisee. Sans cet init, `getSnapshot()` renvoie `[]` et + // la boucle de restauration ci-dessous appelle `remover()` sur + // chaque item charge par `toArray()` → **vide silencieusement la + // collection** au lieu de la preserver. Idempotent si deja initialisee. + if (!$currentCollection->isInitialized()) { + $currentCollection->initialize(); + } + // Snapshot = etat charge depuis la BDD avant denormalisation. // On restaure en retirant les items actuels et en ajoutant les // originaux via l'adder/remover pour que les collections inverses diff --git a/tests/Module/Core/Api/PermissionApiTest.php b/tests/Module/Core/Api/PermissionApiTest.php index dc06f3d..5d8ba13 100644 --- a/tests/Module/Core/Api/PermissionApiTest.php +++ b/tests/Module/Core/Api/PermissionApiTest.php @@ -166,16 +166,19 @@ final class PermissionApiTest extends AbstractApiTestCase self::assertResponseStatusCodeSame(401); } - public function testStandardUserCanListPermissions(): void + public function testStandardUserWithoutPermissionIsForbiddenOnCollection(): void { - // Le catalogue de permissions est accessible a tout utilisateur authentifie. + // Le catalogue de permissions est protege par `core.permissions.view` : + // un user authentifie sans cette permission (ni flag admin) doit + // recevoir un 403. Alice n'a que le role systeme "user" (zero + // permission attachee) — elle est le cobaye ideal pour ce test. $client = $this->authenticatedClient('alice', 'alice'); $client->request('GET', '/api/permissions'); - self::assertResponseIsSuccessful(); + self::assertResponseStatusCodeSame(403); } - public function testStandardUserCanGetPermission(): void + public function testStandardUserWithoutPermissionIsForbiddenOnItem(): void { $permission = $this->getEm()->getRepository(Permission::class) ->findOneBy(['code' => 'test.core.users.view']) @@ -185,7 +188,7 @@ final class PermissionApiTest extends AbstractApiTestCase $client = $this->authenticatedClient('alice', 'alice'); $client->request('GET', '/api/permissions/'.$permission->getId()); - self::assertResponseIsSuccessful(); + self::assertResponseStatusCodeSame(403); } private function cleanupTestPermissions(): void