feature #40284 [RateLimiter][Security] Allow to use no lock in the rate limiter/login throttling (wouterj)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[RateLimiter][Security] Allow to use no lock in the rate limiter/login throttling

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix -
| License       | MIT
| Doc PR        | tbd

This PR adds support for disabling lock in rate limiters. This was brought up by @Seldaek. In most cases (e.g. login throttling), it's not critical to strictly avoid even a single overflow of the window/token. At least, it's probably not always worth the extra load on the lock storage (e.g. redis).

It also directly disables locking by default for login throttling. I'm not sure about this, but I feel like this fits the 80% case where it's definitely not needed (and it's easier to use if you don't need to set-up locking first).

Commits
-------

45be875e84 [Security][RateLimiter] Allow to use no lock in the rate limiter/login throttling

UPGRADE-5.3.md

@@ -84,6 +84,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,

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -1874,7 +1874,7 @@ class Configuration implements ConfigurationInterface
                             ->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')

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 @@ class FrameworkExtension extends Extension
             $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 @@ class FrameworkExtension extends Extension
 
     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 @@ class FrameworkExtension extends Extension
 
         $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;

src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php

@@ -24,6 +24,7 @@ return static function (ContainerConfigurator $container) {
             ->args([
                 abstract_arg('config'),
                 abstract_arg('storage'),
+                null,
             ])
     ;
 };

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php

@@ -13,6 +13,8 @@ namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection;
 
 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 @@ class PhpFrameworkExtensionTest extends FrameworkExtensionTest
             ]);
         });
     }
+
+    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);
+    }
 }

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",

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

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php

@@ -54,6 +54,7 @@ class LoginThrottlingFactory implements AuthenticatorFactoryInterface, SecurityF
             ->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 @@ class LoginThrottlingFactory implements AuthenticatorFactoryInterface, SecurityF
                 'policy' => 'fixed_window',
                 'limit' => $config['max_attempts'],
                 'interval' => '1 minute',
+                'lock_factory' => $config['lock_factory'],
             ];
             FrameworkExtension::registerRateLimiter($container, $localId = '_login_local_'.$firewallName, $limiterOptions);