8000 feature #27914 [Security][SecurityBundle] Add voter individual decisi… · symfony/symfony@2a30a69 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2a30a69

Browse files
fabpotfabpot
committed
feature #27914 [Security][SecurityBundle] Add voter individual decisions to profiler (l-vo)
This PR was merged into the 4.2-dev branch. Discussion ---------- [Security][SecurityBundle] Add voter individual decisions to profiler | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | On the profiler (security tab), the decisions made by the AccessDecisionManager are displayed with the voters registered in the application. But there is no information about which voter was really used for each AccessDecisionManager choice and which result the voters returned. This PR allows to display for each AccessDecisionManager choice, the voters implicated and the vote they returned. Screenshot profiler before PR: ![capture d ecran 2018-07-10 a 13 26 46](https://user-images.githubusercontent.com/15314293/42507425-7320156c-8445-11e8-9f73-a91bdae2eca5.png) Screenshot profiler after PR: <img width="1093" alt="capture d ecran 2018-10-10 a 21 35 30" src="https://user-images.githubusercontent.com/15314293/46761807-c16c4a00-ccd5-11e8-85b1-8cc0ea3eb9b8.png"> Commits ------- 8abb056 [Security][SecurityBundle] Add voter individual decisions to profiler
2 parents edcd627 + 8abb056 commit 2a30a69

File tree

17 files changed

+832
-22
lines changed

17 files changed

+832
-22
lines changed

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ CHANGELOG
1616
* Deprecated the `simple_form` and `simple_preauth` authentication listeners, use Guard instead.
1717
* Deprecated the `SimpleFormFactory` and `SimplePreAuthenticationFactory` classes, use Guard instead.
1818
* Added `port` in access_control
19+
* Added individual voter decisions to the profiler
1920

2021
4.1.0
2122
-----

‎src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
2121
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
2222
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
23+
use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter;
2324
use Symfony\Component\Security\Core\Role\Role;
2425
use Symfony\Component\Security\Core\Role\RoleHierarchyInterface;
2526
use Symfony\Component\Security\Core\Role\SwitchUserRole;
@@ -136,12 +137,33 @@ public function collect(Request $request, Response $response, \Exception $except
136137

137138
// collect voters and access decision manager information
138139
if ($this->accessDecisionManager instanceof TraceableAccessDecisionManager) {
139-
$this->data['access_decision_log'] = $this->accessDecisionManager->getDecisionLog();
140140
$this->data['voter_strategy'] = $this->accessDecisionManager->getStrategy();
141141

142142
foreach ($this->accessDecisionManager->getVoters() as $voter) {
143+
if ($voter instanceof TraceableVoter) {
144+
$voter = $voter->getDecoratedVoter();
145+
}
146+
143147
$this->data['voters'][] = $this->hasVarDumper ? new ClassStub(\get_class($voter)) : \get_class($voter);
144148
}
149+
150+
// collect voter details
151+
$decisionLog = $this->accessDecisionManager->getDecisionLog();
152+
foreach ($decisionLog as $key => $log) {
153+
$decisionLog[$key]['voter_details'] = array();
154+
foreach ($log['voterDetails'] as $voterDetail) {
155+
$voterClass = \get_class($voterDetail['voter']);
156+
$classData = $this->hasVarDumper ? new ClassStub($voterClass) : $voterClass;
157+
$decisionLog[$key]['voter_details'][] = array(
158+
'class' => $classData,
159+
'attributes' => $voterDetail['attributes'], // Only displayed for unanimous strategy
160+
'vote' => $voterDetail['vote'],
161+
);
162+
}
163+
unset($decisionLog[$key]['voterDetails']);
164+
}
165+
166+
$this->data['access_decision_log'] = $decisionLog;
145167
} else {
146168
$this->data['access_decision_log'] = array();
147169
$this->data['voter_strategy'] = 'unknown';

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSecurityVotersPass.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
1717
use Symfony\Component\DependencyInjection\ContainerBuilder;
1818
use Symfony\Component\DependencyInjection\Exception\LogicException;
19+
use Symfony\Component\DependencyInjection\Reference;
20+
use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter;
1921
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
2022

2123
/**
@@ -41,13 +43,27 @@ public function process(ContainerBuilder $container)
4143
throw new LogicException('No security voters found. You need to tag at least one with "security.voter".');
4244
}
4345

46+
$debug = $container->getParameter('kernel.debug');
47+
4448
foreach ($voters as $voter) {
45-
$definition = $container->getDefinition((string) $voter);
49+
$voterServiceId = (string) $voter;
50+
$definition = $container->getDefinition($voterServiceId);
51+
4652
$class = $container->getParameterBag()->resolveValue($definition->getClass());
4753

4854
if (!is_a($class, VoterInterface::class, true)) {
4955
throw new LogicException(sprintf('%s must implement the %s when used as a voter.', $class, VoterInterface::class));
5056
}
57+
58+
if ($debug) {
59+
// Decorate original voters with TraceableVoter
60+
$debugVoterServiceId = '.debug.security.voter.'.$voterServiceId;
61+
$container
62+
->register($debugVoterServiceId, TraceableVoter::class)
63+
->setDecoratedService($voterServiceId)
64+
->addArgument(new Reference($debugVoterServiceId.'.inner'))
65+
->addArgument(new Reference('event_dispatcher'));
66+
}
5167
}
5268

5369
$adm = $container->getDefinition('security.access.decision_manager');

‎src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15+
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
16+
use Symfony\Component\Security\Core\Event\VoteEvent;
17+
18+
/**
19+
* Listen to vote events from traceable voters.
20+
*
21+
* @author Laurent VOULLEMIER <laurent.voullemier@gmail.com>
22+
*
23+
* @internal
24+
*/
25+
class VoteListener implements EventSubscriberInterface
26+
{
27+
private $traceableAccessDecisionManager;
28+
29+
public function __construct(TraceableAccessDecisionManager $traceableAccessDecisionManager)
30+
{
31+
$this->traceableAccessDecisionManager = $traceableAccessDecisionManager;
32+
}
33+
34+
/**
35+
* Event dispatched by a voter during access manager decision.
36+
*
37+
* @param VoteEvent $event event with voter data
38+
*/
39+
public function onVoterVote(VoteEvent $event)
40+
{
41+
$this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVote());
42+
}
43+
44+
public static function getSubscribedEvents()
45+
{
46+
return array('debug.security.authorization.vote' => 'onVoterVote');
47+
}
48+
}

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
<argument type="service" id="debug.security.access.decision_manager.inner" />
1212
</service>
1313

14+
<service id="debug.security.voter.vote_listener" class="Symfony\Bundle\SecurityBundle\EventListener\VoteListener">
15+
<tag name="kernel.event_subscriber" />
16+
<argument type="service" id="debug.security.access.decision_manager" />
17+
</service>
18+
1419
<service id="debug.security.firewall" class="Symfony\Bundle\SecurityBundle\Debug\TraceableFirewallListener">
1520
<tag name="kernel.event_subscriber" />
1621
<argument type="service" id="security.firewall.map" />

‎src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@
307307

308308
<tbody>
309309
{% for decision in collector.accessDecisionLog %}
310-
<tr>
310+
<tr class="voter_result">
311311
<td class="font-normal text-small text-muted nowrap">{{ loop.index }}</td>
312312
<td class="font-normal">
313313
{{ decision.result
@@ -331,6 +331,40 @@
331331
</td>
332332
<td>{{ profiler_dump(decision.seek('object')) }}</td>
333333
</tr>
334+
<tr class="voter_details">
335+
<td></td>
336+
<td colspan="3">
337+
{% if decision.voter_details is not empty %}
338+
{% set voter_details_id = 'voter-details-' ~ loop.index %}
339+
<div id="{{ voter_details_id }}" class="sf-toggle-content sf-toggle-hidden">
340+
<table>
341+
<tbody>
342+
{% for voter_detail in decision.voter_details %}
343+
<tr>
344+
<td class="font-normal">{{ profiler_dump(voter_detail['class']) }}</td>
345+
{% if collector.voterStrategy == constant('Symfony\\Component\\Security\\Core\\Authorization\\AccessDecisionManager::STRATEGY_UNANIMOUS') %}
346+
<td class="font-normal text-small">attribute {{ voter_detail['attributes'][0] }}</td>
347+
{% endif %}
348+
<td class="font-normal text-small">
349+
{% if voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %}
350+
ACCESS GRANTED
351+
{% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %}
352+
ACCESS ABSTAIN
353+
{% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %}
354+
ACCESS DENIED
355+
{% else %}
356+
unknown ({{ voter_detail['vote'] }})
357+
{% endif %}
358+
</td>
359+
</tr>
360+
{% endfor %}
361+
</tbody>
362+
</table>
363+
</div>
364+
<a class="btn btn-link text-small sf-toggle" data-toggle-selector="#{{ voter_details_id }}" data-toggle-alt-content="Hide voter details">Show voter details</a>
365+
{% endif %}
366+
</td>
367+
</tr>
334368
{% endfor %}
335369
</tbody>
336370
</table>

‎src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@
1717
use Symfony\Bundle\SecurityBundle\Security\FirewallConfig;
1818
use Symfony\Bundle\SecurityBundle\Security\FirewallMap;
1919
use Symfony\Component\EventDispatcher\EventDispatcher;
20+
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
2021
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
2122
use Symfony\Component\HttpKernel\HttpKernelInterface;
2223
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
2324
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
25+
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
26+
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
27+
use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter;
28+
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
2429
use Symfony\Component\Security\Core\Role\Role;
2530
use Symfony\Component\Security\Core\Role\RoleHierarchy;
2631
use Symfony\Component\Security\Core\Role\SwitchUserRole;
@@ -221,6 +226,137 @@ public function testGetListeners()
221226
$this->addToAssertionCount(1);
222227
}
223228

229+
public function providerCollectDecisionLog(): \Generator
230+
{
231+
$voter1 = $this->getMockBuilder(VoterInterface::class)->getMockForAbstractClass();
232+
$voter2 = $this->getMockBuilder(VoterInterface::class)->getMockForAbstractClass();
233+
234+
$eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass();
235+
$decoratedVoter1 = new TraceableVoter($voter1, $eventDispatcher);
236+
$decoratedVoter2 = new TraceableVoter($voter2, $eventDispatcher);
237+
238+
yield array(
239+
AccessDecisionManager::STRATEGY_AFFIRMATIVE,
240+
array(array(
241+
'attributes' => array('view'),
242+
'object' => new \stdClass(),
243+
'result' => true,
244+
'voterDetails' => array(
245+
array('voter' => $voter1, 'attributes' => array('view'), 'vote' => VoterInterface::ACCESS_ABSTAIN),
246+
array('voter' => $voter2, 'attributes' => array('view'), 'vote 3C99 9; => VoterInterface::ACCESS_ABSTAIN),
247+
),
248+
)),
249+
array($decoratedVoter1, $decoratedVoter1),
250+
array(\get_class($voter1), \get_class($voter2)),
251+
array(array(
252+
'attributes' => array('view'),
253+
'object' => new \stdClass(),
254+
'result' => true,
255+
'voter_details' => array(
256+
array('class' => \get_class($voter1), 'attributes' => array('view'), 'vote' => Vot CF27 erInterface::ACCESS_ABSTAIN),
257+
array('class' => \get_class($voter2), 'attributes' => array('view'), 'vote' => VoterInterface::ACCESS_ABSTAIN),
258+
),
259+
)),
260+
);
261+
262+
yield array(
263+
AccessDecisionManager::STRATEGY_UNANIMOUS,
264+
array(
265+
array(
266+
'attributes' => array('view', 'edit'),
267+
'object' => new \stdClass(),
268+
'result' => false,
269+
'voterDetails' => array(
270+
array('voter' => $voter1, 'attributes' => array('view'), 'vote' => VoterInterface::ACCESS_DENIED),
271+
array('voter' => $voter1, 'attributes' => array('edit'), 'vote' => VoterInterface::ACCESS_DENIED),
272+
array('voter' => $voter2, 'attributes' => array('view'), 'vote' => VoterInterface::ACCESS_GRANTED),
273+
array('voter' => $voter2, 'attributes' => array('edit'), 'vote' => VoterInterface::ACCESS_GRANTED),
274+
),
275+
),
276+
array(
277+
'attributes' => array('update'),
278+
'object' => new \stdClass(),
279+
'result' => true,
280+
'voterDetails' => array(
281+
array('voter' => $voter1, 'attributes' => array('update'), 'vote' => VoterInterface::ACCESS_GRANTED),
282+
array('voter' => $voter2, 'attributes' => array('update'), 'vote' => VoterInterface::ACCESS_GRANTED),
283+
),
284+
),
285+
),
286+
array($decoratedVoter1, $decoratedVoter1),
287+
array(\get_class($voter1), \get_class($voter2)),
288+
array(
289+
array(
290+
'attributes' => array('view', 'edit'),
291+
'object' => new \stdClass(),
292+
'result' => false,
293+
'voter_details' => array(
294+
array('class' => \get_class($voter1), 'attributes' => array('view'), 'vote' => VoterInterface::ACCESS_DENIED),
295+
array('class' => \get_class($voter1), 'attributes' => array('edit'), 'vote' => VoterInterface::ACCESS_DENIED),
296+
array('class' => \get_class($voter2), 'attributes' => array('view'), 'vote' => VoterInterface::ACCESS_GRANTED),
297+
array('class' => \get_class($voter2), 'attributes' => array('edit'), 'vote' => VoterInterface::ACCESS_GRANTED),
298+
),
299+
),
300+
array(
301+
'attributes' => array('update'),
302+
'object' => new \stdClass(),
303+
'result' => true,
304+
'voter_details' => array(
305+
array('class' => \get_class($voter1), 'attributes' => array('update'), 'vote' => VoterInterface::ACCESS_GRANTED),
306+
array('class' => \get_class($voter2), 'attributes' => array('update'), 'vote' => VoterInterface::ACCESS_GRANTED),
307+
),
308+
),
309+
),
310+
);
311+
}
312+
313+
/**
314+
* Test the returned data when AccessDecisionManager is a TraceableAccessDecisionManager.
315+
*
316+
* @param string $strategy strategy returned by the AccessDecisionManager
317+
* @param array $voters voters returned by AccessDecisionManager
318+
* @param array $decisionLog log of the votes and final decisions from AccessDecisionManager
319+
* @param array $expectedVoterClasses expected voter classes returned by the collector
320+
* @param array $expectedDecisionLog expected decision log returned by the collector
321+
*
322+
* @dataProvider providerCollectDecisionLog
323+
*/
324+
public function testCollectDecisionLog(string $strategy, array $decisionLog, array $voters, array $expectedVoterClasses, array $expectedDecisionLog): void
325+
{
326+
$accessDecisionManager = $this
327+
->getMockBuilder(TraceableAccessDecisionManager::class)
328+
->disableOriginalConstructor()
329+
->setMethods(array('getStrategy', 'getVoters', 'getDecisionLog'))
330+
->getMock();
331+
332+
$accessDecisionManager
333+
->expects($this->any())
334+
->method('getStrategy')
335+
->willReturn($strategy);
336+
337+
$accessDecisionManager
338+
->expects($this->any())
339+
->method('getVoters')
340+
->willReturn($voters);
341+
342+
$accessDecisionManager
343+
->expects($this->any())
344+
->method('getDecisionLog')
345+
->willReturn($decisionLog);
346+
347+
$dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager);
348+
$dataCollector->collect($this->getRequest(), $this->getResponse());
349+
350+
$this->assertEquals($dataCollector->getAccessDecisionLog(), $expectedDecisionLog, 'Wrong value returned by getAccessDecisionLog');
351+
352+
$this->assertSame(
353+
array_map(function ($classStub) { return (string) $classStub; }, $dataCollector->getVoters()),
354+
$expectedVoterClasses,
355+
'Wrong value returned by getVoters'
356+
);
357+
$this->assertSame($dataCollector->getVoterStrategy(), $strategy, 'Wrong value returned by getVoterStrategy');
358+
}
359+
224360
public function provideRoles()
225361
{
226362
return array(

0 commit comments

Comments
 (0)
0