Changes according to review

core/js/lostpassword.js

@@ -30,24 +30,26 @@ OC.Lostpassword = {
 			$('#submit').trigger('click');
 		} else {
 			$.post(
-					OC.filePath('core', 'ajax', 'password/lost'), 
-					{ 
+					OC.filePath('core', 'ajax', 'password/lost'),
+					{
 						user : $('#user').val(),
 						proceed: $('#encrypted-continue').attr('checked') ? 'Yes' : 'No'
-					}, 
+					},
 					OC.Lostpassword.sendLinkDone
 			);
 		}
 	},
 			
 	sendLinkDone : function(result){
+		var sendErrorMsg;
+		
 		if (result && result.status === 'success'){
 			OC.Lostpassword.sendLinkSuccess();
 		} else {
 			if (result && result.msg){
-				var sendErrorMsg = result.msg;
+				sendErrorMsg = result.msg;
 			} else {
-				var sendErrorMsg = OC.Lostpassword.sendErrorMsg;
+				sendErrorMsg = OC.Lostpassword.sendErrorMsg;
 			}
 			OC.Lostpassword.sendLinkError(sendErrorMsg);
 		}
@@ -80,7 +82,7 @@ OC.Lostpassword = {
 		if ($('#password').val()){
 			$.post(
 					$('#password').parents('form').attr('action'),
-					{ 
+					{
 						password : $('#password').val()
 					},
 					OC.Lostpassword.resetDone
@@ -89,6 +91,7 @@ OC.Lostpassword = {
 	},
 			
 	resetDone : function(result){
+		var resetErrorMsg;
 		if (result && result.status === 'success'){
 			$.post(
 					OC.webroot + '/',
@@ -100,11 +103,11 @@ OC.Lostpassword = {
 			);
 		} else {
 			if (result && result.msg){
-				var resetErrorMsg = result.msg;
+				resetErrorMsg = result.msg;
 			} else if (result && result.encryption) {
-				var sendErrorMsg = OC.Lostpassword.encryptedMsg;
+				resetErrorMsg = OC.Lostpassword.encryptedMsg;
 			} else {
-				var resetErrorMsg = OC.Lostpassword.resetErrorMsg;
+				resetErrorMsg = OC.Lostpassword.resetErrorMsg;
 			}
 			OC.Lostpassword.resetError(resetErrorMsg);
 		}

core/lostpassword/application.php

@@ -0,0 +1,38 @@
+<?php
+/**
+ * @author Victor Dubiniuk
+ * @copyright 2014 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;
+
+use \OCP\AppFramework\App;
+use OC\Core\LostPassword\Controller\LostController;
+
+class Application extends App {
+	public function __construct(array $urlParams=array()){
+		parent::__construct('core', $urlParams);
+
+		$container = $this->getContainer();
+
+		/**
+		 * Controllers
+		 */
+		$container->registerService('LostController', function($c) {
+			return new LostController(
+				$c->query('AppName'),
+				$c->query('ServerContainer')->getRequest(),
+				$c->query('ServerContainer')->getURLGenerator(),
+				$c->query('ServerContainer')->getUserManager(),
+				new \OC_Defaults(),
+				$c->query('ServerContainer')->getL10N('core'),
+				\OCP\Util::getDefaultEmailAddress('lostpassword-noreply'),
+				\OC_App::isEnabled('files_encryption')
+			);
+		});
+	}
+}

core/lostpassword/controller/ajaxcontroller.php

@@ -1,101 +0,0 @@
-<?php
-/**
- * @author Victor Dubiniuk
- * @copyright 2014 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\Controller;
-
-use \OCP\AppFramework\Controller;
-use \OCP\AppFramework\Http\JSONResponse;
-
-class AjaxController extends LostController {
-
-	/**
-	 * @PublicPage
-	 */
-	public function lost(){
-		$response = new JSONResponse(array('status'=>'success'));
-		try {
-			$this->sendEmail($this->params('user', ''), $this->params('proceed', ''));
-		} catch (EncryptedDataException $e){
-			$response->setData(array(
-				'status' => 'error',
-				'encryption' => '1'
-			));
-		} catch (\Exception $e){
-			$response->setData(array(
-				'status' => 'error',
-				'msg' => $e->getMessage()
-			));
-		}
-		
-		return $response;
-	}
-	
-	/**
-	 * @PublicPage
-	 */
-	public function resetPassword() {
-		$response = new JSONResponse(array('status'=>'success'));
-		try {
-			$user = $this->params('user');
-			$newPassword = $this->params('password');
-			if (!$this->checkToken()) {
-				throw new \RuntimeException('');
-			}
-			if (!\OC_User::setPassword($user, $newPassword)) {
-				throw new \RuntimeException('');
-			}
-			\OC_Preferences::deleteKey($user, 'owncloud', 'lostpassword');
-			\OC_User::unsetMagicInCookie();
-		} catch (Exception $e){
-			$response->setData(array(
-				'status' => 'error',
-				'msg' => $e->getMessage()
-			));
-		}
-		return $response;
-	}
-	
-	protected function sendEmail($user, $proceed) {
-		$l = \OC_L10N::get('core');
-		$isEncrypted = \OC_App::isEnabled('files_encryption');
-
-		if ($isEncrypted && $proceed !== 'Yes'){
-			throw new EncryptedDataException();
-		}
-
-		if (!\OC_User::userExists($user)) {
-			throw new \Exception($l->t('Couldn’t send reset email. Please make sure your username is correct.'));
-		}
-		$token = hash('sha256', \OC_Util::generateRandomBytes(30).\OC_Config::getValue('passwordsalt', ''));
-		\OC_Preferences::setValue($user, 'owncloud', 'lostpassword',
-			hash('sha256', $token)); // Hash the token again to prevent timing attacks
-		$email = \OC_Preferences::getValue($user, 'settings', 'email', '');
-		if (empty($email)) {
-			throw new \Exception($l->t('Couldn’t send reset email because there is no email address for this username. Please contact your administrator.'));
-		}
-		
-		$parameters = array('token' => $token, 'user' => $user);
-		$link = $this->urlGenerator->linkToRoute('core.lost.reset', $parameters);
-		$link = $this->urlGenerator->getAbsoluteUrl($link);
-		
-		$tmpl = new \OC_Template('core/lostpassword', 'email');
-		$tmpl->assign('link', $link, false);
-		$msg = $tmpl->fetchPage();
-		echo $link;
-		$from = \OCP\Util::getDefaultEmailAddress('lostpassword-noreply');
-		try {
-			$defaults = new \OC_Defaults();
-			\OC_Mail::send($email, $user, $l->t('%s password reset', array($defaults->getName())), $msg, $from, $defaults->getName());
-		} catch (\Exception $e) {
-			throw new \Exception( $l->t('Couldn’t send reset email. Please contact your administrator.'));
-		}
-	}
-	
-}

core/lostpassword/controller/lostcontroller.php

@@ -5,27 +5,43 @@
  * later.
  * See the COPYING-README file.
  */
+
 namespace OC\Core\LostPassword\Controller;
 
 use \OCP\AppFramework\Controller;
+use \OCP\AppFramework\Http\JSONResponse;
 use \OCP\AppFramework\Http\TemplateResponse;
 
 class LostController extends Controller {
 	
 	protected $urlGenerator;
+	protected $userManager;
+	protected $defaults;
+	protected $l10n;
+	protected $from;
+	protected $isDataEncrypted;
 	
-	public function __construct($appName, IRequest $request, IURLGenerator $urlGenerator) {
+	public function __construct($appName, IRequest $request, IURLGenerator $urlGenerator, $userManager,
+			$defaults, $l10n, $from, $isDataEncrypted) {
 		parent::__construct($appName, $request);
 		$this->urlGenerator = $urlGenerator;
+		$this->userManager = $userManager;
+		$this->defaults = $defaults;
+		$this->l10n = $l10n;
+		$this->from = $from;
+		$this->isDataEncrypted = $isDataEncrypted;
 	}
 
 	/**
 	 * @PublicPage
 	 * @NoCSRFRequired
+	 * 
+	 * @param string $token
+	 * @param string $uid
 	 */
-	public function reset() {
+	public function reset($token, $uid) {
 		// Someone wants to reset their password:
-		if($this->checkToken()) {
+		if($this->checkToken($uid, $token)) {
 			return new TemplateResponse(
 				'core/lostpassword', 
 				'resetpassword', 
@@ -36,31 +52,102 @@ class LostController extends Controller {
 			);
 		} else {
 			// Someone lost their password
-			$isEncrypted = \OC_App::isEnabled('files_encryption');
 			return new TemplateResponse(
 				'core/lostpassword', 
 				'lostpassword', 
 				array(
-					'isEncrypted' => $isEncrypted, 
-					'link' => $this->getResetPasswordLink()
+					'isEncrypted' => $this->isDataEncrypted, 
+					'link' => $this->getResetPasswordLink($uid, $token)
 				),
 				'guest'
 			);
 		}
 	}
+	
+	/**
+	 * @PublicPage
+	 * 
+	 * @param bool $proceed
+	 */
+	public function lost($user, $proceed){
+		$response = new JSONResponse(array('status'=>'success'));
+		try {
+			$this->sendEmail($user, $proceed);
+		} catch (EncryptedDataException $e){
+			$response->setData(array(
+				'status' => 'error',
+				'encryption' => '1'
+			));
+		} catch (\Exception $e){
+			$response->setData(array(
+				'status' => 'error',
+				'msg' => $e->getMessage()
+			));
+		}
+		
+		return $response;
+	}
+	
+	/**
+	 * @PublicPage
+	 */
+	public function resetPassword($user, $password, $token) {
+		$response = new JSONResponse(array('status'=>'success'));
+		try {
+			if (!$this->checkToken($user, $token)) {
+				throw new \RuntimeException('');
+			}
+			if (!$this->userManager->setPassword($user, $newPassword)) {
+				throw new \RuntimeException('');
+			}
+			\OC_Preferences::deleteKey($user, 'owncloud', 'lostpassword');
+			$this->userManager->unsetMagicInCookie();
+		} catch (Exception $e){
+			$response->setData(array(
+				'status' => 'error',
+				'msg' => $e->getMessage()
+			));
+		}
+		return $response;
+	}
+	
+	protected function sendEmail($user, $proceed) {
+		if ($this->isDataEncrypted && $proceed !== 'Yes'){
+			throw new EncryptedDataException();
+		}
+
+		if (!$this->userManager->userExists($user)) {
+			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));
+		\OC_Preferences::setValue($user, 'owncloud', 'lostpassword', hash('sha256', $token)); // Hash the token again to prevent timing attacks
+		$email = \OC_Preferences::getValue($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.'));
+		}
+		
+		$link = $this->getResetPasswordLink($user, $token);
+		echo $link;
+		$tmpl = new \OC_Template('core/lostpassword', 'email');
+		$tmpl->assign('link', $link, false);
+		$msg = $tmpl->fetchPage();
+		try {
+			\OC_Mail::send($email, $user, $this->l10n->t('%s password reset', array($this->defaults->getName())), $msg, $this->from, $this->defaults->getName());
+		} catch (\Exception $e) {
+			throw new \Exception( $this->l10n->t('Couldn’t send reset email. Please contact your administrator.'));
+		}
+	}
 
-	protected function getResetPasswordLink(){
+	protected function getResetPasswordLink($user, $token){
 		$parameters = array(
-			'token' => $this->params('token'), 
-			'user' => $this->params('user')
+			'token' => $token, 
+			'uid' => $user
 		);
-		$link = $this->urlGenerator->linkToRoute('core.ajax.reset', $parameters);
+		$link = $this->urlGenerator->linkToRoute('core.lost.reset', $parameters);
 		return $this->urlGenerator->getAbsoluteUrl($link);
 	}
 
-	protected function checkToken() {
-		$user = $this->params('user');
-		$token = $this->params('token');
+	protected function checkToken($user, $token) {
 		return \OC_Preferences::getValue($user, 'owncloud', 'lostpassword') === hash('sha256', $token);
 	}
 }

core/routes.php

@@ -6,45 +6,16 @@
  * See the COPYING-README file.
  */
 
-use \OCP\AppFramework\App;
-use OC\Core\LostPassword\Controller\LostController;
-use OC\Core\LostPassword\Controller\AjaxController;
-
-class Application extends App {
-	public function __construct(array $urlParams=array()){
-		parent::__construct('core', $urlParams);
-
-		$container = $this->getContainer();
-
-		/**
-		 * Controllers
-		 */
-		$container->registerService('LostController', function($c) {
-			return new LostController(
-				$c->query('AppName'),
-				$c->query('ServerContainer')->getRequest(),
-				$c->query('ServerContainer')->getURLGenerator()
-			);
-		});
-		$container->registerService('AjaxController', function($c) {
-			return new AjaxController(
-				$c->query('AppName'),
-				$c->query('ServerContainer')->getRequest(),
-				$c->query('ServerContainer')->getURLGenerator()
-			);
-		});
-	}
-}
+use OC\Core\LostPassword\Application;
 
 $application = new Application();
 $application->registerRoutes($this, array('routes' => array(
-		array('name' => 'ajax#lost', 'url' => '/core/ajax/password/lost', 'verb' => 'POST'),
-		array('name' => 'ajax#reset', 'url' => '/core/ajax/password/reset/{token}/{user}', 'verb' => 'POST'),
-		array('name' => 'lost#reset', 'url' => '/lostpassword/reset/{token}/{user}', 'verb' => 'GET'),
+		array('name' => 'lost#lost', 'url' => '/core/ajax/password/lost', 'verb' => 'POST'),
+		array('name' => 'lost#reset', 'url' => '/lostpassword/reset/{token}/{uid}', 'verb' => 'GET'),
+		array('name' => 'lost#resetPassword', 'url' => '/core/ajax/password/reset/{token}/{user}', 'verb' => 'POST'),
 	)
 ));
 
-
 // Post installation check
 
 /** @var $this OCP\Route\IRouter */