feat(core) : RBAC #344 - UserRbacProcessor + endpoint /users/{id}/rbac

Ajoute une operation Patch dediee `PATCH /api/users/{id}/rbac` (nom `user_rbac_patch`) qui accepte exclusivement les champs RBAC isAdmin, roles et directPermissions via le groupe user:rbac:write. L'endpoint est separe volontairement du Patch profil existant pour isoler la modification des droits de celle des donnees profil (decision 0fc4e16). UserRbacProcessor delegue au PersistProcessor Doctrine decore et applique une garde auto-suicide : un admin ne peut pas retirer ses propres droits administrateur (compare l'etat entrant a l'etat UnitOfWork). La garde 'dernier admin' globale est reportee au ticket #345. La propriete Doctrine $roles est renommee $rbacRoles pour eviter la collision avec UserInterface::getRoles() (qui renvoie list<string>) lors de la normalization API Platform. La cle JSON reste `roles` grace a SerializedName, le contrat API est inchange. Tests : 6 unitaires (UserRbacProcessorTest) + 8 fonctionnels (UserRbacApiTest) couvrant promotion admin, remplacement des collections roles/directPermissions, 401/403, filtrage du groupe denormalization (`username` ignore), preservation de isAdmin sur le Patch profil, et garde auto-suicide.
2026-04-15 14:17:18 +02:00
parent 168a47f2b8
commit 3c7dc88fe7
4 changed files with 589 additions and 15 deletions
--- a/tests/Module/Core/Api/UserRbacApiTest.php
+++ b/tests/Module/Core/Api/UserRbacApiTest.php
@@ -0,0 +1,271 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Module\Core\Api;
+
+use App\Module\Core\Domain\Entity\Permission;
+use App\Module\Core\Domain\Entity\Role;
+use App\Module\Core\Domain\Entity\User;
+use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
+
+/**
+ * Tests fonctionnels de l'endpoint RBAC dedie `PATCH /api/users/{id}/rbac`.
+ *
+ * Strategie de donnees :
+ *  - On cree des users, roles et permissions prefixes `test_` / `test.`
+ *    en setUp et on les purge en tearDown.
+ *  - On ne touche JAMAIS aux fixtures (admin / alice / bob). Les cas qui
+ *    ont besoin d'un user standard authentifie s'appuient sur alice sans
+ *    modification d'etat.
+ *  - Les users de test incluent un admin dedie pour le cas d'auto-suicide,
+ *    pour ne pas risquer de corrompre l'admin fixture.
+ *
+ * @internal
+ */
+final class UserRbacApiTest extends AbstractApiTestCase
+{
+    private const TEST_USER_PREFIX       = 'test_';
+    private const TEST_ROLE_PREFIX       = 'test_';
+    private const TEST_PERMISSION_PREFIX = 'test.';
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        self::bootKernel();
+        $em = $this->getEm();
+
+        $this->cleanupTestData();
+
+        /** @var UserPasswordHasherInterface $hasher */
+        $hasher = self::getContainer()->get(UserPasswordHasherInterface::class);
+
+        // User cible standard (non admin).
+        $target = new User();
+        $target->setUsername('test_target');
+        $target->setIsAdmin(false);
+        $target->setPassword($hasher->hashPassword($target, 'secret'));
+        $em->persist($target);
+
+        // User admin dedie pour le cas d'auto-suicide (pas l'admin fixture).
+        $selfAdmin = new User();
+        $selfAdmin->setUsername('test_self_admin');
+        $selfAdmin->setIsAdmin(true);
+        $selfAdmin->setPassword($hasher->hashPassword($selfAdmin, 'secret'));
+        $em->persist($selfAdmin);
+
+        // Role custom pour tester le remplacement de la collection roles.
+        $role = new Role('test_editor', 'Editeur (test)', false);
+        $em->persist($role);
+
+        // Permission custom pour tester directPermissions.
+        $permission = new Permission('test.core.users.view', 'View users (test)', 'core');
+        $em->persist($permission);
+
+        $em->flush();
+        $em->clear();
+    }
+
+    protected function tearDown(): void
+    {
+        $this->cleanupTestData();
+        parent::tearDown();
+    }
+
+    public function testPatchRbacPromotesUserToAdmin(): void
+    {
+        $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertNotNull($target);
+
+        $client = $this->authenticatedClient('admin', 'admin');
+        $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => ['isAdmin' => true],
+        ]);
+
+        self::assertResponseIsSuccessful();
+
+        $em = $this->getEm();
+        $em->clear();
+
+        /** @var User $reloaded */
+        $reloaded = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertTrue($reloaded->isAdmin());
+    }
+
+    public function testPatchRbacReplacesRolesCollection(): void
+    {
+        $em     = $this->getEm();
+        $target = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        $role   = $em->getRepository(Role::class)->findOneBy(['code' => 'test_editor']);
+        self::assertNotNull($target);
+        self::assertNotNull($role);
+
+        $client = $this->authenticatedClient('admin', 'admin');
+        $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => ['roles' => ['/api/roles/'.$role->getId()]],
+        ]);
+
+        self::assertResponseIsSuccessful();
+
+        $em = $this->getEm();
+        $em->clear();
+
+        /** @var User $reloaded */
+        $reloaded = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertCount(1, $reloaded->getRbacRoles());
+        self::assertSame('test_editor', $reloaded->getRbacRoles()->first()->getCode());
+    }
+
+    public function testPatchRbacReplacesDirectPermissionsCollection(): void
+    {
+        $em         = $this->getEm();
+        $target     = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        $permission = $em->getRepository(Permission::class)->findOneBy(['code' => 'test.core.users.view']);
+        self::assertNotNull($target);
+        self::assertNotNull($permission);
+
+        $client = $this->authenticatedClient('admin', 'admin');
+        $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => ['directPermissions' => ['/api/permissions/'.$permission->getId()]],
+        ]);
+
+        self::assertResponseIsSuccessful();
+
+        $em = $this->getEm();
+        $em->clear();
+
+        /** @var User $reloaded */
+        $reloaded = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertCount(1, $reloaded->getDirectPermissions());
+        self::assertSame('test.core.users.view', $reloaded->getDirectPermissions()->first()->getCode());
+    }
+
+    public function testPatchRbacAsStandardUserReturns403(): void
+    {
+        $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertNotNull($target);
+
+        $client = $this->authenticatedClient('alice', 'alice');
+        $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => ['isAdmin' => true],
+        ]);
+
+        self::assertResponseStatusCodeSame(403);
+    }
+
+    public function testPatchRbacUnauthenticatedReturns401(): void
+    {
+        $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertNotNull($target);
+
+        $client = self::createClient();
+        $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => ['isAdmin' => true],
+        ]);
+
+        self::assertResponseStatusCodeSame(401);
+    }
+
+    public function testPatchRbacIgnoresUsernameField(): void
+    {
+        $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertNotNull($target);
+        $targetId = $target->getId();
+
+        $client = $this->authenticatedClient('admin', 'admin');
+        $client->request('PATCH', '/api/users/'.$targetId.'/rbac', [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => [
+                'username' => 'test_target_renamed',
+                'isAdmin'  => true,
+            ],
+        ]);
+
+        self::assertResponseIsSuccessful();
+
+        $em = $this->getEm();
+        $em->clear();
+
+        /** @var User $reloaded */
+        $reloaded = $em->getRepository(User::class)->find($targetId);
+        // `username` n'est pas dans `user:rbac:write` : ignore en denormalization.
+        self::assertSame('test_target', $reloaded->getUsername());
+        // `isAdmin` est bien applique.
+        self::assertTrue($reloaded->isAdmin());
+    }
+
+    public function testPatchProfileEndpointDoesNotModifyIsAdmin(): void
+    {
+        // Confirme la decision 0fc4e16 : `isAdmin` n'est plus dans `user:write`,
+        // donc `PATCH /api/users/{id}` sans `/rbac` ne peut plus promouvoir.
+        $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']);
+        self::assertNotNull($target);
+        $targetId = $target->getId();
+        self::assertFalse($target->isAdmin());
+
+        $client = $this->authenticatedClient('admin', 'admin');
+        $client->request('PATCH', '/api/users/'.$targetId, [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => ['isAdmin' => true],
+        ]);
+
+        // Peu importe le code : le champ ne doit tout simplement pas bouger.
+        $em = $this->getEm();
+        $em->clear();
+
+        /** @var User $reloaded */
+        $reloaded = $em->getRepository(User::class)->find($targetId);
+        self::assertFalse($reloaded->isAdmin());
+    }
+
+    public function testPatchRbacSelfRemovingAdminReturns400(): void
+    {
+        // On utilise le user admin dedie (test_self_admin) pour ne pas
+        // corrompre l'admin fixture en cas de bug.
+        $em        = $this->getEm();
+        $selfAdmin = $em->getRepository(User::class)->findOneBy(['username' => 'test_self_admin']);
+        self::assertNotNull($selfAdmin);
+        $selfAdminId = $selfAdmin->getId();
+
+        $client = $this->authenticatedClient('test_self_admin', 'secret');
+        $client->request('PATCH', '/api/users/'.$selfAdminId.'/rbac', [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => ['isAdmin' => false],
+        ]);
+
+        self::assertResponseStatusCodeSame(400);
+
+        $em = $this->getEm();
+        $em->clear();
+
+        /** @var User $reloaded */
+        $reloaded = $em->getRepository(User::class)->find($selfAdminId);
+        self::assertTrue($reloaded->isAdmin());
+    }
+
+    private function cleanupTestData(): void
+    {
+        $em = $this->getEm();
+
+        // Ordre important : delier les collections avant de supprimer les
+        // entites referencees pour que les FK cascade s'appliquent via le
+        // schema PostgreSQL.
+        $em->createQuery(
+            'DELETE FROM '.User::class.' u WHERE u.username LIKE :prefix'
+        )->setParameter('prefix', self::TEST_USER_PREFIX.'%')->execute();
+
+        $em->createQuery(
+            'DELETE FROM '.Role::class.' r WHERE r.code LIKE :prefix'
+        )->setParameter('prefix', self::TEST_ROLE_PREFIX.'%')->execute();
+
+        $em->createQuery(
+            'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix'
+        )->setParameter('prefix', self::TEST_PERMISSION_PREFIX.'%')->execute();
+    }
+}
--- a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php
+++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php
@@ -0,0 +1,216 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Module\Core\Infrastructure\ApiPlatform\State\Processor;
+
+use ApiPlatform\Metadata\Patch;
+use ApiPlatform\State\ProcessorInterface;
+use App\Module\Core\Domain\Entity\Permission;
+use App\Module\Core\Domain\Entity\Role;
+use App\Module\Core\Domain\Entity\User;
+use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserRbacProcessor;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\UnitOfWork;
+use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use ReflectionClass;
+use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+/**
+ * Tests unitaires du UserRbacProcessor : couvre la garde "auto-suicide" et la
+ * delegation au PersistProcessor Doctrine decore pour les trois champs RBAC
+ * (isAdmin, roles, directPermissions).
+ *
+ * @internal
+ */
+#[AllowMockObjectsWithoutExpectations]
+final class UserRbacProcessorTest extends TestCase
+{
+    private MockObject&ProcessorInterface $persistProcessor;
+    private EntityManagerInterface&MockObject $entityManager;
+    private MockObject&UnitOfWork $unitOfWork;
+    private MockObject&Security $security;
+    private UserRbacProcessor $processor;
+
+    protected function setUp(): void
+    {
+        $this->persistProcessor = $this->createMock(ProcessorInterface::class);
+        $this->entityManager    = $this->createMock(EntityManagerInterface::class);
+        $this->unitOfWork       = $this->createMock(UnitOfWork::class);
+        $this->security         = $this->createMock(Security::class);
+
+        $this->entityManager->method('getUnitOfWork')->willReturn($this->unitOfWork);
+
+        $this->processor = new UserRbacProcessor(
+            $this->persistProcessor,
+            $this->entityManager,
+            $this->security,
+        );
+    }
+
+    public function testPatchPromotesUserToAdminDelegatesToPersistProcessor(): void
+    {
+        $target = $this->buildUser(42, 'alice', false);
+        $target->setIsAdmin(true);
+
+        $currentAdmin = $this->buildUser(1, 'admin', true);
+        $this->security->method('getUser')->willReturn($currentAdmin);
+
+        // Cible != user courant : pas de lecture d'UnitOfWork necessaire.
+        $this->unitOfWork->expects(self::never())->method('getOriginalEntityData');
+
+        $this->persistProcessor
+            ->expects(self::once())
+            ->method('process')
+            ->with($target)
+            ->willReturn($target)
+        ;
+
+        $result = $this->processor->process($target, new Patch());
+
+        self::assertSame($target, $result);
+    }
+
+    public function testPatchUpdatesRolesCollectionDelegatesToPersistProcessor(): void
+    {
+        $target = $this->buildUser(42, 'alice', false);
+        $target->addRbacRole(new Role('editor', 'Editor', false));
+
+        $currentAdmin = $this->buildUser(1, 'admin', true);
+        $this->security->method('getUser')->willReturn($currentAdmin);
+
+        $this->persistProcessor
+            ->expects(self::once())
+            ->method('process')
+            ->with($target)
+            ->willReturn($target)
+        ;
+
+        $result = $this->processor->process($target, new Patch());
+
+        self::assertSame($target, $result);
+        self::assertCount(1, $result->getRbacRoles());
+    }
+
+    public function testPatchUpdatesDirectPermissionsCollectionDelegatesToPersistProcessor(): void
+    {
+        $target = $this->buildUser(42, 'alice', false);
+        $target->addDirectPermission(new Permission('core.users.view', 'View', 'core'));
+
+        $currentAdmin = $this->buildUser(1, 'admin', true);
+        $this->security->method('getUser')->willReturn($currentAdmin);
+
+        $this->persistProcessor
+            ->expects(self::once())
+            ->method('process')
+            ->with($target)
+            ->willReturn($target)
+        ;
+
+        $result = $this->processor->process($target, new Patch());
+
+        self::assertSame($target, $result);
+        self::assertCount(1, $result->getDirectPermissions());
+    }
+
+    public function testPatchSelfRemovingAdminThrowsBadRequestHttpException(): void
+    {
+        // Meme identifiant : l'user courant PATCH sa propre ressource.
+        $self = $this->buildUser(1, 'admin', false);
+
+        $this->security->method('getUser')->willReturn($self);
+
+        $this->unitOfWork
+            ->expects(self::once())
+            ->method('getOriginalEntityData')
+            ->with($self)
+            ->willReturn([
+                'id'       => 1,
+                'username' => 'admin',
+                'isAdmin'  => true,
+            ])
+        ;
+
+        $this->persistProcessor->expects(self::never())->method('process');
+
+        $this->expectException(BadRequestHttpException::class);
+        $this->expectExceptionMessage('Vous ne pouvez pas retirer vos propres droits administrateur.');
+
+        $this->processor->process($self, new Patch());
+    }
+
+    public function testPatchAdminDemotingAnotherUserIsAllowed(): void
+    {
+        // Un admin qui retire isAdmin a quelqu'un d'autre : autorise.
+        $target  = $this->buildUser(42, 'alice', false);
+        $current = $this->buildUser(1, 'admin', true);
+
+        $this->security->method('getUser')->willReturn($current);
+
+        // Cible != user courant : pas de verification d'auto-suicide.
+        $this->unitOfWork->expects(self::never())->method('getOriginalEntityData');
+
+        $this->persistProcessor
+            ->expects(self::once())
+            ->method('process')
+            ->with($target)
+            ->willReturn($target)
+        ;
+
+        $result = $this->processor->process($target, new Patch());
+
+        self::assertSame($target, $result);
+    }
+
+    public function testPatchSelfKeepingAdminIsAllowed(): void
+    {
+        // L'user courant se PATCH lui-meme mais garde isAdmin = true :
+        // aucun auto-suicide, on delegue au PersistProcessor.
+        $self = $this->buildUser(1, 'admin', true);
+
+        $this->security->method('getUser')->willReturn($self);
+
+        $this->unitOfWork
+            ->expects(self::once())
+            ->method('getOriginalEntityData')
+            ->with($self)
+            ->willReturn([
+                'id'       => 1,
+                'username' => 'admin',
+                'isAdmin'  => true,
+            ])
+        ;
+
+        $this->persistProcessor
+            ->expects(self::once())
+            ->method('process')
+            ->with($self)
+            ->willReturn($self)
+        ;
+
+        $result = $this->processor->process($self, new Patch());
+
+        self::assertSame($self, $result);
+    }
+
+    /**
+     * Construit un User avec un id force via reflection (les mocks
+     * d'UnitOfWork n'alimentent pas l'id tout seul).
+     */
+    private function buildUser(int $id, string $username, bool $isAdmin): User
+    {
+        $user = new User();
+        $user->setUsername($username);
+        $user->setIsAdmin($isAdmin);
+
+        $refl = new ReflectionClass($user);
+        $prop = $refl->getProperty('id');
+        $prop->setAccessible(true);
+        $prop->setValue($user, $id);
+
+        return $user;
+    }
+}