diff --git a/src/Admin/Extension/WorkflowExtension.php b/src/Admin/Extension/WorkflowExtension.php index 6ffd85e..67831ac 100644 --- a/src/Admin/Extension/WorkflowExtension.php +++ b/src/Admin/Extension/WorkflowExtension.php @@ -7,6 +7,7 @@ use Sonata\AdminBundle\Admin\AdminInterface; use Sonata\AdminBundle\Route\RouteCollection; use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Workflow\Exception\InvalidArgumentException; use Symfony\Component\Workflow\Registry; use Symfony\Component\Workflow\Transition; @@ -77,7 +78,7 @@ public function configureSideMenu( } $subject = $admin->getSubject(); - if (null === $subject) { + if (null === $subject || !$this->isGrantedView($admin, $subject)) { return; } @@ -96,6 +97,17 @@ public function configureSideMenu( } } + /** + * @inheritdoc + */ + public function getAccessMapping(AdminInterface $admin) + { + return [ + 'viewTransitions' => $this->options['view_transitions_role'], + 'applyTransitions' => $this->options['apply_transitions_role'], + ]; + } + /** * @param object $subject * @param string|null $workflowName @@ -124,6 +136,8 @@ protected function configureOptions(OptionsResolver $resolver) 'dropdown_transitions_icon' => 'fa fa-code-fork', 'transitions_default_icon' => null, 'transitions_icons' => [], + 'view_transitions_role' => 'EDIT', + 'apply_transitions_role' => 'EDIT', ]) ->setAllowedTypes('render_actions', ['string[]']) ->setAllowedTypes('workflow_name', ['string', 'null']) @@ -134,6 +148,8 @@ protected function configureOptions(OptionsResolver $resolver) ->setAllowedTypes('dropdown_transitions_icon', ['string', 'null']) ->setAllowedTypes('transitions_default_icon', ['string', 'null']) ->setAllowedTypes('transitions_icons', ['array']) + ->setAllowedTypes('view_transitions_role', ['string']) + ->setAllowedTypes('apply_transitions_role', ['string']) ; } @@ -188,13 +204,16 @@ protected function transitionsDropdown(MenuItemInterface $menu, AdminInterface $ protected function transitionsItem(MenuItemInterface $menu, AdminInterface $admin, Transition $transition, $subject) { $options = [ - 'uri' => $this->generateTransitionUri($admin, $transition, $subject), 'attributes' => [], 'extras' => [ 'translation_domain' => $admin->getTranslationDomain(), ], ]; + if ($this->isGrantedApply($admin, $subject)) { + $options['uri'] = $this->generateTransitionUri($admin, $transition, $subject); + } + if ($icon = $this->getTransitionIcon($transition)) { $options['attributes']['icon'] = $icon; } @@ -234,4 +253,38 @@ protected function generateTransitionUri(AdminInterface $admin, Transition $tran ['transition' => $transition->getName()] ); } + + /** + * @param AdminInterface $admin + * @param object $subject + * + * @return bool + */ + protected function isGrantedView(AdminInterface $admin, $subject) + { + try { + $admin->checkAccess('viewTransitions', $subject); + } catch (AccessDeniedException $exception) { + return false; + } + + return true; + } + + /** + * @param AdminInterface $admin + * @param object $subject + * + * @return bool + */ + protected function isGrantedApply(AdminInterface $admin, $subject) + { + try { + $admin->checkAccess('applyTransitions', $subject); + } catch (AccessDeniedException $exception) { + return false; + } + + return true; + } } diff --git a/src/Controller/WorkflowControllerTrait.php b/src/Controller/WorkflowControllerTrait.php index 76ac307..2981b46 100644 --- a/src/Controller/WorkflowControllerTrait.php +++ b/src/Controller/WorkflowControllerTrait.php @@ -64,7 +64,7 @@ public function workflowApplyTransitionAction(Request $request) } $this->admin->setSubject($existingObject); - $this->admin->checkAccess('edit', $existingObject); + $this->admin->checkAccess('applyTransitions', $existingObject); $objectId = $this->admin->getNormalizedIdentifier($existingObject); diff --git a/tests/Admin/Extension/WorkflowExtensionTest.php b/tests/Admin/Extension/WorkflowExtensionTest.php index 37e7b70..ff28326 100644 --- a/tests/Admin/Extension/WorkflowExtensionTest.php +++ b/tests/Admin/Extension/WorkflowExtensionTest.php @@ -9,6 +9,7 @@ use Sonata\AdminBundle\Route\RouteCollection; use Sonata\AdminBundle\Translator\LabelTranslatorStrategyInterface; use Symfony\Component\Routing\Route; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Workflow\Registry; use Symfony\Component\Workflow\StateMachine; use Yokai\SonataWorkflow\Admin\Extension\WorkflowExtension; @@ -35,7 +36,8 @@ public function testConfigureRoutes() self::assertSame('/pull-request/{id}/workflow/transition/{transition}/apply', $route->getPath()); self::assertNotEmpty($defaults = $route->getDefaults()); self::assertArrayHasKey('_controller', $defaults); - self::assertSame(WorkflowController::class.'::workflowApplyTransitionAction', $defaults['_controller']); + self::assertStringStartsWith(WorkflowController::class, $defaults['_controller']); + self::assertStringEndsWith('workflowApplyTransitionAction', $defaults['_controller']); self::assertArrayHasKey('_sonata_admin', $defaults); self::assertSame('pull_request', $defaults['_sonata_admin']); } @@ -68,6 +70,18 @@ public function testAlterNewInstance() self::assertSame('opened', $pullRequest->getMarking()); } + public function testAccessMapping() + { + /** @var AdminInterface|ObjectProphecy $admin */ + $admin = $this->prophesize(AdminInterface::class); + + $extension = new WorkflowExtension(new Registry()); + self::assertSame( + ['viewTransitions' => 'EDIT', 'applyTransitions' => 'EDIT'], + $extension->getAccessMapping($admin->reveal()) + ); + } + public function testConfigureSideMenuWithoutSubject() { /** @var AdminInterface|ObjectProphecy $admin */ @@ -80,11 +94,25 @@ public function testConfigureSideMenuWithoutSubject() self::assertFalse($menu->hasChildren()); } + public function testConfigureSideMenuWithoutPermission() + { + /** @var AdminInterface|ObjectProphecy $admin */ + $admin = $this->prophesize(AdminInterface::class); + $admin->getSubject()->willReturn($pullRequest = new PullRequest()); + $admin->checkAccess('viewTransitions', $pullRequest)->willThrow(new AccessDeniedException()); + + $extension = new WorkflowExtension(new Registry()); + $extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); + + self::assertFalse($menu->hasChildren()); + } + public function testConfigureSideMenuWithoutWorkflow() { /** @var AdminInterface|ObjectProphecy $admin */ $admin = $this->prophesize(AdminInterface::class); - $admin->getSubject()->willReturn(new PullRequest()); + $admin->getSubject()->willReturn($pullRequest = new PullRequest()); + $admin->checkAccess('viewTransitions', $pullRequest)->shouldBeCalled(); $extension = new WorkflowExtension(new Registry()); $extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); @@ -95,7 +123,7 @@ public function testConfigureSideMenuWithoutWorkflow() /** * @dataProvider markingToTransition */ - public function testConfigureSideMenu($marking, array $transitions) + public function testConfigureSideMenu($marking, array $transitions, $grantedApply) { $pullRequest = new PullRequest(); $pullRequest->setMarking($marking); @@ -108,14 +136,25 @@ public function testConfigureSideMenu($marking, array $transitions) $admin->getTranslationDomain()->willReturn('admin'); $admin->getLabelTranslatorStrategy()->willReturn($labelStrategy->reveal()); $admin->getSubject()->willReturn($pullRequest); + $admin->checkAccess('viewTransitions', $pullRequest)->shouldBeCalled(); + if ($grantedApply) { + $admin->checkAccess('applyTransitions', $pullRequest)->shouldBeCalledTimes(count($transitions)); + } else { + $admin->checkAccess('applyTransitions', $pullRequest)->willThrow(new AccessDeniedException()); + } foreach ($transitions as $transition) { $labelStrategy->getLabel($transition, 'workflow', 'transition') ->shouldBeCalledTimes(1) ->willReturn('workflow.transition.'.$transition); - $admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition]) - ->shouldBeCalledTimes(1) - ->willReturn('/pull-request/42/workflow/transition/'.$transition.'/apply'); + if ($grantedApply) { + $admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition]) + ->shouldBeCalledTimes(1) + ->willReturn('/pull-request/42/workflow/transition/'.$transition.'/apply'); + } else { + $admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition]) + ->shouldNotBeCalled(); + } } $registry = new Registry(); @@ -153,7 +192,11 @@ public function testConfigureSideMenu($marking, array $transitions) } self::assertNotNull($item = $child->getChild('workflow.transition.'.$transition)); - self::assertSame('/pull-request/42/workflow/transition/'.$transition.'/apply', $item->getUri()); + if ($grantedApply) { + self::assertSame('/pull-request/42/workflow/transition/'.$transition.'/apply', $item->getUri()); + } else { + self::assertNull($item->getUri()); + } self::assertSame('admin', $item->getExtra('translation_domain')); self::assertSame($icon, $item->getAttribute('icon')); } @@ -162,10 +205,12 @@ public function testConfigureSideMenu($marking, array $transitions) public function markingToTransition() { - return [ - 'opened' => ['opened', ['start_review']], - 'pending_review' => ['pending_review', ['merge', 'close']], - 'closed' => ['closed', []], - ]; + foreach ([true, false] as $grantedApply) { + $grantedApplyStr = $grantedApply ? 'with links' : 'without links'; + + yield 'opened '.$grantedApplyStr => ['opened', ['start_review'], $grantedApply]; + yield 'pending_review '.$grantedApplyStr => ['pending_review', ['merge', 'close'], $grantedApply]; + yield 'closed '.$grantedApplyStr => ['closed', [], $grantedApply]; + } } } diff --git a/tests/Controller/WorkflowControllerTest.php b/tests/Controller/WorkflowControllerTest.php index d2fe63b..3725e71 100644 --- a/tests/Controller/WorkflowControllerTest.php +++ b/tests/Controller/WorkflowControllerTest.php @@ -124,7 +124,7 @@ public function testWorkflowApplyTransitionActionWorkflowNotFound() $this->admin->getObject(42)->shouldBeCalledTimes(1) ->willReturn($subject = new PullRequest()); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->controller()->workflowApplyTransitionAction($this->request); @@ -144,7 +144,7 @@ public function testWorkflowApplyTransitionActionMissingTransition() $this->admin->getObject(42)->shouldBeCalledTimes(1) ->willReturn($subject = new PullRequest()); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->controller()->workflowApplyTransitionAction($this->request); @@ -167,7 +167,7 @@ public function testWorkflowApplyTransitionActionTransitionException() ->willReturn($subject = new PullRequest()); $this->admin->toString($subject)->shouldBeCalledTimes(1)->willReturn('pr42'); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->controller()->workflowApplyTransitionAction($this->request); @@ -189,7 +189,7 @@ public function testWorkflowApplyTransitionActionModelManagerException() $this->admin->getObject(42)->shouldBeCalledTimes(1) ->willReturn($subject = new PullRequest()); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->admin->update($subject)->shouldBeCalledTimes(1)->willThrow(new ModelManagerException('phpunit error')); @@ -213,7 +213,7 @@ public function testWorkflowApplyTransitionActionLockException() $this->admin->generateObjectUrl('edit', $subject)->shouldBeCalledTimes(1) ->willReturn('/pull-request/42/edit'); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->admin->hasRoute('edit')->shouldBeCalledTimes(1)->willReturn(false); $this->admin->hasRoute('show')->shouldBeCalledTimes(1)->willReturn(false); @@ -248,7 +248,7 @@ public function testWorkflowApplyTransitionActionSuccessXmlHttp() ->willReturn($subject = new PullRequest()); $this->admin->toString($subject)->shouldBeCalledTimes(1)->willReturn('pr42'); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->admin->update($subject)->shouldBeCalledTimes(1)->willReturn($subject); @@ -276,7 +276,7 @@ public function testWorkflowApplyTransitionActionSuccessHttp() ->willReturn($subject = new PullRequest()); $this->admin->toString($subject)->shouldBeCalledTimes(1)->willReturn('pr42'); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->admin->hasRoute('edit')->shouldBeCalledTimes(1)->willReturn(false); $this->admin->hasRoute('show')->shouldBeCalledTimes(1)->willReturn(false); @@ -311,7 +311,7 @@ public function testWorkflowApplyTransitionActionPreApply() $this->admin->getObject(42)->shouldBeCalledTimes(1) ->willReturn($subject = new PullRequest()); $this->admin->setSubject($subject)->shouldBeCalledTimes(1); - $this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1); + $this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1); $this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42); $this->admin->update($subject)->shouldNotBeCalled();