Skip to content

Commit d6ee1aa

Browse files
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 19005ec commit d6ee1aa

File tree

4 files changed

+72
-35
lines changed

4 files changed

+72
-35
lines changed

EventListener/LoginThrottlingListener.php

Lines changed: 8 additions & 24 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 <[email protected]>
@@ -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 [
67-
CheckPassportEvent::class => ['checkPassport', 64],
68-
LoginSuccessEvent::class => 'onSuccessfulLogin',
57+
CheckPassportEvent::class => ['checkPassport', 2080],
6958
];
7059
}
71-
72-
private function createLimiterKey($username, Request $request): string
73-
{
74-
return $username.$request->getClientIp();
75-
}
7660
}
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 <[email protected]>
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 <[email protected]>
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+
}

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)

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
"php": ">=7.2.5",
2020
"symfony/deprecation-contracts": "^2.1",
2121
"symfony/security-core": "^5.2",
22-
"symfony/http-foundation": "^4.4.7|^5.0.7",
22+
"symfony/http-foundation": "^5.2",
2323
"symfony/http-kernel": "^5.2",
2424
"symfony/polyfill-php80": "^1.15",
2525
"symfony/property-access": "^4.4|^5.0"

0 commit comments

Comments
 (0)