Commit a67a2272 authored by Lukas Reschke's avatar Lukas Reschke
Browse files

Merge pull request #18491 from owncloud/expire-token-after-12h-or-login

Expire token after 12h and if user logged-in again
parents 642ec8e3 db4cb1dd
......@@ -27,6 +27,7 @@
namespace OC\Core;
use OC\AppFramework\Utility\SimpleContainer;
use OC\AppFramework\Utility\TimeFactory;
use \OCP\AppFramework\App;
use OC\Core\LostPassword\Controller\LostController;
use OC\Core\User\UserController;
......@@ -63,7 +64,8 @@ class Application extends App {
$c->query('SecureRandom'),
$c->query('DefaultEmailAddress'),
$c->query('IsEncryptionEnabled'),
$c->query('Mailer')
$c->query('Mailer'),
$c->query('TimeFactory')
);
});
$container->registerService('UserController', function(SimpleContainer $c) {
......@@ -120,15 +122,15 @@ class Application extends App {
$container->registerService('UserFolder', function(SimpleContainer $c) {
return $c->query('ServerContainer')->getUserFolder();
});
$container->registerService('Defaults', function() {
return new \OC_Defaults;
});
$container->registerService('Mailer', function(SimpleContainer $c) {
return $c->query('ServerContainer')->getMailer();
});
$container->registerService('TimeFactory', function(SimpleContainer $c) {
return new TimeFactory();
});
$container->registerService('DefaultEmailAddress', function() {
return Util::getDefaultEmailAddress('lostpassword-noreply');
});
......
......@@ -28,6 +28,7 @@ namespace OC\Core\LostPassword\Controller;
use \OCP\AppFramework\Controller;
use \OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use \OCP\IURLGenerator;
use \OCP\IRequest;
use \OCP\IL10N;
......@@ -66,6 +67,8 @@ class LostController extends Controller {
protected $secureRandom;
/** @var IMailer */
protected $mailer;
/** @var ITimeFactory */
protected $timeFactory;
/**
* @param string $appName
......@@ -79,6 +82,7 @@ class LostController extends Controller {
* @param string $from
* @param string $isDataEncrypted
* @param IMailer $mailer
* @param ITimeFactory $timeFactory
*/
public function __construct($appName,
IRequest $request,
......@@ -90,7 +94,8 @@ class LostController extends Controller {
ISecureRandom $secureRandom,
$from,
$isDataEncrypted,
IMailer $mailer) {
IMailer $mailer,
ITimeFactory $timeFactory) {
parent::__construct($appName, $request);
$this->urlGenerator = $urlGenerator;
$this->userManager = $userManager;
......@@ -101,6 +106,7 @@ class LostController extends Controller {
$this->isDataEncrypted = $isDataEncrypted;
$this->config = $config;
$this->mailer = $mailer;
$this->timeFactory = $timeFactory;
}
/**
......@@ -173,7 +179,17 @@ class LostController extends Controller {
try {
$user = $this->userManager->get($userId);
if (!StringUtils::equals($this->config->getUserValue($userId, 'owncloud', 'lostpassword', null), $token)) {
$splittedToken = explode(':', $this->config->getUserValue($userId, 'owncloud', 'lostpassword', null));
if(count($splittedToken) !== 2) {
throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
}
if ($splittedToken[0] < ($this->timeFactory->getTime() - 60*60*12) ||
$user->getLastLogin() > $splittedToken[0]) {
throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is expired'));
}
if (!StringUtils::equals($splittedToken[1], $token)) {
throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
}
......@@ -216,12 +232,12 @@ class LostController extends Controller {
ISecureRandom::CHAR_DIGITS.
ISecureRandom::CHAR_LOWER.
ISecureRandom::CHAR_UPPER);
$this->config->setUserValue($user, 'owncloud', 'lostpassword', $token);
$this->config->setUserValue($user, 'owncloud', 'lostpassword', $this->timeFactory->getTime() .':'. $token);
$link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', array('userId' => $user, 'token' => $token));
$tmpl = new \OC_Template('core/lostpassword', 'email');
$tmpl->assign('link', $link, false);
$tmpl->assign('link', $link);
$msg = $tmpl->fetchPage();
try {
......
<?php
/**
* Copyright (c) 2014-2015 Lukas Reschke <lukas@owncloud.com>
* This file is licensed under the Affero General Public License version 3 or
* later.
* See the COPYING-README file.
* @author Lukas Reschke <lukas@owncloud.com>
*
* @copyright Copyright (c) 2015, ownCloud, Inc.
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OC\Core\LostPassword\Controller;
......@@ -47,6 +60,8 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
->disableOriginalConstructor()->getMock();
$this->container['SecureRandom'] = $this->getMockBuilder('\OCP\Security\ISecureRandom')
->disableOriginalConstructor()->getMock();
$this->container['TimeFactory'] = $this->getMockBuilder('\OCP\AppFramework\Utility\ITimeFactory')
->disableOriginalConstructor()->getMock();
$this->container['IsEncryptionEnabled'] = true;
$this->lostController = $this->container['LostController'];
}
......@@ -116,6 +131,10 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
->method('userExists')
->with('ExistingUser')
->will($this->returnValue(true));
$this->container['TimeFactory']
->expects($this->once())
->method('getTime')
->will($this->returnValue(12348));
$this->container['Config']
->expects($this->once())
->method('getUserValue')
......@@ -128,7 +147,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->container['Config']
->expects($this->once())
->method('setUserValue')
->with('ExistingUser', 'owncloud', 'lostpassword', 'ThisIsMaybeANotSoSecretToken!');
->with('ExistingUser', 'owncloud', 'lostpassword', '12348:ThisIsMaybeANotSoSecretToken!');
$this->container['URLGenerator']
->expects($this->once())
->method('linkToRouteAbsolute')
......@@ -190,7 +209,11 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->container['Config']
->expects($this->once())
->method('setUserValue')
->with('ExistingUser', 'owncloud', 'lostpassword', 'ThisIsMaybeANotSoSecretToken!');
->with('ExistingUser', 'owncloud', 'lostpassword', '12348:ThisIsMaybeANotSoSecretToken!');
$this->container['TimeFactory']
->expects($this->once())
->method('getTime')
->will($this->returnValue(12348));
$this->container['URLGenerator']
->expects($this->once())
->method('linkToRouteAbsolute')
......@@ -256,9 +279,13 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->will($this->returnValue('TheOnlyAndOnlyOneTokenToResetThePassword'));
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
$user
->expects($this->once())
->method('getLastLogin')
->will($this->returnValue(12344));
$user->expects($this->once())
->method('setPassword')
->with('NewPassword')
......@@ -272,12 +299,94 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
->expects($this->once())
->method('deleteUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword');
$this->container['TimeFactory']
->expects($this->once())
->method('getTime')
->will($this->returnValue(12348));
$response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true);
$expectedResponse = array('status' => 'success');
$this->assertSame($expectedResponse, $response);
}
public function testSetPasswordExpiredToken() {
$this->container['Config']
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
$this->container['UserManager']
->expects($this->once())
->method('get')
->with('ValidTokenUser')
->will($this->returnValue($user));
$this->container['TimeFactory']
->expects($this->once())
->method('getTime')
->will($this->returnValue(55546));
$response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true);
$expectedResponse = [
'status' => 'error',
'msg' => 'Couldn\'t reset password because the token is expired',
];
$this->assertSame($expectedResponse, $response);
}
public function testSetPasswordInvalidDataInDb() {
$this->container['Config']
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->will($this->returnValue('TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
$this->container['UserManager']
->expects($this->once())
->method('get')
->with('ValidTokenUser')
->will($this->returnValue($user));
$response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true);
$expectedResponse = [
'status' => 'error',
'msg' => 'Couldn\'t reset password because the token is invalid',
];
$this->assertSame($expectedResponse, $response);
}
public function testSetPasswordExpiredTokenDueToLogin() {
$this->container['Config']
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
$user
->expects($this->once())
->method('getLastLogin')
->will($this->returnValue(12346));
$this->container['UserManager']
->expects($this->once())
->method('get')
->with('ValidTokenUser')
->will($this->returnValue($user));
$this->container['TimeFactory']
->expects($this->once())
->method('getTime')
->will($this->returnValue(12345));
$response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true);
$expectedResponse = [
'status' => 'error',
'msg' => 'Couldn\'t reset password because the token is expired',
];
$this->assertSame($expectedResponse, $response);
}
public function testIsSetPasswordWithoutTokenFailing() {
$this->container['Config']
->expects($this->once())
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment