Skip to content

Commit 17da1f4

Browse files
tPl0chtPl0ch
committed
[Security] Make it possible to give voters a weight in consensus decisions
After watching the "Dig in Security with Symfony" talk at SymfonyCon today, I realized that there was no weighting mechanism in the current implementation. It is not always true that each voter has the equal weight within a majority vote. This feature PR accounts for that in a BC way. | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | None | License | MIT | Doc PR | None Commit recap from rebase: - Introduced the `WeightedVoterInterface` that extends the `VoterInterface` - Added the abstract `WeightedVoter` as an easily usable base class - Added the `Weight` decorator to extend existing Voters with weighting - Refactored test structures for `Voter` - Added missing methods in `Weight` decorator required by `VoterInterface` - Added test case for `Weight` decorator - Added test case for abstract `WeightedVoter` - Added feature tests to `AccessDecisionManagerTest` - Implemented feature in `AccessDecisionManager` - Fixed CodingStandards
1 parent dd78303 commit 17da1f4

File tree

9 files changed

+395
-38
lines changed

9 files changed

+395
-38
lines changed

‎src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
1515
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
16+
use Symfony\Component\Security\Core\Authorization\Voter\WeightedVoterInterface;
17+
use Symfony\Component\Security\Core\Exception\RuntimeException;
1618

1719
/**
1820
* AccessDecisionManager is the base class for all access decision managers
@@ -155,16 +157,17 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje
155157
$grant = 0;
156158
$deny = 0;
157159
foreach ($this->voters as $voter) {
160+
$weight = $this->getWeightForVoter($voter);
158161
$result = $voter->vote($token, $object, $attributes);
159162

160163
switch ($result) {
161164
case VoterInterface::ACCESS_GRANTED:
162-
++$grant;
165+
$grant += $weight;
163166

164167
break;
165168

166169
case VoterInterface::ACCESS_DENIED:
167-
++$deny;
170+
$deny += $weight;
168171

169172
break;
170173
}
@@ -220,4 +223,25 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje
220223

221224
return $this->allowIfAllAbstainDecisions;
222225
}
226+
227+
/**
228+
* @param VoterInterface $voter
229+
*
230+
* @return int
231+
*
232+
* @throws RuntimeException
233+
*/
234+
private function getWeightForVoter(VoterInterface $voter)
235+
{
236+
$weight = 1;
237+
if ($voter instanceof WeightedVoterInterface) {
238+
$weight = (int) $voter->getWeight();
239+
}
240+
241+
if ($weight < 1) {
242+
throw new RuntimeException(sprintf('Weighted voter of class "%s" needs to have an integer weight >= 1', get_class($voter)));
243+
}
244+
245+
return $weight;
246+
}
223247
}

‎src/Symfony/Component/Security/Core/Authorization/Voter/Decorator/Weight.php

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
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\Component\Security\Core\Authorization\Voter\Decorator;
13+
14+
use InvalidArgumentException;
15+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
16+
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
17+
use Symfony\Component\Security\Core\Authorization\Voter\WeightedVoterInterface;
18+
19+
/**
20+
* A decorator to decorate existing voters with the weighted feature.
21+
*
22+
* @author Thomas Ploch <profiploch@gmail.com>
23+
*/
24+
final class Weight implements WeightedVoterInterface
25+
{
26+
private $voter;
27+
28+
private $weight;
29+
30+
/**
31+
* Weight constructor.
32+
*
33+
* @param VoterInterface $voter
34+
* @param int $weight
35+
*
36+
* @throws InvalidArgumentException
37+
*/
38+
public function __construct(VoterInterface $voter, $weight)
39+
{
40+
$this->voter = $voter;
41+
$this->weight = (int) $weight;
42+
43+
if ($this->weight < 1) {
44+
throw new InvalidArgumentException(sprintf('Weight decorator for class "%s" needs to have an integer weight >= 1', get_class($this->voter)));
45+
}
46+
}
47+
48+
/**
49+
* {@inheritdoc}
50+
*/
51+
public function vote(TokenInterface $token, $subject, array $attributes)
52+
{
53+
return $this->voter->vote($token, $subject, $attributes);
54+
}
55+
56+
/**
57+
* {@inheritdoc}
58+
*/
59+
public function getWeight()
60+
{
61+
return $this->weight;
62+
}
63+
64+
/**
65+
* {@inheritdoc}
66+
*/
67+
public function supportsAttribute($attribute)
68+
{
69+
return $this->voter->supportsAttribute($attribute);
70+
}
71+
72+
/**
73+
* {@inheritdoc}
74+
*/
75+
public function supportsClass($class)
76+
{
77+
return $this->voter->supportsClass($class);
78+
}
79+
}

‎src/Symfony/Component/Security/Core/Authorization/Voter/WeightedVoter.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
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\Component\Security\Core\Authorization\Voter;
13+
14+
/**
15+
* WeightedVoter is an abstract implementation of a weighted voter.
16+
*
17+
* @author Thomas Ploch <profiploch@gmail.com>
18+
*/
19+
abstract class WeightedVoter extends Voter implements WeightedVoterInterface
20+
{
21+
/**
22+
* {@inheritdoc}
23+
*/
24+
abstract public function getWeight();
25+
}

‎src/Symfony/Component/Security/Core/Authorization/Voter/WeightedVoterInterface.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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\Component\Security\Core\Authorization\Voter;
13+
14+
/**
15+
* WeightedVoterInterface is the interface implemented by voters that have a higher weight in decisions.
16+
*
17+
* @author Thomas Ploch <profiploch@gmail.com>
18+
*/
19+
interface WeightedVoterInterface extends VoterInterface
20+
{
21+
/**
22+
* This method provides the weight used to come to a weighted authorization decision.
23+
*
24+
* The weight has to be an integer value >= 1.
25+
*
26+
* @return int
27+
*/
28+
public function getWeight();
29+
}

‎src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,17 @@ public function testStrategies($strategy, $voters, $allowIfAllAbstainDecisions,
7171
$this->assertSame($expected, $manager->decide($token, array('ROLE_FOO')));
7272
}
7373

74+
/**
75+
* @expectedException \Symfony\Component\Security\Core\Exception\RuntimeException
76+
*/
77+
public function testInvalidWeight()
78+
{
79+
$token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
80+
$voter = $this->getWeightedVoter(VoterInterface::ACCESS_GRANTED, 0);
81+
$manager = new AccessDecisionManager(array($voter), 'consensus');
82+
$manager->decide($token, array('ROLE_FOO'));
83+
}
84+
7485
/**
7586
* @dataProvider getStrategiesWith2RolesTests
7687
*/
@@ -138,6 +149,30 @@ public function getStrategyTests()
138149
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(2, 2, 0), false, false, false),
139150
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(2, 2, 1), false, false, false),
140151

152+
// weighted consensus
153+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,1), array()), false, true, true),
154+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,1), array(), true), false, true, true),
155+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,1), array(4)), false, true, true),
156+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,1), array(4), true), false, true, true),
157+
158+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,4), array()), false, true, false),
159+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,4), array(), true), false, true, false),
160+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,4), array(6)), false, true, false),
161+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,4), array(6), true), false, true, false),
162+
163+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array()), false, false, false),
164+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array(), true), false, false, false),
165+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array(4)), false, false, false),
166+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array(4), true), false, false, false),
167+
168+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array()), false, true, true),
169+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array(), true), false, true, true),
170+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array(4)), false, true, true),
171+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(3), array(1,2), array(4), true), false, true, true),
172+
173+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(), array(), array(1,2,4)), true, false, true),
174+
array(AccessDecisionManager::STRATEGY_CONSENSUS, $this->getWeightedVoters(array(), array(), array(1,2,4)), false, false, false),
175+
141176
// unanimous
142177
array(AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(1, 0, 0), false, true, true),
143178
array(AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(1, 0, 1), false, true, true),
@@ -164,6 +199,32 @@ protected function getVoters($grants, $denies, $abstains)
164199
return $voters;
165200
}
166201

202+
protected function getWeightedVoters($grantWeights, $denyWeights, $abstainWeights, $mixInDefaultVoters = false)
203+
{
204+
$voters = array();
205+
$grants = count($grantWeights);
206+
$denies = count($denyWeights);
207+
$abstains = count($abstainWeights);
208+
209+
for ($i = 0; $i < $grants; ++$i) {
210+
$voters[] = $this->getWeightedVoter(VoterInterface::ACCESS_GRANTED, $grantWeights[$i]);
211+
}
212+
for ($i = 0; $i < $denies; ++$i) {
213+
$voters[] = $this->getWeightedVoter(VoterInterface::ACCESS_DENIED, $denyWeights[$i]);
214+
}
215+
for ($i = 0; $i < $abstains; ++$i) {
216+
$voters[] = $this->getWeightedVoter(VoterInterface::ACCESS_ABSTAIN, $abstainWeights[$i]);
217+
}
218+
219+
if (true === $mixInDefaultVoters) {
220+
$voters[] = $this->getVoter(VoterInterface::ACCESS_GRANTED);
221+
$voters[] = $this->getVoter(VoterInterface::ACCESS_DENIED);
222+
$voters[] = $this->getVoter(VoterInterface::ACCESS_ABSTAIN);
223+
}
224+
225+
return $voters;
226+
}
227+
167228
protected function getVoter($vote)
168229
{
169230
$voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface');
@@ -174,6 +235,20 @@ protected function getVoter($vote)
174235
return $voter;
175236
}
176237

238+
protected function getWeightedVoter($vote, $weight)
239+
{
240+
$voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\WeightedVoterInterface');
241+
$voter->expects($this->any())
242+
->method('vote')
243+
->will($this->returnValue($vote));
244+
245+
$voter->expects($this->any())
246+
->method('getWeight')
247+
->will($this->returnValue($weight));
248+
249+
return $voter;
250+
}
251+
177252
protected function getVoterSupportsClass($ret)
178253
{
179254
$voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface');

‎src/Symfony/Component/Security/Core/Tests/Authorization/Voter/BaseVoterTest.php

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
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\Component\Security\Core\Tests\Authorization\Voter;
13+
14+
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
15+
16+
abstract class BaseVoterTest extends \PHPUnit_Framework_TestCase
17+
{
18+
protected $token;
19+
20+
protected function setUp()
21+
{
22+
$this->token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
23+
}
24+
25+
public function getTests()
26+
{
27+
return array(
28+
array(array('EDIT'), VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'),
29+
array(array('CREATE'), VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'),
30+
31+
array(array('DELETE', 'EDIT'), VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access'),
32+
array(array('DELETE', 'CREATE'), VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access'),
33+
34+
array(array('CREATE', 'EDIT'), VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute grants access'),
35+
36+
array(array('DELETE'), VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'),
37+
38+
array(array('EDIT'), VoterInterface::ACCESS_ABSTAIN, $this, 'ACCESS_ABSTAIN if class is not supported'),
39+
40+
array(array('EDIT'), VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null'),
41+
42+
array(array(), VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided'),
43+
);
44+
}
45+
46+
/**
47+
* @dataProvider getTests
48+
*/
49+
public function testVote(array $attributes, $expectedVote, $object, $message)
50+
{
51+
$voter = $this->getVoter();
52+
53+
$this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message);
54+
}
55+
56+
/**
57+
* @return VoterInterface
58+
*/
59+
abstract protected function getVoter();
60+
}

‎src/Symfony/Component/Security/Core/Tests/Authorization/Voter/Decorator/WeightTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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\Component\Security\Core\Tests\Authorization\Voter\Decorator;
13+
14+
use Symfony\Component\Security\Core\Authorization\Voter\Decorator\Weight;
15+
use Symfony\Component\Security\Core\Tests\Authorization\Voter\BaseVoterTest;
16+
use Symfony\Component\Security\Core\Tests\Authorization\Voter\VoterTest_Voter;
17+
18+
class WeightTest extends BaseVoterTest
19+
{
20+
public function testInterface()
21+
{
22+
$voter = $this->getVoter();
23+
24+
$this->assertInstanceOf('\Symfony\Component\Security\Core\Authorization\Voter\WeightedVoterInterface', $voter);
25+
$this->assertInstanceOf('\Symfony\Component\Security\Core\Authorization\Voter\VoterInterface', $voter);
26+
}
27+
28+
public function testWeight()
29+
{
30+
$voter = $this->getVoter();
31+
32+
$this->assertEquals(3, $voter->getWeight());
33+
}
34+
35+
protected function getVoter()
36+
{
37+
$baseVoter = new VoterTest_Voter();
38+
39+
return new Weight($baseVoter, 3);
40+
}
41+
}

0 commit comments

Comments
 (0)