Merge pull request #12160 from Dagefoerde/stable14-oauth-backports

[stable14] Bruteforce protection handling in combination with
nextcloud · Nov 1, 2018 · d1d82fc · d1d82fc
2 parents 41c842f + e3f3212
commit d1d82fc
Show file tree

Hide file tree

Showing 13 changed files with 73 additions and 17 deletions.
diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php
@@ -22,8 +22,9 @@
 namespace OCA\OAuth2\Controller;
 
 use OC\Authentication\Exceptions\InvalidTokenException;
-use OC\Authentication\Token\ExpiredTokenException;
+use OC\Authentication\Exceptions\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,14 +71,16 @@ 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;
 		$this->clientMapper = $clientMapper;
 		$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
@@ -22,11 +22,10 @@
 namespace OCA\OAuth2\Tests\Controller;
 
 use OC\Authentication\Exceptions\InvalidTokenException;
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Token\DefaultToken;
-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 +56,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 +71,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 +81,8 @@ public function setUp() {
 			$this->clientMapper,
 			$this->tokenProvider,
 			$this->secureRandom,
-			$this->time
+			$this->time,
+			$this->throttler
 		);
 	}
 
@@ -286,6 +289,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 +384,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 +476,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/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -430,6 +430,7 @@
     'OC\\Archive\\Archive' => $baseDir . '/lib/private/Archive/Archive.php',
     'OC\\Archive\\TAR' => $baseDir . '/lib/private/Archive/TAR.php',
     'OC\\Archive\\ZIP' => $baseDir . '/lib/private/Archive/ZIP.php',
+    'OC\\Authentication\\Exceptions\\ExpiredTokenException' => $baseDir . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php',
     'OC\\Authentication\\Exceptions\\InvalidTokenException' => $baseDir . '/lib/private/Authentication/Exceptions/InvalidTokenException.php',
     'OC\\Authentication\\Exceptions\\LoginRequiredException' => $baseDir . '/lib/private/Authentication/Exceptions/LoginRequiredException.php',
     'OC\\Authentication\\Exceptions\\PasswordLoginForbiddenException' => $baseDir . '/lib/private/Authentication/Exceptions/PasswordLoginForbiddenException.php',
@@ -442,7 +443,6 @@
     'OC\\Authentication\\Token\\DefaultTokenCleanupJob' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenCleanupJob.php',
     'OC\\Authentication\\Token\\DefaultTokenMapper' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenMapper.php',
     'OC\\Authentication\\Token\\DefaultTokenProvider' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenProvider.php',
-    'OC\\Authentication\\Token\\ExpiredTokenException' => $baseDir . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php',
     'OC\\Authentication\\Token\\IProvider' => $baseDir . '/lib/private/Authentication/Token/IProvider.php',
     'OC\\Authentication\\Token\\IToken' => $baseDir . '/lib/private/Authentication/Token/IToken.php',
     'OC\\Authentication\\Token\\Manager' => $baseDir . '/lib/private/Authentication/Token/Manager.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -460,6 +460,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Archive\\Archive' => __DIR__ . '/../../..' . '/lib/private/Archive/Archive.php',
         'OC\\Archive\\TAR' => __DIR__ . '/../../..' . '/lib/private/Archive/TAR.php',
         'OC\\Archive\\ZIP' => __DIR__ . '/../../..' . '/lib/private/Archive/ZIP.php',
+        'OC\\Authentication\\Exceptions\\ExpiredTokenException' => __DIR__ . '/../../..' . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php',
         'OC\\Authentication\\Exceptions\\InvalidTokenException' => __DIR__ . '/../../..' . '/lib/private/Authentication/Exceptions/InvalidTokenException.php',
         'OC\\Authentication\\Exceptions\\LoginRequiredException' => __DIR__ . '/../../..' . '/lib/private/Authentication/Exceptions/LoginRequiredException.php',
         'OC\\Authentication\\Exceptions\\PasswordLoginForbiddenException' => __DIR__ . '/../../..' . '/lib/private/Authentication/Exceptions/PasswordLoginForbiddenException.php',
@@ -472,7 +473,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Authentication\\Token\\DefaultTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenCleanupJob.php',
         'OC\\Authentication\\Token\\DefaultTokenMapper' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenMapper.php',
         'OC\\Authentication\\Token\\DefaultTokenProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenProvider.php',
-        'OC\\Authentication\\Token\\ExpiredTokenException' => __DIR__ . '/../../..' . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php',
         'OC\\Authentication\\Token\\IProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IProvider.php',
         'OC\\Authentication\\Token\\IToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IToken.php',
         'OC\\Authentication\\Token\\Manager' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/Manager.php',

diff --git a/lib/private/Authentication/Exceptions/ExpiredTokenException.php b/lib/private/Authentication/Exceptions/ExpiredTokenException.php
@@ -21,9 +21,9 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
-namespace OC\Authentication\Token;
+namespace OC\Authentication\Exceptions;
 
-use OC\Authentication\Exceptions\InvalidTokenException;
+use OC\Authentication\Token\IToken;
 
 class ExpiredTokenException extends InvalidTokenException {
 	/** @var IToken */

diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php
@@ -29,6 +29,7 @@
 namespace OC\Authentication\Token;
 
 use Exception;
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 use OCP\AppFramework\Db\DoesNotExistException;

diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php
@@ -26,6 +26,7 @@
 
 namespace OC\Authentication\Token;
 
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 

diff --git a/lib/private/Authentication/Token/Manager.php b/lib/private/Authentication/Token/Manager.php
@@ -23,6 +23,7 @@
 
 namespace OC\Authentication\Token;
 
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 

diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php
@@ -23,6 +23,7 @@
 
 namespace OC\Authentication\Token;
 
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 use OCP\AppFramework\Db\DoesNotExistException;

diff --git a/lib/private/Server.php b/lib/private/Server.php
@@ -753,14 +753,15 @@ 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(),
 				$c->getLogger(),
 				$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.

diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
@@ -38,6 +38,7 @@
 namespace OC\User;
 
 use OC;
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
@@ -401,7 +402,13 @@ public function logClientIn($user,
 			$this->manager->emit('\OC\User', 'preLogin', array($user, $password));
 		}
 
-		$isTokenPassword = $this->isTokenPassword($password);
+		try {
+			$isTokenPassword = $this->isTokenPassword($password);
+		} catch (ExpiredTokenException $e) {
+			// Just return on an expired token no need to check further or record a failed login
+			return false;
+		}
+
 		if (!$isTokenPassword && $this->isTokenAuthEnforced()) {
 			throw new PasswordLoginForbiddenException();
 		}
@@ -474,11 +481,14 @@ protected function isTwoFactorEnforced($username) {
 	 *
 	 * @param string $password
 	 * @return boolean
+	 * @throws ExpiredTokenException
 	 */
 	public function isTokenPassword($password) {
 		try {
 			$this->tokenProvider->getToken($password);
 			return true;
+		} catch (ExpiredTokenException $e) {
+			throw $e;
 		} catch (InvalidTokenException $ex) {
 			return false;
 		}

diff --git a/tests/lib/Authentication/Token/DefaultTokenProviderTest.php b/tests/lib/Authentication/Token/DefaultTokenProviderTest.php
@@ -22,17 +22,16 @@
 
 namespace Test\Authentication\Token;
 
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Token\DefaultToken;
 use OC\Authentication\Token\DefaultTokenMapper;
 use OC\Authentication\Token\DefaultTokenProvider;
-use OC\Authentication\Token\ExpiredTokenException;
 use OC\Authentication\Token\IToken;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IConfig;
 use OCP\ILogger;
-use OCP\IUser;
 use OCP\Security\ICrypto;
 use Test\TestCase;
 

diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php
@@ -23,19 +23,17 @@
 
 namespace Test\Authentication\Token;
 
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
-use OC\Authentication\Exceptions\PasswordlessTokenException;
 use OC\Authentication\Token\DefaultToken;
 use OC\Authentication\Token\PublicKeyToken;
 use OC\Authentication\Token\PublicKeyTokenMapper;
 use OC\Authentication\Token\PublicKeyTokenProvider;
-use OC\Authentication\Token\ExpiredTokenException;
 use OC\Authentication\Token\IToken;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IConfig;
 use OCP\ILogger;
-use OCP\IUser;
 use OCP\Security\ICrypto;
 use Test\TestCase;