diff --git a/frontend/modules/core/pages/admin/roles.vue b/frontend/modules/core/pages/admin/roles.vue index ec1beff..afbe178 100644 --- a/frontend/modules/core/pages/admin/roles.vue +++ b/frontend/modules/core/pages/admin/roles.vue @@ -114,6 +114,10 @@ async function loadRoles() { { toast: false }, ) roles.value = data.member + } catch { + // Reset sur echec pour ne pas afficher de donnees stale (ancienne + // requete reussie avant une perte reseau ou 403). + roles.value = [] } finally { loading.value = false } diff --git a/frontend/modules/core/pages/admin/users.vue b/frontend/modules/core/pages/admin/users.vue index 562c217..1d2a00f 100644 --- a/frontend/modules/core/pages/admin/users.vue +++ b/frontend/modules/core/pages/admin/users.vue @@ -78,6 +78,11 @@ async function loadUsers() { try { const usersData = await api.get<{ member: UserListItem[] }>('/users', {}, { toast: false }) users.value = usersData.member + } catch { + // Reset sur echec pour ne pas afficher de donnees stale (ancienne + // requete reussie avant une perte reseau ou 403). Pas de toast par + // design ici : on laisse la liste vide parler d'elle-meme. + users.value = [] } finally { loading.value = false } diff --git a/frontend/modules/sites/pages/admin/sites.vue b/frontend/modules/sites/pages/admin/sites.vue index 205dec1..96f28f7 100644 --- a/frontend/modules/sites/pages/admin/sites.vue +++ b/frontend/modules/sites/pages/admin/sites.vue @@ -118,6 +118,10 @@ async function loadSites() { { toast: false }, ) sites.value = data.member + } catch { + // Reset sur echec pour ne pas afficher de donnees stale (ancienne + // requete reussie avant une perte reseau ou 403). + sites.value = [] } finally { loading.value = false } diff --git a/makefile b/makefile index 60318db..c7b3ffe 100644 --- a/makefile +++ b/makefile @@ -73,8 +73,10 @@ seed-e2e: # Bootstrap one-time pour les tests E2E sur un nouveau poste : # 1. Telecharge Chromium dans ~/.cache/ms-playwright -# 2. Installe les deps systeme (libnss3, libasound, libatk, etc.) via -# la liste officielle Playwright — demande sudo. +# 2. Installe les deps systeme (libnss3, libasound, libatk, etc.) : +# - Ubuntu/Debian : `playwright install-deps` (officiel) +# - Fedora/RHEL : liste dnf equivalente (playwright ne gere pas dnf) +# - Autre : avertissement, a faire a la main. # # Le `sudo env "PATH=$$PATH"` est necessaire car avec NVM, `sudo npx` ne # trouve pas npx (le PATH de sudo est vide par defaut). On preserve @@ -84,7 +86,21 @@ seed-e2e: # bouger entre versions majeures). install-e2e-deps: cd frontend && npx playwright install chromium - cd frontend && sudo env "PATH=$$PATH" npx playwright install-deps chromium + @if command -v apt-get >/dev/null 2>&1; then \ + echo ">> Detected apt-get — using playwright install-deps"; \ + cd frontend && sudo env "PATH=$$PATH" npx playwright install-deps chromium; \ + elif command -v dnf >/dev/null 2>&1; then \ + echo ">> Detected dnf — installing Chromium deps via dnf"; \ + sudo dnf install -y \ + nss nspr dbus-libs atk at-spi2-atk at-spi2-core cups-libs \ + libdrm libX11 libXcomposite libXdamage libXfixes libXrandr \ + libXext libXtst libxkbcommon mesa-libgbm alsa-lib \ + pango cairo libwayland-client; \ + else \ + echo ">> No supported package manager detected (apt-get / dnf)."; \ + echo ">> Install Chromium system libs manually, then re-run test-e2e."; \ + exit 1; \ + fi # Lance les tests E2E Playwright sur l'host. Pre-requis : # - `make install-e2e-deps` (une fois par poste) diff --git a/src/Module/Core/Domain/Entity/Permission.php b/src/Module/Core/Domain/Entity/Permission.php index 284652b..def8a9a 100644 --- a/src/Module/Core/Domain/Entity/Permission.php +++ b/src/Module/Core/Domain/Entity/Permission.php @@ -18,13 +18,20 @@ use Symfony\Component\Serializer\Attribute\Groups; #[ApiResource( operations: [ + // Guard RBAC du catalogue de permissions : accepte les gestionnaires + // de users et de roles en plus du code dedie `core.permissions.view`. + // Justification : les drawers `UserRbacDrawer`/`RoleDrawer` fetchent + // systematiquement ce catalogue pour afficher les checkboxes de + // permissions ; exiger uniquement `core.permissions.view` casserait + // ces workflows pour tout gestionnaire non-admin. L'endpoint n'expose + // que des codes/libelles (pas de secret), le bypass reste acceptable. new GetCollection( normalizationContext: ['groups' => ['permission:read']], - security: "is_granted('core.permissions.view')", + security: "is_granted('core.permissions.view') or is_granted('core.users.manage') or is_granted('core.roles.manage')", ), new Get( normalizationContext: ['groups' => ['permission:read']], - security: "is_granted('core.permissions.view')", + security: "is_granted('core.permissions.view') or is_granted('core.users.manage') or is_granted('core.roles.manage')", ), ], )] diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php index 1528e71..a4ead6b 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -93,19 +93,6 @@ final class UserRbacProcessor implements ProcessorInterface )); } - // Garde anti-ecrasement (defense in depth) : PATCH merge-patch+json impose - // que les cles absentes du payload ne mutent PAS les proprietes - // correspondantes. La denormalisation API Platform ne respecte pas cet - // invariant pour les collections ManyToMany — elle reinstancie une - // ArrayCollection vide des que la cle n'est pas presente. Sans cette - // garde, un client qui PATCHe juste `{ "isAdmin": true }` verrait toutes - // ses roles/directPermissions/sites detruits. - // - // On lit le body brut de la requete pour connaitre les cles envoyees, - // puis on restaure les collections absentes a partir de l'etat d'origine - // charge par Doctrine (snapshot des PersistentCollection). - $this->restoreAbsentCollections($data); - $currentUser = $this->security->getUser(); // Calcul partage entre les deux gardes : l'user perdait-il le flag admin ? @@ -164,6 +151,20 @@ final class UserRbacProcessor implements ProcessorInterface $originalCurrentSiteId, &$result, ): void { + // Garde anti-ecrasement (defense in depth) : PATCH merge-patch+json impose + // que les cles absentes du payload ne mutent PAS les proprietes + // correspondantes. La denormalisation API Platform ne respecte pas cet + // invariant pour les collections ManyToMany — elle reinstancie une + // ArrayCollection vide des que la cle n'est pas presente. Sans cette + // garde, un client qui PATCHe juste `{ "isAdmin": true }` verrait toutes + // ses roles/directPermissions/sites detruits. + // + // Execute dans la transaction (et non avant) : garantit que le snapshot + // Doctrine lu pour restauration reflete le meme etat BDD que celui sur + // lequel le persist va operer. Evite toute fenetre de race entre la + // lecture du snapshot et le flush. + $this->restoreAbsentCollections($data); + $result = $this->persistProcessor->process($data, $operation, $uriVariables, $context); // Garde coherence currentSite (ticket 2 module Sites). diff --git a/tests/Module/Core/Api/PermissionApiTest.php b/tests/Module/Core/Api/PermissionApiTest.php index 5d8ba13..7d66ffc 100644 --- a/tests/Module/Core/Api/PermissionApiTest.php +++ b/tests/Module/Core/Api/PermissionApiTest.php @@ -191,6 +191,41 @@ final class PermissionApiTest extends AbstractApiTestCase self::assertResponseStatusCodeSame(403); } + public function testNonAdminWithPermissionViewCanListPermissions(): void + { + // Chemin positif : un user non-admin qui porte la permission + // `core.permissions.view` (via un role dedie) doit recevoir 200 sur + // le catalogue. Couvre l'habilitation sans bypass isAdmin. + $creds = $this->createUserWithPermission('core.permissions.view'); + $client = $this->authenticatedClient($creds['username'], $creds['password']); + $client->request('GET', '/api/permissions'); + + self::assertResponseIsSuccessful(); + } + + public function testNonAdminWithUsersManageCanListPermissions(): void + { + // Bypass pragmatique : les gestionnaires d'users ont besoin du + // catalogue pour les drawers RBAC. Couvre la branche OR de la + // security expression `core.users.manage`. + $creds = $this->createUserWithPermission('core.users.manage'); + $client = $this->authenticatedClient($creds['username'], $creds['password']); + $client->request('GET', '/api/permissions'); + + self::assertResponseIsSuccessful(); + } + + public function testNonAdminWithRolesManageCanListPermissions(): void + { + // Meme logique que ci-dessus pour les gestionnaires de roles + // (branche OR `core.roles.manage` de la security expression). + $creds = $this->createUserWithPermission('core.roles.manage'); + $client = $this->authenticatedClient($creds['username'], $creds['password']); + $client->request('GET', '/api/permissions'); + + self::assertResponseIsSuccessful(); + } + private function cleanupTestPermissions(): void { $em = $this->getEm(); diff --git a/tests/Module/Core/Api/UserRbacSitesApiTest.php b/tests/Module/Core/Api/UserRbacSitesApiTest.php index 29fa75d..2595360 100644 --- a/tests/Module/Core/Api/UserRbacSitesApiTest.php +++ b/tests/Module/Core/Api/UserRbacSitesApiTest.php @@ -166,6 +166,12 @@ final class UserRbacSitesApiTest extends AbstractApiTestCase $reloaded = $em->getRepository(User::class)->find($aliceId); self::assertNotNull($reloaded->getCurrentSite()); self::assertSame('Chatellerault', $reloaded->getCurrentSite()->getName()); + + // Garde non-regression : la collection `sites` elle-meme doit etre + // preservee (cf. fix restoreAbsentCollections + initialize LAZY). Un + // PATCH /rbac sans cle `sites` ne doit en aucun cas vider la relation. + self::assertCount(1, $reloaded->getSites()); + self::assertSame('Chatellerault', $reloaded->getSites()->first()->getName()); } /**