From 41552cd89625f91e6d31e9807dda125a015ae94a Mon Sep 17 00:00:00 2001 From: David Maicher Date: Wed, 4 Apr 2018 20:31:33 +0200 Subject: [PATCH] [SecurityBundle] allow using custom function inside allow_if expressions --- UPGRADE-4.1.md | 1 + UPGRADE-5.0.md | 1 + .../CacheWarmer/ExpressionCacheWarmer.php | 43 +++++++++++++ .../DependencyInjection/SecurityExtension.php | 33 ++++------ .../Resources/config/security.xml | 16 ++++- .../CacheWarmer/ExpressionCacheWarmerTest.php | 35 +++++++++++ .../SecurityExtensionTest.php | 63 +++++++++++++++++++ .../Authorization/Voter/ExpressionVoter.php | 5 ++ 8 files changed, 176 insertions(+), 21 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/CacheWarmer/ExpressionCacheWarmerTest.php diff --git a/UPGRADE-4.1.md b/UPGRADE-4.1.md index 8d96b710ed..807c028e5d 100644 --- a/UPGRADE-4.1.md +++ b/UPGRADE-4.1.md @@ -81,6 +81,7 @@ Security functionality, create a custom user-checker based on the `Symfony\Component\Security\Core\User\UserChecker`. * `AuthenticationUtils::getLastUsername()` now always returns a string. + * The `ExpressionVoter::addExpressionLanguageProvider()` method is deprecated. Register the provider directly on the injected ExpressionLanguage instance instead. SecurityBundle -------------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 2141dbb1df..d178fcdc0d 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -76,6 +76,7 @@ Security * The `ContextListener::setLogoutOnUserChange()` method has been removed. * The `Symfony\Component\Security\Core\User\AdvancedUserInterface` has been removed. + * The `ExpressionVoter::addExpressionLanguageProvider()` method has been removed. SecurityBundle -------------- diff --git a/src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php b/src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php new file mode 100644 index 0000000000..de5a75b3b0 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php @@ -0,0 +1,43 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\CacheWarmer; + +use Symfony\Component\ExpressionLanguage\Expression; +use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; +use Symfony\Component\Security\Core\Authorization\ExpressionLanguage; + +class ExpressionCacheWarmer implements CacheWarmerInterface +{ + private $expressions; + private $expressionLanguage; + + /** + * @param iterable|Expression[] $expressions + */ + public function __construct(iterable $expressions, ExpressionLanguage $expressionLanguage) + { + $this->expressions = $expressions; + $this->expressionLanguage = $expressionLanguage; + } + + public function isOptional() + { + return true; + } + + public function warmUp($cacheDir) + { + foreach ($this->expressions as $expression) { + $this->expressionLanguage->parse($expression, array('token', 'user', 'object', 'subject', 'roles', 'request', 'trust_resolver')); + } + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 3b56ac54dd..88ab775770 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -26,7 +26,6 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Parameter; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\Config\FileLocator; -use Symfony\Component\Security\Core\Authorization\ExpressionLanguage; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder; use Symfony\Component\Security\Http\Controller\UserValueResolver; @@ -45,7 +44,6 @@ class SecurityExtension extends Extension private $listenerPositions = array('pre_auth', 'form', 'http', 'remember_me'); private $factories = array(); private $userProviderFactories = array(); - private $expressionLanguage; public function __construct() { @@ -136,10 +134,6 @@ class SecurityExtension extends Extension private function createAuthorization($config, ContainerBuilder $container) { - if (!$config['access_control']) { - return; - } - foreach ($config['access_control'] as $access) { $matcher = $this->createRequestMatcher( $container, @@ -157,6 +151,14 @@ class SecurityExtension extends Extension $container->getDefinition('security.access_map') ->addMethodCall('add', array($matcher, $attributes, $access['requires_channel'])); } + + // allow cache warm-up for expressions + if (count($this->expressions)) { + $container->getDefinition('security.cache_warmer.expression') + ->replaceArgument(0, new IteratorArgument(array_values($this->expressions))); + } else { + $container->removeDefinition('security.cache_warmer.expression'); + } } private function createFirewalls($config, ContainerBuilder $container) @@ -636,11 +638,14 @@ class SecurityExtension extends Extension return $this->expressions[$id]; } + if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { + throw new \RuntimeException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.'); + } + $container - ->register($id, 'Symfony\Component\ExpressionLanguage\SerializedParsedExpression') + ->register($id, 'Symfony\Component\ExpressionLanguage\Expression') ->setPublic(false) ->addArgument($expression) - ->addArgument(serialize($this->getExpressionLanguage()->parse($expression, array('token', 'user', 'object', 'roles', 'request', 'trust_resolver'))->getNodes())) ; return $this->expressions[$id] = new Reference($id); @@ -703,16 +708,4 @@ class SecurityExtension extends Extension // first assemble the factories return new MainConfiguration($this->factories, $this->userProviderFactories); } - - private function getExpressionLanguage() - { - if (null === $this->expressionLanguage) { - if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { - throw new \RuntimeException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.'); - } - $this->expressionLanguage = new ExpressionLanguage(); - } - - return $this->expressionLanguage; - } } diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index 26db528aed..2659619d49 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -80,7 +80,9 @@ - + + + @@ -195,5 +197,17 @@ + + + + + + + + + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/CacheWarmer/ExpressionCacheWarmerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/CacheWarmer/ExpressionCacheWarmerTest.php new file mode 100644 index 0000000000..30ead76927 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/CacheWarmer/ExpressionCacheWarmerTest.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\CacheWarmer; + +use PHPUnit\Framework\TestCase; +use Symfony\Bundle\SecurityBundle\CacheWarmer\ExpressionCacheWarmer; +use Symfony\Component\ExpressionLanguage\Expression; +use Symfony\Component\Security\Core\Authorization\ExpressionLanguage; + +class ExpressionCacheWarmerTest extends TestCase +{ + public function testWarmUp() + { + $expressions = array(new Expression('A'), new Expression('B')); + + $expressionLang = $this->createMock(ExpressionLanguage::class); + $expressionLang->expects($this->exactly(2)) + ->method('parse') + ->withConsecutive( + array($expressions[0], array('token', 'user', 'object', 'subject', 'roles', 'request', 'trust_resolver')), + array($expressions[1], array('token', 'user', 'object', 'subject', 'roles', 'request', 'trust_resolver')) + ); + + (new ExpressionCacheWarmer($expressions, $expressionLang))->warmUp(''); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php index 718c459f68..b1ff1c2125 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php @@ -15,7 +15,10 @@ use PHPUnit\Framework\TestCase; use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\UserProvider\DummyProvider; +use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\ExpressionLanguage\Expression; class SecurityExtensionTest extends TestCase { @@ -207,6 +210,66 @@ class SecurityExtensionTest extends TestCase $this->addToAssertionCount(1); } + public function testRegisterRequestMatchersWithAllowIfExpression() + { + $container = $this->getRawContainer(); + + $rawExpression = "'foo' == 'bar' or 1 in [1, 3, 3]"; + + $container->loadFromExtension('security', array( + 'providers' => array( + 'default' => array('id' => 'foo'), + ), + 'firewalls' => array( + 'some_firewall' => array( + 'pattern' => '/.*', + 'http_basic' => array(), + ), + ), + 'access_control' => array( + array('path' => '/', 'allow_if' => $rawExpression), + ), + )); + + $container->compile(); + $accessMap = $container->getDefinition('security.access_map'); + $this->assertCount(1, $accessMap->getMethodCalls()); + $call = $accessMap->getMethodCalls()[0]; + $this->assertSame('add', $call[0]); + $args = $call[1]; + $this->assertCount(3, $args); + $expressionId = $args[1][0]; + $this->assertTrue($container->hasDefinition($expressionId)); + $expressionDef = $container->getDefinition($expressionId); + $this->assertSame(Expression::class, $expressionDef->getClass()); + $this->assertSame($rawExpression, $expressionDef->getArgument(0)); + + $this->assertTrue($container->hasDefinition('security.cache_warmer.expression')); + $this->assertEquals( + new IteratorArgument(array(new Reference($expressionId))), + $container->getDefinition('security.cache_warmer.expression')->getArgument(0) + ); + } + + public function testRemovesExpressionCacheWarmerDefinitionIfNoExpressions() + { + $container = $this->getRawContainer(); + $container->loadFromExtension('security', array( + 'providers' => array( + 'default' => array('id' => 'foo'), + ), + 'firewalls' => array( + 'some_firewall' => array( + 'pattern' => '/.*', + 'http_basic' => array(), + ), + ), + )); + $container->compile(); + + $this->assertFalse($container->hasDefinition('security.cache_warmer.expression')); + } + protected function getRawContainer() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php index 00633397d2..cbee938667 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php @@ -37,8 +37,13 @@ class ExpressionVoter implements VoterInterface $this->roleHierarchy = $roleHierarchy; } + /** + * @deprecated since Symfony 4.1, register the provider directly on the injected ExpressionLanguage instance instead. + */ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterface $provider) { + @trigger_error(sprintf('The %s() method is deprecated since Symfony 4.1, register the provider directly on the injected ExpressionLanguage instance instead.', __METHOD__), E_USER_DEPRECATED); + $this->expressionLanguage->registerProvider($provider); }