Reset bruteforce on token refresh OAuth

When using atoken obtained via OAuth the token expires. Resulting in
brute force attempts hitting the requesting IP.

This resets the brute force attempts for that UID on a valid refresh of
the token.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php
index 2083741fa0c89ce95ca47ec0a1d34eda64c0de85..978ca76d75b4fb9ff65352e80b138059624918da 100644 (file)
--- a/apps/oauth2/lib/Controller/OauthApiController.php
+++ b/apps/oauth2/lib/Controller/OauthApiController.php
@@ -24,6 +24,7 @@ namespace OCA\OAuth2\Controller;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Token\ExpiredTokenException;
 use OC\Authentication\Token\IProvider as TokenProvider;
+use OC\Security\Bruteforce\Throttler;
 use OCA\OAuth2\Db\AccessTokenMapper;
 use OCA\OAuth2\Db\ClientMapper;
 use OCA\OAuth2\Exceptions\AccessTokenNotFoundException;
@@ -49,6 +50,8 @@ class OauthApiController extends Controller {
        private $secureRandom;
        /** @var ITimeFactory */
        private $time;
+       /** @var Throttler */
+       private $throttler;
 
        /**
         * @param string $appName
@@ -59,6 +62,7 @@ class OauthApiController extends Controller {
         * @param TokenProvider $tokenProvider
         * @param ISecureRandom $secureRandom
         * @param ITimeFactory $time
+        * @param Throttler $throttler
         */
        public function __construct($appName,
                                                                IRequest $request,
@@ -67,7 +71,8 @@ class OauthApiController extends Controller {
                                                                ClientMapper $clientMapper,
                                                                TokenProvider $tokenProvider,
                                                                ISecureRandom $secureRandom,
-                                                               ITimeFactory $time) {
+                                                               ITimeFactory $time,
+                                                               Throttler $throttler) {
                parent::__construct($appName, $request);
                $this->crypto = $crypto;
                $this->accessTokenMapper = $accessTokenMapper;
@@ -75,6 +80,7 @@ class OauthApiController extends Controller {
                $this->tokenProvider = $tokenProvider;
                $this->secureRandom = $secureRandom;
                $this->time = $time;
+               $this->throttler = $throttler;
        }
 
        /**
@@ -164,6 +170,8 @@ class OauthApiController extends Controller {
                $accessToken->setEncryptedToken($this->crypto->encrypt($newToken, $newCode));
                $this->accessTokenMapper->update($accessToken);
 
+               $this->throttler->resetDelay($this->request->getRemoteAddress(), 'login', ['user' => $appToken->getUID()]);
+
                return new JSONResponse(
                        [
                                'access_token' => $newToken,

diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php
index 1074848597179ab89a308f0478d056ebbb3b3fe0..7d5dc9be258f784736f98b8b9fa9ce86244190cc 100644 (file)
--- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php
+++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php
@@ -27,6 +27,7 @@ use OC\Authentication\Token\DefaultTokenMapper;
 use OC\Authentication\Token\ExpiredTokenException;
 use OC\Authentication\Token\IProvider as TokenProvider;
 use OC\Authentication\Token\IToken;
+use OC\Security\Bruteforce\Throttler;
 use OCA\OAuth2\Controller\OauthApiController;
 use OCA\OAuth2\Db\AccessToken;
 use OCA\OAuth2\Db\AccessTokenMapper;
@@ -57,6 +58,8 @@ class OauthApiControllerTest extends TestCase {
        private $secureRandom;
        /** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */
        private $time;
+       /** @var Throttler|\PHPUnit_Framework_MockObject_MockObject */
+       private $throttler;
        /** @var OauthApiController */
        private $oauthApiController;
 
@@ -70,6 +73,7 @@ class OauthApiControllerTest extends TestCase {
                $this->tokenProvider = $this->createMock(TokenProvider::class);
                $this->secureRandom = $this->createMock(ISecureRandom::class);
                $this->time = $this->createMock(ITimeFactory::class);
+               $this->throttler = $this->createMock(Throttler::class);
 
                $this->oauthApiController = new OauthApiController(
                        'oauth2',
@@ -79,7 +83,8 @@ class OauthApiControllerTest extends TestCase {
                        $this->clientMapper,
                        $this->tokenProvider,
                        $this->secureRandom,
-                       $this->time
+                       $this->time,
+                       $this->throttler
                );
        }
 
@@ -286,6 +291,17 @@ class OauthApiControllerTest extends TestCase {
                        'user_id' => 'userId',
                ]);
 
+               $this->request->method('getRemoteAddress')
+                       ->willReturn('1.2.3.4');
+
+               $this->throttler->expects($this->once())
+                       ->method('resetDelay')
+                       ->with(
+                               '1.2.3.4',
+                               'login',
+                               ['user' => 'userId']
+                       );
+
                $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret'));
        }
 
@@ -370,6 +386,17 @@ class OauthApiControllerTest extends TestCase {
                $this->request->server['PHP_AUTH_USER'] = 'clientId';
                $this->request->server['PHP_AUTH_PW'] = 'clientSecret';
 
+               $this->request->method('getRemoteAddress')
+                       ->willReturn('1.2.3.4');
+
+               $this->throttler->expects($this->once())
+                       ->method('resetDelay')
+                       ->with(
+                               '1.2.3.4',
+                               'login',
+                               ['user' => 'userId']
+                       );
+
                $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', null, null));
        }
 
@@ -451,6 +478,17 @@ class OauthApiControllerTest extends TestCase {
                        'user_id' => 'userId',
                ]);
 
+               $this->request->method('getRemoteAddress')
+                       ->willReturn('1.2.3.4');
+
+               $this->throttler->expects($this->once())
+                       ->method('resetDelay')
+                       ->with(
+                               '1.2.3.4',
+                               'login',
+                               ['user' => 'userId']
+                       );
+
                $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret'));
        }
 }

diff --git a/lib/private/Server.php b/lib/private/Server.php
index 5962012604a0970d9272044176228e3bab458530..c9a2d1d0ae7553b9ae249f5a6675111b06ffc965 100644 (file)
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -753,7 +753,7 @@ class Server extends ServerContainer implements IServerContainer {
                $this->registerService('TrustedDomainHelper', function ($c) {
                        return new TrustedDomainHelper($this->getConfig());
                });
-               $this->registerService('Throttler', function (Server $c) {
+               $this->registerService(Throttler::class, function (Server $c) {
                        return new Throttler(
                                $c->getDatabaseConnection(),
                                new TimeFactory(),
@@ -761,6 +761,7 @@ class Server extends ServerContainer implements IServerContainer {
                                $c->getConfig()
                        );
                });
+               $this->registerAlias('Throttler', Throttler::class);
                $this->registerService('IntegrityCodeChecker', function (Server $c) {
                        // IConfig and IAppManager requires a working database. This code
                        // might however be called when ownCloud is not yet setup.