8000 Added request rate limiters and improved login throttling · symfony/symfony@9eaab61 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9eaab61

Browse files
wouterjwouterj
committed
Added request rate limiters and improved login throttling
This allows limiting on different elements of a request. This is usefull to e.g. prevent breadth-first attacks, by allowing to enforce a limit on both IP and IP+username.
1 parent f06f2f0 commit 9eaab61

File tree

11 files changed

+210
-40
lines changed

11 files changed

+210
-40
lines changed

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
use Symfony\Component\DependencyInjection\ChildDefinition;
1818
use Symfony\Component\DependencyInjection\ContainerBuilder;
1919
use Symfony\Component\DependencyInjection\Reference;
20+
use Symfony\Component\HttpFoundation\RateLimiter\RequestRateLimiterInterface;
2021
use Symfony\Component\Security\Http\EventListener\LoginThrottlingListener;
22+
use Symfony\Component\Security\Http\RateLimiter\DefaultLoginRateLimiter;
2123

2224
/**
2325
* @author Wouter de Jong <wouter@wouterj.nl>
@@ -49,7 +51,7 @@ public function addConfiguration(NodeDefinition $builder)
4951
{
5052
$builder
5153
->children()
52-
->scalarNode('limiter')->info('The name of the limiter that you defined under "framework.rate_limiter".')->end()
54+
->scalarNode('limiter')->info(sprintf('A service id implementing "%s".', RequestRateLimiterInterface::class))->end()
5355
->integerNode('max_attempts')->defaultValue(5)->end()
5456
->end();
5557
}
@@ -65,18 +67,27 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
6567
throw new \LogicException('You must either configure a rate limiter for "security.firewalls.'.$firewallName.'.login_throttling" or install symfony/framework-bundle:^5.2');
6668
}
6769

68-
FrameworkExtension::registerRateLimiter($container, $config['limiter'] = '_login_'.$firewallName, [
70+
$limiterOptions = [
6971
'strategy' => 'fixed_window',
7072
'limit' => $config['max_attempts'],
7173
'interval' => '1 minute',
7274
'lock_factory' => 'lock.factory',
7375
'cache_pool' => 'cache.app',
74-
]);
76+
];
77+
FrameworkExtension::registerRateLimiter($container, $localId = '_login_local_'.$firewallName, $limiterOptions);
78+
79+
$limiterOptions['limit'] = 5 * $config['max_attempts'];
80+
FrameworkExtension::registerRateLimiter($container, $globalId = '_login_global_'.$firewallName, $limiterOptions);
81+
82+
$container->register($config['limiter'] = 'security.login_throttling.'.$firewallName.'.limiter', DefaultLoginRateLimiter::class)
83+
->addArgument(new Reference('limiter.'.$globalId))
84+
->addArgument(new Reference('limiter.'.$localId))
85+
;
7586
}
7687

7788
$container
7889
->setDefinition('security.listener.login_throttling.'.$firewallName, new ChildDefinition('security.listener.login_throttling'))
79-
->replaceArgument(1, new Reference('limiter.'.$config['limiter']))
90+
->replaceArgument(1, new Reference($config['limiter']))
8091
->addTag('kernel.event_subscriber', ['dispatcher' => 'security.event_dispatcher.'.$firewallName]);
8192

8293
return [];

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@
118118
->abstract()
119119
->args([
120120
service('request_stack'),
121-
abstract_arg('rate limiter'),
121+
abstract_arg('request rate limiter'),
122122
])
123123

124124
// Authenticators

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/views/Login/login.html.twig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
{% if error %}
66
<div>{{ error.messageKey }}</div>
7+
<div>{{ error.messageKey|replace(error.messageData) }}</div>
78
{% endif %}
89

910
<form action="{{ path('form_login_check') }}" method="post">

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public function testLoginThrottling()
127127
$client->submit($form);
128128

129129
$text = $client->followRedirect()->text(null, true);
130-
$this->assertStringContainsString('Too many failed login attempts, please try again later.', $text);
130+
$this->assertStringContainsString('Too many failed login attempts, please try again in 1 minute.', $text);
131131
}
132132

133133
public function provideClientOptions()

‎src/Symfony/Component/HttpFoundation/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
* added `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names
99
* added `File::getContent()`
1010
* added ability to use comma separated ip addresses for `RequestMatcher::matchIps()`
11+
* added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter`
1112

1213
5.1.0
1314
-----

‎src/Symfony/Component/HttpFoundation/RateLimiter/AbstractRequestRateLimiter.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\RateLimiter;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\RateLimiter\Limit;
16+
use Symfony\Component\RateLimiter\LimiterInterface;
17+
18+
/**
19+
* An implementation of RequestRateLimiterInterface that
20+
* fits most use-cases.
21+
*
22+
* @author Wouter de Jong <wouter@wouterj.nl>
23+
*
24+
* @experimental in Symfony 5.2
25+
*/
26+
abstract class AbstractRequestRateLimiter implements RequestRateLimiterInterface
27+
{
28+
public function consume(Request $request): Limit
29+
{
30+
$minimalLimit = null;
31+
foreach ($this->getLimiters($request) as $limiter) {
32+
$limit = $limiter->consume(1);
33+
34+
if (null === $minimalLimit || $limit->getRemainingTokens() < $minimalLimit->getRemainingTokens()) {
35+
$minimalLimit = $limit;
36+
}
37+
}
38+
39+
return $minimalLimit;
40+
}
41+
42+
public function reset(): void
43+
{
44+
foreach ($this->getLimiters($request) as $limiter) {
45+
$limiter->reset();
46+
}
47+
}
48+
49+
/**
50+
* @return LimiterInterface[] a set of limiters using keys extracted from the request
51+
*/
52+
abstract protected function getLimiters(Request $request): array;
53+
}

‎src/Symfony/Component/HttpFoundation/RateLimiter/RequestRateLimiterInterface.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\RateLimiter;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\RateLimiter\Limit;
16+
17+
/**
18+
* A special type of limiter that deals with requests.
19+
*
20+
* This allows to limit on different types of information
21+
* from the requests.
22+
*
23+
* @author Wouter de Jong <wouter@wouterj.nl>
24+
*
25+
* @experimental in Symfony 5.2
26+
*/
27+
interface RequestRateLimiterInterface
28+
{
29+
public function consume(Request $request): Limit;
30+
31+
public function reset(): void;
32+
}

‎src/Symfony/Component/Security/Core/Exception/TooManyLoginAttemptsAuthenticationException.php

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,46 @@
1919
*/
2020
class TooManyLoginAttemptsAuthenticationException extends AuthenticationException
2121
{
22+
private $threshold;
23+
24+
public function __construct(int $threshold = null)
25+
{
26+
$this->threshold = $threshold;
27+
}
28+
29+
/**
30+
* {@inheritdoc}
31+
*/
32+
public function getMessageData(): array
33+
{
34+
return [
35+
'%minutes%' => $this->threshold,
36+
];
37+
}
38+
2239
/**
2340
* {@inheritdoc}
2441
*/
2542
public function getMessageKey(): string
2643
{
27-
return 'Too many failed login attempts, please try again later.';
44+
return 'Too many failed login attempts, please try again '.($this->threshold ? 'in %minutes% minute'.($this->threshold > 1 ? 's' : '').'.' : 'later.');
45+
}
46+
47+
/**
48+
* {@inheritdoc}
49+
*/
50+
public function __serialize(): array
51+
{
52+
return [$this->threshold, parent::__serialize()];
53+
}
54+
55+
/**
56+
* {@inheritdoc}
57+
*/
58+
public function __unserialize(array $data): void
59+
{
60+
[$this->threshold, $parentData] = $data;
61+
$parentData = \is_array($parentData) ? $parentData : unserialize($parentData);
62+
parent::__unserialize($parentData);
2863
}
2964
}

‎src/Symfony/Component/Security/Http/EventListener/LoginThrottlingListener.php

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@
1212
namespace Symfony\Component\Security\Http\EventListener;
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15-
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\HttpFoundation\RateLimiter\RequestRateLimiterInterface;
1616
use Symfony\Component\HttpFoundation\RequestStack;
17-
use Symfony\Component\RateLimiter\Limiter;
1817
use Symfony\Component\Security\Core\Exception\TooManyLoginAttemptsAuthenticationException;
18+
use Symfony\Component\Security\Core\Security;
1919
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
2020
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
21-
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
2221

2322
/**
2423
* @author Wouter de Jong <wouter@wouterj.nl>
@@ -30,7 +29,7 @@ final class LoginThrottlingListener implements EventSubscriberInterface
3029
private $requestStack;
3130
private $limiter;
3231

33-
public function __construct(RequestStack $requestStack, Limiter $limiter)
32+
public function __construct(RequestStack $requestStack, RequestRateLimiterInterface $limiter)
3433
{
3534
$this->requestStack = $requestStack;
3635
$this->limiter = $limiter;
@@ -44,33 +43,18 @@ public function checkPassport(CheckPassportEvent $event): void
4443
}
4544

4645
$request = $this->requestStack->getMasterRequest();
47-
$username = $passport->getBadge(UserBadge::class)->getUserIdentifier();
48-
$limiterKey = $this->createLimiterKey($username, $request);
46+
$request->attributes->set(Security::LAST_USERNAME, $passport->getBadge(UserBadge::class)->getUserIdentifier());
4947

50-
$limiter = $this->limiter->create($limiterKey);
51-
if (!$limiter->consume()->isAccepted()) {
52-
throw new TooManyLoginAttemptsAuthenticationException();
48+
$limit = $this->limiter->consume($request);
49+
if (!$limit->isAccepted()) {
50+
throw new TooManyLoginAttemptsAuthenticationException(ceil(($limit->getRetryAfter()->getTimestamp() - time()) / 60));
5351
}
5452
}
5553

56-
public function onSuccessfulLogin(LoginSuccessEvent $event): void
57-
{
58-
$limiterKey = $this->createLimiterKey($event->getAuthenticatedToken()->getUsername(), $event->getRequest());
59-
$limiter = $this->limiter->create($limiterKey);
60-
61-
$limiter->reset();
62-
}
63-
6454
public static function getSubscribedEvents(): array
6555
{
6656
return [
6757
CheckPassportEvent::class => ['checkPassport', 64],
68-
LoginSuccessEvent::class => 'onSuccessfulLogin',
6958
];
7059
}
71-
72-
private function createLimiterKey($username, Request $request): string
73-
{
74-
return $username.$request->getClientIp();
75-
}
7660
}

‎src/Symfony/Component/Security/Http/RateLimiter/DefaultLoginRateLimiter.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Http\RateLimiter;
13+
14+
use Symfony\Component\HttpFoundation\RateLimiter\AbstractRequestRateLimiter;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\RateLimiter\Limiter;
17+
use Symfony\Component\Security\Core\Security;
18+
19+
/**
20+
* A default login throttling limiter.
21+
*
22+
* This limiter prevents breadth-first attacks by enforcing
23+
* a limit on username+IP and a (higher) limit on IP.
24+
*
25+
* @author Wouter de Jong <wouter@wouterj.nl>
26+
*
27+
* @experimental in Symfony 5.2
28+
*/
29+
final class DefaultLoginRateLimiter extends AbstractRequestRateLimiter
30+
{
31+
private $globalLimiter;
32+
private $localLimiter;
33+
34+
public function __construct(Limiter $globalLimiter, Limiter $localLimiter)
35+
{
36+
$this->globalLimiter = $globalLimiter;
37+
$this->localLimiter = $localLimiter;
38+
}
39+
40+
protected function getLimiters(Request $request): array
41+
{
42+
return [
43+
$this->globalLimiter->create($request->getClientIp()),
44+
$this->localLimiter->create($request->attributes->get(Security::LAST_USERNAME).$request->getClientIp()),
45+
];
46+
}
47+
}

‎src/Symfony/Component/Security/Http/Tests/EventListener/LoginThrottlingListenerTest.php

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
2525
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
2626
use Symfony\Component\Security\Http\EventListener\LoginThrottlingListener;
27+
use Symfony\Component\Security\Http\RateLimiter\DefaultLoginRateLimiter;
2728

2829
class LoginThrottlingListenerTest extends TestCase
2930
{
@@ -34,17 +35,24 @@ protected function setUp(): void
3435
{
3536
$this->requestStack = new RequestStack();
3637

37-
$limiter = new Limiter([
38+
$localLimiter = new Limiter([
3839
'id' => 'login',
3940
'strategy' => 'fixed_window',
4041
'limit' => 3,
4142
'interval' => '1 minute',
4243
], new InMemoryStorage());
44+
$globalLimiter = new Limiter([
45+
'id' => 'login',
46+
'strategy' => 'fixed_window',
47+
'limit' => 6,
48+
'interval' => '1 minute',
49+
], new InMemoryStorage());
50+
$limiter = new DefaultLoginRateLimiter($globalLimiter, $localLimiter);
4351

4452
$this->listener = new LoginThrottlingListener($this->requestStack, $limiter);
4553
}
4654

47-
public function testPreventsLoginWhenOverThreshold()
55+
public function testPreventsLoginWhenOverLocalThreshold()
4856
{
4957
$request = $this->createRequest();
5058
$passport = $this->createPassport('wouter');
@@ -59,21 +67,19 @@ public function testPreventsLoginWhenOverThreshold()
5967
$this->listener->checkPassport($this->createCheckPassportEvent($passport));
6068
}
6169

62-
public function testSuccessfulLoginResetsCount()
70+
public function testPreventsLoginWhenOverGlobalThreshold()
6371
{
64-
$this->expectNotToPerformAssertions();
65-
6672
$request = $this->createRequest();
67-
$passport = $this->createPassport('wouter');
73+
$passports = [$this->createPassport('wouter'), $this->createPassport('ryan')];
6874

6975
$this->requestStack->push($request);
7076

71-
for ($i = 0; $i < 3; ++$i) {
72-
$this->listener->checkPassport($this->createCheckPassportEvent($passport));
77+
for ($i = 0; $i < 6; ++$i) {
78+
$this->listener->checkPassport($this->createCheckPassportEvent($passports[$i % 2]));
7379
}
7480

75-
$this->listener->onSuccessfulLogin($this->createLoginSuccessfulEvent($passport));
76-
$this->listener->checkPassport($this->createCheckPassportEvent($passport));
81+
$this->expectException(TooManyLoginAttemptsAuthenticationException::class);
82+
$this->listener->checkPassport($this->createCheckPassportEvent($passports[0]));
7783
}
7884

7985
private function createPassport($username)

0 commit comments

Comments
 (0)
0