Merge pull request #4123 from nextcloud/allow-password-reset-with-email

Allow to reset the password with the email as an input

core/Controller/LostController.php

@@ -39,6 +39,7 @@ use \OCP\IURLGenerator;
 use \OCP\IRequest;
 use \OCP\IL10N;
 use \OCP\IConfig;
+use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Mail\IMailer;
 use OCP\Security\ICrypto;
@@ -253,16 +254,12 @@ class LostController extends Controller {
 	}
 
 	/**
-	 * @param string $user
+	 * @param string $input
 	 * @throws \Exception
 	 */
-	protected function sendEmail($user) {
+	protected function sendEmail($input) {
-		if (!$this->userManager->userExists($user)) {
+		$user = $this->findUserByIdOrMail($input);
-			throw new \Exception($this->l10n->t('Couldn\'t send reset email. Please make sure your username is correct.'));
+		$email = $user->getEMailAddress();
-		}
-
-		$userObject = $this->userManager->get($user);
-		$email = $userObject->getEMailAddress();
 
 		if (empty($email)) {
 			throw new \Exception(
@@ -281,11 +278,10 @@ class LostController extends Controller {
 			ISecureRandom::CHAR_UPPER
 		);
 		$tokenValue = $this->timeFactory->getTime() .':'. $token;
-		$mailAddress = !is_null($userObject->getEMailAddress()) ? $userObject->getEMailAddress() : '';
+		$encryptedValue = $this->crypto->encrypt($tokenValue, $email . $this->config->getSystemValue('secret'));
-		$encryptedValue = $this->crypto->encrypt($tokenValue, $mailAddress.$this->config->getSystemValue('secret'));
+		$this->config->setUserValue($user->getUID(), 'core', 'lostpassword', $encryptedValue);
-		$this->config->setUserValue($user, 'core', 'lostpassword', $encryptedValue);
 
-		$link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', array('userId' => $user, 'token' => $token));
+		$link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', array('userId' => $user->getUID(), 'token' => $token));
 
 		$tmpl = new \OC_Template('core', 'lostpassword/email');
 		$tmpl->assign('link', $link);
@@ -293,7 +289,7 @@ class LostController extends Controller {
 
 		try {
 			$message = $this->mailer->createMessage();
-			$message->setTo([$email => $user]);
+			$message->setTo([$email => $user->getUID()]);
 			$message->setSubject($this->l10n->t('%s password reset', [$this->defaults->getName()]));
 			$message->setPlainBody($msg);
 			$message->setFrom([$this->from => $this->defaults->getName()]);
@@ -305,4 +301,21 @@ class LostController extends Controller {
 		}
 	}
 
+	/**
+	 * @param string $input
+	 * @return IUser
+	 * @throws \Exception
+	 */
+	protected function findUserByIdOrMail($input) {
+		$user = $this->userManager->get($input);
+		if ($user instanceof IUser) {
+			return $user;
+		}
+		$users = $this->userManager->getByEmail($input);
+		if (count($users) === 1) {
+			return $users[0];
+		}
+
+		throw new \InvalidArgumentException($this->l10n->t('Couldn\'t send reset email. Please make sure your username is correct.'));
+	}
 }

tests/Core/Controller/LostControllerTest.php

@@ -76,8 +76,12 @@ class LostControllerTest extends \Test\TestCase {
 		parent::setUp();
 
 		$this->existingUser = $this->createMock(IUser::class);
-		$this->existingUser->method('getEMailAddress')
+		$this->existingUser->expects($this->any())
+			->method('getEMailAddress')
 			->willReturn('test@example.com');
+		$this->existingUser->expects($this->any())
+			->method('getUID')
+			->willReturn('ExistingUser');
 
 		$this->config = $this->createMock(IConfig::class);
 		$this->config->method('getSystemValue')
@@ -279,11 +283,6 @@ class LostControllerTest extends \Test\TestCase {
 			->method('generate')
 			->with('21')
 			->will($this->returnValue('ThisIsMaybeANotSoSecretToken!'));
-		$this->userManager
-				->expects($this->once())
-				->method('userExists')
-				->with('ExistingUser')
-				->will($this->returnValue(true));
 		$this->userManager
 				->expects($this->any())
 				->method('get')
@@ -344,17 +343,83 @@ class LostControllerTest extends \Test\TestCase {
 		$this->assertSame($expectedResponse, $response);
 	}
 
-	public function testEmailCantSendException() {
+	public function testEmailWithMailSuccessful() {
 		$this->secureRandom
 			->expects($this->once())
 			->method('generate')
 			->with('21')
 			->will($this->returnValue('ThisIsMaybeANotSoSecretToken!'));
 		$this->userManager
+				->expects($this->any())
+				->method('get')
+				->with('test@example.com')
+				->willReturn(null);
+		$this->userManager
+				->expects($this->any())
+				->method('getByEmail')
+				->with('test@example.com')
+				->willReturn([$this->existingUser]);
+		$this->timeFactory
 			->expects($this->once())
-			->method('userExists')
+			->method('getTime')
-			->with('ExistingUser')
+			->will($this->returnValue(12348));
-			->will($this->returnValue(true));
+		$this->config
+			->expects($this->once())
+			->method('setUserValue')
+			->with('ExistingUser', 'core', 'lostpassword', 'encryptedToken');
+		$this->urlGenerator
+			->expects($this->once())
+			->method('linkToRouteAbsolute')
+			->with('core.lost.resetform', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!'))
+			->will($this->returnValue('https://example.tld/index.php/lostpassword/'));
+		$message = $this->getMockBuilder('\OC\Mail\Message')
+			->disableOriginalConstructor()->getMock();
+		$message
+			->expects($this->at(0))
+			->method('setTo')
+			->with(['test@example.com' => 'ExistingUser']);
+		$message
+			->expects($this->at(1))
+			->method('setSubject')
+			->with(' password reset');
+		$message
+			->expects($this->at(2))
+			->method('setPlainBody')
+			->with('Use the following link to reset your password: https://example.tld/index.php/lostpassword/');
+		$message
+			->expects($this->at(3))
+			->method('setFrom')
+			->with(['lostpassword-noreply@localhost' => null]);
+		$this->mailer
+			->expects($this->at(0))
+			->method('createMessage')
+			->will($this->returnValue($message));
+		$this->mailer
+			->expects($this->at(1))
+			->method('send')
+			->with($message);
+
+		$this->config->method('getSystemValue')
+			->with('secret', '')
+			->willReturn('SECRET');
+
+		$this->crypto->method('encrypt')
+			->with(
+				$this->equalTo('12348:ThisIsMaybeANotSoSecretToken!'),
+				$this->equalTo('test@example.comSECRET')
+			)->willReturn('encryptedToken');
+
+		$response = $this->lostController->email('test@example.com');
+		$expectedResponse = array('status' => 'success');
+		$this->assertSame($expectedResponse, $response);
+	}
+
+	public function testEmailCantSendException() {
+		$this->secureRandom
+			->expects($this->once())
+			->method('generate')
+			->with('21')
+			->will($this->returnValue('ThisIsMaybeANotSoSecretToken!'));
 		$this->userManager
 				->expects($this->any())
 				->method('get')