[12.x] Introduce "after" rate limiting (#57125)

* [12.x] Allow rate limit hits to only be recorded after success

* Make after callback generic

---------

Co-authored-by: Joseph Silber <[email protected]>

Author: timacdonald

src/Illuminate/Cache/RateLimiting/Limit.php

@@ -25,6 +25,13 @@ class Limit
      */
     public $decaySeconds;
 
+    /**
+     * The after callback used to determine if the limiter should be hit.
+     *
+     * @var ?callable
+     */
+    public $afterCallback = null;
+
     /**
      * The response generator callback.
      *
@@ -129,6 +136,19 @@ public function by($key)
         return $this;
     }
 
+    /**
+     * Set the callback to determine if the limiter should be hit.
+     *
+     * @param  callable  $callback
+     * @return $this
+     */
+    public function after($callback)
+    {
+        $this->afterCallback = $callback;
+
+        return $this;
+    }
+
     /**
      * Set the callback that should generate the response when the limit is exceeded.
      *

src/Illuminate/Routing/Middleware/ThrottleRequests.php

@@ -98,6 +98,7 @@ public function handle($request, Closure $next, $maxAttempts = 60, $decayMinutes
                     'key' => $prefix.$this->resolveRequestSignature($request),
                     'maxAttempts' => $this->resolveMaxAttempts($request, $maxAttempts),
                     'decaySeconds' => 60 * $decayMinutes,
+                    'afterCallback' => null,
                     'responseCallback' => null,
                 ],
             ]
@@ -133,6 +134,7 @@ protected function handleRequestUsingNamedLimiter($request, Closure $next, $limi
                     'key' => self::$shouldHashKeys ? md5($limiterName.$limit->key) : $limiterName.':'.$limit->key,
                     'maxAttempts' => $limit->maxAttempts,
                     'decaySeconds' => $limit->decaySeconds,
+                    'afterCallback' => $limit->afterCallback,
                     'responseCallback' => $limit->responseCallback,
                 ];
             })->all()
@@ -156,12 +158,18 @@ protected function handleRequest($request, Closure $next, array $limits)
                 throw $this->buildException($request, $limit->key, $limit->maxAttempts, $limit->responseCallback);
             }
 
-            $this->limiter->hit($limit->key, $limit->decaySeconds);
+            if (! $limit->afterCallback) {
+                $this->limiter->hit($limit->key, $limit->decaySeconds);
+            }
         }
 
         $response = $next($request);
 
         foreach ($limits as $limit) {
+            if ($limit->afterCallback && ($limit->afterCallback)($response)) {
+                $this->limiter->hit($limit->key, $limit->decaySeconds);
+            }
+
             $response = $this->addHeaders(
                 $response,
                 $limit->maxAttempts,

tests/Integration/Http/ThrottleRequestsTest.php

@@ -10,14 +10,19 @@
 use Illuminate\Foundation\Auth\User;
 use Illuminate\Foundation\Testing\RefreshDatabase;
 use Illuminate\Http\Exceptions\ThrottleRequestsException;
+use Illuminate\Http\Request;
 use Illuminate\Routing\Exceptions\MissingRateLimiterException;
 use Illuminate\Routing\Middleware\ThrottleRequests;
 use Illuminate\Support\Carbon;
+use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Date;
+use Illuminate\Support\Facades\RateLimiter as RateLimiterFacade;
 use Illuminate\Support\Facades\Route;
 use Orchestra\Testbench\Attributes\WithConfig;
 use Orchestra\Testbench\Attributes\WithMigration;
 use Orchestra\Testbench\TestCase;
 use PHPUnit\Framework\Attributes\DataProvider;
+use Symfony\Component\HttpFoundation\Response;
 use Throwable;
 
 #[WithConfig('hashing.driver', 'bcrypt')]
@@ -410,6 +415,81 @@ public function testItFallbacksToUserAccessorWhenThereIsNoNamedLimiterWhenAuthen
             $this->assertEquals(Carbon::now()->addSeconds(2)->getTimestamp(), $e->getHeaders()['X-RateLimit-Reset']);
         }
     }
+
+    public function testItCanThrottleBasedOnResponse()
+    {
+        RateLimiterFacade::for('throttle-not-found', function (Request $request) {
+            return Limit::perMinute(1)->after(fn ($response) => $response->status() === 404);
+        });
+        Route::get('/', fn () => match(request('status')) {
+            '404' => abort(404),
+            default => 'ok',
+        })->middleware(ThrottleRequests::using('throttle-not-found'));
+
+        $this->travelTo('2000-01-01 00:00:00');
+        $this->get('?status=404')->assertNotFound();
+        $this->get('?status=404')->assertTooManyRequests();
+        $this->get('?status=404')->assertTooManyRequests();
+
+        $this->travelTo('2000-01-01 00:00:59');
+        $this->get('?status=404')->assertTooManyRequests();
+        $this->get('?status=404')->assertTooManyRequests();
+
+        $this->travelTo('2000-01-01 00:01:00');
+        $this->get('?status=404')->assertNotFound();
+        $this->get('?status=404')->assertTooManyRequests();
+        $this->get('?status=404')->assertTooManyRequests();
+
+        $this->travelTo('2000-01-01 00:01:59');
+        $this->get('?status=404')->assertTooManyRequests();
+        $this->get('?status=404')->assertTooManyRequests();
+
+        $this->travelTo('2000-01-01 00:02:00');
+        $this->get('?status=404')->assertNotFound();
+        $this->get('?status=404')->assertTooManyRequests();
+        $this->get('?status=404')->assertTooManyRequests();
+    }
+
+    public function testItDoesNotHitLimiterUntilResponseHasBeenGenerated()
+    {
+        ThrottleRequests::shouldHashKeys(false);
+        RateLimiterFacade::for('throttle-not-found', function (Request $request) {
+            return Limit::perMinute(1)->after(fn ($response) => $response->status() === 404);
+        });
+        $duringRequest = null;
+        Route::get('/', function () use (&$duringRequest) {
+            $duringRequest = [
+                Cache::get('throttle-not-found:'),
+                Cache::get('throttle-not-found::timer'),
+            ];
+
+            abort(404);
+        })->middleware(ThrottleRequests::using('throttle-not-found'));
+
+        $this->travelTo('2000-01-01 00:00:00');
+        $this->get('?status=404')->assertNotFound();
+
+        $this->assertSame([null, null], $duringRequest);
+        $this->assertSame([1, 946684860], [
+            Cache::get('throttle-not-found:'),
+            Cache::get('throttle-not-found::timer'),
+        ]);
+    }
+
+    public function testItReturnsConfiguredResponseWhenUsingAfterLimit(): void
+    {
+        ThrottleRequests::shouldHashKeys(false);
+        RateLimiterFacade::for('throttle-not-found', function (Request $request) {
+            return Limit::perMinute(1)
+                ->after(fn ($response) => $response->status() === 404)
+                ->response(fn () => response('ah ah ah', status: 429));
+        });
+        Route::get('/', fn () => abort(404))->middleware(ThrottleRequests::using('throttle-not-found'));
+
+        $this->travelTo('2000-01-01 00:00:00');
+        $this->get('?status=404')->assertNotFound();
+        $this->get('?status=404')->assertTooManyRequests()->assertContent('ah ah ah');
+    }
 }
 
 class UserWithAccessor extends User