diff options
author | Julien Veyssier <julien-nc@posteo.net> | 2023-06-12 17:36:49 +0200 |
---|---|---|
committer | Julien Veyssier <julien-nc@posteo.net> | 2023-06-21 11:35:43 +0200 |
commit | bebf19951149f32931e18f0ec3990761eb342a6a (patch) | |
tree | e2c5b7cf68a32a334042ec4b2917d4610812564d /apps/oauth2 | |
parent | c7a2ff2baa8d7cfd9f99cdfc9ff45a11d66a8720 (diff) | |
download | nextcloud-server-bebf19951149f32931e18f0ec3990761eb342a6a.tar.gz nextcloud-server-bebf19951149f32931e18f0ec3990761eb342a6a.zip |
add bruteforce protection in OauthApiController
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Diffstat (limited to 'apps/oauth2')
-rw-r--r-- | apps/oauth2/lib/Controller/OauthApiController.php | 22 | ||||
-rw-r--r-- | apps/oauth2/tests/Controller/OauthApiControllerTest.php | 6 |
2 files changed, 23 insertions, 5 deletions
diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index badafd3bb77..e07a2c2de15 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -64,6 +64,7 @@ class OauthApiController extends Controller { /** * @PublicPage * @NoCSRFRequired + * @BruteForceProtection(action=oauth2GetToken) * * @param string $grant_type * @param string $code @@ -76,9 +77,11 @@ class OauthApiController extends Controller { // 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 @@ -89,17 +92,21 @@ class OauthApiController extends Controller { 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'])) { @@ -111,15 +118,18 @@ class OauthApiController extends Controller { $storedClientSecret = $this->crypto->decrypt($client->getSecret()); } catch (\Exception $e) { $this->logger->error('OAuth client secret decryption error', ['exception' => $e]); + // we don't throttle here because it might not be a bruteforce attack return new JSONResponse([ 'error' => 'invalid_client', ], Http::STATUS_BAD_REQUEST); } // The client id and secret must match. Else we don't provide an access token! if ($client->getClientIdentifier() !== $client_id || $storedClientSecret !== $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); @@ -132,9 +142,11 @@ class OauthApiController extends Controller { } 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 eb9311dbbc7..c65302532a9 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -104,6 +104,7 @@ class OauthApiControllerTest extends TestCase { $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)); } @@ -112,6 +113,7 @@ class OauthApiControllerTest extends TestCase { $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') @@ -124,6 +126,7 @@ class OauthApiControllerTest extends TestCase { $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') @@ -136,6 +139,7 @@ class OauthApiControllerTest extends TestCase { $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); @@ -169,6 +173,7 @@ class OauthApiControllerTest extends TestCase { $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); @@ -191,6 +196,7 @@ class OauthApiControllerTest extends TestCase { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'token is invalid']); $accessToken = new AccessToken(); $accessToken->setClientId(42); |