Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 <[email protected]>
  • Loading branch information
rullzer committed Jan 30, 2019
commit 931056e711aa61c35a2f970686ebad4b4b45f192
10 changes: 8 additions & 2 deletions core/Controller/LostController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -57,7 +58,6 @@
* @package OC\Core\Controller
*/
class LostController extends Controller {

/** @var IURLGenerator */
protected $urlGenerator;
/** @var IUserManager */
Expand All @@ -80,6 +80,8 @@ class LostController extends Controller {
protected $timeFactory;
/** @var ICrypto */
protected $crypto;
/** @var Manager */
private $twoFactorManager;

/**
* @param string $appName
Expand Down Expand Up @@ -108,7 +110,8 @@ public function __construct($appName,
IManager $encryptionManager,
IMailer $mailer,
ITimeFactory $timeFactory,
ICrypto $crypto) {
ICrypto $crypto,
Manager $twoFactorManager) {
parent::__construct($appName, $request);
$this->urlGenerator = $urlGenerator;
$this->userManager = $userManager;
Expand All @@ -121,6 +124,7 @@ public function __construct($appName,
$this->mailer = $mailer;
$this->timeFactory = $timeFactory;
$this->crypto = $crypto;
$this->twoFactorManager = $twoFactorManager;
}

/**
Expand Down Expand Up @@ -284,6 +288,8 @@ public function setPassword($token, $userId, $password, $proceed) {

\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){
Expand Down
9 changes: 9 additions & 0 deletions lib/private/Authentication/TwoFactorAuth/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use function array_filter;
use BadMethodCallException;
use Exception;
use OC\Authentication\Exceptions\ExpiredTokenException;
use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Token\IProvider as TokenProvider;
use OCP\Activity\IManager;
Expand Down Expand Up @@ -378,4 +379,12 @@ public function prepareTwoFactorLogin(IUser $user, bool $rememberMe) {
$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);
}
}

}
7 changes: 6 additions & 1 deletion tests/Core/Controller/LostControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -124,6 +127,7 @@ protected function setUp() {
->method('isEnabled')
->willReturn(true);
$this->crypto = $this->createMock(ICrypto::class);
$this->twofactorManager = $this->createMock(Manager::class);
$this->lostController = new LostController(
'Core',
$this->request,
Expand All @@ -137,7 +141,8 @@ protected function setUp() {
$this->encryptionManager,
$this->mailer,
$this->timeFactory,
$this->crypto
$this->crypto,
$this->twofactorManager
);
}

Expand Down