Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 79f6a5c

Browse filesBrowse files
fabpotfabpot
committed
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 ------- 45be875 [Security][RateLimiter] Allow to use no lock in the rate limiter/login throttling
2 parents 59fbe57 + 45be875 commit 79f6a5c
Copy full SHA for 79f6a5c

File tree

Expand file treeCollapse file tree

8 files changed

+64
-8
lines changed
Filter options
Expand file treeCollapse file tree

8 files changed

+64
-8
lines changed

‎UPGRADE-5.3.md

Copy file name to clipboardExpand all lines: UPGRADE-5.3.md
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ Security
8484
SecurityBundle
8585
--------------
8686

87+
* [BC break] Add `login_throttling.lock_factory` setting defaulting to `null`. Set this option
88+
to `lock.factory` if you need precise login rate limiting with synchronous requests.
8789
* Deprecate `UserPasswordEncoderCommand` class and the corresponding `user:encode-password` command,
8890
use `UserPasswordHashCommand` and `user:hash-password` instead
8991
* 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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1874,7 +1874,7 @@ private function addRateLimiterSection(ArrayNodeDefinition $rootNode, callable $
18741874
->arrayPrototype()
18751875
->children()
18761876
->scalarNode('lock_factory')
1877-
->info('The service ID of the lock factory used by this limiter')
1877+
->info('The service ID of the lock factory used by this limiter (or null to disable locking)')
18781878
->defaultValue('lock.factory')
18791879
->end()
18801880
->scalarNode('cache_pool')

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+9-7Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ class FrameworkExtension extends Extension
205205
private $httpClientConfigEnabled = false;
206206
private $notifierConfigEnabled = false;
207207
private $propertyAccessConfigEnabled = false;
208-
private $lockConfigEnabled = false;
208+
private static $lockConfigEnabled = false;
209209

210210
/**
211211
* Responds to the app.config configuration parameter.
@@ -438,7 +438,7 @@ public function load(array $configs, ContainerBuilder $container)
438438
$this->registerPropertyInfoConfiguration($container, $loader);
439439
}
440440

441-
if ($this->lockConfigEnabled = $this->isConfigEnabled($container, $config['lock'])) {
441+
if (self::$lockConfigEnabled = $this->isConfigEnabled($container, $config['lock'])) {
442442
$this->registerLockConfiguration($config['lock'], $container, $loader);
443443
}
444444

@@ -2344,10 +2344,6 @@ private function registerNotifierConfiguration(array $config, ContainerBuilder $
23442344

23452345
private function registerRateLimiterConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader)
23462346
{
2347-
if (!$this->lockConfigEnabled) {
2348-
throw new LogicException('Rate limiter support cannot be enabled without enabling the Lock component.');
2349-
}
2350-
23512347
$loader->load('rate_limiter.php');
23522348

23532349
foreach ($config['limiters'] as $name => $limiterConfig) {
@@ -2362,7 +2358,13 @@ public static function registerRateLimiter(ContainerBuilder $container, string $
23622358

23632359
$limiter = $container->setDefinition($limiterId = 'limiter.'.$name, new ChildDefinition('limiter'));
23642360

2365-
$limiter->addArgument(new Reference($limiterConfig['lock_factory']));
2361+
if (null !== $limiterConfig['lock_factory']) {
2362+
if (!self::$lockConfigEnabled) {
2363+
throw new LogicException(sprintf('Rate limiter "%s" requires the Lock component to be installed and configured.', $name));
2364+
}
2365+
2366+
$limiter->replaceArgument(2, new Reference($limiterConfig['lock_factory']));
2367+
}
23662368
unset($limiterConfig['lock_factory']);
23672369

23682370
$storageId = $limiterConfig['storage_service'] ?? null;

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
->args([
2525
abstract_arg('config'),
2626
abstract_arg('storage'),
27+
null,
2728
])
2829
;
2930
};

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php
+47Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
use Symfony\Component\Config\FileLocator;
1515
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\DependencyInjection\Exception\LogicException;
17+
use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException;
1618
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
1719
use Symfony\Component\Workflow\Exception\InvalidDefinitionException;
1820

@@ -82,4 +84,49 @@ public function testWorkflowValidationStateMachine()
8284
]);
8385
});
8486
}
87+
88+
public function testRateLimiterWithLockFactory()
89+
{
90+
try {
91+
$this->createContainerFromClosure(function (ContainerBuilder $container) {
92+
$container->loadFromExtension('framework', [
93+
'rate_limiter' => [
94+
'with_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour'],
95+
],
96+
]);
97+
});
98+
99+
$this->fail('No LogicException thrown');
100+
} catch (LogicException $e) {
101+
$this->assertEquals('Rate limiter "with_lock" requires the Lock component to be installed and configured.', $e->getMessage());
102+
}
103+
104+
$container = $this->createContainerFromClosure(function (ContainerBuilder $container) {
105+
$container->loadFromExtension('framework', [
106+
'lock' => true,
107+
'rate_limiter' => [
108+
'with_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour'],
109+
],
110+
]);
111+
});
112+
113+
$withLock = $container->getDefinition('limiter.with_lock');
114+
$this->assertEquals('lock.factory', (string) $withLock->getArgument(2));
115+
}
116+
117+
public function testRateLimiterLockFactory()
118+
{
119+
$container = $this->createContainerFromClosure(function (ContainerBuilder $container) {
120+
$container->loadFromExtension('framework', [
121+
'rate_limiter' => [
122+
'without_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour', 'lock_factory' => null],
123+
],
124+
]);
125+
});
126+
127+
$this->expectException(OutOfBoundsException::class);
128+
$this->expectExceptionMessage('The argument "2" doesn\'t exist.');
129+
130+
$container->getDefinition('limiter.without_lock')->getArgument(2);
131+
}
85132
}

‎src/Symfony/Bundle/FrameworkBundle/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/composer.json
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
"symfony/messenger": "^5.2",
5252
"symfony/mime": "^4.4|^5.0",
5353
"symfony/process": "^4.4|^5.0",
54+
"symfony/rate-limiter": "^5.2",
5455
"symfony/security-bundle": "^5.3",
5556
"symfony/serializer": "^5.2",
5657
"symfony/stopwatch": "^4.4|^5.0",

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
5.3
55
---
66

7+
* [BC break] Add `login_throttling.lock_factory` setting defaulting to `null` (instead of `lock.factory`)
78
* Add the `debug:firewall` command.
89
* Deprecate `UserPasswordEncoderCommand` class and the corresponding `user:encode-password` command,
910
use `UserPasswordHashCommand` and `user:hash-password` instead

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public function addConfiguration(NodeDefinition $builder)
5454
->children()
5555
->scalarNode('limiter')->info(sprintf('A service id implementing "%s".', RequestRateLimiterInterface::class))->end()
5656
->integerNode('max_attempts')->defaultValue(5)->end()
57+
->scalarNode('lock_factory')->info('The service ID of the lock factory used by the login rate limiter (or null to disable locking)')->defaultNull()->end()
5758
->end();
5859
}
5960

@@ -76,6 +77,7 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
7677
'policy' => 'fixed_window',
7778
'limit' => $config['max_attempts'],
7879
'interval' => '1 minute',
80+
'lock_factory' => $config['lock_factory'],
7981
];
8082
FrameworkExtension::registerRateLimiter($container, $localId = '_login_local_'.$firewallName, $limiterOptions);
8183

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.