From 0a17836fec59120cef54cdacf1535d45013dd6f8 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Mon, 28 Jan 2019 16:12:06 +0100
Subject: Clean pending 2FA authentication on password reset

When a password is reste we should make sure that all users are properly
logged in. Pending states should be cleared. For example a session where
the 2FA code is not entered yet should be cleared.

The token is now removed so the session will be killed the next time
this is checked (within 5 minutes).

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
---
 core/Controller/LostController.php                   | 10 ++++++++--
 lib/private/Authentication/TwoFactorAuth/Manager.php |  9 +++++++++
 tests/Core/Controller/LostControllerTest.php         |  7 ++++++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php
index d30c6ae1ebd..4fdb63484f5 100644
--- a/core/Controller/LostController.php
+++ b/core/Controller/LostController.php
@@ -31,6 +31,7 @@
 
 namespace OC\Core\Controller;
 
+use OC\Authentication\TwoFactorAuth\Manager;
 use OC\HintException;
 use \OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\JSONResponse;
@@ -57,7 +58,6 @@ use OCP\Security\ISecureRandom;
  * @package OC\Core\Controller
  */
 class LostController extends Controller {
-
 	/** @var IURLGenerator */
 	protected $urlGenerator;
 	/** @var IUserManager */
@@ -80,6 +80,8 @@ class LostController extends Controller {
 	protected $timeFactory;
 	/** @var ICrypto */
 	protected $crypto;
+	/** @var Manager */
+	private $twoFactorManager;
 
 	/**
 	 * @param string $appName
@@ -108,7 +110,8 @@ class LostController extends Controller {
 								IManager $encryptionManager,
 								IMailer $mailer,
 								ITimeFactory $timeFactory,
-								ICrypto $crypto) {
+								ICrypto $crypto,
+								Manager $twoFactorManager) {
 		parent::__construct($appName, $request);
 		$this->urlGenerator = $urlGenerator;
 		$this->userManager = $userManager;
@@ -121,6 +124,7 @@ class LostController extends Controller {
 		$this->mailer = $mailer;
 		$this->timeFactory = $timeFactory;
 		$this->crypto = $crypto;
+		$this->twoFactorManager = $twoFactorManager;
 	}
 
 	/**
@@ -283,6 +287,8 @@ class LostController extends Controller {
 
 			\OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', array('uid' => $userId, 'password' => $password));
 
+			$this->twoFactorManager->clearTwoFactorPending($userId);
+
 			$this->config->deleteUserValue($userId, 'core', 'lostpassword');
 			@\OC::$server->getUserSession()->unsetMagicInCookie();
 		} catch (HintException $e){
diff --git a/lib/private/Authentication/TwoFactorAuth/Manager.php b/lib/private/Authentication/TwoFactorAuth/Manager.php
index d527359b2ff..77a7d456e12 100644
--- a/lib/private/Authentication/TwoFactorAuth/Manager.php
+++ b/lib/private/Authentication/TwoFactorAuth/Manager.php
@@ -30,6 +30,7 @@ use Exception;
 use OC;
 use OC\App\AppManager;
 use OC_App;
+use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Token\IProvider as TokenProvider;
 use OCP\Activity\IManager;
@@ -335,4 +336,12 @@ class Manager {
 		$this->config->setUserValue($user->getUID(), 'login_token_2fa', $token->getId(), $this->timeFactory->getTime());
 	}
 
+	public function clearTwoFactorPending(string $userId) {
+		$tokensNeeding2FA = $this->config->getUserKeys($userId, 'login_token_2fa');
+
+		foreach ($tokensNeeding2FA as $tokenId) {
+			$this->tokenProvider->invalidateTokenById($userId, $tokenId);
+		}
+	}
+
 }
diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php
index d372ec559d0..c95e6f96382 100644
--- a/tests/Core/Controller/LostControllerTest.php
+++ b/tests/Core/Controller/LostControllerTest.php
@@ -21,6 +21,7 @@
 
 namespace Tests\Core\Controller;
 
+use OC\Authentication\TwoFactorAuth\Manager;
 use OC\Core\Controller\LostController;
 use OC\Mail\Message;
 use OCP\AppFramework\Http\JSONResponse;
@@ -74,6 +75,8 @@ class LostControllerTest extends \Test\TestCase {
 	private $request;
 	/** @var ICrypto|\PHPUnit_Framework_MockObject_MockObject */
 	private $crypto;
+	/** @var Manager|\PHPUnit_Framework_MockObject_MockObject */
+	private $twofactorManager;
 
 	protected function setUp() {
 		parent::setUp();
@@ -124,6 +127,7 @@ class LostControllerTest extends \Test\TestCase {
 			->method('isEnabled')
 			->willReturn(true);
 		$this->crypto = $this->createMock(ICrypto::class);
+		$this->twofactorManager = $this->createMock(Manager::class);
 		$this->lostController = new LostController(
 			'Core',
 			$this->request,
@@ -137,7 +141,8 @@ class LostControllerTest extends \Test\TestCase {
 			$this->encryptionManager,
 			$this->mailer,
 			$this->timeFactory,
-			$this->crypto
+			$this->crypto,
+			$this->twofactorManager
 		);
 	}
 
-- 
cgit v1.2.3