add bruteforce protection in OauthApiController

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>

diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php
index badafd3bb77f8d72556e70edcce9a513c98fe515..e07a2c2de1579a1655c96ec5fc975248e0b3d421 100644 (file)
--- 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 eb9311dbbc7552bdb55f6c5cebfb3c58bdb07481..c65302532a93794a316cd5337452c4112118c1e6 100644 (file)
--- 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);