Add PeekableRequestRateLimiterInterface and fix the LoginThrottlingListener to reduce amount of writes on the cache backend, fixes #40371

Author: Seldaek

EventListener/LoginThrottlingListener.php

@@ -12,11 +12,13 @@
 namespace Symfony\Component\Security\Http\EventListener;
 
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpFoundation\RateLimiter\PeekableRequestRateLimiterInterface;
 use Symfony\Component\HttpFoundation\RateLimiter\RequestRateLimiterInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Security\Core\Exception\TooManyLoginAttemptsAuthenticationException;
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
 use Symfony\Component\Security\Http\Event\CheckPassportEvent;
+use Symfony\Component\Security\Http\Event\LoginFailureEvent;
 use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
 use Symfony\Component\Security\Http\SecurityRequestAttributes;
 
@@ -44,21 +46,41 @@ public function checkPassport(CheckPassportEvent $event): void
         $request = $this->requestStack->getMainRequest();
         $request->attributes->set(SecurityRequestAttributes::LAST_USERNAME, $passport->getBadge(UserBadge::class)->getUserIdentifier());
 
-        $limit = $this->limiter->consume($request);
-        if (!$limit->isAccepted()) {
-            throw new TooManyLoginAttemptsAuthenticationException(ceil(($limit->getRetryAfter()->getTimestamp() - time()) / 60));
+        if ($this->limiter instanceof PeekableRequestRateLimiterInterface) {
+            $limit = $this->limiter->peek($request);
+            // Checking isAccepted here is not enough as peek consumes 0 token, it will
+            // be accepted even if there are 0 tokens remaining to be consumed. We check both
+            // anyway for safety in case third party implementations behave unexpectedly.
+            if (!$limit->isAccepted() || 0 === $limit->getRemainingTokens()) {
+                throw new TooManyLoginAttemptsAuthenticationException(ceil(($limit->getRetryAfter()->getTimestamp() - time()) / 60));
+            }
+        } else {
+            $limit = $this->limiter->consume($request);
+            if (!$limit->isAccepted()) {
+                throw new TooManyLoginAttemptsAuthenticationException(ceil(($limit->getRetryAfter()->getTimestamp() - time()) / 60));
+            }
         }
     }
 
     public function onSuccessfulLogin(LoginSuccessEvent $event): void
     {
-        $this->limiter->reset($event->getRequest());
+        if (!$this->limiter instanceof PeekableRequestRateLimiterInterface) {
+            $this->limiter->reset($event->getRequest());
+        }
+    }
+
+    public function onFailedLogin(LoginFailureEvent $event): void
+    {
+        if ($this->limiter instanceof PeekableRequestRateLimiterInterface) {
+            $this->limiter->consume($event->getRequest());
+        }
     }
 
     public static function getSubscribedEvents(): array
     {
         return [
             CheckPassportEvent::class => ['checkPassport', 2080],
+            LoginFailureEvent::class => 'onFailedLogin',
             LoginSuccessEvent::class => 'onSuccessfulLogin',
         ];
     }

Tests/EventListener/LoginThrottlingListenerTest.php

@@ -16,13 +16,13 @@
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\RateLimiter\RateLimiterFactory;
 use Symfony\Component\RateLimiter\Storage\InMemoryStorage;
-use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\Exception\TooManyLoginAttemptsAuthenticationException;
 use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
 use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
 use Symfony\Component\Security\Http\Event\CheckPassportEvent;
-use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
+use Symfony\Component\Security\Http\Event\LoginFailureEvent;
 use Symfony\Component\Security\Http\EventListener\LoginThrottlingListener;
 use Symfony\Component\Security\Http\RateLimiter\DefaultLoginRateLimiter;
 
@@ -61,12 +61,7 @@ public function testPreventsLoginWhenOverLocalThreshold()
 
         for ($i = 0; $i < 3; ++$i) {
             $this->listener->checkPassport($this->createCheckPassportEvent($passport));
-        }
-
-        $this->listener->onSuccessfulLogin($this->createLoginSuccessfulEvent($passport));
-
-        for ($i = 0; $i < 3; ++$i) {
-            $this->listener->checkPassport($this->createCheckPassportEvent($passport));
+            $this->listener->onFailedLogin($this->createLoginFailedEvent($passport));
         }
 
         $this->expectException(TooManyLoginAttemptsAuthenticationException::class);
@@ -82,6 +77,7 @@ public function testPreventsLoginWithMultipleCase()
 
         for ($i = 0; $i < 3; ++$i) {
             $this->listener->checkPassport($this->createCheckPassportEvent($passports[$i % 3]));
+            $this->listener->onFailedLogin($this->createLoginFailedEvent($passports[$i % 3]));
         }
 
         $this->expectException(TooManyLoginAttemptsAuthenticationException::class);
@@ -97,6 +93,7 @@ public function testPreventsLoginWhenOverGlobalThreshold()
 
         for ($i = 0; $i < 6; ++$i) {
             $this->listener->checkPassport($this->createCheckPassportEvent($passports[$i % 2]));
+            $this->listener->onFailedLogin($this->createLoginFailedEvent($passports[$i % 2]));
         }
 
         $this->expectException(TooManyLoginAttemptsAuthenticationException::class);
@@ -108,9 +105,9 @@ private function createPassport($username)
         return new SelfValidatingPassport(new UserBadge($username));
     }
 
-    private function createLoginSuccessfulEvent($passport)
+    private function createLoginFailedEvent($passport)
     {
-        return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), $passport, $this->createMock(TokenInterface::class), $this->requestStack->getCurrentRequest(), null, 'main');
+        return new LoginFailureEvent($this->createMock(AuthenticationException::class), $this->createMock(AuthenticatorInterface::class), $this->requestStack->getCurrentRequest(), null, 'main', $passport);
     }
 
     private function createCheckPassportEvent($passport)