diff --git a/UPGRADE-5.3.md b/UPGRADE-5.3.md index fcf81d72ad38a..057e5b3f600ae 100644 --- a/UPGRADE-5.3.md +++ b/UPGRADE-5.3.md @@ -83,6 +83,8 @@ Security SecurityBundle -------------- + * [BC break] Add `login_throttling.lock_factory` setting defaulting to `null`. Set this option + to `lock.factory` if you need precise login rate limiting with synchronous requests. * Deprecate `UserPasswordEncoderCommand` class and the corresponding `user:encode-password` command, use `UserPasswordHashCommand` and `user:hash-password` instead * Deprecate the `security.encoder_factory.generic` service, the `security.encoder_factory` and `Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface` aliases, diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index ee44ac7d56a0e..565b9d3960d6e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1874,7 +1874,7 @@ private function addRateLimiterSection(ArrayNodeDefinition $rootNode, callable $ ->arrayPrototype() ->children() ->scalarNode('lock_factory') - ->info('The service ID of the lock factory used by this limiter') + ->info('The service ID of the lock factory used by this limiter (or null to disable locking)') ->defaultValue('lock.factory') ->end() ->scalarNode('cache_pool') diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 57a1d057937f2..07c033736771f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -205,7 +205,7 @@ class FrameworkExtension extends Extension private $httpClientConfigEnabled = false; private $notifierConfigEnabled = false; private $propertyAccessConfigEnabled = false; - private $lockConfigEnabled = false; + private static $lockConfigEnabled = false; /** * Responds to the app.config configuration parameter. @@ -438,7 +438,7 @@ public function load(array $configs, ContainerBuilder $container) $this->registerPropertyInfoConfiguration($container, $loader); } - if ($this->lockConfigEnabled = $this->isConfigEnabled($container, $config['lock'])) { + if (self::$lockConfigEnabled = $this->isConfigEnabled($container, $config['lock'])) { $this->registerLockConfiguration($config['lock'], $container, $loader); } @@ -2344,10 +2344,6 @@ private function registerNotifierConfiguration(array $config, ContainerBuilder $ private function registerRateLimiterConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader) { - if (!$this->lockConfigEnabled) { - throw new LogicException('Rate limiter support cannot be enabled without enabling the Lock component.'); - } - $loader->load('rate_limiter.php'); foreach ($config['limiters'] as $name => $limiterConfig) { @@ -2362,7 +2358,13 @@ public static function registerRateLimiter(ContainerBuilder $container, string $ $limiter = $container->setDefinition($limiterId = 'limiter.'.$name, new ChildDefinition('limiter')); - $limiter->addArgument(new Reference($limiterConfig['lock_factory'])); + if (null !== $limiterConfig['lock_factory']) { + if (!self::$lockConfigEnabled) { + throw new LogicException(sprintf('Rate limiter "%s" requires the Lock component to be installed and configured.', $name)); + } + + $limiter->replaceArgument(2, new Reference($limiterConfig['lock_factory'])); + } unset($limiterConfig['lock_factory']); $storageId = $limiterConfig['storage_service'] ?? null; diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php index 39f92323f09a5..727a1f6364456 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php @@ -24,6 +24,7 @@ ->args([ abstract_arg('config'), abstract_arg('storage'), + null, ]) ; }; diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php index 0b8b4eb8fa406..b46bcf7713c95 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php @@ -13,6 +13,8 @@ use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Exception\LogicException; +use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; use Symfony\Component\Workflow\Exception\InvalidDefinitionException; @@ -82,4 +84,49 @@ public function testWorkflowValidationStateMachine() ]); }); } + + public function testRateLimiterWithLockFactory() + { + try { + $this->createContainerFromClosure(function (ContainerBuilder $container) { + $container->loadFromExtension('framework', [ + 'rate_limiter' => [ + 'with_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour'], + ], + ]); + }); + + $this->fail('No LogicException thrown'); + } catch (LogicException $e) { + $this->assertEquals('Rate limiter "with_lock" requires the Lock component to be installed and configured.', $e->getMessage()); + } + + $container = $this->createContainerFromClosure(function (ContainerBuilder $container) { + $container->loadFromExtension('framework', [ + 'lock' => true, + 'rate_limiter' => [ + 'with_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour'], + ], + ]); + }); + + $withLock = $container->getDefinition('limiter.with_lock'); + $this->assertEquals('lock.factory', (string) $withLock->getArgument(2)); + } + + public function testRateLimiterLockFactory() + { + $container = $this->createContainerFromClosure(function (ContainerBuilder $container) { + $container->loadFromExtension('framework', [ + 'rate_limiter' => [ + 'without_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour', 'lock_factory' => null], + ], + ]); + }); + + $this->expectException(OutOfBoundsException::class); + $this->expectExceptionMessage('The argument "2" doesn\'t exist.'); + + $container->getDefinition('limiter.without_lock')->getArgument(2); + } } diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index 5cada9c672733..a313abdf891f6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -51,6 +51,7 @@ "symfony/messenger": "^5.2", "symfony/mime": "^4.4|^5.0", "symfony/process": "^4.4|^5.0", + "symfony/rate-limiter": "^5.2", "symfony/security-bundle": "^5.3", "symfony/serializer": "^5.2", "symfony/stopwatch": "^4.4|^5.0", diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index 2473dfd9ced74..d36da75e80e25 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 5.3 --- + * [BC break] Add `login_throttling.lock_factory` setting defaulting to `null` (instead of `lock.factory`) * Add the `debug:firewall` command. * Deprecate `UserPasswordEncoderCommand` class and the corresponding `user:encode-password` command, use `UserPasswordHashCommand` and `user:hash-password` instead diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php index c0aa37ec88712..f8e3416e68819 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php @@ -54,6 +54,7 @@ public function addConfiguration(NodeDefinition $builder) ->children() ->scalarNode('limiter')->info(sprintf('A service id implementing "%s".', RequestRateLimiterInterface::class))->end() ->integerNode('max_attempts')->defaultValue(5)->end() + ->scalarNode('lock_factory')->info('The service ID of the lock factory used by the login rate limiter (or null to disable locking)')->defaultNull()->end() ->end(); } @@ -76,6 +77,7 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal 'policy' => 'fixed_window', 'limit' => $config['max_attempts'], 'interval' => '1 minute', + 'lock_factory' => $config['lock_factory'], ]; FrameworkExtension::registerRateLimiter($container, $localId = '_login_local_'.$firewallName, $limiterOptions);