mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #1024 from nextcloud/remove-password-reset-link-after-successful-login 

Remove "password reset token" after successful login
This commit is contained in:
Lukas Reschke 2016-08-24 13:44:41 +02:00 committed by GitHub
parent 8d8366762b 736e884e9a
commit a27a8d733b
4 changed files with 44 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
core/Controller/LoginController.php
View File

@ -237,6 +237,9 @@ class LoginController extends Controller {
$this->userSession->login($user, $password);
$this->userSession->createSessionToken($this->request, $loginResult->getUID(), $user, $password);
// User has successfully logged in, now remove the password reset link, when it is available
$this->config->deleteUserValue($loginResult->getUID(), 'core', 'lostpassword');
if ($this->twoFactorManager->isTwoFactorAuthenticated($loginResult)) {
$this->twoFactorManager->prepareTwoFactorLogin($loginResult);
if (!is_null($redirect_url)) {

6
core/Controller/LostController.php
View File

@ -151,7 +151,7 @@ class LostController extends Controller {
private function checkPasswordResetToken($token, $userId) {
$user = $this->userManager->get($userId);
$splittedToken = explode(':', $this->config->getUserValue($userId, 'owncloud', 'lostpassword', null));
$splittedToken = explode(':', $this->config->getUserValue($userId, 'core', 'lostpassword', null));
if(count($splittedToken) !== 2) {
throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
}
@ -222,7 +222,7 @@ class LostController extends Controller {
\OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', array('uid' => $userId, 'password' => $password));
$this->config->deleteUserValue($userId, 'owncloud', 'lostpassword');
$this->config->deleteUserValue($userId, 'core', 'lostpassword');
@\OC_User::unsetMagicInCookie();
} catch (\Exception $e){
return $this->error($e->getMessage());
@ -253,7 +253,7 @@ class LostController extends Controller {
ISecureRandom::CHAR_DIGITS.
ISecureRandom::CHAR_LOWER.
ISecureRandom::CHAR_UPPER);
$this->config->setUserValue($user, 'owncloud', 'lostpassword', $this->timeFactory->getTime() .':'. $token);
$this->config->setUserValue($user, 'core', 'lostpassword', $this->timeFactory->getTime() .':'. $token);
$link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', array('userId' => $user, 'token' => $token));

20
tests/Core/Controller/LoginControllerTest.php
View File

@ -322,6 +322,8 @@ class LoginControllerTest extends TestCase {
$this->userSession->expects($this->never())
->method('createSessionToken');
$this->config->expects($this->never())
->method('deleteUserValue');
$expected = new \OCP\AppFramework\Http\RedirectResponse($loginPageUrl);
$this->assertEquals($expected, $this->loginController->tryLogin($user, $password, ''));
@ -330,6 +332,9 @@ class LoginControllerTest extends TestCase {
public function testLoginWithValidCredentials() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->getMockBuilder('\OCP\IUser')->getMock();
$user->expects($this->any())
->method('getUID')
->will($this->returnValue('uid'));
$password = 'secret';
$indexPageUrl = \OC_Util::getDefaultPageUrl();
@ -363,6 +368,9 @@ class LoginControllerTest extends TestCase {
->method('isTwoFactorAuthenticated')
->with($user)
->will($this->returnValue(false));
$this->config->expects($this->once())
->method('deleteUserValue')
->with('uid', 'core', 'lostpassword');
$expected = new \OCP\AppFramework\Http\RedirectResponse($indexPageUrl);
$this->assertEquals($expected, $this->loginController->tryLogin($user, $password, null));
@ -398,6 +406,8 @@ class LoginControllerTest extends TestCase {
->method('isLoggedIn')
->with()
->will($this->returnValue(false));
$this->config->expects($this->never())
->method('deleteUserValue');
$expected = new \OCP\AppFramework\Http\RedirectResponse(\OC_Util::getDefaultPageUrl());
$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
@ -438,6 +448,8 @@ class LoginControllerTest extends TestCase {
->method('getAbsoluteURL')
->with(urldecode($originalUrl))
->will($this->returnValue($redirectUrl));
$this->config->expects($this->never())
->method('deleteUserValue');
$expected = new \OCP\AppFramework\Http\RedirectResponse($redirectUrl);
$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
@ -485,6 +497,9 @@ class LoginControllerTest extends TestCase {
->method('getAbsoluteURL')
->with(urldecode($originalUrl))
->will($this->returnValue($redirectUrl));
$this->config->expects($this->once())
->method('deleteUserValue')
->with('jane', 'core', 'lostpassword');
$expected = new \OCP\AppFramework\Http\RedirectResponse(urldecode($redirectUrl));
$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
@ -536,6 +551,9 @@ class LoginControllerTest extends TestCase {
->method('linkToRoute')
->with('core.TwoFactorChallenge.selectChallenge')
->will($this->returnValue($challengeUrl));
$this->config->expects($this->once())
->method('deleteUserValue')
->with('john', 'core', 'lostpassword');
$expected = new RedirectResponse($challengeUrl);
$this->assertEquals($expected, $this->loginController->tryLogin('john@doe.com', $password, null));
@ -586,6 +604,8 @@ class LoginControllerTest extends TestCase {
->expects($this->once())
->method('registerAttempt')
->with('login', '192.168.0.1', ['user' => 'john@doe.com']);
$this->config->expects($this->never())
->method('deleteUserValue');
$expected = new RedirectResponse('');
$this->assertEquals($expected, $this->loginController->tryLogin('john@doe.com', 'just wrong', null));

36
tests/Core/Controller/LostControllerTest.php
View File

@ -133,7 +133,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
@ -180,7 +180,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$response = $this->lostController->resetform($token, $userId);
$expectedResponse = new TemplateResponse('core',
@ -217,19 +217,19 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$this->urlGenerator
->expects($this->once())
->method('linkToRouteAbsolute')
->with('core.lost.setPassword', array('userId' => 'ValidTokenUser', 'token' => 'TheOnlyAndOnlyOneTokenToResetThePassword'))
->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/'));
->will($this->returnValue('https://example.tld/index.php/lostpassword/'));
$response = $this->lostController->resetform($token, $userId);
$expectedResponse = new TemplateResponse('core',
'lostpassword/resetpassword',
array(
'link' => 'https://ownCloud.com/index.php/lostpassword/',
'link' => 'https://example.tld/index.php/lostpassword/',
),
'guest');
$this->assertEquals($expectedResponse, $response);
@ -291,12 +291,12 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('setUserValue')
->with('ExistingUser', 'owncloud', 'lostpassword', '12348:ThisIsMaybeANotSoSecretToken!');
->with('ExistingUser', 'core', 'lostpassword', '12348:ThisIsMaybeANotSoSecretToken!');
$this->urlGenerator
->expects($this->once())
->method('linkToRouteAbsolute')
->with('core.lost.resetform', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!'))
->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/'));
->will($this->returnValue('https://example.tld/index.php/lostpassword/'));
$message = $this->getMockBuilder('\OC\Mail\Message')
->disableOriginalConstructor()->getMock();
$message
@ -310,7 +310,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$message
->expects($this->at(2))
->method('setPlainBody')
->with('Use the following link to reset your password: https://ownCloud.com/index.php/lostpassword/');
->with('Use the following link to reset your password: https://example.tld/index.php/lostpassword/');
$message
->expects($this->at(3))
->method('setFrom')
@ -348,7 +348,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('setUserValue')
->with('ExistingUser', 'owncloud', 'lostpassword', '12348:ThisIsMaybeANotSoSecretToken!');
->with('ExistingUser', 'core', 'lostpassword', '12348:ThisIsMaybeANotSoSecretToken!');
$this->timeFactory
->expects($this->once())
->method('getTime')
@ -357,7 +357,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
->expects($this->once())
->method('linkToRouteAbsolute')
->with('core.lost.resetform', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!'))
->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/'));
->will($this->returnValue('https://example.tld/index.php/lostpassword/'));
$message = $this->getMockBuilder('\OC\Mail\Message')
->disableOriginalConstructor()->getMock();
$message
@ -371,7 +371,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$message
->expects($this->at(2))
->method('setPlainBody')
->with('Use the following link to reset your password: https://ownCloud.com/index.php/lostpassword/');
->with('Use the following link to reset your password: https://example.tld/index.php/lostpassword/');
$message
->expects($this->at(3))
->method('setFrom')
@ -395,7 +395,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('InvalidTokenUser', 'owncloud', 'lostpassword', null)
->with('InvalidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('TheOnlyAndOnlyOneTokenToResetThePassword'));
// With an invalid token
@ -417,7 +417,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
@ -437,7 +437,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('deleteUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword');
->with('ValidTokenUser', 'core', 'lostpassword');
$this->timeFactory
->expects($this->once())
->method('getTime')
@ -452,7 +452,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
@ -478,7 +478,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
@ -500,7 +500,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue('12345:TheOnlyAndOnlyOneTokenToResetThePassword'));
$user = $this->getMockBuilder('\OCP\IUser')
->disableOriginalConstructor()->getMock();
@ -530,7 +530,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase {
$this->config
->expects($this->once())
->method('getUserValue')
->with('ValidTokenUser', 'owncloud', 'lostpassword', null)
->with('ValidTokenUser', 'core', 'lostpassword', null)
->will($this->returnValue(null));
$response = $this->lostController->setPassword('', 'ValidTokenUser', 'NewPassword', true);