From 60e15e934c6556d34d27ff214f6a148247605bc2 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 6 Jun 2016 15:09:42 +0200 Subject: [PATCH] do not generate device token if 2FA is enable for user --- core/Application.php | 3 +- core/Controller/TokenController.php | 36 ++++++++++++----- .../Token/DefaultTokenCleanupJob.php | 1 + tests/Core/Controller/TokenControllerTest.php | 40 +++++++++++++++---- 4 files changed, 61 insertions(+), 19 deletions(-) diff --git a/core/Application.php b/core/Application.php index 25e2fa7627..a87917b626 100644 --- a/core/Application.php +++ b/core/Application.php @@ -120,7 +120,8 @@ class Application extends App { $c->query('AppName'), $c->query('Request'), $c->query('UserManager'), - $c->query('OC\Authentication\Token\DefaultTokenProvider'), + $c->query('ServerContainer')->query('OC\Authentication\Token\IProvider'), + $c->query('TwoFactorAuthManager'), $c->query('SecureRandom') ); }); diff --git a/core/Controller/TokenController.php b/core/Controller/TokenController.php index 42cc29bad1..13b1db9044 100644 --- a/core/Controller/TokenController.php +++ b/core/Controller/TokenController.php @@ -1,4 +1,5 @@ * @@ -23,22 +24,27 @@ namespace OC\Core\Controller; use OC\AppFramework\Http; use OC\Authentication\Token\DefaultTokenProvider; +use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; -use OC\User\Manager; +use OC\Authentication\TwoFactorAuth\Manager as TwoFactorAuthManager; +use OC\User\Manager as UserManager; +use OCA\User_LDAP\User\Manager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; -use OCP\AppFramework\Http\Response; use OCP\IRequest; use OCP\Security\ISecureRandom; class TokenController extends Controller { - /** @var Manager */ + /** @var UserManager */ private $userManager; - /** @var DefaultTokenProvider */ + /** @var IProvider */ private $tokenProvider; + /** @var TwoFactorAuthManager */ + private $twoFactorAuthManager; + /** @var ISecureRandom */ private $secureRandom; @@ -49,12 +55,12 @@ class TokenController extends Controller { * @param DefaultTokenProvider $tokenProvider * @param ISecureRandom $secureRandom */ - public function __construct($appName, IRequest $request, Manager $userManager, DefaultTokenProvider $tokenProvider, - ISecureRandom $secureRandom) { + public function __construct($appName, IRequest $request, UserManager $userManager, IProvider $tokenProvider, TwoFactorAuthManager $twoFactorAuthManager, ISecureRandom $secureRandom) { parent::__construct($appName, $request); $this->userManager = $userManager; $this->tokenProvider = $tokenProvider; $this->secureRandom = $secureRandom; + $this->twoFactorAuthManager = $twoFactorAuthManager; } /** @@ -70,18 +76,26 @@ class TokenController extends Controller { */ public function generateToken($user, $password, $name = 'unknown client') { if (is_null($user) || is_null($password)) { - $response = new Response(); + $response = new JSONResponse(); $response->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); return $response; } - $loginResult = $this->userManager->checkPassword($user, $password); - if ($loginResult === false) { - $response = new Response(); + $loginName = $user; + $user = $this->userManager->checkPassword($loginName, $password); + if ($user === false) { + $response = new JSONResponse(); $response->setStatus(Http::STATUS_UNAUTHORIZED); return $response; } + + if ($this->twoFactorAuthManager->isTwoFactorAuthenticated($user)) { + $resp = new JSONResponse(); + $resp->setStatus(Http::STATUS_UNAUTHORIZED); + return $resp; + } + $token = $this->secureRandom->generate(128); - $this->tokenProvider->generateToken($token, $loginResult->getUID(), $user, $password, $name, IToken::PERMANENT_TOKEN); + $this->tokenProvider->generateToken($token, $user->getUID(), $loginName, $password, $name, IToken::PERMANENT_TOKEN); return [ 'token' => $token, ]; diff --git a/lib/private/Authentication/Token/DefaultTokenCleanupJob.php b/lib/private/Authentication/Token/DefaultTokenCleanupJob.php index 04b98c6c5a..7746d6be91 100644 --- a/lib/private/Authentication/Token/DefaultTokenCleanupJob.php +++ b/lib/private/Authentication/Token/DefaultTokenCleanupJob.php @@ -28,6 +28,7 @@ class DefaultTokenCleanupJob extends Job { protected function run($argument) { /* @var $provider DefaultTokenProvider */ + // TODO: add OC\Authentication\Token\IProvider::invalidateOldTokens and query interface $provider = OC::$server->query('OC\Authentication\Token\DefaultTokenProvider'); $provider->invalidateOldTokens(); } diff --git a/tests/Core/Controller/TokenControllerTest.php b/tests/Core/Controller/TokenControllerTest.php index 386140a8a4..b6b54b14fa 100644 --- a/tests/Core/Controller/TokenControllerTest.php +++ b/tests/Core/Controller/TokenControllerTest.php @@ -23,8 +23,9 @@ namespace Tests\Core\Controller; use OC\AppFramework\Http; +use OC\Authentication\Token\IToken; use OC\Core\Controller\TokenController; -use OCP\AppFramework\Http\Response; +use OCP\AppFramework\Http\JSONResponse; use Test\TestCase; class TokenControllerTest extends TestCase { @@ -34,6 +35,7 @@ class TokenControllerTest extends TestCase { private $request; private $userManager; private $tokenProvider; + private $twoFactorAuthManager; private $secureRandom; protected function setUp() { @@ -43,17 +45,17 @@ class TokenControllerTest extends TestCase { $this->userManager = $this->getMockBuilder('\OC\User\Manager') ->disableOriginalConstructor() ->getMock(); - $this->tokenProvider = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenProvider') + $this->tokenProvider = $this->getMock('\OC\Authentication\Token\IProvider'); + $this->twoFactorAuthManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager') ->disableOriginalConstructor() ->getMock(); $this->secureRandom = $this->getMock('\OCP\Security\ISecureRandom'); - $this->tokenController = new TokenController('core', $this->request, $this->userManager, $this->tokenProvider, - $this->secureRandom); + $this->tokenController = new TokenController('core', $this->request, $this->userManager, $this->tokenProvider, $this->twoFactorAuthManager, $this->secureRandom); } public function testWithoutCredentials() { - $expected = new Response(); + $expected = new JSONResponse(); $expected->setStatus(Http::STATUS_UNPROCESSABLE_ENTITY); $actual = $this->tokenController->generateToken(null, null); @@ -66,7 +68,7 @@ class TokenControllerTest extends TestCase { ->method('checkPassword') ->with('john', 'passme') ->will($this->returnValue(false)); - $expected = new Response(); + $expected = new JSONResponse(); $expected->setStatus(Http::STATUS_UNAUTHORIZED); $actual = $this->tokenController->generateToken('john', 'passme'); @@ -83,13 +85,17 @@ class TokenControllerTest extends TestCase { $user->expects($this->once()) ->method('getUID') ->will($this->returnValue('john')); + $this->twoFactorAuthManager->expects($this->once()) + ->method('isTwoFactorAuthenticated') + ->with($user) + ->will($this->returnValue(false)); $this->secureRandom->expects($this->once()) ->method('generate') ->with(128) ->will($this->returnValue('verysecurerandomtoken')); $this->tokenProvider->expects($this->once()) ->method('generateToken') - ->with('verysecurerandomtoken', 'john', 'john', '123456', 'unknown client', \OC\Authentication\Token\IToken::PERMANENT_TOKEN); + ->with('verysecurerandomtoken', 'john', 'john', '123456', 'unknown client', IToken::PERMANENT_TOKEN); $expected = [ 'token' => 'verysecurerandomtoken' ]; @@ -99,4 +105,24 @@ class TokenControllerTest extends TestCase { $this->assertEquals($expected, $actual); } + public function testWithValidCredentialsBut2faEnabled() { + $user = $this->getMock('\OCP\IUser'); + $this->userManager->expects($this->once()) + ->method('checkPassword') + ->with('john', '123456') + ->will($this->returnValue($user)); + $this->twoFactorAuthManager->expects($this->once()) + ->method('isTwoFactorAuthenticated') + ->with($user) + ->will($this->returnValue(true)); + $this->secureRandom->expects($this->never()) + ->method('generate'); + $expected = new JSONResponse(); + $expected->setStatus(Http::STATUS_UNAUTHORIZED); + + $actual = $this->tokenController->generateToken('john', '123456'); + + $this->assertEquals($expected, $actual); + } + }