From 38b8cf5f64e07b46a507ce868957fb48f43e561c Mon Sep 17 00:00:00 2001 From: Miquel Company Rodriguez Date: Sun, 12 Jul 2015 23:05:03 +0200 Subject: [PATCH 1/7] [Security] Fixed #14049 HighestNotAbstainedVoterStrategy has been added. Also this commit makes access decision strategy extensible. --- .../AddAccessDecisionStrategyPass.php | 36 ++++ .../Resources/config/security.xml | 25 +++ .../Authorization/AccessDecisionManager.php | 167 +++++------------- .../AccessDecisionStrategyInterface.php | 45 +++++ .../Strategy/AbstractDecideStrategy.php | 36 ++++ .../Strategy/DecideAffirmativeStrategy.php | 46 +++++ .../Strategy/DecideConsensusStrategy.php | 62 +++++++ ...DecideHighestNotAbstainedVoterStrategy.php | 36 ++++ .../Strategy/DecideUnanimousStrategy.php | 50 ++++++ .../AccessDecisionManagerTest.php | 50 ++++-- 10 files changed, 417 insertions(+), 136 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php new file mode 100644 index 0000000000000..c9493d7a568c1 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php @@ -0,0 +1,36 @@ +hasDefinition('security.access.decision_manager')) { + return; + } + + $strategies = array(); + foreach ($container->findTaggedServiceIds('security.access_strategy') as $id => $attributes) { + $strategyName = isset($attributes[0]['strategy']) ? $attributes[0]['strategy'] : 0; + $strategies[$strategyName] = new Reference($id); + } + + if (!$strategies) { + throw new LogicException('No access decision strategies found. You need to tag at least one with "security.access_strategy"'); + } + + $container->getDefinition('security.access.decision_manager')->addMethodCall('setStrategies', $strategies); + } + +} \ No newline at end of file diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index b7c1407c1cc56..d17f502e47980 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -29,6 +29,13 @@ Symfony\Component\Security\Core\Authorization\AccessDecisionManager + Symfony\Component\Security\Core\Authorization\Strategy\DecideAffirmativeStrategy + Symfony\Component\Security\Core\Authorization\Strategy\DecideConsensusStrategy + Symfony\Component\Security\Core\Authorization\Strategy\DecideUnanimousStrategy + + Symfony\Component\Security\Core\Authorization\Strategy\DecideHighestNotAbstainedVoterStrategy + + Symfony\Component\Security\Core\Authorization\Voter\RoleVoter Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter Symfony\Component\Security\Core\Authorization\Voter\RoleHierarchyVoter @@ -111,6 +118,24 @@ %security.role_hierarchy.roles% + + + + + + + + + + + + + + + + + diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index e021cc73547c8..1afc87c5b9b4b 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -11,6 +11,10 @@ namespace Symfony\Component\Security\Core\Authorization; +use Symfony\Component\Security\Core\Authorization\Strategy\DecideAffirmativeStrategy; +use Symfony\Component\Security\Core\Authorization\Strategy\DecideConsensusStrategy; +use Symfony\Component\Security\Core\Authorization\Strategy\DecideHighestNotAbstainedVoterStrategy; +use Symfony\Component\Security\Core\Authorization\Strategy\DecideUnanimousStrategy; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; @@ -25,8 +29,10 @@ class AccessDecisionManager implements AccessDecisionManagerInterface const STRATEGY_AFFIRMATIVE = 'affirmative'; const STRATEGY_CONSENSUS = 'consensus'; const STRATEGY_UNANIMOUS = 'unanimous'; + const STRATEGY_HIGHEST_NOT_ABSTAINED = 'highest'; private $voters; + private $strategies; private $strategy; private $allowIfAllAbstainDecisions; private $allowIfEqualGrantedDeniedDecisions; @@ -43,13 +49,9 @@ class AccessDecisionManager implements AccessDecisionManagerInterface */ public function __construct(array $voters = array(), $strategy = self::STRATEGY_AFFIRMATIVE, $allowIfAllAbstainDecisions = false, $allowIfEqualGrantedDeniedDecisions = true) { - $strategyMethod = 'decide'.ucfirst($strategy); - if (!is_callable(array($this, $strategyMethod))) { - throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategy)); - } - + $this->strategies = array(); $this->voters = $voters; - $this->strategy = $strategyMethod; + $this->strategy = $strategy; $this->allowIfAllAbstainDecisions = (bool) $allowIfAllAbstainDecisions; $this->allowIfEqualGrantedDeniedDecisions = (bool) $allowIfEqualGrantedDeniedDecisions; } @@ -65,155 +67,74 @@ public function setVoters(array $voters) } /** - * {@inheritdoc} + * @param mixed $strategies */ - public function decide(TokenInterface $token, array $attributes, $object = null) + public function addStrategy($name,$strategy) { - return $this->{$this->strategy}($token, $attributes, $object); + $this->strategies[$name] = $strategy; } - /** - * {@inheritdoc} - */ - public function supportsAttribute($attribute) + private function getStrategy($strategyName) { - foreach ($this->voters as $voter) { - if ($voter->supportsAttribute($attribute)) { - return true; + if(!array_key_exists($strategyName,$this->strategies)) + { + switch($strategyName){ + case self::STRATEGY_UNANIMOUS: + return new DecideUnanimousStrategy(); + case self::STRATEGY_CONSENSUS: + return new DecideConsensusStrategy(); + case self::STRATEGY_AFFIRMATIVE: + return new DecideAffirmativeStrategy(); + case self::STRATEGY_HIGHEST_NOT_ABSTAINED: + return new DecideHighestNotAbstainedVoterStrategy(); + default: + break; } + } elseif($this->strategies[$strategyName] instanceof AccessDecisionStrategyInterface) { + return $this->strategies[$strategyName]; } - return false; + throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategyName)); } /** * {@inheritdoc} */ - public function supportsClass($class) + public function decide(TokenInterface $token, array $attributes, $object = null) { - foreach ($this->voters as $voter) { - if ($voter->supportsClass($class)) { - return true; - } - } + $strategy = $this->getStrategy($this->strategy); + $strategy->setVoters($this->voters); + $strategy->setAllowIfAllAbstainDecisions($this->allowIfAllAbstainDecisions); + $strategy->setAllowIfEqualGrantedDeniedDecisions($this->allowIfEqualGrantedDeniedDecisions); - return false; + return $strategy->decide($token, $attributes, $object); } /** - * Grants access if any voter returns an affirmative response. - * - * If all voters abstained from voting, the decision will be based on the - * allowIfAllAbstainDecisions property value (defaults to false). + * {@inheritdoc} */ - private function decideAffirmative(TokenInterface $token, array $attributes, $object = null) + public function supportsAttribute($attribute) { - $deny = 0; foreach ($this->voters as $voter) { - $result = $voter->vote($token, $object, $attributes); - switch ($result) { - case VoterInterface::ACCESS_GRANTED: - return true; - - case VoterInterface::ACCESS_DENIED: - ++$deny; - - break; - - default: - break; + if ($voter->supportsAttribute($attribute)) { + return true; } } - if ($deny > 0) { - return false; - } - - return $this->allowIfAllAbstainDecisions; + return false; } /** - * Grants access if there is consensus of granted against denied responses. - * - * Consensus means majority-rule (ignoring abstains) rather than unanimous - * agreement (ignoring abstains). If you require unanimity, see - * UnanimousBased. - * - * If there were an equal number of grant and deny votes, the decision will - * be based on the allowIfEqualGrantedDeniedDecisions property value - * (defaults to true). - * - * If all voters abstained from voting, the decision will be based on the - * allowIfAllAbstainDecisions property value (defaults to false). + * {@inheritdoc} */ - private function decideConsensus(TokenInterface $token, array $attributes, $object = null) + public function supportsClass($class) { - $grant = 0; - $deny = 0; foreach ($this->voters as $voter) { - $result = $voter->vote($token, $object, $attributes); - - switch ($result) { - case VoterInterface::ACCESS_GRANTED: - ++$grant; - - break; - - case VoterInterface::ACCESS_DENIED: - ++$deny; - - break; - } - } - - if ($grant > $deny) { - return true; - } - - if ($deny > $grant) { - return false; - } - - if ($grant > 0) { - return $this->allowIfEqualGrantedDeniedDecisions; - } - - return $this->allowIfAllAbstainDecisions; - } - - /** - * Grants access if only grant (or abstain) votes were received. - * - * If all voters abstained from voting, the decision will be based on the - * allowIfAllAbstainDecisions property value (defaults to false). - */ - private function decideUnanimous(TokenInterface $token, array $attributes, $object = null) - { - $grant = 0; - foreach ($attributes as $attribute) { - foreach ($this->voters as $voter) { - $result = $voter->vote($token, $object, array($attribute)); - - switch ($result) { - case VoterInterface::ACCESS_GRANTED: - ++$grant; - - break; - - case VoterInterface::ACCESS_DENIED: - return false; - - default: - break; - } + if ($voter->supportsClass($class)) { + return true; } } - // no deny votes - if ($grant > 0) { - return true; - } - - return $this->allowIfAllAbstainDecisions; + return false; } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php new file mode 100644 index 0000000000000..50c08f03c83e9 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php @@ -0,0 +1,45 @@ + + */ +interface AccessDecisionStrategyInterface +{ + /** + * Configures the voters. + * + * @param VoterInterface[] $voters An array of VoterInterface instances + */ + public function setVoters(array $voters); + + /** + * Set whether to grant access if all voters abstained or not. + * + * @param bool $allowIfAllAbstainDecisions + */ + public function setAllowIfAllAbstainDecisions($allowIfAllAbstainDecisions); + + /** + * Set whether to grant access if result are equals. + * + * @param bool $allowIfEqualGrantedDeniedDecisions + */ + public function setAllowIfEqualGrantedDeniedDecisions($allowIfEqualGrantedDeniedDecisions); + + /** + * Decides whether the access is possible or not. + * + * @param TokenInterface $token + * @param array $attributes + * @param null $object + * + * @return true if this decision strategy decides that the access can be made + */ + public function decide(TokenInterface $token, array $attributes, $object = null); +} \ No newline at end of file diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php new file mode 100644 index 0000000000000..63368edd4d2d0 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php @@ -0,0 +1,36 @@ +voters = $voters; + } + + /** + * {@inheritdoc} + */ + public function setAllowIfAllAbstainDecisions($allowIfAllAbstainDecisions) + { + $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + } + + /** + * {@inheritdoc} + */ + public function setAllowIfEqualGrantedDeniedDecisions($allowIfEqualGrantedDeniedDecisions) + { + $this->allowIfEqualGrantedDeniedDecisions = $allowIfEqualGrantedDeniedDecisions; + } +} \ No newline at end of file diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php new file mode 100644 index 0000000000000..4d81d24b7c99a --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php @@ -0,0 +1,46 @@ +voters as $voter) { + $result = $voter->vote($token, $object, $attributes); + switch ($result) { + case VoterInterface::ACCESS_GRANTED: + return true; + + case VoterInterface::ACCESS_DENIED: + ++$deny; + + break; + + default: + break; + } + } + + if ($deny > 0) { + return false; + } + + return $this->allowIfAllAbstainDecisions; + } + +} \ No newline at end of file diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php new file mode 100644 index 0000000000000..577c3a0be8279 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php @@ -0,0 +1,62 @@ +voters as $voter) { + $result = $voter->vote($token, $object, $attributes); + + switch ($result) { + case VoterInterface::ACCESS_GRANTED: + ++$grant; + + break; + case VoterInterface::ACCESS_DENIED: + ++$deny; + + break; + } + } + + if ($grant > $deny) { + return true; + } + + if ($deny > $grant) { + return false; + } + + if ($grant > 0) { + return $this->allowIfEqualGrantedDeniedDecisions; + } + + return $this->allowIfAllAbstainDecisions; + } + +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php new file mode 100644 index 0000000000000..cc44540f96799 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php @@ -0,0 +1,36 @@ +voters as $voter) { + $result = $voter->vote($token, $object, $attributes); + switch ($result) { + case VoterInterface::ACCESS_GRANTED: + return true; + case VoterInterface::ACCESS_DENIED: + return false; + default: + break; + } + } + + return $this->allowIfAllAbstainDecisions; + } + +} \ No newline at end of file diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php new file mode 100644 index 0000000000000..7822e1a4cd625 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php @@ -0,0 +1,50 @@ +voters as $voter) { + $result = $voter->vote($token, $object, array($attribute)); + + switch ($result) { + case VoterInterface::ACCESS_GRANTED: + ++$grant; + + break; + + case VoterInterface::ACCESS_DENIED: + return false; + + default: + break; + } + } + } + + // no deny votes + if ($grant > 0) { + return true; + } + + return $this->allowIfAllAbstainDecisions; + } + +} \ No newline at end of file diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index bd876c729f1d8..72ec216c30b6f 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -51,7 +51,25 @@ public function testSupportsAttribute() */ public function testSetUnsupportedStrategy() { - new AccessDecisionManager(array($this->getVoter(VoterInterface::ACCESS_GRANTED)), 'fooBar'); + $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); + $manager = new AccessDecisionManager(array($this->getVoter(VoterInterface::ACCESS_GRANTED)), 'fooBar'); + $manager->decide($token, array('ROLE_FOO')); + + } + + public function testCustomStrategy() + { + $decisionStrategy = $this->getMock('Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface'); + $decisionStrategy->expects($this->once()) + ->method('decide') + ->will($this->returnValue(true)); + + $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); + $manager = new AccessDecisionManager(array($this->getVoter(VoterInterface::ACCESS_GRANTED)), 'testStrategy'); + $manager->addStrategy('testStrategy', $decisionStrategy); + + $this->assertTrue($manager->decide($token, array('ROLE_FOO'))); + } /** @@ -97,12 +115,11 @@ protected function getVoterFor2Roles($token, $vote1, $vote2) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->exactly(2)) - ->method('vote') - ->will($this->returnValueMap(array( - array($token, null, array('ROLE_FOO'), $vote1), - array($token, null, array('ROLE_BAR'), $vote2), - ))) - ; + ->method('vote') + ->will($this->returnValueMap(array( + array($token, null, array('ROLE_FOO'), $vote1), + array($token, null, array('ROLE_BAR'), $vote2), + ))); return $voter; } @@ -139,6 +156,13 @@ public function getStrategyTests() array(AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(0, 0, 2), false, true, false), array(AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(0, 0, 2), true, true, true), + + //highest not abstained strategy + array(AccessDecisionManager::STRATEGY_HIGHEST_NOT_ABSTAINED, $this->getVoters(1, 0, 0), false, false, true), + array(AccessDecisionManager::STRATEGY_HIGHEST_NOT_ABSTAINED, $this->getVoters(0, 1, 0), false, false, false), + array(AccessDecisionManager::STRATEGY_HIGHEST_NOT_ABSTAINED, $this->getVoters(0, 0, 1), false, false, false), + array(AccessDecisionManager::STRATEGY_HIGHEST_NOT_ABSTAINED, $this->getVoters(0, 0, 1), true, false, true), + array(AccessDecisionManager::STRATEGY_HIGHEST_NOT_ABSTAINED, $this->getVoters(1, 1, 1), false, false, true), ); } @@ -162,8 +186,8 @@ protected function getVoter($vote) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->any()) - ->method('vote') - ->will($this->returnValue($vote)); + ->method('vote') + ->will($this->returnValue($vote)); return $voter; } @@ -172,8 +196,8 @@ protected function getVoterSupportsClass($ret) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->any()) - ->method('supportsClass') - ->will($this->returnValue($ret)); + ->method('supportsClass') + ->will($this->returnValue($ret)); return $voter; } @@ -182,8 +206,8 @@ protected function getVoterSupportsAttribute($ret) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->any()) - ->method('supportsAttribute') - ->will($this->returnValue($ret)); + ->method('supportsAttribute') + ->will($this->returnValue($ret)); return $voter; } From 66457b05ee113e58810d0c869fb435576162b8a1 Mon Sep 17 00:00:00 2001 From: Miquel Company Rodriguez Date: Fri, 17 Jul 2015 00:12:25 +0200 Subject: [PATCH 2/7] Make AccessDecisionManager work as a proxy class for other concrete access decision strategies --- .../AddAccessDecisionStrategyPass.php | 36 -------- .../DependencyInjection/MainConfiguration.php | 2 +- .../Resources/config/security.xml | 25 ------ .../Authorization/AccessDecisionManager.php | 83 +++++++------------ .../Strategy/AbstractDecideStrategy.php | 24 ++++-- .../Strategy/DecideAffirmativeStrategy.php | 15 +++- .../Strategy/DecideConsensusStrategy.php | 19 ++++- ...DecideHighestNotAbstainedVoterStrategy.php | 15 +++- .../Strategy/DecideUnanimousStrategy.php | 15 +++- .../AccessDecisionManagerTest.php | 43 +++------- 10 files changed, 118 insertions(+), 159 deletions(-) delete mode 100644 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php deleted file mode 100644 index c9493d7a568c1..0000000000000 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddAccessDecisionStrategyPass.php +++ /dev/null @@ -1,36 +0,0 @@ -hasDefinition('security.access.decision_manager')) { - return; - } - - $strategies = array(); - foreach ($container->findTaggedServiceIds('security.access_strategy') as $id => $attributes) { - $strategyName = isset($attributes[0]['strategy']) ? $attributes[0]['strategy'] : 0; - $strategies[$strategyName] = new Reference($id); - } - - if (!$strategies) { - throw new LogicException('No access decision strategies found. You need to tag at least one with "security.access_strategy"'); - } - - $container->getDefinition('security.access.decision_manager')->addMethodCall('setStrategies', $strategies); - } - -} \ No newline at end of file diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 7b6ac4186ac03..091c16f60e1d4 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -72,7 +72,7 @@ public function getConfigTreeBuilder() ->addDefaultsIfNotSet() ->children() ->enumNode('strategy') - ->values(array(AccessDecisionManager::STRATEGY_AFFIRMATIVE, AccessDecisionManager::STRATEGY_CONSENSUS, AccessDecisionManager::STRATEGY_UNANIMOUS)) + ->values(array(AccessDecisionManager::STRATEGY_AFFIRMATIVE, AccessDecisionManager::STRATEGY_CONSENSUS, AccessDecisionManager::STRATEGY_UNANIMOUS, AccessDecisionManager::STRATEGY_HIGHEST_NOT_ABSTAINED)) ->defaultValue(AccessDecisionManager::STRATEGY_AFFIRMATIVE) ->end() ->booleanNode('allow_if_all_abstain')->defaultFalse()->end() diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index d17f502e47980..b7c1407c1cc56 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -29,13 +29,6 @@ Symfony\Component\Security\Core\Authorization\AccessDecisionManager - Symfony\Component\Security\Core\Authorization\Strategy\DecideAffirmativeStrategy - Symfony\Component\Security\Core\Authorization\Strategy\DecideConsensusStrategy - Symfony\Component\Security\Core\Authorization\Strategy\DecideUnanimousStrategy - - Symfony\Component\Security\Core\Authorization\Strategy\DecideHighestNotAbstainedVoterStrategy - - Symfony\Component\Security\Core\Authorization\Voter\RoleVoter Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter Symfony\Component\Security\Core\Authorization\Voter\RoleHierarchyVoter @@ -118,24 +111,6 @@ %security.role_hierarchy.roles% - - - - - - - - - - - - - - - - - diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 1afc87c5b9b4b..363d29512a4cb 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -32,7 +32,6 @@ class AccessDecisionManager implements AccessDecisionManagerInterface const STRATEGY_HIGHEST_NOT_ABSTAINED = 'highest'; private $voters; - private $strategies; private $strategy; private $allowIfAllAbstainDecisions; private $allowIfEqualGrantedDeniedDecisions; @@ -49,11 +48,10 @@ class AccessDecisionManager implements AccessDecisionManagerInterface */ public function __construct(array $voters = array(), $strategy = self::STRATEGY_AFFIRMATIVE, $allowIfAllAbstainDecisions = false, $allowIfEqualGrantedDeniedDecisions = true) { - $this->strategies = array(); - $this->voters = $voters; - $this->strategy = $strategy; $this->allowIfAllAbstainDecisions = (bool) $allowIfAllAbstainDecisions; $this->allowIfEqualGrantedDeniedDecisions = (bool) $allowIfEqualGrantedDeniedDecisions; + $this->strategy = $this->createStrategy($strategy); + $this->setVoters($voters); } /** @@ -63,38 +61,23 @@ public function __construct(array $voters = array(), $strategy = self::STRATEGY_ */ public function setVoters(array $voters) { - $this->voters = $voters; + $this->strategy->setVoters($voters); } - /** - * @param mixed $strategies - */ - public function addStrategy($name,$strategy) - { - $this->strategies[$name] = $strategy; - } - - private function getStrategy($strategyName) + private function createStrategy($strategyName) { - if(!array_key_exists($strategyName,$this->strategies)) - { - switch($strategyName){ - case self::STRATEGY_UNANIMOUS: - return new DecideUnanimousStrategy(); - case self::STRATEGY_CONSENSUS: - return new DecideConsensusStrategy(); - case self::STRATEGY_AFFIRMATIVE: - return new DecideAffirmativeStrategy(); - case self::STRATEGY_HIGHEST_NOT_ABSTAINED: - return new DecideHighestNotAbstainedVoterStrategy(); - default: - break; - } - } elseif($this->strategies[$strategyName] instanceof AccessDecisionStrategyInterface) { - return $this->strategies[$strategyName]; + switch($strategyName){ + case self::STRATEGY_UNANIMOUS: + return new DecideUnanimousStrategy($this->allowIfAllAbstainDecisions); + case self::STRATEGY_CONSENSUS: + return new DecideConsensusStrategy($this->allowIfEqualGrantedDeniedDecisions,$this->allowIfAllAbstainDecisions); + case self::STRATEGY_AFFIRMATIVE: + return new DecideAffirmativeStrategy($this->allowIfAllAbstainDecisions); + case self::STRATEGY_HIGHEST_NOT_ABSTAINED: + return new DecideHighestNotAbstainedVoterStrategy($this->allowIfAllAbstainDecisions); + default: + throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategyName)); } - - throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategyName)); } /** @@ -102,39 +85,29 @@ private function getStrategy($strategyName) */ public function decide(TokenInterface $token, array $attributes, $object = null) { - $strategy = $this->getStrategy($this->strategy); - $strategy->setVoters($this->voters); - $strategy->setAllowIfAllAbstainDecisions($this->allowIfAllAbstainDecisions); - $strategy->setAllowIfEqualGrantedDeniedDecisions($this->allowIfEqualGrantedDeniedDecisions); - - return $strategy->decide($token, $attributes, $object); + return $this->strategy->decide($token, $attributes, $object); } + /** * {@inheritdoc} */ - public function supportsAttribute($attribute) + public function supportsClass($class) { - foreach ($this->voters as $voter) { - if ($voter->supportsAttribute($attribute)) { - return true; - } - } - - return false; + return $this->strategy->supportsClass($class); } /** - * {@inheritdoc} + * Checks if the access decision manager supports the given attribute. + * + * @param string $attribute An attribute + * + * @return bool true if this decision manager supports the attribute, false otherwise */ - public function supportsClass($class) + public function supportsAttribute($attribute) { - foreach ($this->voters as $voter) { - if ($voter->supportsClass($class)) { - return true; - } - } - - return false; + return $this->strategy->supportsAttribute($attribute); } + + } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php index 63368edd4d2d0..b74b77f91d66a 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php @@ -6,10 +6,6 @@ abstract class AbstractDecideStrategy { protected $voters; - protected $allowIfAllAbstainDecisions; - - protected $allowIfEqualGrantedDeniedDecisions; - /** * {@inheritdoc} */ @@ -21,16 +17,28 @@ public function setVoters(array $voters) /** * {@inheritdoc} */ - public function setAllowIfAllAbstainDecisions($allowIfAllAbstainDecisions) + public function supportsAttribute($attribute) { - $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + foreach ($this->voters as $voter) { + if ($voter->supportsAttribute($attribute)) { + return true; + } + } + + return false; } /** * {@inheritdoc} */ - public function setAllowIfEqualGrantedDeniedDecisions($allowIfEqualGrantedDeniedDecisions) + public function supportsClass($class) { - $this->allowIfEqualGrantedDeniedDecisions = $allowIfEqualGrantedDeniedDecisions; + foreach ($this->voters as $voter) { + if ($voter->supportsClass($class)) { + return true; + } + } + + return false; } } \ No newline at end of file diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php index 4d81d24b7c99a..8e9db2d63cdd5 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php @@ -3,6 +3,7 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; @@ -12,8 +13,20 @@ * If all voters abstained from voting, the decision will be based on the * allowIfAllAbstainDecisions property value (defaults to false). */ -class DecideAffirmativeStrategy extends AbstractDecideStrategy implements AccessDecisionStrategyInterface +class DecideAffirmativeStrategy extends AbstractDecideStrategy implements AccessDecisionManagerInterface { + private $allowIfAllAbstainDecisions; + + /** + * DecideAffirmativeStrategy constructor. + * @param $allowIfAllAbstainDecisions + */ + public function __construct($allowIfAllAbstainDecisions) + { + $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + } + + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php index 577c3a0be8279..6d6c46c6f3279 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php @@ -3,6 +3,7 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; @@ -20,8 +21,24 @@ * If all voters abstained from voting, the decision will be based on the * allowIfAllAbstainDecisions property value (defaults to false). */ -class DecideConsensusStrategy extends AbstractDecideStrategy implements AccessDecisionStrategyInterface +class DecideConsensusStrategy extends AbstractDecideStrategy implements AccessDecisionManagerInterface { + private $allowIfEqualGrantedDeniedDecisions; + + private $allowIfAllAbstainDecisions; + + /** + * DecideConsensusStrategy constructor. + * @param $allowIfEqualGrantedDeniedDecisions + * @param $allowIfAllAbstainDecisions + */ + public function __construct($allowIfEqualGrantedDeniedDecisions, $allowIfAllAbstainDecisions) + { + $this->allowIfEqualGrantedDeniedDecisions = $allowIfEqualGrantedDeniedDecisions; + $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + } + + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php index cc44540f96799..81e1f0f4e9b69 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php @@ -3,6 +3,7 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; @@ -11,8 +12,20 @@ * * If all voters abstain from voting, the decission will be base on the allowIfAllAbstainDecisionsProperty */ -class DecideHighestNotAbstainedVoterStrategy extends AbstractDecideStrategy implements AccessDecisionStrategyInterface +class DecideHighestNotAbstainedVoterStrategy extends AbstractDecideStrategy implements AccessDecisionManagerInterface { + private $allowIfAllAbstainDecisions; + + /** + * DecideHighestNotAbstainedVoterStrategy constructor. + * @param $allowIfAllAbstainDecisions + */ + public function __construct($allowIfAllAbstainDecisions) + { + $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + } + + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php index 7822e1a4cd625..9101fe383d606 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php @@ -3,6 +3,7 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; @@ -12,8 +13,20 @@ * If all voters abstained from voting, the decision will be based on the * allowIfAllAbstainDecisions property value (defaults to false). */ -class DecideUnanimousStrategy extends AbstractDecideStrategy implements AccessDecisionStrategyInterface +class DecideUnanimousStrategy extends AbstractDecideStrategy implements AccessDecisionManagerInterface { + private $allowIfAllAbstainDecisions; + + /** + * DecideUnanimousStrategy constructor. + * @param $allowIfAllAbstainDecisions + */ + public function __construct($allowIfAllAbstainDecisions) + { + $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; + } + + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index 72ec216c30b6f..f0b142bb8e8a7 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -51,25 +51,7 @@ public function testSupportsAttribute() */ public function testSetUnsupportedStrategy() { - $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); - $manager = new AccessDecisionManager(array($this->getVoter(VoterInterface::ACCESS_GRANTED)), 'fooBar'); - $manager->decide($token, array('ROLE_FOO')); - - } - - public function testCustomStrategy() - { - $decisionStrategy = $this->getMock('Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface'); - $decisionStrategy->expects($this->once()) - ->method('decide') - ->will($this->returnValue(true)); - - $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); - $manager = new AccessDecisionManager(array($this->getVoter(VoterInterface::ACCESS_GRANTED)), 'testStrategy'); - $manager->addStrategy('testStrategy', $decisionStrategy); - - $this->assertTrue($manager->decide($token, array('ROLE_FOO'))); - + new AccessDecisionManager(array($this->getVoter(VoterInterface::ACCESS_GRANTED)), 'fooBar'); } /** @@ -115,11 +97,12 @@ protected function getVoterFor2Roles($token, $vote1, $vote2) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->exactly(2)) - ->method('vote') - ->will($this->returnValueMap(array( - array($token, null, array('ROLE_FOO'), $vote1), - array($token, null, array('ROLE_BAR'), $vote2), - ))); + ->method('vote') + ->will($this->returnValueMap(array( + array($token, null, array('ROLE_FOO'), $vote1), + array($token, null, array('ROLE_BAR'), $vote2), + ))) + ; return $voter; } @@ -186,8 +169,8 @@ protected function getVoter($vote) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->any()) - ->method('vote') - ->will($this->returnValue($vote)); + ->method('vote') + ->will($this->returnValue($vote)); return $voter; } @@ -196,8 +179,8 @@ protected function getVoterSupportsClass($ret) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->any()) - ->method('supportsClass') - ->will($this->returnValue($ret)); + ->method('supportsClass') + ->will($this->returnValue($ret)); return $voter; } @@ -206,8 +189,8 @@ protected function getVoterSupportsAttribute($ret) { $voter = $this->getMock('Symfony\Component\Security\Core\Authorization\Voter\VoterInterface'); $voter->expects($this->any()) - ->method('supportsAttribute') - ->will($this->returnValue($ret)); + ->method('supportsAttribute') + ->will($this->returnValue($ret)); return $voter; } From 58947310f43c9b60986f61430359225af7908506 Mon Sep 17 00:00:00 2001 From: Miquel Company Rodriguez Date: Fri, 17 Jul 2015 00:14:10 +0200 Subject: [PATCH 3/7] Remove AccessDecisionStrategyInterface --- .../AccessDecisionStrategyInterface.php | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php deleted file mode 100644 index 50c08f03c83e9..0000000000000 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionStrategyInterface.php +++ /dev/null @@ -1,45 +0,0 @@ - - */ -interface AccessDecisionStrategyInterface -{ - /** - * Configures the voters. - * - * @param VoterInterface[] $voters An array of VoterInterface instances - */ - public function setVoters(array $voters); - - /** - * Set whether to grant access if all voters abstained or not. - * - * @param bool $allowIfAllAbstainDecisions - */ - public function setAllowIfAllAbstainDecisions($allowIfAllAbstainDecisions); - - /** - * Set whether to grant access if result are equals. - * - * @param bool $allowIfEqualGrantedDeniedDecisions - */ - public function setAllowIfEqualGrantedDeniedDecisions($allowIfEqualGrantedDeniedDecisions); - - /** - * Decides whether the access is possible or not. - * - * @param TokenInterface $token - * @param array $attributes - * @param null $object - * - * @return true if this decision strategy decides that the access can be made - */ - public function decide(TokenInterface $token, array $attributes, $object = null); -} \ No newline at end of file From 78c18ab4001524e9880208955986cd9dfa66437d Mon Sep 17 00:00:00 2001 From: Miquel Company Rodriguez Date: Fri, 17 Jul 2015 00:15:54 +0200 Subject: [PATCH 4/7] Fix wrong coding standards --- .../Security/Core/Authorization/AccessDecisionManager.php | 2 -- .../Core/Authorization/Strategy/DecideAffirmativeStrategy.php | 3 +-- .../Core/Authorization/Strategy/DecideConsensusStrategy.php | 3 +-- .../Strategy/DecideHighestNotAbstainedVoterStrategy.php | 3 +-- .../Core/Authorization/Strategy/DecideUnanimousStrategy.php | 3 +-- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 363d29512a4cb..f9af85ca304bd 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -88,7 +88,6 @@ public function decide(TokenInterface $token, array $attributes, $object = null) return $this->strategy->decide($token, $attributes, $object); } - /** * {@inheritdoc} */ @@ -109,5 +108,4 @@ public function supportsAttribute($attribute) return $this->strategy->supportsAttribute($attribute); } - } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php index 8e9db2d63cdd5..1cb5507dc050d 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideAffirmativeStrategy.php @@ -4,7 +4,6 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; -use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -19,6 +18,7 @@ class DecideAffirmativeStrategy extends AbstractDecideStrategy implements Access /** * DecideAffirmativeStrategy constructor. + * * @param $allowIfAllAbstainDecisions */ public function __construct($allowIfAllAbstainDecisions) @@ -26,7 +26,6 @@ public function __construct($allowIfAllAbstainDecisions) $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; } - /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php index 6d6c46c6f3279..fa58d513e7459 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php @@ -4,7 +4,6 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; -use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -29,6 +28,7 @@ class DecideConsensusStrategy extends AbstractDecideStrategy implements AccessDe /** * DecideConsensusStrategy constructor. + * * @param $allowIfEqualGrantedDeniedDecisions * @param $allowIfAllAbstainDecisions */ @@ -38,7 +38,6 @@ public function __construct($allowIfEqualGrantedDeniedDecisions, $allowIfAllAbst $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; } - /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php index 81e1f0f4e9b69..249a9eb779c2a 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php @@ -4,7 +4,6 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; -use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -18,6 +17,7 @@ class DecideHighestNotAbstainedVoterStrategy extends AbstractDecideStrategy impl /** * DecideHighestNotAbstainedVoterStrategy constructor. + * * @param $allowIfAllAbstainDecisions */ public function __construct($allowIfAllAbstainDecisions) @@ -25,7 +25,6 @@ public function __construct($allowIfAllAbstainDecisions) $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; } - /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php index 9101fe383d606..73b0a59c88e4f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php @@ -4,7 +4,6 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; -use Symfony\Component\Security\Core\Authorization\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -19,6 +18,7 @@ class DecideUnanimousStrategy extends AbstractDecideStrategy implements AccessDe /** * DecideUnanimousStrategy constructor. + * * @param $allowIfAllAbstainDecisions */ public function __construct($allowIfAllAbstainDecisions) @@ -26,7 +26,6 @@ public function __construct($allowIfAllAbstainDecisions) $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; } - /** * {@inheritdoc} */ From f78b3df4ffbb0139116645898839bd8a0bd7893e Mon Sep 17 00:00:00 2001 From: Miquel Company Rodriguez Date: Fri, 17 Jul 2015 23:37:24 +0200 Subject: [PATCH 5/7] Make access decision manager configurable. --- .../CustomAccessDecisionManagerPass.php | 36 +++++++++++++++++++ .../DependencyInjection/MainConfiguration.php | 6 ++++ .../DependencyInjection/SecurityExtension.php | 5 +++ .../Bundle/SecurityBundle/SecurityBundle.php | 2 ++ .../Authorization/AccessDecisionManager.php | 33 +++++++++-------- 5 files changed, 65 insertions(+), 17 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php new file mode 100644 index 0000000000000..1cce1145cc5a8 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php @@ -0,0 +1,36 @@ +hasParameter('security.access.manager.service')) { + $serviceName = $container->getParameter('security.access.manager.service'); + + if ($container->hasDefinition('security.authorization_checker') && $container->hasDefinition( + $serviceName + ) + ) { + if ($container->get($serviceName) instanceof AccessDecisionManagerInterface) { + $definition = $container->getDefinition('security.authorization_checker'); + $definition->replaceArgument(2, new Reference($serviceName)); + } else { + throw new InvalidConfigurationException(); + } + } + } + } + +} \ No newline at end of file diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 091c16f60e1d4..3131465b429cd 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -68,6 +68,12 @@ public function getConfigTreeBuilder() ->booleanNode('hide_user_not_found')->defaultTrue()->end() ->booleanNode('always_authenticate_before_granting')->defaultFalse()->end() ->booleanNode('erase_credentials')->defaultTrue()->end() + ->arrayNode('authorization_checker') + ->canBeEnabled() + ->children() + ->scalarNode('access_decision_manager_service')->end() + ->end() + ->end() ->arrayNode('access_decision_manager') ->addDefaultsIfNotSet() ->children() diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index e82e3ca8779bc..f5739af476f2c 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -75,6 +75,11 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter('security.access.denied_url', $config['access_denied_url']); $container->setParameter('security.authentication.manager.erase_credentials', $config['erase_credentials']); $container->setParameter('security.authentication.session_strategy.strategy', $config['session_fixation_strategy']); + + if(isset($config['authorization_checker']['access_decision_manager_service'])) { + $container->setParameter('security.access.manager.service', $config['authorization_checker']['access_decision_manager_service']); + } + $container ->getDefinition('security.access.decision_manager') ->addArgument($config['access_decision_manager']['strategy']) diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php index 72f7b68de959d..c41f16354f6f6 100644 --- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php +++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php @@ -14,6 +14,7 @@ use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\CustomAccessDecisionManagerPass; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpDigestFactory; @@ -47,5 +48,6 @@ public function build(ContainerBuilder $container) $extension->addUserProviderFactory(new InMemoryFactory()); $container->addCompilerPass(new AddSecurityVotersPass()); + $container->addCompilerPass(new CustomAccessDecisionManagerPass()); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index f9af85ca304bd..8f7388c47aab0 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -31,7 +31,6 @@ class AccessDecisionManager implements AccessDecisionManagerInterface const STRATEGY_UNANIMOUS = 'unanimous'; const STRATEGY_HIGHEST_NOT_ABSTAINED = 'highest'; - private $voters; private $strategy; private $allowIfAllAbstainDecisions; private $allowIfEqualGrantedDeniedDecisions; @@ -64,22 +63,6 @@ public function setVoters(array $voters) $this->strategy->setVoters($voters); } - private function createStrategy($strategyName) - { - switch($strategyName){ - case self::STRATEGY_UNANIMOUS: - return new DecideUnanimousStrategy($this->allowIfAllAbstainDecisions); - case self::STRATEGY_CONSENSUS: - return new DecideConsensusStrategy($this->allowIfEqualGrantedDeniedDecisions,$this->allowIfAllAbstainDecisions); - case self::STRATEGY_AFFIRMATIVE: - return new DecideAffirmativeStrategy($this->allowIfAllAbstainDecisions); - case self::STRATEGY_HIGHEST_NOT_ABSTAINED: - return new DecideHighestNotAbstainedVoterStrategy($this->allowIfAllAbstainDecisions); - default: - throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategyName)); - } - } - /** * {@inheritdoc} */ @@ -108,4 +91,20 @@ public function supportsAttribute($attribute) return $this->strategy->supportsAttribute($attribute); } + private function createStrategy($strategyName) + { + switch($strategyName){ + case self::STRATEGY_UNANIMOUS: + return new DecideUnanimousStrategy($this->allowIfAllAbstainDecisions); + case self::STRATEGY_CONSENSUS: + return new DecideConsensusStrategy($this->allowIfEqualGrantedDeniedDecisions,$this->allowIfAllAbstainDecisions); + case self::STRATEGY_AFFIRMATIVE: + return new DecideAffirmativeStrategy($this->allowIfAllAbstainDecisions); + case self::STRATEGY_HIGHEST_NOT_ABSTAINED: + return new DecideHighestNotAbstainedVoterStrategy($this->allowIfAllAbstainDecisions); + default: + throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategyName)); + } + } + } From ffb411b01207558b2bd12bcf758d433a4cbd79b9 Mon Sep 17 00:00:00 2001 From: Miquel Company Rodriguez Date: Mon, 27 Jul 2015 16:14:33 +0200 Subject: [PATCH 6/7] Improve CustomAccessDecisionManagerCompilerPass --- .../CustomAccessDecisionManagerPass.php | 40 ++++++++++--------- .../DependencyInjection/MainConfiguration.php | 6 ++- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php index 1cce1145cc5a8..62b3f0c9a350f 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/CustomAccessDecisionManagerPass.php @@ -1,36 +1,40 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler; -use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; -use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; class CustomAccessDecisionManagerPass implements CompilerPassInterface { /** - * @inheritDoc + * {@inheritdoc} */ public function process(ContainerBuilder $container) { - if ($container->hasParameter('security.access.manager.service')) { - $serviceName = $container->getParameter('security.access.manager.service'); + $serviceName = $container->getParameter('security.access.manager.service'); - if ($container->hasDefinition('security.authorization_checker') && $container->hasDefinition( - $serviceName - ) - ) { - if ($container->get($serviceName) instanceof AccessDecisionManagerInterface) { - $definition = $container->getDefinition('security.authorization_checker'); - $definition->replaceArgument(2, new Reference($serviceName)); - } else { - throw new InvalidConfigurationException(); - } + if ($container->hasDefinition('security.authorization_checker') && $container->hasDefinition($serviceName)) { + // We must assume that the class value has been correctly filled, even if the service is created by a factory + $class = $container->getParameterBag()->resolveValue($container->getDefinition($serviceName)->getClass()); + $refClass = new \ReflectionClass($class); + + if ($refClass->implementsInterface('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')) { + $definition = $container->getDefinition('security.authorization_checker'); + $definition->replaceArgument(2, new Reference($serviceName)); + } else { + throw new \InvalidArgumentException(sprintf('Service "%s" must implement interface "AccessDecisionManagerInterface".', $serviceName)); } } } - -} \ No newline at end of file +} diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 3131465b429cd..97bea12944add 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -69,9 +69,11 @@ public function getConfigTreeBuilder() ->booleanNode('always_authenticate_before_granting')->defaultFalse()->end() ->booleanNode('erase_credentials')->defaultTrue()->end() ->arrayNode('authorization_checker') - ->canBeEnabled() + ->addDefaultsIfNotSet() ->children() - ->scalarNode('access_decision_manager_service')->end() + ->scalarNode('access_decision_manager_service') + ->defaultValue('security.access.decision_manager') + ->end() ->end() ->end() ->arrayNode('access_decision_manager') From 5d5f4ef94fa0ea761dfe82a3b9c532c60dddff8e Mon Sep 17 00:00:00 2001 From: Miquel Company Rodriguez Date: Fri, 31 Jul 2015 11:36:41 +0200 Subject: [PATCH 7/7] Change strategy namespace and naming --- .../Authorization/AccessDecisionManager.php | 19 +++++++++---------- .../AbstractAccessDecisionManager.php} | 14 +++++++++++--- .../AffirmativeAccessDecisionManager.php} | 8 +++----- .../ConsensusAccessDecisionManager.php} | 6 ++---- ...otAbstainedVoterAccessDecisionManager.php} | 8 +++----- .../UnanimousAccessDecisionManager.php} | 8 +++----- 6 files changed, 31 insertions(+), 32 deletions(-) rename src/Symfony/Component/Security/Core/Authorization/{Strategy/AbstractDecideStrategy.php => AccessDecisionManager/AbstractAccessDecisionManager.php} (60%) rename src/Symfony/Component/Security/Core/Authorization/{Strategy/DecideAffirmativeStrategy.php => AccessDecisionManager/AffirmativeAccessDecisionManager.php} (84%) rename src/Symfony/Component/Security/Core/Authorization/{Strategy/DecideConsensusStrategy.php => AccessDecisionManager/ConsensusAccessDecisionManager.php} (89%) rename src/Symfony/Component/Security/Core/Authorization/{Strategy/DecideHighestNotAbstainedVoterStrategy.php => AccessDecisionManager/HighestNotAbstainedVoterAccessDecisionManager.php} (82%) rename src/Symfony/Component/Security/Core/Authorization/{Strategy/DecideUnanimousStrategy.php => AccessDecisionManager/UnanimousAccessDecisionManager.php} (85%) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 8f7388c47aab0..8cafa4b7bda6e 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -11,10 +11,10 @@ namespace Symfony\Component\Security\Core\Authorization; -use Symfony\Component\Security\Core\Authorization\Strategy\DecideAffirmativeStrategy; -use Symfony\Component\Security\Core\Authorization\Strategy\DecideConsensusStrategy; -use Symfony\Component\Security\Core\Authorization\Strategy\DecideHighestNotAbstainedVoterStrategy; -use Symfony\Component\Security\Core\Authorization\Strategy\DecideUnanimousStrategy; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManager\AffirmativeAccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManager\ConsensusAccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManager\HighestNotAbstainedVoterAccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManager\UnanimousAccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; @@ -93,18 +93,17 @@ public function supportsAttribute($attribute) private function createStrategy($strategyName) { - switch($strategyName){ + switch ($strategyName) { case self::STRATEGY_UNANIMOUS: - return new DecideUnanimousStrategy($this->allowIfAllAbstainDecisions); + return new UnanimousAccessDecisionManager($this->allowIfAllAbstainDecisions); case self::STRATEGY_CONSENSUS: - return new DecideConsensusStrategy($this->allowIfEqualGrantedDeniedDecisions,$this->allowIfAllAbstainDecisions); + return new ConsensusAccessDecisionManager($this->allowIfEqualGrantedDeniedDecisions, $this->allowIfAllAbstainDecisions); case self::STRATEGY_AFFIRMATIVE: - return new DecideAffirmativeStrategy($this->allowIfAllAbstainDecisions); + return new AffirmativeAccessDecisionManager($this->allowIfAllAbstainDecisions); case self::STRATEGY_HIGHEST_NOT_ABSTAINED: - return new DecideHighestNotAbstainedVoterStrategy($this->allowIfAllAbstainDecisions); + return new HighestNotAbstainedVoterAccessDecisionManager($this->allowIfAllAbstainDecisions); default: throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategyName)); } } - } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/AbstractAccessDecisionManager.php similarity index 60% rename from src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php rename to src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/AbstractAccessDecisionManager.php index b74b77f91d66a..01beb82bcf511 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AbstractDecideStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/AbstractAccessDecisionManager.php @@ -1,8 +1,11 @@ allowIfAllAbstainDecisions; } - -} \ No newline at end of file +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/ConsensusAccessDecisionManager.php similarity index 89% rename from src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php rename to src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/ConsensusAccessDecisionManager.php index fa58d513e7459..8158b83b10e20 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideConsensusStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/ConsensusAccessDecisionManager.php @@ -1,9 +1,8 @@ allowIfAllAbstainDecisions; } - } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/HighestNotAbstainedVoterAccessDecisionManager.php similarity index 82% rename from src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php rename to src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/HighestNotAbstainedVoterAccessDecisionManager.php index 249a9eb779c2a..d2379dafc8e82 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideHighestNotAbstainedVoterStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/HighestNotAbstainedVoterAccessDecisionManager.php @@ -1,9 +1,8 @@ allowIfAllAbstainDecisions; } - -} \ No newline at end of file +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/UnanimousAccessDecisionManager.php similarity index 85% rename from src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php rename to src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/UnanimousAccessDecisionManager.php index 73b0a59c88e4f..45cb33c4cdcb5 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/DecideUnanimousStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager/UnanimousAccessDecisionManager.php @@ -1,9 +1,8 @@ allowIfAllAbstainDecisions; } - -} \ No newline at end of file +}