From 031fdfb1fc3b99c7a7dd93ee20fe000e9cf7fda6 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Sat, 18 Aug 2018 16:51:59 +0200 Subject: [PATCH] Enable password reset for user with same email address when only one is active When two or more user share the same email address its not possible to reset password by email. Even when only one account is active. This pr reduce list of users returned by getByEmail by disabled users. Signed-off-by: Daniel Kesselberg --- core/Controller/LostController.php | 25 +++--- tests/Core/Controller/LostControllerTest.php | 84 ++++++++++++++++++++ 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index eacd5847c6..d663559dc6 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -364,24 +364,27 @@ class LostController extends Controller { * @throws \InvalidArgumentException */ protected function findUserByIdOrMail($input) { + $userNotFound = new \InvalidArgumentException( + $this->l10n->t('Couldn\'t send reset email. Please make sure your username is correct.') + ); + $user = $this->userManager->get($input); if ($user instanceof IUser) { if (!$user->isEnabled()) { - throw new \InvalidArgumentException($this->l10n->t('Couldn\'t send reset email. Please make sure your username is correct.')); - } - - return $user; - } - $users = $this->userManager->getByEmail($input); - if (count($users) === 1) { - $user = $users[0]; - if (!$user->isEnabled()) { - throw new \InvalidArgumentException($this->l10n->t('Couldn\'t send reset email. Please make sure your username is correct.')); + throw $userNotFound; } return $user; } - throw new \InvalidArgumentException($this->l10n->t('Couldn\'t send reset email. Please make sure your username is correct.')); + $users = \array_filter($this->userManager->getByEmail($input), function (IUser $user) { + return $user->isEnabled(); + }); + + if (\count($users) === 1) { + return $users[0]; + } + + throw $userNotFound; } } diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php index d6afa5959a..682229111e 100644 --- a/tests/Core/Controller/LostControllerTest.php +++ b/tests/Core/Controller/LostControllerTest.php @@ -759,4 +759,88 @@ class LostControllerTest extends \Test\TestCase { $this->assertSame($expectedResponse, $response); } + public function testTwoUsersWithSameEmail() { + $user1 = $this->createMock(IUser::class); + $user1->expects($this->any()) + ->method('getEMailAddress') + ->willReturn('test@example.com'); + $user1->expects($this->any()) + ->method('getUID') + ->willReturn('User1'); + $user1->expects($this->any()) + ->method('isEnabled') + ->willReturn(true); + + $user2 = $this->createMock(IUser::class); + $user2->expects($this->any()) + ->method('getEMailAddress') + ->willReturn('test@example.com'); + $user2->expects($this->any()) + ->method('getUID') + ->willReturn('User2'); + $user2->expects($this->any()) + ->method('isEnabled') + ->willReturn(true); + + $this->userManager + ->method('get') + ->willReturn(null); + + $this->userManager + ->method('getByEmail') + ->willReturn([$user1, $user2]); + + // request password reset for test@example.com + $response = $this->lostController->email('test@example.com'); + + $expectedResponse = new JSONResponse([ + 'status' => 'error', + 'msg' => 'Couldn\'t send reset email. Please make sure your username is correct.' + ]); + $expectedResponse->throttle(); + + $this->assertEquals($expectedResponse, $response); + } + + public function testTwoUsersWithSameEmailOneDisabled() { + $user1 = $this->createMock(IUser::class); + $user1->expects($this->any()) + ->method('getEMailAddress') + ->willReturn('test@example.com'); + $user1->expects($this->any()) + ->method('getUID') + ->willReturn('User1'); + $user1->expects($this->any()) + ->method('isEnabled') + ->willReturn(true); + + $user2 = $this->createMock(IUser::class); + $user2->expects($this->any()) + ->method('getEMailAddress') + ->willReturn('test@example.com'); + $user2->expects($this->any()) + ->method('getUID') + ->willReturn('User2'); + $user2->expects($this->any()) + ->method('isEnabled') + ->willReturn(false); + + $this->userManager + ->method('get') + ->willReturn(null); + + $this->userManager + ->method('getByEmail') + ->willReturn([$user1, $user2]); + + // request password reset for test@example.com + $response = $this->lostController->email('test@example.com'); + + $expectedResponse = new JSONResponse([ + 'status' => 'success' + ]); + $expectedResponse->throttle(); + + $this->assertEquals($expectedResponse, $response); + } }