Merge pull request #11696 from owncloud/addWarningToEncryptionLostPassword

Warn for password reset when files_encryption is enabled

8 files changed, 163 insertions, 149 deletions

diff --git a/core/application.php b/core/application.php
index 33801847758..c36ab559c27 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/css/styles.css b/core/css/styles.css
index c45588cece6..2859399b59e 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;
diff --git a/core/js/lostpassword.js b/core/js/lostpassword.js
index ad221cb30fc..35173fd3d33 100644
--- a/core/js/lostpassword.js
+++ b/core/js/lostpassword.js
@@ -8,19 +8,12 @@ OC.Lostpassword = {
 			+ ('<br /><input type="checkbox" id="encrypted-continue" value="Yes" />')
 			+ '<label for="encrypted-continue">'
 			+ t('core', 'I know what I\'m doing')
-			+ '</label><br />'
-			+ '<a id="lost-password-encryption" href>'
-			+ t('core', 'Reset password')
-			+ '</a>',
+			+ '</label><br />',
 
 	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,11 +76,16 @@ OC.Lostpassword = {
 			$.post(
 					$('#password').parents('form').attr('action'),
 					{
-						password : $('#password').val()
+						password : $('#password').val(),
+						proceed: $('#encrypted-continue').attr('checked') ? 'true' : 'false'
 					},
 					OC.Lostpassword.resetDone
 			);
 		}
+		if($('#encrypted-continue').attr('checked')) {
+			$('#reset-password #submit').hide();
+			$('#reset-password #float-spinner').removeClass('hidden');
+		}
 	},
 
 	resetDone : function(result){
@@ -126,7 +123,7 @@ OC.Lostpassword = {
 
 	getResetStatusNode : function (){
 		if (!$('#lost-password').length){
-			$('<p id="lost-password"></p>').insertAfter($('#submit'));
+			$('<p id="lost-password"></p>').insertBefore($('#reset-password fieldset'));
 		} else {
 			$('#lost-password').replaceWith($('<p id="lost-password"></p>'));
 		}
diff --git a/core/lostpassword/controller/lostcontroller.php b/core/lostpassword/controller/lostcontroller.php
index e4d51fde077..aee4001ed37 100644
--- a/core/lostpassword/controller/lostcontroller.php
+++ b/core/lostpassword/controller/lostcontroller.php
@@ -9,68 +9,73 @@
 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 \OC\Core\LostPassword\EncryptedDataException;
+use OCP\IUserManager;
+use OCP\Security\ISecureRandom;
+use \OC_Defaults;
+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 {
 
-	/**
-	 * @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 IUserManager $userManager
+	 * @param OC_Defaults $defaults
+	 * @param IL10N $l10n
+	 * @param IConfig $config
+	 * @param ISecureRandom $secureRandom
+	 * @param string $from
+	 * @param string $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 +86,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 +119,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 +132,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 +156,10 @@ 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();
+			\OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', array('uid' => $userId, 'password' => $password));
+
+			$this->config->deleteUserValue($userId, 'owncloud', 'lostpassword');
+			@\OC_User::unsetMagicInCookie();
 
 		} catch (\Exception $e){
 			return $this->error($e->getMessage());
@@ -148,36 +168,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.resetform', array('userId' => $user, 'token' => $token));
 
 		$tmpl = new \OC_Template('core/lostpassword', 'email');
 		$tmpl->assign('link', $link, false);
@@ -200,23 +216,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/css/resetpassword.css b/core/lostpassword/css/resetpassword.css
index 012af672d97..29a7e875537 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/encrypteddataexception.php b/core/lostpassword/encrypteddataexception.php
deleted file mode 100644
index 99d19445b6c..00000000000
--- a/core/lostpassword/encrypteddataexception.php
+++ /dev/null
@@ -1,14 +0,0 @@
-<?php
-/**
- * @author Victor Dubiniuk
- * @copyright 2013 Victor Dubiniuk victor.dubiniuk@gmail.com
- *
- * This file is licensed under the Affero General Public License version 3 or
- * later.
- * See the COPYING-README file.
- */
-
-namespace OC\Core\LostPassword;
- 
-class EncryptedDataException extends \Exception{
-}
diff --git a/core/lostpassword/templates/lostpassword.php b/core/lostpassword/templates/lostpassword.php
deleted file mode 100644
index 00dd139e71f..00000000000
--- a/core/lostpassword/templates/lostpassword.php
+++ /dev/null
@@ -1,20 +0,0 @@
-<?php
-//load the file we need
-OCP\Util::addStyle('lostpassword', 'lostpassword'); ?>
-<form action="<?php print_unescaped($_['link']) ?>" method="post">
-	<fieldset>
-		<div class="update"><?php p($l->t('You will receive a link to reset your password via Email.')); ?></div>
-		<p>
-			<input type="text" name="user" id="user" placeholder="<?php p($l->t( 'Username' )); ?>" value="" autocomplete="off" required autofocus />
-			<label for="user" class="infield"><?php p($l->t( 'Username' )); ?></label>
-			<img class="svg" src="<?php print_unescaped(image_path('', 'actions/user.svg')); ?>" alt=""/>
-			<?php if ($_['isEncrypted']): ?>
-				<br />
-				<p class="warning"><?php p($l->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?")); ?><br />
-				<input type="checkbox" name="continue" value="Yes" />
-				<?php p($l->t('Yes, I really want to reset my password now')); ?></p>
-			<?php endif; ?>
-		</p>
-		<input type="submit" id="submit" value="<?php p($l->t('Reset')); ?>" />
-	</fieldset>
-</form>
diff --git a/core/lostpassword/templates/resetpassword.php b/core/lostpassword/templates/resetpassword.php
index 118fe787116..498c692f12e 100644
--- a/core/lostpassword/templates/resetpassword.php
+++ b/core/lostpassword/templates/resetpassword.php
@@ -1,4 +1,10 @@
-<?php OCP\Util::addStyle('lostpassword', 'resetpassword'); ?>
+<?php
+/** @var array $_ */
+/** @var $l OC_L10N */
+style('lostpassword', 'resetpassword');
+script('core', 'lostpassword');
+?>
+
 <form action="<?php print_unescaped($_['link']) ?>" id="reset-password" method="post">
 	<fieldset>
 		<p>
@@ -7,6 +13,8 @@
 			<img class="svg" id="password-icon" src="<?php print_unescaped(image_path('', 'actions/password.svg')); ?>" alt=""/>
 		</p>
 		<input type="submit" id="submit" value="<?php p($l->t('Reset password')); ?>" />
+		<p class="text-center">
+			<img class="hidden" id="float-spinner" src="<?php p(\OCP\Util::imagePath('core', 'loading-dark.gif'));?>"/>
+		</p>
 	</fieldset>
 </form>
-<?php OCP\Util::addScript('core', 'lostpassword'); ?>