Skip to content

Commit 886e07d

Browse files
yann-eugoneyann-eugone
authored
Merge pull request #15 from yokai-php/issue-14-transition-permissions
Added extension options to handle view & apply transitions permissions
2 parents 3705681 + e696777 commit 886e07d

File tree

4 files changed

+121
-23
lines changed

4 files changed

+121
-23
lines changed

‎src/Admin/Extension/WorkflowExtension.php‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Sonata\AdminBundle\Admin\AdminInterface;
88
use Sonata\AdminBundle\Route\RouteCollection;
99
use Symfony\Component\OptionsResolver\OptionsResolver;
10+
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
1011
use Symfony\Component\Workflow\Exception\InvalidArgumentException;
1112
use Symfony\Component\Workflow\Registry;
1213
use Symfony\Component\Workflow\Transition;
@@ -77,7 +78,7 @@ public function configureSideMenu(
7778
}
7879

7980
$subject = $admin->getSubject();
80-
if (null === $subject) {
81+
if (null === $subject || !$this->isGrantedView($admin, $subject)) {
8182
return;
8283
}
8384

@@ -96,6 +97,17 @@ public function configureSideMenu(
9697
}
9798
}
9899

100+
/**
101+
* @inheritdoc
102+
*/
103+
public function getAccessMapping(AdminInterface $admin)
104+
{
105+
return [
106+
'viewTransitions' => $this->options['view_transitions_role'],
107+
'applyTransitions' => $this->options['apply_transitions_role'],
108+
];
109+
}
110+
99111
/**
100112
* @param object $subject
101113
* @param string|null $workflowName
@@ -124,6 +136,8 @@ protected function configureOptions(OptionsResolver $resolver)
124136
'dropdown_transitions_icon' => 'fa fa-code-fork',
125137
'transitions_default_icon' => null,
126138
'transitions_icons' => [],
139+
'view_transitions_role' => 'EDIT',
140+
'apply_transitions_role' => 'EDIT',
127141
])
128142
->setAllowedTypes('render_actions', ['string[]'])
129143
->setAllowedTypes('workflow_name', ['string', 'null'])
@@ -134,6 +148,8 @@ protected function configureOptions(OptionsResolver $resolver)
134148
->setAllowedTypes('dropdown_transitions_icon', ['string', 'null'])
135149
->setAllowedTypes('transitions_default_icon', ['string', 'null'])
136150
->setAllowedTypes('transitions_icons', ['array'])
151+
->setAllowedTypes('view_transitions_role', ['string'])
152+
->setAllowedTypes('apply_transitions_role', ['string'])
137153
;
138154
}
139155

@@ -188,13 +204,16 @@ protected function transitionsDropdown(MenuItemInterface $menu, AdminInterface $
188204
protected function transitionsItem(MenuItemInterface $menu, AdminInterface $admin, Transition $transition, $subject)
189205
{
190206
$options = [
191-
'uri' => $this->generateTransitionUri($admin, $transition, $subject),
192207
'attributes' => [],
193208
'extras' => [
194209
'translation_domain' => $admin->getTranslationDomain(),
195210
],
196211
];
197212

213+
if ($this->isGrantedApply($admin, $subject)) {
214+
$options['uri'] = $this->generateTransitionUri($admin, $transition, $subject);
215+
}
216+
198217
if ($icon = $this->getTransitionIcon($transition)) {
199218
$options['attributes']['icon'] = $icon;
200219
}
@@ -234,4 +253,38 @@ protected function generateTransitionUri(AdminInterface $admin, Transition $tran
234253
['transition' => $transition->getName()]
235254
);
236255
}
256+
257+
/**
258+
* @param AdminInterface $admin
259+
* @param object $subject
260+
*
261+
* @return bool
262+
*/
263+
protected function isGrantedView(AdminInterface $admin, $subject)
264+
{
265+
try {
266+
$admin->checkAccess('viewTransitions', $subject);
267+
} catch (AccessDeniedException $exception) {
268+
return false;
269+
}
270+
271+
return true;
272+
}
273+
274+
/**
275+
* @param AdminInterface $admin
276+
* @param object $subject
277+
*
278+
* @return bool
279+
*/
280+
protected function isGrantedApply(AdminInterface $admin, $subject)
281+
{
282+
try {
283+
$admin->checkAccess('applyTransitions', $subject);
284+
} catch (AccessDeniedException $exception) {
285+
return false;
286+
}
287+
288+
return true;
289+
}
237290
}

‎src/Controller/WorkflowControllerTrait.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public function workflowApplyTransitionAction(Request $request)
6464
}
6565

6666
$this->admin->setSubject($existingObject);
67-
$this->admin->checkAccess('edit', $existingObject);
67+
$this->admin->checkAccess('applyTransitions', $existingObject);
6868

6969
$objectId = $this->admin->getNormalizedIdentifier($existingObject);
7070

‎tests/Admin/Extension/WorkflowExtensionTest.php‎

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Sonata\AdminBundle\Route\RouteCollection;
1010
use Sonata\AdminBundle\Translator\LabelTranslatorStrategyInterface;
1111
use Symfony\Component\Routing\Route;
12+
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
1213
use Symfony\Component\Workflow\Registry;
1314
use Symfony\Component\Workflow\StateMachine;
1415
use Yokai\SonataWorkflow\Admin\Extension\WorkflowExtension;
@@ -36,7 +37,8 @@ public function testConfigureRoutes()
3637
self::assertSame('/pull-request/{id}/workflow/transition/{transition}/apply', $route->getPath());
3738
self::assertNotEmpty($defaults = $route->getDefaults());
3839
self::assertArrayHasKey('_controller', $defaults);
39-
self::assertSame(WorkflowController::class.'::workflowApplyTransitionAction', $defaults['_controller']);
40+
self::assertStringStartsWith(WorkflowController::class, $defaults['_controller']);
41+
self::assertStringEndsWith('workflowApplyTransitionAction', $defaults['_controller']);
4042
self::assertArrayHasKey('_sonata_admin', $defaults);
4143
self::assertSame('pull_request', $defaults['_sonata_admin']);
4244
}
@@ -69,6 +71,18 @@ public function testAlterNewInstance()
6971
self::assertSame('opened', $pullRequest->getMarking());
7072
}
7173

74+
public function testAccessMapping()
75+
{
76+
/** @var AdminInterface|ObjectProphecy $admin */
77+
$admin = $this->prophesize(AdminInterface::class);
78+
79+
$extension = new WorkflowExtension(new Registry());
80+
self::assertSame(
81+
['viewTransitions' => 'EDIT', 'applyTransitions' => 'EDIT'],
82+
$extension->getAccessMapping($admin->reveal())
83+
);
84+
}
85+
7286
public function testConfigureSideMenuWithoutSubject()
7387
{
7488
/** @var AdminInterface|ObjectProphecy $admin */
@@ -81,11 +95,25 @@ public function testConfigureSideMenuWithoutSubject()
8195
self::assertFalse($menu->hasChildren());
8296
}
8397

98+
public function testConfigureSideMenuWithoutPermission()
99+
{
100+
/** @var AdminInterface|ObjectProphecy $admin */
101+
$admin = $this->prophesize(AdminInterface::class);
102+
$admin->getSubject()->willReturn($pullRequest = new PullRequest());
103+
$admin->checkAccess('viewTransitions', $pullRequest)->willThrow(new AccessDeniedException());
104+
105+
$extension = new WorkflowExtension(new Registry());
106+
$extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit');
107+
108+
self::assertFalse($menu->hasChildren());
109+
}
110+
84111
public function testConfigureSideMenuWithoutWorkflow()
85112
{
86113
/** @var AdminInterface|ObjectProphecy $admin */
87114
$admin = $this->prophesize(AdminInterface::class);
88-
$admin->getSubject()->willReturn(new PullRequest());
115+
$admin->getSubject()->willReturn($pullRequest = new PullRequest());
116+
$admin->checkAccess('viewTransitions', $pullRequest)->shouldBeCalled();
89117

90118
$extension = new WorkflowExtension(new Registry());
91119
$extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit');
@@ -96,7 +124,7 @@ public function testConfigureSideMenuWithoutWorkflow()
96124
/**
97125
* @dataProvider markingToTransition
98126
*/
99-
public function testConfigureSideMenu($marking, array $transitions)
127+
public function testConfigureSideMenu($marking, array $transitions, $grantedApply)
100128
{
101129
$pullRequest = new PullRequest();
102130
$pullRequest->setMarking($marking);
@@ -109,14 +137,25 @@ public function testConfigureSideMenu($marking, array $transitions)
109137
$admin->getTranslationDomain()->willReturn('admin');
110138
$admin->getLabelTranslatorStrategy()->willReturn($labelStrategy->reveal());
111139
$admin->getSubject()->willReturn($pullRequest);
140+
$admin->checkAccess('viewTransitions', $pullRequest)->shouldBeCalled();
141+
if ($grantedApply) {
142+
$admin->checkAccess('applyTransitions', $pullRequest)->shouldBeCalledTimes(count($transitions));
143+
} else {
144+
$admin->checkAccess('applyTransitions', $pullRequest)->willThrow(new AccessDeniedException());
145+
}
112146

113147
foreach ($transitions as $transition) {
114148
$labelStrategy->getLabel($transition, 'workflow', 'transition')
115149
->shouldBeCalledTimes(1)
116150
->willReturn('workflow.transition.'.$transition);
117-
$admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition])
118-
->shouldBeCalledTimes(1)
119-
->willReturn('/pull-request/42/workflow/transition/'.$transition.'/apply');
151+
if ($grantedApply) {
152+
$admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition])
153+
->shouldBeCalledTimes(1)
154+
->willReturn('/pull-request/42/workflow/transition/'.$transition.'/apply');
155+
} else {
156+
$admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition])
157+
->shouldNotBeCalled();
158+
}
120159
}
121160

122161
$registry = new LegacyWorkflowRegistry();
@@ -154,7 +193,11 @@ public function testConfigureSideMenu($marking, array $transitions)
154193
}
155194

156195
self::assertNotNull($item = $child->getChild('workflow.transition.'.$transition));
157-
self::assertSame('/pull-request/42/workflow/transition/'.$transition.'/apply', $item->getUri());
196+
if ($grantedApply) {
197+
self::assertSame('/pull-request/42/workflow/transition/'.$transition.'/apply', $item->getUri());
198+
} else {
199+
self::assertNull($item->getUri());
200+
}
158201
self::assertSame('admin', $item->getExtra('translation_domain'));
159202
self::assertSame($icon, $item->getAttribute('icon'));
160203
}
@@ -163,10 +206,12 @@ public function testConfigureSideMenu($marking, array $transitions)
163206

164207
public function markingToTransition()
165208
{
166-
return [
167-
'opened' => ['opened', ['start_review']],
168-
'pending_review' => ['pending_review', ['merge', 'close']],
169-
'closed' => ['closed', []],
170-
];
209+
foreach ([true, false] as $grantedApply) {
210+
$grantedApplyStr = $grantedApply ? 'with links' : 'without links';
211+
212+
yield 'opened '.$grantedApplyStr => ['opened', ['start_review'], $grantedApply];
213+
yield 'pending_review '.$grantedApplyStr => ['pending_review', ['merge', 'close'], $grantedApply];
214+
yield 'closed '.$grantedApplyStr => ['closed', [], $grantedApply];
215+
}
171216
}
172217
}

‎tests/Controller/WorkflowControllerTest.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public function testWorkflowApplyTransitionActionWorkflowNotFound()
125125
$this->admin->getObject(42)->shouldBeCalledTimes(1)
126126
->willReturn($subject = new PullRequest());
127127
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
128-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
128+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
129129
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
130130

131131
$this->controller()->workflowApplyTransitionAction($this->request);
@@ -145,7 +145,7 @@ public function testWorkflowApplyTransitionActionMissingTransition()
145145
$this->admin->getObject(42)->shouldBeCalledTimes(1)
146146
->willReturn($subject = new PullRequest());
147147
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
148-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
148+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
149149
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
150150

151151
$this->controller()->workflowApplyTransitionAction($this->request);
@@ -168,7 +168,7 @@ public function testWorkflowApplyTransitionActionTransitionException()
168168
->willReturn($subject = new PullRequest());
169169
$this->admin->toString($subject)->shouldBeCalledTimes(1)->willReturn('pr42');
170170
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
171-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
171+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
172172
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
173173

174174
$this->controller()->workflowApplyTransitionAction($this->request);
@@ -190,7 +190,7 @@ public function testWorkflowApplyTransitionActionModelManagerException()
190190
$this->admin->getObject(42)->shouldBeCalledTimes(1)
191191
->willReturn($subject = new PullRequest());
192192
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
193-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
193+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
194194
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
195195
$this->admin->update($subject)->shouldBeCalledTimes(1)->willThrow(new ModelManagerException('phpunit error'));
196196

@@ -214,7 +214,7 @@ public function testWorkflowApplyTransitionActionLockException()
214214
$this->admin->generateObjectUrl('edit', $subject)->shouldBeCalledTimes(1)
215215
->willReturn('/pull-request/42/edit');
216216
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
217-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
217+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
218218
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
219219
$this->admin->hasRoute('edit')->shouldBeCalledTimes(1)->willReturn(false);
220220
$this->admin->hasRoute('show')->shouldBeCalledTimes(1)->willReturn(false);
@@ -249,7 +249,7 @@ public function testWorkflowApplyTransitionActionSuccessXmlHttp()
249249
->willReturn($subject = new PullRequest());
250250
$this->admin->toString($subject)->shouldBeCalledTimes(1)->willReturn('pr42');
251251
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
252-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
252+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
253253
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
254254
$this->admin->update($subject)->shouldBeCalledTimes(1)->willReturn($subject);
255255

@@ -277,7 +277,7 @@ public function testWorkflowApplyTransitionActionSuccessHttp()
277277
->willReturn($subject = new PullRequest());
278278
$this->admin->toString($subject)->shouldBeCalledTimes(1)->willReturn('pr42');
279279
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
280-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
280+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
281281
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
282282
$this->admin->hasRoute('edit')->shouldBeCalledTimes(1)->willReturn(false);
283283
$this->admin->hasRoute('show')->shouldBeCalledTimes(1)->willReturn(false);
@@ -312,7 +312,7 @@ public function testWorkflowApplyTransitionActionPreApply()
312312
$this->admin->getObject(42)->shouldBeCalledTimes(1)
313313
->willReturn($subject = new PullRequest());
314314
$this->admin->setSubject($subject)->shouldBeCalledTimes(1);
315-
$this->admin->checkAccess('edit', $subject)->shouldBeCalledTimes(1);
315+
$this->admin->checkAccess('applyTransitions', $subject)->shouldBeCalledTimes(1);
316316
$this->admin->getNormalizedIdentifier($subject)->shouldBeCalledTimes(1)->willReturn(42);
317317
$this->admin->update($subject)->shouldNotBeCalled();
318318

0 commit comments

Comments
 (0)