Skip to content

Commit

Permalink
Merge pull request #38901 from nextcloud/backport/38773/stable24
Browse files Browse the repository at this point in the history
[stable24] Add bruteforce protection in OauthApiController
  • Loading branch information
julien-nc authored Jun 28, 2023
2 parents 603d464 + 42e08d6 commit 62c7dd0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
23 changes: 17 additions & 6 deletions apps/oauth2/lib/Controller/OauthApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public function __construct(string $appName,
/**
* @PublicPage
* @NoCSRFRequired
* @BruteForceProtection(action=oauth2GetToken)
*
* @param string $grant_type
* @param string $code
Expand All @@ -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
Expand All @@ -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'])) {
Expand All @@ -126,24 +133,28 @@ 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) {
$appToken = $e->getToken();
} 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)
Expand Down
6 changes: 6 additions & 0 deletions apps/oauth2/tests/Controller/OauthApiControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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')
Expand All @@ -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')
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 62c7dd0

Please sign in to comment.