yokai-php · yann-eugone · Feb 28, 2020 · Oct 22, 2019 · Oct 22, 2019
diff --git 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
@@ -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
@@ -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
@@ -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();