From 9c63f391dce48f48edaf859cc8eca857c9625e5a Mon Sep 17 00:00:00 2001 From: Mathieu MARCHOIS Date: Thu, 8 Aug 2024 13:28:40 +0200 Subject: [PATCH] Restrict publication (#910) --- .../CanUserPublishRegulation.php | 28 ++++++++ .../Fragments/AddMeasureController.php | 1 + .../PublishRegulationController.php | 5 ++ .../Regulation/RegulationDetailController.php | 5 +- .../Voter/RegulationOrderRecordVoter.php | 51 ++++++++++++++ templates/regulation/detail.html.twig | 20 +++--- .../fragments/_measure.added.stream.html.twig | 2 +- .../fragments/_publication_button.html.twig | 26 +++---- .../PublishRegulationControllerTest.php | 12 +++- .../RegulationDetailControllerTest.php | 12 +++- .../CanUserPublishRegulationTest.php | 68 +++++++++++++++++++ 11 files changed, 203 insertions(+), 27 deletions(-) create mode 100644 src/Domain/User/Specification/CanUserPublishRegulation.php create mode 100644 src/Infrastructure/Security/Voter/RegulationOrderRecordVoter.php create mode 100644 tests/Unit/Domain/User/Specification/CanUserPublishRegulationTest.php diff --git a/src/Domain/User/Specification/CanUserPublishRegulation.php b/src/Domain/User/Specification/CanUserPublishRegulation.php new file mode 100644 index 000000000..d2d6c04fd --- /dev/null +++ b/src/Domain/User/Specification/CanUserPublishRegulation.php @@ -0,0 +1,28 @@ +getOrganization(); + + foreach ($user->getOrganizationUsers() as $userOrganization) { + if ($userOrganization->uuid !== $organization->getUuid()) { + continue; + } + + return \in_array(OrganizationRolesEnum::ROLE_ORGA_ADMIN->value, $userOrganization->roles) + || \in_array(OrganizationRolesEnum::ROLE_ORGA_PUBLISHER->value, $userOrganization->roles); + } + + return false; + } +} diff --git a/src/Infrastructure/Controller/Regulation/Fragments/AddMeasureController.php b/src/Infrastructure/Controller/Regulation/Fragments/AddMeasureController.php index 8977906c3..ebe41a392 100644 --- a/src/Infrastructure/Controller/Regulation/Fragments/AddMeasureController.php +++ b/src/Infrastructure/Controller/Regulation/Fragments/AddMeasureController.php @@ -86,6 +86,7 @@ public function __invoke(Request $request, string $uuid): Response context: [ 'measure' => MeasureView::fromEntity($measure), 'regulationOrderRecordUuid' => $uuid, + 'regulationOrderRecord' => $regulationOrderRecord, 'generalInfo' => $generalInfo, 'canDelete' => ($regulationOrderRecord->countMeasures() + 1) > 1, 'preExistingMeasureUuids' => $preExistingMeasureUuids, diff --git a/src/Infrastructure/Controller/Regulation/PublishRegulationController.php b/src/Infrastructure/Controller/Regulation/PublishRegulationController.php index 87d6738cd..29b6763bc 100644 --- a/src/Infrastructure/Controller/Regulation/PublishRegulationController.php +++ b/src/Infrastructure/Controller/Regulation/PublishRegulationController.php @@ -9,6 +9,7 @@ use App\Application\Regulation\Command\PublishRegulationCommand; use App\Domain\Regulation\Exception\RegulationOrderRecordCannotBePublishedException; use App\Domain\Regulation\Specification\CanOrganizationAccessToRegulation; +use App\Infrastructure\Security\Voter\RegulationOrderRecordVoter; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; @@ -49,6 +50,10 @@ public function __invoke(Request $request, string $uuid): Response $regulationOrderRecord = $this->getRegulationOrderRecord($uuid); + if (!$this->security->isGranted(RegulationOrderRecordVoter::PUBLISH, $regulationOrderRecord)) { + throw new AccessDeniedHttpException(); + } + try { $this->commandBus->handle(new PublishRegulationCommand($regulationOrderRecord)); } catch (RegulationOrderRecordCannotBePublishedException) { diff --git a/src/Infrastructure/Controller/Regulation/RegulationDetailController.php b/src/Infrastructure/Controller/Regulation/RegulationDetailController.php index 839ee8ddf..b04c6fbec 100644 --- a/src/Infrastructure/Controller/Regulation/RegulationDetailController.php +++ b/src/Infrastructure/Controller/Regulation/RegulationDetailController.php @@ -6,7 +6,6 @@ use App\Application\QueryBusInterface; use App\Application\Regulation\Query\GetGeneralInfoQuery; -use App\Application\Regulation\Query\GetOrganizationUuidByRegulationOrderRecordQuery; use App\Application\Regulation\Query\Measure\GetMeasuresQuery; use App\Application\Regulation\View\GeneralInfoView; use App\Domain\Regulation\ArrayRegulationMeasures; @@ -55,7 +54,8 @@ public function __invoke(string $uuid): Response throw new AccessDeniedHttpException(); } - $organizationUuid = $this->queryBus->handle(new GetOrganizationUuidByRegulationOrderRecordQuery($uuid)); + $regulationOrderRecord = $this->getRegulationOrderRecord($uuid, requireUserSameOrg: false); + $organizationUuid = $regulationOrderRecord->getOrganizationUuid(); $measures = $this->queryBus->handle(new GetMeasuresQuery($uuid)); $isReadOnly = !($currentUser && $this->canOrganizationAccessToRegulation->isSatisfiedBy($organizationUuid, $currentUser->getOrganizationUuids())); @@ -67,6 +67,7 @@ public function __invoke(string $uuid): Response 'isReadOnly' => $isReadOnly, 'generalInfo' => $generalInfo, 'measures' => $measures, + 'regulationOrderRecord' => $regulationOrderRecord, ]; return new Response( diff --git a/src/Infrastructure/Security/Voter/RegulationOrderRecordVoter.php b/src/Infrastructure/Security/Voter/RegulationOrderRecordVoter.php new file mode 100644 index 000000000..b6b56345e --- /dev/null +++ b/src/Infrastructure/Security/Voter/RegulationOrderRecordVoter.php @@ -0,0 +1,51 @@ +getUser(); + + if (!$user instanceof SymfonyUser) { + return false; + } + + /** @var RegulationOrderRecord $regulationOrderRecord */ + $regulationOrderRecord = $subject; + + return match ($attribute) { + self::PUBLISH => $this->canUserPublishRegulation->isSatisfiedBy($regulationOrderRecord, $user), + default => throw new \LogicException('This code should not be reached!') + }; + } +} diff --git a/templates/regulation/detail.html.twig b/templates/regulation/detail.html.twig index a54f1bbfd..9a99c1d30 100644 --- a/templates/regulation/detail.html.twig +++ b/templates/regulation/detail.html.twig @@ -68,7 +68,7 @@ {% if isDraft %}
  • - {% include 'regulation/fragments/_publication_button.html.twig' with { canPublish, uuid: generalInfo.uuid } only %} + {% include 'regulation/fragments/_publication_button.html.twig' with { canPublish, uuid: generalInfo.uuid, regulationOrderRecord } only %}
    • {% endif %} @@ -127,14 +127,16 @@ {{ parent() }} {% if not isReadOnly %} {% if isDraft %} - {% include "common/confirmation_modal.html.twig" with { - id: 'regulation-publish-modal', - title: 'regulation.publish_modal.title'|trans, - buttons: [ - { label: 'common.publish'|trans, attr: {type: 'submit', class: 'fr-btn'} }, - { label: 'common.do_not_publish'|trans, attr: {value: 'close', class: 'fr-btn fr-btn--secondary'} }, - ], - } only %} + {% if is_granted(constant('App\\Infrastructure\\Security\\Voter\\RegulationOrderRecordVoter::PUBLISH'), regulationOrderRecord) %} + {% include "common/confirmation_modal.html.twig" with { + id: 'regulation-publish-modal', + title: 'regulation.publish_modal.title'|trans, + buttons: [ + { label: 'common.publish'|trans, attr: {type: 'submit', class: 'fr-btn'} }, + { label: 'common.do_not_publish'|trans, attr: {value: 'close', class: 'fr-btn fr-btn--secondary'} }, + ], + } only %} + {% endif %} {% include "common/confirmation_modal.html.twig" with { id: 'measure-delete-modal', diff --git a/templates/regulation/fragments/_measure.added.stream.html.twig b/templates/regulation/fragments/_measure.added.stream.html.twig index a195d8a7f..97b2801bf 100644 --- a/templates/regulation/fragments/_measure.added.stream.html.twig +++ b/templates/regulation/fragments/_measure.added.stream.html.twig @@ -33,6 +33,6 @@ diff --git a/templates/regulation/fragments/_publication_button.html.twig b/templates/regulation/fragments/_publication_button.html.twig index e414ac60a..a47dd7df6 100644 --- a/templates/regulation/fragments/_publication_button.html.twig +++ b/templates/regulation/fragments/_publication_button.html.twig @@ -1,14 +1,16 @@ -{% if canPublish %} - {% set publishCsrfToken = csrf_token('publish-regulation') %} -
    - - +{% if is_granted(constant('App\\Infrastructure\\Security\\Voter\\RegulationOrderRecordVoter::PUBLISH'), regulationOrderRecord) %} + {% if canPublish %} + {% set publishCsrfToken = csrf_token('publish-regulation') %} + + + - - - -{% else %} - + +
    + + {% else %} + + {% endif %} {% endif %} diff --git a/tests/Integration/Infrastructure/Controller/Regulation/PublishRegulationControllerTest.php b/tests/Integration/Infrastructure/Controller/Regulation/PublishRegulationControllerTest.php index 72deaf334..8f1ae7a26 100644 --- a/tests/Integration/Infrastructure/Controller/Regulation/PublishRegulationControllerTest.php +++ b/tests/Integration/Infrastructure/Controller/Regulation/PublishRegulationControllerTest.php @@ -15,7 +15,7 @@ final class PublishRegulationControllerTest extends AbstractWebTestCase public function testPublish(): void { - $client = $this->login(); + $client = $this->login(UserFixture::MAIN_ORG_ADMIN_EMAIL); $client->request('POST', '/regulations/' . RegulationOrderRecordFixture::UUID_TYPICAL . '/publish', [ 'token' => $this->generateCsrfToken($client, 'publish-regulation'), ]); @@ -25,6 +25,16 @@ public function testPublish(): void $this->assertRouteSame('app_regulation_detail', ['uuid' => RegulationOrderRecordFixture::UUID_TYPICAL]); } + public function testPublishAsContributor(): void + { + $client = $this->login(); + $client->request('POST', '/regulations/' . RegulationOrderRecordFixture::UUID_TYPICAL . '/publish', [ + 'token' => $this->generateCsrfToken($client, 'publish-regulation'), + ]); + + $this->assertResponseStatusCodeSame(403); + } + public function testCannotBePublishedBecauseNoMeasures(): void { $client = $this->login(); diff --git a/tests/Integration/Infrastructure/Controller/Regulation/RegulationDetailControllerTest.php b/tests/Integration/Infrastructure/Controller/Regulation/RegulationDetailControllerTest.php index 522572bb7..1fa55e9a4 100644 --- a/tests/Integration/Infrastructure/Controller/Regulation/RegulationDetailControllerTest.php +++ b/tests/Integration/Infrastructure/Controller/Regulation/RegulationDetailControllerTest.php @@ -12,9 +12,9 @@ final class RegulationDetailControllerTest extends AbstractWebTestCase { - public function testDraftRegulationDetail(): void + public function testDraftRegulationDetailAsAdmin(): void { - $client = $this->login(); + $client = $this->login(UserFixture::MAIN_ORG_ADMIN_EMAIL); $crawler = $client->request('GET', '/regulations/' . RegulationOrderRecordFixture::UUID_TYPICAL); $this->assertSecurityHeaders(); @@ -82,6 +82,14 @@ public function testDraftRegulationDetail(): void $this->assertSame('/regulations?tab=temporary', $goBackLink->extract(['href'])[0]); } + public function testDraftRegulationDetailAsContributor(): void + { + $client = $this->login(); + $crawler = $client->request('GET', '/regulations/' . RegulationOrderRecordFixture::UUID_TYPICAL); + + $this->assertSame(0, $crawler->selectButton('Publier')->count()); + } + public function testPermanentRegulationDetail(): void { $client = $this->login(); diff --git a/tests/Unit/Domain/User/Specification/CanUserPublishRegulationTest.php b/tests/Unit/Domain/User/Specification/CanUserPublishRegulationTest.php new file mode 100644 index 000000000..e142a1526 --- /dev/null +++ b/tests/Unit/Domain/User/Specification/CanUserPublishRegulationTest.php @@ -0,0 +1,68 @@ +createMock(Organization::class); + $organization + ->expects(self::once()) + ->method('getUuid') + ->willReturn('c1790745-b915-4fb5-96e7-79b104092a55'); + + $regulationOrderRecord = $this->createMock(RegulationOrderRecord::class); + $regulationOrderRecord + ->expects(self::once()) + ->method('getOrganization') + ->willReturn($organization); + + $symfonyUser = $this->createMock(SymfonyUser::class); + $symfonyUser + ->expects(self::once()) + ->method('getOrganizationUsers') + ->willReturn([ + new OrganizationView('c1790745-b915-4fb5-96e7-79b104092a55', 'Dialog', [OrganizationRolesEnum::ROLE_ORGA_PUBLISHER->value]), + ]); + + $pattern = new CanUserPublishRegulation(); + $this->assertTrue($pattern->isSatisfiedBy($regulationOrderRecord, $symfonyUser)); + } + + public function testCantPublish(): void + { + $organization = $this->createMock(Organization::class); + $organization + ->expects(self::once()) + ->method('getUuid') + ->willReturn('c1790745-b915-4fb5-96e7-79b104092a55'); + + $regulationOrderRecord = $this->createMock(RegulationOrderRecord::class); + $regulationOrderRecord + ->expects(self::once()) + ->method('getOrganization') + ->willReturn($organization); + + $symfonyUser = $this->createMock(SymfonyUser::class); + $symfonyUser + ->expects(self::once()) + ->method('getOrganizationUsers') + ->willReturn([ + new OrganizationView('c1790745-b915-4fb5-96e7-79b104092a55', 'Dialog', [OrganizationRolesEnum::ROLE_ORGA_CONTRIBUTOR->value]), + ]); + + $pattern = new CanUserPublishRegulation(); + $this->assertFalse($pattern->isSatisfiedBy($regulationOrderRecord, $symfonyUser)); + } +}