From ecd6dc304d2cf47121ef5f0c2f1075a05ff47680 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 19 Jun 2023 15:00:37 +0200 Subject: [PATCH] add bruteforce protection in OauthApiController Signed-off-by: Julien Veyssier --- .../lib/Controller/OauthApiController.php | 23 ++++++++++++++----- .../Controller/OauthApiControllerTest.php | 6 +++++ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 392eb09e89e12..9ef26026b16e7 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -81,6 +81,7 @@ public function __construct(string $appName, /** * @PublicPage * @NoCSRFRequired + * @BruteForceProtection(action=oauth2GetToken) * * @param string $grant_type * @param string $code @@ -93,9 +94,11 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client // We only handle two types if ($grant_type !== 'authorization_code' && $grant_type !== 'refresh_token') { - return new JSONResponse([ + $response = new JSONResponse([ 'error' => 'invalid_grant', ], Http::STATUS_BAD_REQUEST); + $response->throttle(['invalid_grant' => $grant_type]); + return $response; } // We handle the initial and refresh tokens the same way @@ -106,17 +109,21 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client try { $accessToken = $this->accessTokenMapper->getByCode($code); } catch (AccessTokenNotFoundException $e) { - return new JSONResponse([ + $response = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $response->throttle(['invalid_request' => 'token not found', 'code' => $code]); + return $response; } try { $client = $this->clientMapper->getByUid($accessToken->getClientId()); } catch (ClientNotFoundException $e) { - return new JSONResponse([ + $response = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $response->throttle(['invalid_request' => 'client not found', 'client_id' => $accessToken->getClientId()]); + return $response; } if (isset($this->request->server['PHP_AUTH_USER'])) { @@ -126,14 +133,16 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client // The client id and secret must match. Else we don't provide an access token! if ($client->getClientIdentifier() !== $client_id || $client->getSecret() !== $client_secret) { - return new JSONResponse([ + $response = new JSONResponse([ 'error' => 'invalid_client', ], Http::STATUS_BAD_REQUEST); + $response->throttle(['invalid_client' => 'client ID or secret does not match']); + return $response; } $decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code); - // Obtain the appToken assoicated + // Obtain the appToken associated try { $appToken = $this->tokenProvider->getTokenById($accessToken->getTokenId()); } catch (ExpiredTokenException $e) { @@ -141,9 +150,11 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client } catch (InvalidTokenException $e) { //We can't do anything... $this->accessTokenMapper->delete($accessToken); - return new JSONResponse([ + $response = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $response->throttle(['invalid_request' => 'token is invalid']); + return $response; } // Rotate the apptoken (so the old one becomes invalid basically) diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index 24385e785e50e..280db573c7fc5 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -94,6 +94,7 @@ public function testGetTokenInvalidGrantType() { $expected = new JSONResponse([ 'error' => 'invalid_grant', ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_grant' => 'foo']); $this->assertEquals($expected, $this->oauthApiController->getToken('foo', null, null, null, null)); } @@ -102,6 +103,7 @@ public function testGetTokenInvalidCode() { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'token not found', 'code' => 'invalidcode']); $this->accessTokenMapper->method('getByCode') ->with('invalidcode') @@ -114,6 +116,7 @@ public function testGetTokenInvalidRefreshToken() { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'token not found', 'code' => 'invalidrefresh']); $this->accessTokenMapper->method('getByCode') ->with('invalidrefresh') @@ -126,6 +129,7 @@ public function testGetTokenClientDoesNotExist() { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'client not found', 'client_id' => 42]); $accessToken = new AccessToken(); $accessToken->setClientId(42); @@ -159,6 +163,7 @@ public function testGetTokenInvalidClient($clientId, $clientSecret) { $expected = new JSONResponse([ 'error' => 'invalid_client', ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_client' => 'client ID or secret does not match']); $accessToken = new AccessToken(); $accessToken->setClientId(42); @@ -181,6 +186,7 @@ public function testGetTokenInvalidAppToken() { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'token is invalid']); $accessToken = new AccessToken(); $accessToken->setClientId(42);