Merge pull request #24991 from owncloud/2fa-do-not-generate-token

do not generate device token if 2FA is enable for user

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

core/Controller/TokenController.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @author Christoph Wurst <christoph@owncloud.com>
  *
@@ -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,
 		];

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();
 	}

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);
+	}
+
 }