From 7721ba4746f77a44af239ec2f862e4ee9a9fff20 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Wed, 1 Jun 2016 17:22:18 +0200 Subject: [PATCH 01/11] Fixed some issues of the AccessDecisionManager profiler --- .../SecurityBundle/DependencyInjection/SecurityExtension.php | 2 +- .../Security/Core/Authorization/DebugAccessDecisionManager.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 51a234a288388..4fc88b96d06e6 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -66,7 +66,7 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('collectors.xml'); $loader->load('guard.xml'); - if ($container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug')) { + if ($container->has('profiler')) { $loader->load('security_debug.xml'); } diff --git a/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php index 540d998206047..523b2c36d334a 100644 --- a/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php @@ -29,7 +29,7 @@ class DebugAccessDecisionManager implements AccessDecisionManagerInterface private $voters; private $decisionLog = array(); - public function __construct(AccessDecisionManager $manager) + public function __construct(AccessDecisionManagerInterface $manager) { $this->manager = $manager; From 639c8ead203e988ff6125954ab397c4f6989d397 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Thu, 2 Jun 2016 09:47:25 +0200 Subject: [PATCH 02/11] Improved the code --- .../Authorization/DebugAccessDecisionManager.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php index 523b2c36d334a..6a4ca9b4a3914 100644 --- a/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php @@ -33,10 +33,12 @@ public function __construct(AccessDecisionManagerInterface $manager) { $this->manager = $manager; - // The strategy is stored in a private property of the decorated service - $reflection = new \ReflectionProperty($manager, 'strategy'); - $reflection->setAccessible(true); - $this->strategy = $reflection->getValue($manager); + if ($this->manager instanceof AccessDecisionManager) { + // The strategy is stored in a private property of the decorated service + $reflection = new \ReflectionProperty(AccessDecisionManager::class, 'strategy'); + $reflection->setAccessible(true); + $this->strategy = $reflection->getValue($manager); + } } /** @@ -60,6 +62,10 @@ public function decide(TokenInterface $token, array $attributes, $object = null) */ public function setVoters(array $voters) { + if (!$this->manager instanceof AccessDecisionManager) { + return; + } + $this->voters = $voters; $this->manager->setVoters($voters); } @@ -72,7 +78,7 @@ public function getStrategy() // The $strategy property is misleading because it stores the name of its // method (e.g. 'decideAffirmative') instead of the original strategy name // (e.g. 'affirmative') - return strtolower(substr($this->strategy, 6)); + return null === $this->strategy ? '-' : strtolower(substr($this->strategy, 6)); } /** From 16dd82b34d11851e72f4b3dca7b425280e78866d Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Sun, 5 Jun 2016 21:38:43 +0200 Subject: [PATCH 03/11] Restored the original condition (but simplify it a bit) --- .../SecurityBundle/DependencyInjection/SecurityExtension.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 4fc88b96d06e6..a29534b7226da 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -66,7 +66,7 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('collectors.xml'); $loader->load('guard.xml'); - if ($container->has('profiler')) { + if ($container->getParameter('kernel.debug')) { $loader->load('security_debug.xml'); } From ec9ffeb047edbf3bac7a98b3787e49d4336594c6 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Tue, 28 Jun 2016 08:58:02 +0200 Subject: [PATCH 04/11] Added a compiler pass to enable the debug access manager conditionally --- .../AddDebugAccessDecisionManagerPass.php | 42 +++++++++++++++++++ .../DependencyInjection/SecurityExtension.php | 4 -- .../Resources/config/security_debug.xml | 12 ------ .../Bundle/SecurityBundle/SecurityBundle.php | 2 + 4 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php delete mode 100644 src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php new file mode 100644 index 0000000000000..5d95fdaa5aea3 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php @@ -0,0 +1,42 @@ + + * + * 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\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager; + +/** + * Adds all configured security voters to the access decision manager. + * + * @author Javier Eguiluz + */ +class AddDebugAccessDecisionManagerPass implements CompilerPassInterface +{ + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + if (!$container->hasDefinition('profiler')) { + return; + } + + $definition = new Definition(DebugAccessDecisionManager::class, array(new Reference('debug.security.access.decision_manager.inner'))); + $definition->setPublic(false); + $definition->setDecoratedService('security.access.decision_manager'); + + $container->setDefinition('debug.security.access.decision_manager', $definition); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index a29534b7226da..6043ed6292089 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -66,10 +66,6 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('collectors.xml'); $loader->load('guard.xml'); - if ($container->getParameter('kernel.debug')) { - $loader->load('security_debug.xml'); - } - if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { $container->removeDefinition('security.expression_language'); $container->removeDefinition('security.access.expression_voter'); diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml deleted file mode 100644 index 4312d747417d1..0000000000000 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php index f2dfc991fbcef..545d40944ebbc 100644 --- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php +++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php @@ -13,6 +13,7 @@ use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddDebugAccessDecisionManagerPass; use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginLdapFactory; @@ -55,5 +56,6 @@ public function build(ContainerBuilder $container) $extension->addUserProviderFactory(new InMemoryFactory()); $extension->addUserProviderFactory(new LdapFactory()); $container->addCompilerPass(new AddSecurityVotersPass()); + $container->addCompilerPass(new AddDebugAccessDecisionManagerPass()); } } From 2ee2361f3d4d975cd45570887b2b13fbb4b34901 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Tue, 28 Jun 2016 17:46:35 +0200 Subject: [PATCH 05/11] Pass the decorated access decision manager service to the security collector --- .../Bundle/SecurityBundle/Resources/config/collectors.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml index 738455358b0dd..de157d51824ce 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml @@ -10,7 +10,7 @@ - + From 800ef55823e6e27b8266572a50ccd69f233d8017 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Tue, 28 Jun 2016 17:56:26 +0200 Subject: [PATCH 06/11] Initialize the voters property to aovid errors in tests --- .../Security/Core/Authorization/DebugAccessDecisionManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php index 6a4ca9b4a3914..1a04bc1f60720 100644 --- a/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php @@ -26,7 +26,7 @@ class DebugAccessDecisionManager implements AccessDecisionManagerInterface { private $manager; private $strategy; - private $voters; + private $voters = array(); private $decisionLog = array(); public function __construct(AccessDecisionManagerInterface $manager) From e4cbf0122b3757d57d6293ea58d035f7af9f353f Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Wed, 29 Jun 2016 09:49:59 +0200 Subject: [PATCH 07/11] Trying another way to solve this issue --- .../AddDebugAccessDecisionManagerPass.php | 42 ------------------- .../DependencyInjection/SecurityExtension.php | 1 + .../Resources/config/security_debug.xml | 12 ++++++ .../Bundle/SecurityBundle/SecurityBundle.php | 1 - 4 files changed, 13 insertions(+), 43 deletions(-) delete mode 100644 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php deleted file mode 100644 index 5d95fdaa5aea3..0000000000000 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php +++ /dev/null @@ -1,42 +0,0 @@ - - * - * 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\DependencyInjection\Definition; -use Symfony\Component\DependencyInjection\Reference; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; -use Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager; - -/** - * Adds all configured security voters to the access decision manager. - * - * @author Javier Eguiluz - */ -class AddDebugAccessDecisionManagerPass implements CompilerPassInterface -{ - /** - * {@inheritdoc} - */ - public function process(ContainerBuilder $container) - { - if (!$container->hasDefinition('profiler')) { - return; - } - - $definition = new Definition(DebugAccessDecisionManager::class, array(new Reference('debug.security.access.decision_manager.inner'))); - $definition->setPublic(false); - $definition->setDecoratedService('security.access.decision_manager'); - - $container->setDefinition('debug.security.access.decision_manager', $definition); - } -} diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 6043ed6292089..24c45cbd34386 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -65,6 +65,7 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('templating_twig.xml'); $loader->load('collectors.xml'); $loader->load('guard.xml'); + $loader->load('security_debug.xml'); if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { $container->removeDefinition('security.expression_language'); diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml new file mode 100644 index 0000000000000..eb22cbddf504e --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml @@ -0,0 +1,12 @@ + + + + + + + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php index 545d40944ebbc..8fbc330f4b846 100644 --- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php +++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php @@ -56,6 +56,5 @@ public function build(ContainerBuilder $container) $extension->addUserProviderFactory(new InMemoryFactory()); $extension->addUserProviderFactory(new LdapFactory()); $container->addCompilerPass(new AddSecurityVotersPass()); - $container->addCompilerPass(new AddDebugAccessDecisionManagerPass()); } } From 5d96ec65c7033b82f7cebc607170e459283a95b1 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Wed, 29 Jun 2016 10:02:56 +0200 Subject: [PATCH 08/11] Removed an unused "use" import --- src/Symfony/Bundle/SecurityBundle/SecurityBundle.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php index 8fbc330f4b846..f2dfc991fbcef 100644 --- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php +++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php @@ -13,7 +13,6 @@ use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddDebugAccessDecisionManagerPass; use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginLdapFactory; From 804fcecbc92599818c88aca122d8622beb313e00 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Wed, 29 Jun 2016 10:42:44 +0200 Subject: [PATCH 09/11] Load the debug config conditionally --- .../SecurityBundle/DependencyInjection/SecurityExtension.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 24c45cbd34386..a29534b7226da 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -65,7 +65,10 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('templating_twig.xml'); $loader->load('collectors.xml'); $loader->load('guard.xml'); - $loader->load('security_debug.xml'); + + if ($container->getParameter('kernel.debug')) { + $loader->load('security_debug.xml'); + } if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { $container->removeDefinition('security.expression_language'); From e3c9c29d7a4b9577868131d329dbc07bf763ef3a Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Wed, 29 Jun 2016 11:00:12 +0200 Subject: [PATCH 10/11] Reverted some changes --- .../SecurityBundle/DependencyInjection/SecurityExtension.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index a29534b7226da..51a234a288388 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -66,7 +66,7 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('collectors.xml'); $loader->load('guard.xml'); - if ($container->getParameter('kernel.debug')) { + if ($container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug')) { $loader->load('security_debug.xml'); } From 82f0541ce598331c6854971244c30a8a95cd31b3 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Wed, 29 Jun 2016 11:38:46 +0200 Subject: [PATCH 11/11] Updated the "symfony/security" dependency --- src/Symfony/Bundle/SecurityBundle/composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index 6e22dc91924c5..a67980b64313c 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -17,7 +17,7 @@ ], "require": { "php": ">=5.5.9", - "symfony/security": "~3.1", + "symfony/security": "~3.1,>=3.1.2", "symfony/http-kernel": "~2.8|~3.0", "symfony/polyfill-php70": "~1.0" },