diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php index a4ead6b..08117eb 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -250,7 +250,12 @@ final class UserRbacProcessor implements ProcessorInterface } foreach (self::COLLECTION_MAP as $jsonKey => $accessors) { - if (array_key_exists($jsonKey, $payload)) { + // La garde ne doit sauter la restauration que si le payload fournit + // un VRAI tableau pour cette cle. Un `null`, un scalaire ou un autre + // type doivent etre traites comme "cle absente" : sinon un payload + // `{"sites": null}` contourne la restauration et laisse API Platform + // vider la collection silencieusement (bypass de la garde). + if (array_key_exists($jsonKey, $payload) && is_array($payload[$jsonKey])) { continue; } diff --git a/tests/Module/Core/Api/UserRbacSitesApiTest.php b/tests/Module/Core/Api/UserRbacSitesApiTest.php index fe74668..210e7bd 100644 --- a/tests/Module/Core/Api/UserRbacSitesApiTest.php +++ b/tests/Module/Core/Api/UserRbacSitesApiTest.php @@ -139,6 +139,46 @@ final class UserRbacSitesApiTest extends AbstractApiTestCase self::assertSame('Chatellerault', $reloaded->getCurrentSite()->getName()); } + /** + * Defense-in-depth contre un bypass hypothetique de restoreAbsentCollections. + * + * API Platform rejette deja `sites: null` au denormalize (400 Bad Request, + * type mismatch). Le guard `is_array` dans UserRbacProcessor est une + * deuxieme ligne de defense si la config denormalizer change un jour. + * + * Ce test verrouille deux garanties : + * 1. `{"sites": null}` → 400 (pas un 500, pas un 200 silencieux) + * 2. La collection sites d'alice est intacte apres l'echec + */ + public function testRbacPatchWithNullSitesReturns400AndDoesNotWipeSitesCollection(): void + { + $em = $this->getEm(); + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + $aliceId = $alice->getId(); + self::assertCount(1, $alice->getSites(), 'Pre-condition : alice doit avoir exactement 1 site'); + $em->clear(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'isAdmin' => false, + 'sites' => null, + ], + ]); + + self::assertResponseStatusCodeSame(400); + + $em = $this->getEm(); + $em->clear(); + $reloaded = $em->getRepository(User::class)->find($aliceId); + self::assertCount( + 1, + $reloaded->getSites(), + 'Un payload `sites: null` rejete en 400 ne doit laisser aucune trace en DB.', + ); + } + public function testRbacPatchWithoutSitesFieldDoesNotChangeCurrentSite(): void { // Garde structurelle : si le payload /rbac ne contient pas le champ