8000 Add PeekableRequestRateLimiterInterface and fix the LoginThrottlingLi… · symfony/symfony@495f679 · GitHub
[go: up one dir, main page]

Skip to content

Commit 495f679

Browse files
committed
Add PeekableRequestRateLimiterInterface and fix the LoginThrottlingListener to reduce amount of writes on the cache backend, fixes #40371
1 parent 4c2d670 commit 495f679

File tree

3 files changed

+73
-8
lines changed

3 files changed

+73
-8
lines changed

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

+13-3
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,24 @@
1717
use Symfony\Component\RateLimiter\RateLimit;
1818

1919
/**
20-
* An implementation of RequestRateLimiterInterface that
20+
* An implementation of PeekableRequestRateLimiterInterface that
2121
* fits most use-cases.
2222
*
2323
* @author Wouter de Jong <wouter@wouterj.nl>
2424
*/
25-
abstract class AbstractRequestRateLimiter implements RequestRateLimiterInterface
25+
abstract class AbstractRequestRateLimiter implements PeekableRequestRateLimiterInterface
2626
{
2727
public function consume(Request $request): RateLimit
28+
{
29+
return $this->doConsume($request, 1);
30+
}
31+
32+
public function peek(Request $request): RateLimit
33+
{
34+
return $this->doConsume($request, 0);
35+
}
36+
37+
private function doConsume(Request $request, int $tokens)
2838
{
2939
$limiters = $this->getLimiters($request);
3040
if (0 === \count($limiters)) {
@@ -33,7 +43,7 @@ public function consume(Request $request): RateLimit
3343

3444
$minimalRateLimit = null;
3545
foreach ($limiters as $limiter) {
36-
$rateLimit = $limiter->consume(1);
46+
$rateLimit = $limiter->consume($tokens);
3747

3848
if (null === $minimalRateLimit || $rateLimit->getRemainingTokens() < $minimalRateLimit->getRemainingTokens()) {
3949
$minimalRateLimit = $rateLimit;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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\RateLimit;
16+
17+
/**
18+
* A request limiter which allows peeking ahead.
19+
*
20+
* This is valuable to reduce the cache backend load in scenarios
21+
* like a login when we only want to consume a token on login failure,
22+
* and where the majority of requests will be successful and thus not
23+
* need to consume a token.
24+
*
25+
* This way we can peek ahead before allowing the request through, and
26+
* only consume if the request failed (1 backend op). This is compared
8000
27+
* to always consuming and then resetting the limit if the request
28+
* is successful (2 backend ops).
29+
*
30+
* @author Jordi Boggiano <j.boggiano@seld.be>
31+
*/
32+
interface PeekableRequestRateLimiterInterface extends RequestRateLimiterInterface
33+
{
34+
public function peek(Request $request): RateLimit;
35+
}

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

+25-5
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@
1212
namespace Symfony\Component\Security\Http\EventListener;
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15+
use Symfony\Component\HttpFoundation\RateLimiter\PeekableRequestRateLimiterInterface;
1516
use Symfony\Component\HttpFoundation\RateLimiter\RequestRateLimiterInterface;
1617
use Symfony\Component\HttpFoundation\RequestStack;
1718
use Symfony\Component\Security\Core\Exception\TooManyLoginAttemptsAuthenticationException;
1819
use Symfony\Component\Security\Core\Security;
1920
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
2021
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
22+
use Symfony\Component\Security\Http\Event\LoginFailureEvent;
2123
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
2224

2325
/**
@@ -44,22 +46,40 @@ public function checkPassport(CheckPassportEvent $event): void
4446
$request = $this->requestStack->getMainRequest();
4547
$request->attributes->set(Security::LAST_USERNAME, $passport->getBadge(UserBadge::class)->getUserIdentifier());
4648

47-
$limit = $this->limiter->consume($request);
48-
if (!$limit->isAccepted()) {
49+
if ($this->limiter instanceof PeekableRequestRateLimiterInterface) {
50+
$limit = $this->limiter->peek($request);
51+
} else {
52+
$limit = $this->limiter->consume($request);
53+
}
54+
55+
// Checking isAccepted here is not enough as peek consumes 0 token, it will
56+
// be accepted even if there are 0 tokens remaining to be consumed. We check both
57+
// anyway for safety in case third party implementations behave unexpectedly.
58+
if (!$limit->isAccepted() || 0 === $limit->getRemainingTokens()) {
4959
throw new TooManyLoginAttemptsAuthenticationException(ceil(($limit->getRetryAfter()->getTimestamp() - time()) / 60));
5060
}
5161
}
5262

53-
public function onSuccessfulLogin(LoginSuccessEvent $event): void
63+
public function onLoginFailure(LoginFailureEvent $event): void
5464
{
55-
$this->limiter->reset($event->getRequest());
65+
if ($this->limiter instanceof PeekableRequestRateLimiterInterface) {
66+
$this->limiter->consume($event->getRequest());
67+
}
68+
}
69+
70+
public function onLoginSuccess(LoginSuccessEvent $event): void
71+
{
72+
if (!$this->limiter instanceof PeekableRequestRateLimiterInterface) {
73+
$this->limiter->reset($event->getRequest());
74+
}
5675
}
5776

5877
public static function getSubscribedEvents(): array
5978
{
6079
return [
6180
CheckPassportEvent::class => ['checkPassport', 2080],
62-
LoginSuccessEvent::class => 'onSuccessfulLogin',
81+
LoginFailureEvent::class => 'onLoginFailure',
82+
LoginSuccessEvent::class => 'onLoginSuccess',
6383
];
6484
}
6585
}

0 commit comments

Comments
 (0)
0