feat(core) : RBAC #344 - RoleProcessor + gardes systeme et code immuable
This commit is contained in:
@@ -13,6 +13,7 @@ use ApiPlatform\Metadata\GetCollection;
|
|||||||
use ApiPlatform\Metadata\Patch;
|
use ApiPlatform\Metadata\Patch;
|
||||||
use ApiPlatform\Metadata\Post;
|
use ApiPlatform\Metadata\Post;
|
||||||
use App\Module\Core\Domain\Exception\SystemRoleDeletionException;
|
use App\Module\Core\Domain\Exception\SystemRoleDeletionException;
|
||||||
|
use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\RoleProcessor;
|
||||||
use App\Module\Core\Infrastructure\Doctrine\DoctrineRoleRepository;
|
use App\Module\Core\Infrastructure\Doctrine\DoctrineRoleRepository;
|
||||||
use Doctrine\Common\Collections\ArrayCollection;
|
use Doctrine\Common\Collections\ArrayCollection;
|
||||||
use Doctrine\Common\Collections\Collection;
|
use Doctrine\Common\Collections\Collection;
|
||||||
@@ -47,16 +48,19 @@ use Symfony\Component\Validator\Constraints as Assert;
|
|||||||
denormalizationContext: ['groups' => ['role:write']],
|
denormalizationContext: ['groups' => ['role:write']],
|
||||||
// TODO ticket #345 : remplacer par is_granted('core.roles.manage')
|
// TODO ticket #345 : remplacer par is_granted('core.roles.manage')
|
||||||
security: "is_granted('ROLE_ADMIN')",
|
security: "is_granted('ROLE_ADMIN')",
|
||||||
|
processor: RoleProcessor::class,
|
||||||
),
|
),
|
||||||
new Patch(
|
new Patch(
|
||||||
normalizationContext: ['groups' => ['role:read']],
|
normalizationContext: ['groups' => ['role:read']],
|
||||||
denormalizationContext: ['groups' => ['role:write']],
|
denormalizationContext: ['groups' => ['role:write']],
|
||||||
// TODO ticket #345 : remplacer par is_granted('core.roles.manage')
|
// TODO ticket #345 : remplacer par is_granted('core.roles.manage')
|
||||||
security: "is_granted('ROLE_ADMIN')",
|
security: "is_granted('ROLE_ADMIN')",
|
||||||
|
processor: RoleProcessor::class,
|
||||||
),
|
),
|
||||||
new Delete(
|
new Delete(
|
||||||
// TODO ticket #345 : remplacer par is_granted('core.roles.manage')
|
// TODO ticket #345 : remplacer par is_granted('core.roles.manage')
|
||||||
security: "is_granted('ROLE_ADMIN')",
|
security: "is_granted('ROLE_ADMIN')",
|
||||||
|
processor: RoleProcessor::class,
|
||||||
),
|
),
|
||||||
],
|
],
|
||||||
normalizationContext: ['groups' => ['role:read']],
|
normalizationContext: ['groups' => ['role:read']],
|
||||||
@@ -159,6 +163,19 @@ class Role
|
|||||||
return $this->permissions;
|
return $this->permissions;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Setter expose uniquement a la denormalisation API Platform pour
|
||||||
|
* permettre au RoleProcessor de detecter une tentative de modification
|
||||||
|
* du code (garde "code immuable"). Le code reste en pratique fige apres
|
||||||
|
* creation : le processor refuse toute modification via 400.
|
||||||
|
*/
|
||||||
|
public function setCode(string $code): static
|
||||||
|
{
|
||||||
|
$this->code = $code;
|
||||||
|
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Met a jour le libelle affichable du role. Le code reste immuable pour
|
* Met a jour le libelle affichable du role. Le code reste immuable pour
|
||||||
* garantir la stabilite des references cote fixtures et migrations.
|
* garantir la stabilite des references cote fixtures et migrations.
|
||||||
|
|||||||
@@ -0,0 +1,78 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Module\Core\Infrastructure\ApiPlatform\State\Processor;
|
||||||
|
|
||||||
|
use ApiPlatform\Metadata\DeleteOperationInterface;
|
||||||
|
use ApiPlatform\Metadata\Operation;
|
||||||
|
use ApiPlatform\State\ProcessorInterface;
|
||||||
|
use App\Module\Core\Domain\Entity\Role;
|
||||||
|
use App\Module\Core\Domain\Exception\SystemRoleDeletionException;
|
||||||
|
use Doctrine\ORM\EntityManagerInterface;
|
||||||
|
use Symfony\Component\DependencyInjection\Attribute\Autowire;
|
||||||
|
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
|
||||||
|
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Processor applicatif pour l'entite Role.
|
||||||
|
*
|
||||||
|
* Choix d'implementation : une seule classe qui recoit en dependances les deux
|
||||||
|
* processors Doctrine decores (Persist et Remove) et branche l'un ou l'autre
|
||||||
|
* selon le type d'operation. Ce choix reste plus lisible que deux classes
|
||||||
|
* jumelees et reflete la symetrie des gardes metier (immuabilite du `code`
|
||||||
|
* cote ecriture, protection des roles systeme cote suppression).
|
||||||
|
*
|
||||||
|
* Gardes metier :
|
||||||
|
* - DELETE : delegue a Role::ensureDeletable() et traduit la
|
||||||
|
* SystemRoleDeletionException en AccessDeniedHttpException (403).
|
||||||
|
* - POST/PATCH : refuse toute modification du `code` (champ immuable apres
|
||||||
|
* creation), regle uniforme pour les roles systeme ET custom.
|
||||||
|
*
|
||||||
|
* @implements ProcessorInterface<Role, null|Role>
|
||||||
|
*/
|
||||||
|
final class RoleProcessor implements ProcessorInterface
|
||||||
|
{
|
||||||
|
public function __construct(
|
||||||
|
#[Autowire(service: 'api_platform.doctrine.orm.state.persist_processor')]
|
||||||
|
private readonly ProcessorInterface $persistProcessor,
|
||||||
|
#[Autowire(service: 'api_platform.doctrine.orm.state.remove_processor')]
|
||||||
|
private readonly ProcessorInterface $removeProcessor,
|
||||||
|
private readonly EntityManagerInterface $entityManager,
|
||||||
|
) {}
|
||||||
|
|
||||||
|
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
|
||||||
|
// quand meme au processor approprie pour ne pas etouffer
|
||||||
|
// silencieusement un bug de configuration.
|
||||||
|
return $operation instanceof DeleteOperationInterface
|
||||||
|
? $this->removeProcessor->process($data, $operation, $uriVariables, $context)
|
||||||
|
: $this->persistProcessor->process($data, $operation, $uriVariables, $context);
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($operation instanceof DeleteOperationInterface) {
|
||||||
|
try {
|
||||||
|
$data->ensureDeletable();
|
||||||
|
} catch (SystemRoleDeletionException $e) {
|
||||||
|
// Traduction HTTP : le domaine reste pur, l'API renvoie 403.
|
||||||
|
throw new AccessDeniedHttpException($e->getMessage(), $e);
|
||||||
|
}
|
||||||
|
|
||||||
|
return $this->removeProcessor->process($data, $operation, $uriVariables, $context);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Ecriture (POST/PATCH) : verifier l'immuabilite du `code`.
|
||||||
|
// L'UnitOfWork n'expose un etat d'origine que pour les entites deja
|
||||||
|
// managees (PATCH). Pour un POST (entite nouvelle), `getOriginalEntityData`
|
||||||
|
// retourne un tableau vide : aucune comparaison necessaire.
|
||||||
|
$originalData = $this->entityManager->getUnitOfWork()->getOriginalEntityData($data);
|
||||||
|
|
||||||
|
if (isset($originalData['code']) && $originalData['code'] !== $data->getCode()) {
|
||||||
|
throw new BadRequestHttpException("Le code d'un role est immuable apres creation.");
|
||||||
|
}
|
||||||
|
|
||||||
|
return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -245,6 +245,77 @@ final class RoleApiTest extends ApiTestCase
|
|||||||
self::assertNull($em->getRepository(Role::class)->find($id));
|
self::assertNull($em->getRepository(Role::class)->find($id));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testDeleteSystemRoleReturns403(): void
|
||||||
|
{
|
||||||
|
$role = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]);
|
||||||
|
self::assertNotNull($role);
|
||||||
|
|
||||||
|
$client = $this->authenticatedClient('admin', 'admin');
|
||||||
|
$client->request('DELETE', '/api/roles/'.$role->getId());
|
||||||
|
|
||||||
|
self::assertResponseStatusCodeSame(403);
|
||||||
|
|
||||||
|
// Le role systeme doit toujours exister.
|
||||||
|
$em = $this->getEm();
|
||||||
|
$em->clear();
|
||||||
|
self::assertNotNull($em->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testPatchSystemRoleLabelReturns200(): void
|
||||||
|
{
|
||||||
|
$em = $this->getEm();
|
||||||
|
$role = $em->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]);
|
||||||
|
self::assertNotNull($role);
|
||||||
|
$originalLabel = $role->getLabel();
|
||||||
|
$roleId = $role->getId();
|
||||||
|
|
||||||
|
$client = $this->authenticatedClient('admin', 'admin');
|
||||||
|
|
||||||
|
try {
|
||||||
|
$response = $client->request('PATCH', '/api/roles/'.$roleId, [
|
||||||
|
'headers' => ['Content-Type' => 'application/merge-patch+json'],
|
||||||
|
'json' => ['label' => 'Administrateur (modifie test)'],
|
||||||
|
]);
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
$data = $response->toArray();
|
||||||
|
self::assertSame('Administrateur (modifie test)', $data['label']);
|
||||||
|
self::assertSame(SystemRoles::ADMIN_CODE, $data['code']);
|
||||||
|
self::assertTrue($data['isSystem']);
|
||||||
|
} finally {
|
||||||
|
// Restauration defensive du label original pour ne pas polluer
|
||||||
|
// les tests suivants (les fixtures systeme sont partagees).
|
||||||
|
$em = $this->getEm();
|
||||||
|
|
||||||
|
/** @var null|Role $reloaded */
|
||||||
|
$reloaded = $em->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]);
|
||||||
|
if (null !== $reloaded && $reloaded->getLabel() !== $originalLabel) {
|
||||||
|
$reloaded->setLabel($originalLabel);
|
||||||
|
$em->flush();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testPatchRoleCodeChangeReturns400(): void
|
||||||
|
{
|
||||||
|
$role = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => 'test_editor']);
|
||||||
|
self::assertNotNull($role);
|
||||||
|
|
||||||
|
$client = $this->authenticatedClient('admin', 'admin');
|
||||||
|
$client->request('PATCH', '/api/roles/'.$role->getId(), [
|
||||||
|
'headers' => ['Content-Type' => 'application/merge-patch+json'],
|
||||||
|
'json' => ['code' => 'test_editor_renamed'],
|
||||||
|
]);
|
||||||
|
|
||||||
|
self::assertResponseStatusCodeSame(400);
|
||||||
|
|
||||||
|
// Verification cote base : le code d'origine n'a pas bouge.
|
||||||
|
$em = $this->getEm();
|
||||||
|
$em->clear();
|
||||||
|
self::assertNotNull($em->getRepository(Role::class)->findOneBy(['code' => 'test_editor']));
|
||||||
|
self::assertNull($em->getRepository(Role::class)->findOneBy(['code' => 'test_editor_renamed']));
|
||||||
|
}
|
||||||
|
|
||||||
public function testUnauthenticatedGetCollectionReturns401(): void
|
public function testUnauthenticatedGetCollectionReturns401(): void
|
||||||
{
|
{
|
||||||
$client = self::createClient();
|
$client = self::createClient();
|
||||||
|
|||||||
@@ -0,0 +1,212 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Tests\Module\Core\Infrastructure\ApiPlatform\State\Processor;
|
||||||
|
|
||||||
|
use ApiPlatform\Metadata\Delete;
|
||||||
|
use ApiPlatform\Metadata\Patch;
|
||||||
|
use ApiPlatform\Metadata\Post;
|
||||||
|
use ApiPlatform\State\ProcessorInterface;
|
||||||
|
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 PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations;
|
||||||
|
use PHPUnit\Framework\MockObject\MockObject;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use ReflectionClass;
|
||||||
|
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
|
||||||
|
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tests unitaires du RoleProcessor : couvre les gardes metier
|
||||||
|
* (immuabilite du code, refus de suppression des roles systeme) et la
|
||||||
|
* delegation aux processors Doctrine decores.
|
||||||
|
*
|
||||||
|
* @internal
|
||||||
|
*/
|
||||||
|
#[AllowMockObjectsWithoutExpectations]
|
||||||
|
final class RoleProcessorTest extends TestCase
|
||||||
|
{
|
||||||
|
private MockObject&ProcessorInterface $persistProcessor;
|
||||||
|
private MockObject&ProcessorInterface $removeProcessor;
|
||||||
|
private EntityManagerInterface&MockObject $entityManager;
|
||||||
|
private MockObject&UnitOfWork $unitOfWork;
|
||||||
|
private RoleProcessor $processor;
|
||||||
|
|
||||||
|
protected function setUp(): void
|
||||||
|
{
|
||||||
|
$this->persistProcessor = $this->createMock(ProcessorInterface::class);
|
||||||
|
$this->removeProcessor = $this->createMock(ProcessorInterface::class);
|
||||||
|
$this->entityManager = $this->createMock(EntityManagerInterface::class);
|
||||||
|
$this->unitOfWork = $this->createMock(UnitOfWork::class);
|
||||||
|
|
||||||
|
$this->entityManager->method('getUnitOfWork')->willReturn($this->unitOfWork);
|
||||||
|
|
||||||
|
$this->processor = new RoleProcessor(
|
||||||
|
$this->persistProcessor,
|
||||||
|
$this->removeProcessor,
|
||||||
|
$this->entityManager,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testDeleteCustomRoleDelegatesToRemoveProcessor(): void
|
||||||
|
{
|
||||||
|
$role = new Role('editor', 'Editor', false);
|
||||||
|
|
||||||
|
$this->removeProcessor
|
||||||
|
->expects(self::once())
|
||||||
|
->method('process')
|
||||||
|
->with($role)
|
||||||
|
->willReturn(null)
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->persistProcessor->expects(self::never())->method('process');
|
||||||
|
|
||||||
|
$result = $this->processor->process($role, new Delete());
|
||||||
|
|
||||||
|
self::assertNull($result);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testDeleteSystemRoleThrowsAccessDeniedHttpException(): void
|
||||||
|
{
|
||||||
|
$role = new Role('admin', 'Admin', true);
|
||||||
|
|
||||||
|
$this->removeProcessor->expects(self::never())->method('process');
|
||||||
|
$this->persistProcessor->expects(self::never())->method('process');
|
||||||
|
|
||||||
|
$this->expectException(AccessDeniedHttpException::class);
|
||||||
|
|
||||||
|
$this->processor->process($role, new Delete());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testPostCreatesCustomRoleDelegatesToPersistProcessor(): void
|
||||||
|
{
|
||||||
|
$role = new Role('editor', 'Editor', false);
|
||||||
|
|
||||||
|
// Entite nouvelle : l'UnitOfWork n'a pas d'etat d'origine.
|
||||||
|
$this->unitOfWork
|
||||||
|
->expects(self::once())
|
||||||
|
->method('getOriginalEntityData')
|
||||||
|
->with($role)
|
||||||
|
->willReturn([])
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->persistProcessor
|
||||||
|
->expects(self::once())
|
||||||
|
->method('process')
|
||||||
|
->with($role)
|
||||||
|
->willReturn($role)
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->removeProcessor->expects(self::never())->method('process');
|
||||||
|
|
||||||
|
$result = $this->processor->process($role, new Post());
|
||||||
|
|
||||||
|
self::assertSame($role, $result);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testPatchWithChangedCodeThrowsBadRequestHttpException(): void
|
||||||
|
{
|
||||||
|
// L'entite arrive avec le nouveau code deja applique par le denormalizer.
|
||||||
|
$role = new Role('editor_renamed', 'Editor', false);
|
||||||
|
$this->setRoleId($role, 42);
|
||||||
|
|
||||||
|
$this->unitOfWork
|
||||||
|
->expects(self::once())
|
||||||
|
->method('getOriginalEntityData')
|
||||||
|
->with($role)
|
||||||
|
->willReturn([
|
||||||
|
'id' => 42,
|
||||||
|
'code' => 'editor',
|
||||||
|
'label' => 'Editor',
|
||||||
|
'isSystem' => false,
|
||||||
|
])
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->persistProcessor->expects(self::never())->method('process');
|
||||||
|
$this->removeProcessor->expects(self::never())->method('process');
|
||||||
|
|
||||||
|
$this->expectException(BadRequestHttpException::class);
|
||||||
|
$this->expectExceptionMessage("Le code d'un role est immuable apres creation.");
|
||||||
|
|
||||||
|
$this->processor->process($role, new Patch());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testPatchWithUnchangedCodeDelegatesToPersistProcessor(): void
|
||||||
|
{
|
||||||
|
$role = new Role('editor', 'Editor modifie', false, 'desc');
|
||||||
|
$this->setRoleId($role, 42);
|
||||||
|
|
||||||
|
$this->unitOfWork
|
||||||
|
->expects(self::once())
|
||||||
|
->method('getOriginalEntityData')
|
||||||
|
->with($role)
|
||||||
|
->willReturn([
|
||||||
|
'id' => 42,
|
||||||
|
'code' => 'editor',
|
||||||
|
'label' => 'Editor',
|
||||||
|
'isSystem' => false,
|
||||||
|
])
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->persistProcessor
|
||||||
|
->expects(self::once())
|
||||||
|
->method('process')
|
||||||
|
->with($role)
|
||||||
|
->willReturn($role)
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->removeProcessor->expects(self::never())->method('process');
|
||||||
|
|
||||||
|
$result = $this->processor->process($role, new Patch());
|
||||||
|
|
||||||
|
self::assertSame($role, $result);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testPatchSystemRoleLabelDelegatesToPersistProcessor(): void
|
||||||
|
{
|
||||||
|
// Regle uniforme : un role systeme peut voir son label modifie tant
|
||||||
|
// que son code reste inchange. Seul le DELETE est bloque.
|
||||||
|
$role = new Role('admin', 'Administrateur', true);
|
||||||
|
$this->setRoleId($role, 1);
|
||||||
|
|
||||||
|
$this->unitOfWork
|
||||||
|
->expects(self::once())
|
||||||
|
->method('getOriginalEntityData')
|
||||||
|
->with($role)
|
||||||
|
->willReturn([
|
||||||
|
'id' => 1,
|
||||||
|
'code' => 'admin',
|
||||||
|
'label' => 'Admin',
|
||||||
|
'isSystem' => true,
|
||||||
|
])
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->persistProcessor
|
||||||
|
->expects(self::once())
|
||||||
|
->method('process')
|
||||||
|
->with($role)
|
||||||
|
->willReturn($role)
|
||||||
|
;
|
||||||
|
|
||||||
|
$this->removeProcessor->expects(self::never())->method('process');
|
||||||
|
|
||||||
|
$result = $this->processor->process($role, new Patch());
|
||||||
|
|
||||||
|
self::assertSame($role, $result);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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).
|
||||||
|
*/
|
||||||
|
private function setRoleId(Role $role, int $id): void
|
||||||
|
{
|
||||||
|
$refl = new ReflectionClass($role);
|
||||||
|
$prop = $refl->getProperty('id');
|
||||||
|
$prop->setAccessible(true);
|
||||||
|
$prop->setValue($role, $id);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user