From 94211721a66c9f2c2f36d638b6492d843a6b8711 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 11 May 2023 09:23:50 +0200 Subject: [PATCH] fix(middleware): Also abort the request when reaching max delay in afterController Signed-off-by: Joas Schilling --- .../Middleware/Security/BruteForceMiddleware.php | 10 +++++++++- .../Middleware/Security/BruteForceMiddlewareTest.php | 6 +++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php index 069d04a9e753f..c36c6412fb73b 100644 --- a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php @@ -78,8 +78,16 @@ public function afterController($controller, $methodName, Response $response) { if ($this->reflector->hasAnnotation('BruteForceProtection') && $response->isThrottled()) { $action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action'); $ip = $this->request->getRemoteAddress(); - $this->throttler->sleepDelay($ip, $action); $this->throttler->registerAttempt($action, $ip, $response->getThrottleMetadata()); + try { + $this->throttler->sleepDelayOrThrowOnMax($ip, $action); + } catch (MaxDelayReached $e) { + if ($controller instanceof OCSController) { + throw new OCSException($e->getMessage(), Http::STATUS_TOO_MANY_REQUESTS); + } + + return new TooManyRequestsResponse(); + } } return parent::afterController($controller, $methodName, $response); diff --git a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php index 7dfcfe222611e..7f51c3d3a2a69 100644 --- a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php @@ -125,7 +125,7 @@ public function testAfterControllerWithAnnotationAndThrottledRequest() { ->willReturn('127.0.0.1'); $this->throttler ->expects($this->once()) - ->method('sleepDelay') + ->method('sleepDelayOrThrowOnMax') ->with('127.0.0.1', 'login'); $this->throttler ->expects($this->once()) @@ -157,7 +157,7 @@ public function testAfterControllerWithAnnotationAndNotThrottledRequest() { ->method('getRemoteAddress'); $this->throttler ->expects($this->never()) - ->method('sleepDelay'); + ->method('sleepDelayOrThrowOnMax'); $this->throttler ->expects($this->never()) ->method('registerAttempt'); @@ -181,7 +181,7 @@ public function testAfterControllerWithoutAnnotation() { ->method('getRemoteAddress'); $this->throttler ->expects($this->never()) - ->method('sleepDelay'); + ->method('sleepDelayOrThrowOnMax'); /** @var Controller|\PHPUnit\Framework\MockObject\MockObject $controller */ $controller = $this->createMock(Controller::class);