Merge pull request #12208 from nextcloud/backport/12130/stable13

[13] Reset bruteforce on token refresh OAuth
This commit is contained in:
Roeland Jago Douma 2018-11-02 11:38:13 +01:00 committed by GitHub
commit 3aaeaf1316
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 50 additions and 3 deletions

View File

@ -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,

View File

@ -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'));
}
}

View File

@ -758,7 +758,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(),
@ -766,6 +766,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.