From 1b50d4f7ceb92fffe0d38f823f175cf7e419c69e Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 20 Oct 2014 19:05:48 +0200 Subject: [PATCH 01/11] Warn for password reset when files_encryption is enabled This patch wil warn the user of the consequences when resetting the password and requires checking a checkbox (as we had in the past) to reset a password. Furthermore I updated the code to use our new classes and added some unit tests for it :dancers: Fixes https://github.com/owncloud/core/issues/11438 --- core/application.php | 64 ++++-- core/js/lostpassword.js | 19 +- .../controller/lostcontroller.php | 164 +++++++-------- core/lostpassword/encrypteddataexception.php | 14 -- core/lostpassword/templates/lostpassword.php | 20 -- core/lostpassword/templates/resetpassword.php | 9 +- .../controller/lostcontrollertest.php | 195 ++++++++++++++++++ tests/phpunit-autotest.xml | 1 + tests/phpunit.xml.dist | 2 + .../controller/mailsettingscontrollertest.php | 2 +- 10 files changed, 341 insertions(+), 149 deletions(-) delete mode 100644 core/lostpassword/encrypteddataexception.php delete mode 100644 core/lostpassword/templates/lostpassword.php create mode 100644 tests/core/lostpassword/controller/lostcontrollertest.php diff --git a/core/application.php b/core/application.php index 3380184775..c36ab559c2 100644 --- a/core/application.php +++ b/core/application.php @@ -10,13 +10,22 @@ namespace OC\Core; +use OC\AppFramework\Utility\SimpleContainer; use \OCP\AppFramework\App; use OC\Core\LostPassword\Controller\LostController; use OC\Core\User\UserController; +use \OCP\Util; +/** + * Class Application + * + * @package OC\Core + */ class Application extends App { - + /** + * @param array $urlParams + */ public function __construct(array $urlParams=array()){ parent::__construct('core', $urlParams); @@ -25,29 +34,56 @@ class Application extends App { /** * Controllers */ - $container->registerService('LostController', function($c) { + $container->registerService('LostController', function(SimpleContainer $c) { return new LostController( $c->query('AppName'), $c->query('Request'), - $c->query('ServerContainer')->getURLGenerator(), - $c->query('ServerContainer')->getUserManager(), - new \OC_Defaults(), - $c->query('ServerContainer')->getL10N('core'), - $c->query('ServerContainer')->getConfig(), - $c->query('ServerContainer')->getUserSession(), - \OCP\Util::getDefaultEmailAddress('lostpassword-noreply'), - \OC_App::isEnabled('files_encryption') + $c->query('URLGenerator'), + $c->query('UserManager'), + $c->query('Defaults'), + $c->query('L10N'), + $c->query('Config'), + $c->query('SecureRandom'), + $c->query('DefaultEmailAddress'), + $c->query('IsEncryptionEnabled') ); }); - $container->registerService('UserController', function($c) { + $container->registerService('UserController', function(SimpleContainer $c) { return new UserController( $c->query('AppName'), $c->query('Request'), - $c->query('ServerContainer')->getUserManager(), - new \OC_Defaults() + $c->query('UserManager'), + $c->query('Defaults') ); }); + + /** + * Core class wrappers + */ + $container->registerService('IsEncryptionEnabled', function() { + return \OC_App::isEnabled('files_encryption'); + }); + $container->registerService('URLGenerator', function(SimpleContainer $c) { + return $c->query('ServerContainer')->getURLGenerator(); + }); + $container->registerService('UserManager', function(SimpleContainer $c) { + return $c->query('ServerContainer')->getUserManager(); + }); + $container->registerService('Config', function(SimpleContainer $c) { + return $c->query('ServerContainer')->getConfig(); + }); + $container->registerService('L10N', function(SimpleContainer $c) { + return $c->query('ServerContainer')->getL10N('core'); + }); + $container->registerService('SecureRandom', function(SimpleContainer $c) { + return $c->query('ServerContainer')->getSecureRandom(); + }); + $container->registerService('Defaults', function() { + return new \OC_Defaults; + }); + $container->registerService('DefaultEmailAddress', function() { + return Util::getDefaultEmailAddress('lostpassword-noreply'); + }); } - } diff --git a/core/js/lostpassword.js b/core/js/lostpassword.js index ad221cb30f..aa1a864ffe 100644 --- a/core/js/lostpassword.js +++ b/core/js/lostpassword.js @@ -8,19 +8,12 @@ OC.Lostpassword = { + ('
') + '
' - + '' - + t('core', 'Reset password') - + '', + + '
', resetErrorMsg : t('core', 'Password can not be changed. Please contact your administrator.'), init : function() { - if ($('#lost-password-encryption').length){ - $('#lost-password-encryption').click(OC.Lostpassword.sendLink); - } else { - $('#lost-password').click(OC.Lostpassword.sendLink); - } + $('#lost-password').click(OC.Lostpassword.sendLink); $('#reset-password #submit').click(OC.Lostpassword.resetPassword); }, @@ -32,8 +25,7 @@ OC.Lostpassword = { $.post( OC.generateUrl('/lostpassword/email'), { - user : $('#user').val(), - proceed: $('#encrypted-continue').attr('checked') ? 'Yes' : 'No' + user : $('#user').val() }, OC.Lostpassword.sendLinkDone ); @@ -84,7 +76,8 @@ OC.Lostpassword = { $.post( $('#password').parents('form').attr('action'), { - password : $('#password').val() + password : $('#password').val(), + proceed: $('#encrypted-continue').attr('checked') ? 'true' : 'false' }, OC.Lostpassword.resetDone ); @@ -126,7 +119,7 @@ OC.Lostpassword = { getResetStatusNode : function (){ if (!$('#lost-password').length){ - $('

').insertAfter($('#submit')); + $('

').insertBefore($('#reset-password fieldset')); } else { $('#lost-password').replaceWith($('

')); } diff --git a/core/lostpassword/controller/lostcontroller.php b/core/lostpassword/controller/lostcontroller.php index e4d51fde07..a1a683baa7 100644 --- a/core/lostpassword/controller/lostcontroller.php +++ b/core/lostpassword/controller/lostcontroller.php @@ -9,68 +9,72 @@ namespace OC\Core\LostPassword\Controller; use \OCP\AppFramework\Controller; -use \OCP\AppFramework\Http\JSONResponse; use \OCP\AppFramework\Http\TemplateResponse; use \OCP\IURLGenerator; use \OCP\IRequest; use \OCP\IL10N; use \OCP\IConfig; -use \OCP\IUserSession; +use OCP\IUserManager; use \OC\Core\LostPassword\EncryptedDataException; +use OCP\Security\ISecureRandom; +use \OC_Defaults; +use OCP\Security\StringUtils; +/** + * Class LostController + * + * @package OC\Core\LostPassword\Controller + */ class LostController extends Controller { - /** - * @var \OCP\IURLGenerator - */ + /** @var IURLGenerator */ protected $urlGenerator; - - /** - * @var \OCP\IUserManager - */ + /** @var IUserManager */ protected $userManager; - - /** - * @var \OC_Defaults - */ + /** @var OC_Defaults */ protected $defaults; - - /** - * @var IL10N - */ + /** @var IL10N */ protected $l10n; + /** @var string */ protected $from; + /** @var bool */ protected $isDataEncrypted; - - /** - * @var IConfig - */ + /** @var IConfig */ protected $config; + /** @var ISecureRandom */ + protected $secureRandom; /** - * @var IUserSession + * @param string $appName + * @param IRequest $request + * @param IURLGenerator $urlGenerator + * @param $userManager + * @param $defaults + * @param IL10N $l10n + * @param IConfig $config + * @param ISecureRandom $secureRandom + * @param $from + * @param $isDataEncrypted */ - protected $userSession; - public function __construct($appName, - IRequest $request, - IURLGenerator $urlGenerator, - $userManager, - $defaults, - IL10N $l10n, - IConfig $config, - IUserSession $userSession, - $from, - $isDataEncrypted) { + IRequest $request, + IURLGenerator $urlGenerator, + IUserManager $userManager, + OC_Defaults $defaults, + IL10N $l10n, + IConfig $config, + ISecureRandom $secureRandom, + $from, + $isDataEncrypted) { parent::__construct($appName, $request); $this->urlGenerator = $urlGenerator; $this->userManager = $userManager; $this->defaults = $defaults; $this->l10n = $l10n; + $this->secureRandom = $secureRandom; $this->from = $from; $this->isDataEncrypted = $isDataEncrypted; $this->config = $config; - $this->userSession = $userSession; } /** @@ -81,23 +85,31 @@ class LostController extends Controller { * * @param string $token * @param string $userId + * @return TemplateResponse */ public function resetform($token, $userId) { return new TemplateResponse( 'core/lostpassword', 'resetpassword', array( - 'isEncrypted' => $this->isDataEncrypted, - 'link' => $this->getLink('core.lost.setPassword', $userId, $token), + 'link' => $this->urlGenerator->linkToRouteAbsolute('core.lost.setPassword', array('userId' => $userId, 'token' => $token)), ), 'guest' ); } + /** + * @param $message + * @param array $additional + * @return array + */ private function error($message, array $additional=array()) { return array_merge(array('status' => 'error', 'msg' => $message), $additional); } + /** + * @return array + */ private function success() { return array('status'=>'success'); } @@ -106,14 +118,12 @@ class LostController extends Controller { * @PublicPage * * @param string $user - * @param bool $proceed + * @return array */ - public function email($user, $proceed){ + public function email($user){ // FIXME: use HTTP error codes try { - $this->sendEmail($user, $proceed); - } catch (EncryptedDataException $e){ - return $this->error('', array('encryption' => '1')); + $this->sendEmail($user); } catch (\Exception $e){ return $this->error($e->getMessage()); } @@ -121,15 +131,23 @@ class LostController extends Controller { return $this->success(); } - /** * @PublicPage + * @param string $token + * @param string $userId + * @param string $password + * @param boolean $proceed + * @return array */ - public function setPassword($token, $userId, $password) { + public function setPassword($token, $userId, $password, $proceed) { + if ($this->isDataEncrypted && !$proceed){ + return $this->error('', array('encryption' => true)); + } + try { $user = $this->userManager->get($userId); - if (!$this->checkToken($userId, $token)) { + if (!StringUtils::equals($this->config->getUserValue($userId, 'owncloud', 'lostpassword'), $token)) { throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); } @@ -137,9 +155,8 @@ class LostController extends Controller { throw new \Exception(); } - // FIXME: should be added to the all config at some point - \OC_Preferences::deleteKey($userId, 'owncloud', 'lostpassword'); - $this->userSession->unsetMagicInCookie(); + $this->config->deleteUserValue($userId, 'owncloud', 'lostpassword'); + @\OC_User::unsetMagicInCookie(); } catch (\Exception $e){ return $this->error($e->getMessage()); @@ -148,36 +165,32 @@ class LostController extends Controller { return $this->success(); } - - protected function sendEmail($user, $proceed) { - if ($this->isDataEncrypted && !$proceed){ - throw new EncryptedDataException(); - } - + /** + * @param string $user + * @throws \Exception + */ + protected function sendEmail($user) { if (!$this->userManager->userExists($user)) { - throw new \Exception( - $this->l10n->t('Couldn\'t send reset email. Please make sure '. - 'your username is correct.')); + throw new \Exception($this->l10n->t('Couldn\'t send reset email. Please make sure your username is correct.')); } - $token = hash('sha256', \OC_Util::generateRandomBytes(30)); - - // Hash the token again to prevent timing attacks - $this->config->setUserValue( - $user, 'owncloud', 'lostpassword', hash('sha256', $token) - ); - $email = $this->config->getUserValue($user, 'settings', 'email'); if (empty($email)) { throw new \Exception( $this->l10n->t('Couldn\'t send reset email because there is no '. - 'email address for this username. Please ' . - 'contact your administrator.') + 'email address for this username. Please ' . + 'contact your administrator.') ); } - $link = $this->getLink('core.lost.resetform', $user, $token); + $token = $this->secureRandom->getMediumStrengthGenerator()->generate(21, + ISecureRandom::CHAR_DIGITS. + ISecureRandom::CHAR_LOWER. + ISecureRandom::CHAR_UPPER); + $this->config->setUserValue($user, 'owncloud', 'lostpassword', $token); + + $link = $this->urlGenerator->linkToRouteAbsolute('core.lost.setPassword', array('userId' => $user, 'token' => $token)); $tmpl = new \OC_Template('core/lostpassword', 'email'); $tmpl->assign('link', $link, false); @@ -200,23 +213,4 @@ class LostController extends Controller { } } - - protected function getLink($route, $user, $token){ - $parameters = array( - 'token' => $token, - 'userId' => $user - ); - $link = $this->urlGenerator->linkToRoute($route, $parameters); - - return $this->urlGenerator->getAbsoluteUrl($link); - } - - - protected function checkToken($user, $token) { - return $this->config->getUserValue( - $user, 'owncloud', 'lostpassword' - ) === hash('sha256', $token); - } - - } diff --git a/core/lostpassword/encrypteddataexception.php b/core/lostpassword/encrypteddataexception.php deleted file mode 100644 index 99d19445b6..0000000000 --- a/core/lostpassword/encrypteddataexception.php +++ /dev/null @@ -1,14 +0,0 @@ - -
-
-
t('You will receive a link to reset your password via Email.')); ?>
-

- - - - -
-

t("Your files are encrypted. If you haven't enabled the recovery key, there will be no way to get your data back after your password is reset. If you are not sure what to do, please contact your administrator before you continue. Do you really want to continue?")); ?>
- - t('Yes, I really want to reset my password now')); ?>

- -

- -
-
diff --git a/core/lostpassword/templates/resetpassword.php b/core/lostpassword/templates/resetpassword.php index 118fe78711..ebd7bff266 100644 --- a/core/lostpassword/templates/resetpassword.php +++ b/core/lostpassword/templates/resetpassword.php @@ -1,4 +1,10 @@ - + +

@@ -9,4 +15,3 @@

- diff --git a/tests/core/lostpassword/controller/lostcontrollertest.php b/tests/core/lostpassword/controller/lostcontrollertest.php new file mode 100644 index 0000000000..0a598d1f19 --- /dev/null +++ b/tests/core/lostpassword/controller/lostcontrollertest.php @@ -0,0 +1,195 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\Core\LostPassword\Controller; +use OC\Core\Application; +use OCP\AppFramework\Http\TemplateResponse; + +/** + * Class LostControllerTest + * + * @package OC\Core\LostPassword\Controller + */ +class LostControllerTest extends \PHPUnit_Framework_TestCase { + + private $container; + /** @var LostController */ + private $lostController; + + protected function setUp() { + $app = new Application(); + $this->container = $app->getContainer(); + $this->container['AppName'] = 'core'; + $this->container['Config'] = $this->getMockBuilder('\OCP\IConfig') + ->disableOriginalConstructor()->getMock(); + $this->container['L10N'] = $this->getMockBuilder('\OCP\IL10N') + ->disableOriginalConstructor()->getMock(); + $this->container['Defaults'] = $this->getMockBuilder('\OC_Defaults') + ->disableOriginalConstructor()->getMock(); + $this->container['UserManager'] = $this->getMockBuilder('\OCP\IUserManager') + ->disableOriginalConstructor()->getMock(); + $this->container['Config'] = $this->getMockBuilder('\OCP\IConfig') + ->disableOriginalConstructor()->getMock(); + $this->container['URLGenerator'] = $this->getMockBuilder('\OCP\IURLGenerator') + ->disableOriginalConstructor()->getMock(); + $this->container['SecureRandom'] = $this->getMockBuilder('\OCP\Security\ISecureRandom') + ->disableOriginalConstructor()->getMock(); + $this->container['IsEncryptionEnabled'] = true; + $this->lostController = $this->container['LostController']; + } + + public function testResetFormUnsuccessful() { + $userId = 'admin'; + $token = 'MySecretToken'; + + $this->container['URLGenerator'] + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with('core.lost.setPassword', array('userId' => 'admin', 'token' => 'MySecretToken')) + ->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/')); + + $response = $this->lostController->resetform($token, $userId); + $expectedResponse = new TemplateResponse('core/lostpassword', + 'resetpassword', + array( + 'link' => 'https://ownCloud.com/index.php/lostpassword/', + ), + 'guest'); + $this->assertEquals($expectedResponse, $response); + } + + public function testEmailUnsucessful() { + $existingUser = 'ExistingUser'; + $nonExistingUser = 'NonExistingUser'; + $this->container['UserManager'] + ->expects($this->any()) + ->method('userExists') + ->will($this->returnValueMap(array( + array(true, $existingUser), + array(false, $nonExistingUser) + ))); + $this->container['L10N'] + ->expects($this->any()) + ->method('t') + ->will( + $this->returnValueMap( + array( + array('Couldn\'t send reset email. Please make sure your username is correct.', array(), + 'Couldn\'t send reset email. Please make sure your username is correct.'), + + ) + )); + + // With a non existing user + $response = $this->lostController->email($nonExistingUser); + $expectedResponse = array('status' => 'error', 'msg' => 'Couldn\'t send reset email. Please make sure your username is correct.'); + $this->assertSame($expectedResponse, $response); + + // With no mail address + $this->container['Config'] + ->expects($this->any()) + ->method('getUserValue') + ->with($existingUser, 'settings', 'email') + ->will($this->returnValue(null)); + $response = $this->lostController->email($existingUser); + $expectedResponse = array('status' => 'error', 'msg' => 'Couldn\'t send reset email. Please make sure your username is correct.'); + $this->assertSame($expectedResponse, $response); + } + + public function testEmailSuccessful() { + $randomToken = $this->container['SecureRandom']; + $this->container['SecureRandom'] + ->expects($this->once()) + ->method('generate') + ->with('21') + ->will($this->returnValue('ThisIsMaybeANotSoSecretToken!')); + $this->container['UserManager'] + ->expects($this->once()) + ->method('userExists') + ->with('ExistingUser') + ->will($this->returnValue(true)); + $this->container['Config'] + ->expects($this->once()) + ->method('getUserValue') + ->with('ExistingUser', 'settings', 'email') + ->will($this->returnValue('test@example.com')); + $this->container['SecureRandom'] + ->expects($this->once()) + ->method('getMediumStrengthGenerator') + ->will($this->returnValue($randomToken)); + $this->container['Config'] + ->expects($this->once()) + ->method('setUserValue') + ->with('ExistingUser', 'owncloud', 'lostpassword', 'ThisIsMaybeANotSoSecretToken!'); + $this->container['URLGenerator'] + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with('core.lost.setPassword', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!')) + ->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/')); + + $response = $this->lostController->email('ExistingUser', true); + $expectedResponse = array('status' => 'success'); + $this->assertSame($expectedResponse, $response); + } + + public function testSetPasswordUnsuccessful() { + $this->container['L10N'] + ->expects($this->any()) + ->method('t') + ->will( + $this->returnValueMap( + array( + array('Couldn\'t reset password because the token is invalid', array(), + 'Couldn\'t reset password because the token is invalid'), + ) + )); + $this->container['Config'] + ->expects($this->once()) + ->method('getUserValue') + ->with('InvalidTokenUser', 'owncloud', 'lostpassword') + ->will($this->returnValue('TheOnlyAndOnlyOneTokenToResetThePassword')); + + // With an invalid token + $userName = 'InvalidTokenUser'; + $response = $this->lostController->setPassword('wrongToken', $userName, 'NewPassword', true); + $expectedResponse = array('status' => 'error', 'msg' => 'Couldn\'t reset password because the token is invalid'); + $this->assertSame($expectedResponse, $response); + + // With a valid token and no proceed + $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword!', $userName, 'NewPassword', false); + $expectedResponse = array('status' => 'error', 'msg' => '', 'encryption' => true); + $this->assertSame($expectedResponse, $response); + } + + public function testSetPasswordSuccessful() { + $this->container['Config'] + ->expects($this->once()) + ->method('getUserValue') + ->with('ValidTokenUser', 'owncloud', 'lostpassword') + ->will($this->returnValue('TheOnlyAndOnlyOneTokenToResetThePassword')); + $user = $this->getMockBuilder('\OCP\IUser') + ->disableOriginalConstructor()->getMock(); + $user->expects($this->once()) + ->method('setPassword') + ->with('NewPassword') + ->will($this->returnValue(true)); + $this->container['UserManager'] + ->expects($this->once()) + ->method('get') + ->with('ValidTokenUser') + ->will($this->returnValue($user)); + $this->container['Config'] + ->expects($this->once()) + ->method('deleteUserValue') + ->with('ValidTokenUser', 'owncloud', 'lostpassword'); + + $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); + $expectedResponse = array('status' => 'success'); + $this->assertSame($expectedResponse, $response); + } +} diff --git a/tests/phpunit-autotest.xml b/tests/phpunit-autotest.xml index 3805bb1ac7..282f5477c3 100644 --- a/tests/phpunit-autotest.xml +++ b/tests/phpunit-autotest.xml @@ -9,6 +9,7 @@ lib/ settings/ + core/ apps.php diff --git a/tests/phpunit.xml.dist b/tests/phpunit.xml.dist index 21c63ea046..95abe47396 100644 --- a/tests/phpunit.xml.dist +++ b/tests/phpunit.xml.dist @@ -2,6 +2,8 @@ lib/ + settings/ + core/ apps.php diff --git a/tests/settings/controller/mailsettingscontrollertest.php b/tests/settings/controller/mailsettingscontrollertest.php index 6d3485d28e..789b6ce8fb 100644 --- a/tests/settings/controller/mailsettingscontrollertest.php +++ b/tests/settings/controller/mailsettingscontrollertest.php @@ -14,7 +14,7 @@ use \OC\Settings\Application; /** * @package OC\Settings\Controller */ -class MailSettingscontrollerTest extends \PHPUnit_Framework_TestCase { +class MailSettingsControllerTest extends \PHPUnit_Framework_TestCase { private $container; From 60ae2894aa298cc2ee5bb2a59312303746bbd633 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 21 Oct 2014 18:31:41 +0200 Subject: [PATCH 02/11] Fix scrutinizer issues --- tests/core/lostpassword/controller/lostcontrollertest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/lostpassword/controller/lostcontrollertest.php b/tests/core/lostpassword/controller/lostcontrollertest.php index 0a598d1f19..c513ce17c7 100644 --- a/tests/core/lostpassword/controller/lostcontrollertest.php +++ b/tests/core/lostpassword/controller/lostcontrollertest.php @@ -132,7 +132,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase { ->with('core.lost.setPassword', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!')) ->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/')); - $response = $this->lostController->email('ExistingUser', true); + $response = $this->lostController->email('ExistingUser'); $expectedResponse = array('status' => 'success'); $this->assertSame($expectedResponse, $response); } From 57b5c82eb74a0f6f6f8fe785d0fd05892a12ba57 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 22 Oct 2014 10:29:26 +0200 Subject: [PATCH 03/11] Remove uneeded import --- core/lostpassword/controller/lostcontroller.php | 1 - 1 file changed, 1 deletion(-) diff --git a/core/lostpassword/controller/lostcontroller.php b/core/lostpassword/controller/lostcontroller.php index a1a683baa7..eff7a1f572 100644 --- a/core/lostpassword/controller/lostcontroller.php +++ b/core/lostpassword/controller/lostcontroller.php @@ -15,7 +15,6 @@ use \OCP\IRequest; use \OCP\IL10N; use \OCP\IConfig; use OCP\IUserManager; -use \OC\Core\LostPassword\EncryptedDataException; use OCP\Security\ISecureRandom; use \OC_Defaults; use OCP\Security\StringUtils; From 767b08c6699e88a669b97246497d5315c6a6c063 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 24 Oct 2014 11:45:30 +0200 Subject: [PATCH 04/11] Use correct route instead THX @schiesbn (I should setup a mail server on my local system...) --- core/lostpassword/controller/lostcontroller.php | 2 +- tests/core/lostpassword/controller/lostcontrollertest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/lostpassword/controller/lostcontroller.php b/core/lostpassword/controller/lostcontroller.php index eff7a1f572..ec52108ad4 100644 --- a/core/lostpassword/controller/lostcontroller.php +++ b/core/lostpassword/controller/lostcontroller.php @@ -189,7 +189,7 @@ class LostController extends Controller { ISecureRandom::CHAR_UPPER); $this->config->setUserValue($user, 'owncloud', 'lostpassword', $token); - $link = $this->urlGenerator->linkToRouteAbsolute('core.lost.setPassword', array('userId' => $user, 'token' => $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); diff --git a/tests/core/lostpassword/controller/lostcontrollertest.php b/tests/core/lostpassword/controller/lostcontrollertest.php index c513ce17c7..5da9e5ce48 100644 --- a/tests/core/lostpassword/controller/lostcontrollertest.php +++ b/tests/core/lostpassword/controller/lostcontrollertest.php @@ -129,7 +129,7 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase { $this->container['URLGenerator'] ->expects($this->once()) ->method('linkToRouteAbsolute') - ->with('core.lost.setPassword', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!')) + ->with('core.lost.resetform', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!')) ->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/')); $response = $this->lostController->email('ExistingUser'); From 357465eac9733530c8e78ff0d5e1abca04ac93db Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 24 Oct 2014 13:41:44 +0200 Subject: [PATCH 05/11] Add "postPasswordReset" hook --- core/lostpassword/controller/lostcontroller.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/core/lostpassword/controller/lostcontroller.php b/core/lostpassword/controller/lostcontroller.php index ec52108ad4..0a78063cc3 100644 --- a/core/lostpassword/controller/lostcontroller.php +++ b/core/lostpassword/controller/lostcontroller.php @@ -22,6 +22,8 @@ use OCP\Security\StringUtils; /** * Class LostController * + * Successfully changing a password will emit the post_passwordReset hook. + * * @package OC\Core\LostPassword\Controller */ class LostController extends Controller { @@ -47,13 +49,13 @@ class LostController extends Controller { * @param string $appName * @param IRequest $request * @param IURLGenerator $urlGenerator - * @param $userManager - * @param $defaults + * @param IUserManager $userManager + * @param OC_Defaults $defaults * @param IL10N $l10n * @param IConfig $config * @param ISecureRandom $secureRandom - * @param $from - * @param $isDataEncrypted + * @param string $from + * @param string $isDataEncrypted */ public function __construct($appName, IRequest $request, @@ -154,6 +156,8 @@ class LostController extends Controller { throw new \Exception(); } + \OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', array($userId)); + $this->config->deleteUserValue($userId, 'owncloud', 'lostpassword'); @\OC_User::unsetMagicInCookie(); From 11ab457b7204b68d41337794ab16b71031dd592f Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 29 Oct 2014 12:44:15 +0100 Subject: [PATCH 06/11] add password as parameter to the signal so that the encryption can create a new key-pair --- core/lostpassword/controller/lostcontroller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lostpassword/controller/lostcontroller.php b/core/lostpassword/controller/lostcontroller.php index 0a78063cc3..aee4001ed3 100644 --- a/core/lostpassword/controller/lostcontroller.php +++ b/core/lostpassword/controller/lostcontroller.php @@ -156,7 +156,7 @@ class LostController extends Controller { throw new \Exception(); } - \OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', array($userId)); + \OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', array('uid' => $userId, 'password' => $password)); $this->config->deleteUserValue($userId, 'owncloud', 'lostpassword'); @\OC_User::unsetMagicInCookie(); From f6efbfcf0bb76e16347748666d0c967ad839c5b2 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 29 Oct 2014 12:45:13 +0100 Subject: [PATCH 07/11] listen to the post_passwordReset hook, backup the old keys and create a new key pair for the user --- apps/files_encryption/hooks/hooks.php | 13 +++++++++++++ apps/files_encryption/lib/helper.php | 1 + apps/files_encryption/lib/util.php | 12 ++++++++++++ 3 files changed, 26 insertions(+) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index 3a0a37c0a5..eadd2b64b8 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -263,6 +263,19 @@ class Hooks { } } + /** + * after password reset we create a new key pair for the user + * + * @param array $params + */ + public static function postPasswordReset($params) { + $uid = $params['uid']; + $password = $params['password']; + + $util = new Util(new \OC\Files\View(), $uid); + $util->replaceUserKeys($password); + } + /* * check if files can be encrypted to every user. */ diff --git a/apps/files_encryption/lib/helper.php b/apps/files_encryption/lib/helper.php index 53c380ab2b..7a50ade82f 100644 --- a/apps/files_encryption/lib/helper.php +++ b/apps/files_encryption/lib/helper.php @@ -70,6 +70,7 @@ class Helper { \OCP\Util::connectHook('OC_Filesystem', 'delete', 'OCA\Encryption\Hooks', 'preDelete'); \OCP\Util::connectHook('OC_Filesystem', 'post_umount', 'OCA\Encryption\Hooks', 'postUmount'); \OCP\Util::connectHook('OC_Filesystem', 'umount', 'OCA\Encryption\Hooks', 'preUmount'); + \OCP\Util::connectHook('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', 'OCA\Encryption\Hooks', 'postPasswordReset'); } /** diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index c8697ae7c8..d12b003b22 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -124,6 +124,18 @@ class Util { } } + /** + * create a new public/private key pair for the user + * + * @param string $password password for the private key + */ + public function replaceUserKeys($password) { + $this->backupAllKeys('password_reset'); + $this->view->unlink($this->publicKeyPath); + $this->view->unlink($this->privateKeyPath); + $this->setupServerSide($password); + } + /** * Sets up user folders and keys for serverside encryption * From f530865b3d952e10c5c295f6dbe1138a667fa659 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 29 Oct 2014 13:26:24 +0100 Subject: [PATCH 08/11] Hide submit button after password change Creating a new key pair can take 1-2 seconds. So it could happen that the user click the "Reset password" button again which can lead to many nasty things, e.g. we could create two new key pairs in parallel. --- core/js/lostpassword.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/js/lostpassword.js b/core/js/lostpassword.js index aa1a864ffe..7145e21965 100644 --- a/core/js/lostpassword.js +++ b/core/js/lostpassword.js @@ -82,6 +82,9 @@ OC.Lostpassword = { OC.Lostpassword.resetDone ); } + if($('#encrypted-continue').attr('checked')) { + $('#reset-password #submit').hide(); + } }, resetDone : function(result){ From 68e77f46598429e1d8a96f668e5065e5fce362ce Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Tue, 11 Nov 2014 15:14:04 +0100 Subject: [PATCH 09/11] fix unreadable label in warning box --- core/css/styles.css | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/css/styles.css b/core/css/styles.css index c45588cece..2859399b59 100644 --- a/core/css/styles.css +++ b/core/css/styles.css @@ -353,6 +353,12 @@ input[type="submit"].enabled { filter: alpha(opacity=60); opacity: .6; } +/* overrides another !important statement that sets this to unreadable black */ +#body-login form .warning input[type="checkbox"]:hover+label, +#body-login form .warning input[type="checkbox"]:focus+label, +#body-login form .warning input[type="checkbox"]+label { + color: #fff !important; +} #body-login .update h2 { font-size: 20px; From 9eeea57e3abc4a1cfb1d303eae5f454f3d382b3f Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 17 Nov 2014 17:49:58 +0100 Subject: [PATCH 10/11] Show spinner --- core/js/lostpassword.js | 1 + core/lostpassword/templates/resetpassword.php | 1 + 2 files changed, 2 insertions(+) diff --git a/core/js/lostpassword.js b/core/js/lostpassword.js index 7145e21965..35173fd3d3 100644 --- a/core/js/lostpassword.js +++ b/core/js/lostpassword.js @@ -84,6 +84,7 @@ OC.Lostpassword = { } if($('#encrypted-continue').attr('checked')) { $('#reset-password #submit').hide(); + $('#reset-password #float-spinner').removeClass('hidden'); } }, diff --git a/core/lostpassword/templates/resetpassword.php b/core/lostpassword/templates/resetpassword.php index ebd7bff266..bac8b5c75c 100644 --- a/core/lostpassword/templates/resetpassword.php +++ b/core/lostpassword/templates/resetpassword.php @@ -13,5 +13,6 @@ script('core', 'lostpassword');

+ From 345eb62ffaa969707fc1901d55c0674aceecea3e Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Tue, 18 Nov 2014 10:25:16 +0100 Subject: [PATCH 11/11] center spinner --- core/lostpassword/css/resetpassword.css | 4 ++++ core/lostpassword/templates/resetpassword.php | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/core/lostpassword/css/resetpassword.css b/core/lostpassword/css/resetpassword.css index 012af672d9..29a7e87553 100644 --- a/core/lostpassword/css/resetpassword.css +++ b/core/lostpassword/css/resetpassword.css @@ -2,6 +2,10 @@ position: relative; } +.text-center { + text-align: center; +} + #password-icon { top: 20px; } diff --git a/core/lostpassword/templates/resetpassword.php b/core/lostpassword/templates/resetpassword.php index bac8b5c75c..498c692f12 100644 --- a/core/lostpassword/templates/resetpassword.php +++ b/core/lostpassword/templates/resetpassword.php @@ -13,6 +13,8 @@ script('core', 'lostpassword');

- +

+ +