refactor(core) : RBAC #344 - polish review - narrow rbac read group + fail-fast processors

src/Module/Core/Domain/Entity/User.php

@@ -43,7 +43,7 @@ use Symfony\Component\Serializer\Attribute\SerializedName;
             uriTemplate: '/users/{id}/rbac',
             // TODO ticket #345 : remplacer par is_granted('core.users.manage')
             security: "is_granted('ROLE_ADMIN')",
-            normalizationContext: ['groups' => ['user:list']],
+            normalizationContext: ['groups' => ['user:rbac:read']],
             denormalizationContext: ['groups' => ['user:rbac:write']],
             processor: UserRbacProcessor::class,
         ),
@@ -58,7 +58,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
     #[ORM\Id]
     #[ORM\GeneratedValue]
     #[ORM\Column]
-    #[Groups(['me:read', 'user:list'])]
+    #[Groups(['me:read', 'user:list', 'user:rbac:read'])]
     private ?int $id = null;
 
     #[ORM\Column(length: 180, unique: true)]
@@ -66,7 +66,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
     private ?string $username = null;
 
     #[ORM\Column(name: 'is_admin', options: ['default' => false])]
-    #[Groups(['me:read', 'user:list', 'user:rbac:write'])]
+    #[Groups(['me:read', 'user:list', 'user:rbac:write', 'user:rbac:read'])]
     private bool $isAdmin = false;
 
     /**
@@ -81,7 +81,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
      */
     #[ORM\ManyToMany(targetEntity: Role::class, fetch: 'EAGER')]
     #[ORM\JoinTable(name: 'user_role')]
-    #[Groups(['me:read', 'user:list', 'user:rbac:write'])]
+    #[Groups(['me:read', 'user:list', 'user:rbac:write', 'user:rbac:read'])]
     // La propriete s'appelle `rbacRoles` cote PHP pour ne pas entrer en
     // collision avec UserInterface::getRoles() (qui renvoie list<string>) ;
     // on reexpose la cle JSON sous `roles` via SerializedName pour rester
@@ -99,7 +99,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
      */
     #[ORM\ManyToMany(targetEntity: Permission::class, fetch: 'EAGER')]
     #[ORM\JoinTable(name: 'user_permission')]
-    #[Groups(['me:read', 'user:list', 'user:rbac:write'])]
+    #[Groups(['me:read', 'user:list', 'user:rbac:write', 'user:rbac:read'])]
     private Collection $directPermissions;
 
     #[ORM\Column]
@@ -152,6 +152,8 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
      * la Collection peut ne pas etre hydratee. On se contente d'un calcul
      * base sur un scalaire.
      *
+     * @see getRbacRoles() pour la collection RBAC metier (exposee en JSON sous la cle "roles").
+     *
      * @return list<string>
      */
     public function getRoles(): array

src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php

@@ -10,6 +10,7 @@ use ApiPlatform\State\ProcessorInterface;
 use App\Module\Core\Domain\Entity\Role;
 use App\Module\Core\Domain\Exception\SystemRoleDeletionException;
 use Doctrine\ORM\EntityManagerInterface;
+use LogicException;
 use Symfony\Component\DependencyInjection\Attribute\Autowire;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
@@ -44,12 +45,14 @@ final class RoleProcessor implements ProcessorInterface
     public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed
     {
         if (!$data instanceof Role) {
-            // Securite : si le provider n'a pas fourni un Role, on delegue
+            // Ce processor est wire exclusivement sur les operations Role.
-            // quand meme au processor approprie pour ne pas etouffer
+            // Si on arrive ici avec autre chose, c'est une misconfiguration
-            // silencieusement un bug de configuration.
+            // qu'il faut faire remonter fort.
-            return $operation instanceof DeleteOperationInterface
+            throw new LogicException(sprintf(
-                ? $this->removeProcessor->process($data, $operation, $uriVariables, $context)
+                'RoleProcessor attend une instance de %s, %s recu.',
-                : $this->persistProcessor->process($data, $operation, $uriVariables, $context);
+                Role::class,
+                get_debug_type($data),
+            ));
         }
 
         if ($operation instanceof DeleteOperationInterface) {

src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php

@@ -8,6 +8,7 @@ use ApiPlatform\Metadata\Operation;
 use ApiPlatform\State\ProcessorInterface;
 use App\Module\Core\Domain\Entity\User;
 use Doctrine\ORM\EntityManagerInterface;
+use LogicException;
 use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\DependencyInjection\Attribute\Autowire;
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
@@ -43,9 +44,14 @@ final class UserRbacProcessor implements ProcessorInterface
     public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed
     {
         if (!$data instanceof User) {
-            // Securite : si le provider n'a pas fourni un User, on delegue
+            // Ce processor est wire exclusivement sur l'operation user_rbac_patch
-            // quand meme pour ne pas etouffer un bug de configuration.
+            // qui cible User. Si on arrive ici avec autre chose, c'est une
-            return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
+            // misconfiguration qu'il faut faire remonter fort.
+            throw new LogicException(sprintf(
+                'UserRbacProcessor attend une instance de %s, %s recu.',
+                User::class,
+                get_debug_type($data),
+            ));
         }
 
         $currentUser = $this->security->getUser();

tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php

@@ -12,10 +12,12 @@ use App\Module\Core\Domain\Entity\Role;
 use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\RoleProcessor;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\UnitOfWork;
+use LogicException;
 use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use ReflectionClass;
+use stdClass;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 
@@ -199,6 +201,19 @@ final class RoleProcessorTest extends TestCase
         self::assertSame($role, $result);
     }
 
+    public function testProcessNonRoleDataThrowsLogicException(): void
+    {
+        // Garde-fou contre une misconfiguration : ce processor est wire
+        // exclusivement sur les operations Role.
+        $this->persistProcessor->expects(self::never())->method('process');
+        $this->removeProcessor->expects(self::never())->method('process');
+
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage('RoleProcessor attend une instance de');
+
+        $this->processor->process(new stdClass(), new Patch());
+    }
+
     /**
      * Positionne l'id d'un Role via reflection pour simuler une entite deja
      * persistee (les mocks d'UnitOfWork n'alimentent pas l'id tout seul).

tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php

@@ -12,10 +12,12 @@ 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 LogicException;
 use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use ReflectionClass;
+use stdClass;
 use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 
@@ -196,6 +198,18 @@ final class UserRbacProcessorTest extends TestCase
         self::assertSame($self, $result);
     }
 
+    public function testProcessNonUserDataThrowsLogicException(): void
+    {
+        // Garde-fou contre une misconfiguration : ce processor est wire
+        // exclusivement sur l'operation user_rbac_patch (cible User).
+        $this->persistProcessor->expects(self::never())->method('process');
+
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage('UserRbacProcessor attend une instance de');
+
+        $this->processor->process(new stdClass(), new Patch());
+    }
+
     /**
      * Construit un User avec un id force via reflection (les mocks
      * d'UnitOfWork n'alimentent pas l'id tout seul).