symfony · fabpot · Jul 24, 2022 · Apr 19, 2022 · Apr 19, 2022 · Apr 19, 2022
diff --git a/src/Symfony/Component/HttpFoundation/RateLimiter/AbstractRequestRateLimiter.php b/src/Symfony/Component/HttpFoundation/RateLimiter/AbstractRequestRateLimiter.php
@@ -17,14 +17,24 @@
 use Symfony\Component\RateLimiter\RateLimit;
 
 /**
- * An implementation of RequestRateLimiterInterface that
+ * An implementation of PeekableRequestRateLimiterInterface that
  * fits most use-cases.
  *
  * @author Wouter de Jong <wouter@wouterj.nl>
  */
-abstract class AbstractRequestRateLimiter implements RequestRateLimiterInterface
+abstract class AbstractRequestRateLimiter implements PeekableRequestRateLimiterInterface
 {
     public function consume(Request $request): RateLimit
+    {
+        return $this->doConsume($request, 1);
+    }
+
+    public function peek(Request $request): RateLimit
+    {
+        return $this->doConsume($request, 0);
+    }
+
+    private function doConsume(Request $request, int $tokens): RateLimit
     {
         $limiters = $this->getLimiters($request);
         if (0 === \count($limiters)) {
@@ -33,7 +43,7 @@ public function consume(Request $request): RateLimit
 
         $minimalRateLimit = null;
         foreach ($limiters as $limiter) {
-            $rateLimit = $limiter->consume(1);
+            $rateLimit = $limiter->consume($tokens);
 
             if (null === $minimalRateLimit || $rateLimit->getRemainingTokens() < $minimalRateLimit->getRemainingTokens()) {
                 $minimalRateLimit = $rateLimit;

diff --git a/src/Symfony/Component/HttpFoundation/RateLimiter/PeekableRequestRateLimiterInterface.php b/src/Symfony/Component/HttpFoundation/RateLimiter/PeekableRequestRateLimiterInterface.php
@@ -0,0 +1,35 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpFoundation\RateLimiter;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\RateLimiter\RateLimit;
+
+/**
+ * A request limiter which allows peeking ahead.
+ *
+ * This is valuable to reduce the cache backend load in scenarios
+ * like a login when we only want to consume a token on login failure,
+ * and where the majority of requests will be successful and thus not
+ * need to consume a token.
+ *
+ * This way we can peek ahead before allowing the request through, and
+ * only consume if the request failed (1 backend op). This is compared
+ * to always consuming and then resetting the limit if the request
+ * is successful (2 backend ops).
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+interface PeekableRequestRateLimiterInterface extends RequestRateLimiterInterface
+{
+    public function peek(Request $request): RateLimit;
+}
diff --git a/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php
@@ -59,12 +59,13 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
 
             $now = microtime(true);
             $availableTokens = $window->getAvailableTokens($now);
-            if ($availableTokens >= $tokens) {
+
+            if ($availableTokens >= max(1, $tokens)) {
                 $window->add($tokens, $now);
 
                 $reservation = new Reservation($now, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit));
             } else {
-                $waitDuration = $window->calculateTimeForTokens($tokens);
+                $waitDuration = $window->calculateTimeForTokens(max(1, $tokens));
 
                 if (null !== $maxTime && $waitDuration > $maxTime) {
                     // process needs to wait longer than set interval
@@ -75,7 +76,10 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
 
                 $reservation = new Reservation($now + $waitDuration, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->limit));
             }
-            $this->storage->save($window);
+
+            if (0 < $tokens) {
+                $this->storage->save($window);
+            }
         } finally {
             $this->lock->release();
         }

diff --git a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php
@@ -74,7 +74,10 @@ public function consume(int $tokens = 1): RateLimit
             }
 
             $window->add($tokens);
-            $this->storage->save($window);
+
+            if (0 < $tokens) {
+                $this->storage->save($window);
+            }
 
             return new RateLimit($this->getAvailableTokens($window->getHitCount()), $window->getRetryAfter(), true, $this->limit);
         } finally {

diff --git a/src/Symfony/Component/RateLimiter/Policy/TokenBucketLimiter.php b/src/Symfony/Component/RateLimiter/Policy/TokenBucketLimiter.php
@@ -67,7 +67,8 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
 
             $now = microtime(true);
             $availableTokens = $bucket->getAvailableTokens($now);
-            if ($availableTokens >= $tokens) {
+
+            if ($availableTokens >= max(1, $tokens)) {
                 // tokens are now available, update bucket
                 $bucket->setTokens($availableTokens - $tokens);
                 $bucket->setTimer($now);
@@ -92,7 +93,9 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
                 $reservation = new Reservation($now + $waitDuration, new RateLimit(0, \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->maxBurst));
             }
 
-            $this->storage->save($bucket);
+            if (0 < $tokens) {
+                $this->storage->save($bucket);
+            }
         } finally {
             $this->lock->release();
         }

diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php
@@ -108,6 +108,20 @@ public function testWindowResilientToTimeShifting()
         $this->assertSame(100, $window->getAvailableTokens($serverOneClock));
     }
 
+    public function testPeekConsume()
+    {
+        $limiter = $this->createLimiter();
+
+        $limiter->consume(9);
+
+        // peek by consuming 0 tokens twice (making sure peeking doesn't claim a token)
+        for ($i = 0; $i < 2; ++$i) {
+            $rateLimit = $limiter->consume(0);
+            $this->assertSame(10, $rateLimit->getLimit());
+            $this->assertTrue($rateLimit->isAccepted());
+        }
+    }
+
     public function provideConsumeOutsideInterval(): \Generator
     {
         yield ['PT15S'];

diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php
@@ -76,6 +76,19 @@ public function testReserve()
         $this->createLimiter()->reserve();
     }
 
+    public function testPeekConsume()
+    {
+        $limiter = $this->createLimiter();
+
+        $limiter->consume(9);
+
+        for ($i = 0; $i < 2; ++$i) {
+            $rateLimit = $limiter->consume(0);
+            $this->assertTrue($rateLimit->isAccepted());
+            $this->assertSame(10, $rateLimit->getLimit());
+        }
+    }
+
     private function createLimiter(): SlidingWindowLimiter
     {
         return new SlidingWindowLimiter('test', 10, new \DateInterval('PT12S'), $this->storage);

diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/TokenBucketLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/TokenBucketLimiterTest.php
@@ -128,6 +128,19 @@ public function testBucketResilientToTimeShifting()
         $this->assertSame(100, $bucket->getAvailableTokens($serverOneClock));
     }
 
+    public function testPeekConsume()
+    {
+        $limiter = $this->createLimiter();
+
+        $limiter->consume(9);
+
+        for ($i = 0; $i < 2; ++$i) {
+            $rateLimit = $limiter->consume(0);
+            $this->assertTrue($rateLimit->isAccepted());
+            $this->assertSame(10, $rateLimit->getLimit());
+        }
+    }
+
     private function createLimiter($initialTokens = 10, Rate $rate = null)
     {
         return new TokenBucketLimiter('test', $initialTokens, $rate ?? Rate::perSecond(10), $this->storage);

@@ -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',
         ];
     }

@@ -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)