From f22ab3e665124e79427f51049fea0f937b66cdbb Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 27 Jul 2017 14:14:20 +0200 Subject: [PATCH] Add metadata to \OCP\AppFramework\Http\Response::throttle Fixes https://github.com/nextcloud/server/issues/5891 Signed-off-by: Lukas Reschke --- core/Controller/LoginController.php | 2 +- .../Middleware/Security/BruteForceMiddleware.php | 2 +- lib/public/AppFramework/Http/Response.php | 16 +++++++++++++++- tests/Core/Controller/LoginControllerTest.php | 4 ++-- tests/lib/AppFramework/Http/ResponseTest.php | 5 +++++ .../Security/BruteForceMiddlewareTest.php | 4 ++++ 6 files changed, 28 insertions(+), 5 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 1c75b1f3c8..1243157125 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -248,7 +248,7 @@ class LoginController extends Controller { $args['redirect_url'] = $redirect_url; } $response = new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args)); - $response->throttle(); + $response->throttle(['user' => $user]); $this->session->set('loginMessages', [ ['invalidpassword'], [] ]); diff --git a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php index b361f453bd..75cf40b0ac 100644 --- a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php @@ -75,7 +75,7 @@ class BruteForceMiddleware extends Middleware { $action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action'); $ip = $this->request->getRemoteAddress(); $this->throttler->sleepDelay($ip, $action); - $this->throttler->registerAttempt($action, $ip); + $this->throttler->registerAttempt($action, $ip, $response->getThrottleMetadata()); } return parent::afterController($controller, $methodName, $response); diff --git a/lib/public/AppFramework/Http/Response.php b/lib/public/AppFramework/Http/Response.php index 087522386b..4e79e20a73 100644 --- a/lib/public/AppFramework/Http/Response.php +++ b/lib/public/AppFramework/Http/Response.php @@ -83,6 +83,8 @@ class Response { /** @var bool */ private $throttled = false; + /** @var array */ + private $throttleMetadata = []; /** * Caches the response @@ -328,10 +330,22 @@ class Response { * Marks the response as to throttle. Will be throttled when the * @BruteForceProtection annotation is added. * + * @param array $metadata * @since 12.0.0 */ - public function throttle() { + public function throttle(array $metadata = []) { $this->throttled = true; + $this->throttleMetadata = $metadata; + } + + /** + * Returns the throttle metadata, defaults to empty array + * + * @return array + * @since 13.0.0 + */ + public function getThrottleMetadata() { + return $this->throttleMetadata; } /** diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index bd2d0143ca..493bade9dd 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -307,7 +307,7 @@ class LoginControllerTest extends TestCase { ->method('deleteUserValue'); $expected = new \OCP\AppFramework\Http\RedirectResponse($loginPageUrl); - $expected->throttle(); + $expected->throttle(['user' => 'MyUserName']); $this->assertEquals($expected, $this->loginController->tryLogin($user, $password, '/apps/files')); } @@ -634,7 +634,7 @@ class LoginControllerTest extends TestCase { ->method('createRememberMeToken'); $expected = new RedirectResponse(''); - $expected->throttle(); + $expected->throttle(['user' => 'john']); $this->assertEquals($expected, $this->loginController->tryLogin('john@doe.com', 'just wrong', null)); } } diff --git a/tests/lib/AppFramework/Http/ResponseTest.php b/tests/lib/AppFramework/Http/ResponseTest.php index d8959face8..9267d86260 100644 --- a/tests/lib/AppFramework/Http/ResponseTest.php +++ b/tests/lib/AppFramework/Http/ResponseTest.php @@ -269,4 +269,9 @@ class ResponseTest extends \Test\TestCase { $this->childResponse->throttle(); $this->assertTrue($this->childResponse->isThrottled()); } + + public function testGetThrottleMetadata() { + $this->childResponse->throttle(['foo' => 'bar']); + $this->assertSame(['foo' => 'bar'], $this->childResponse->getThrottleMetadata()); + } } diff --git a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php index 14d3b79684..ae2345764f 100644 --- a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php @@ -112,6 +112,10 @@ class BruteForceMiddlewareTest extends TestCase { ->expects($this->once()) ->method('isThrottled') ->willReturn(true); + $response + ->expects($this->once()) + ->method('getThrottleMetadata') + ->willReturn([]); $this->reflector ->expects($this->once()) ->method('getAnnotationParameter')