From 6971ecef9d5223e9996c14abb93458c33daa5989 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 29 Oct 2018 22:12:18 +0100 Subject: [PATCH] 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 --- .../lib/Controller/OauthApiController.php | 10 ++++- .../Controller/OauthApiControllerTest.php | 40 ++++++++++++++++++- lib/private/Server.php | 3 +- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 2083741fa0c89..978ca76d75b4f 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -24,6 +24,7 @@ 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 @@ public function __construct($appName, 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 @@ public function __construct($appName, $this->tokenProvider = $tokenProvider; $this->secureRandom = $secureRandom; $this->time = $time; + $this->throttler = $throttler; } /** @@ -164,6 +170,8 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client $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 1074848597179..7d5dc9be258f7 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -27,6 +27,7 @@ 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 @@ public function setUp() { $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 @@ public function setUp() { $this->clientMapper, $this->tokenProvider, $this->secureRandom, - $this->time + $this->time, + $this->throttler ); } @@ -286,6 +291,17 @@ public function testGetTokenValidAppToken() { '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 @@ public function testGetTokenValidAppTokenBasicAuth() { $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 @@ public function testGetTokenExpiredAppToken() { '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 84af87f0d2f4c..f66f780ad526d 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -758,7 +758,7 @@ public function __construct($webRoot, \OC\Config $config) { $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(), @@ -766,6 +766,7 @@ public function __construct($webRoot, \OC\Config $config) { $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.