diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index afd547ab2103..4df09ec1564d 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -3,6 +3,7 @@ * @author Christoph Wurst * @author Joas Schilling * @author Lukas Reschke + * @author Semih Serhat Karakaya * @author Thomas Müller * * @copyright Copyright (c) 2017, ownCloud GmbH @@ -197,17 +198,16 @@ public function showLoginForm($user, $redirect_url, $remember_login) { public function tryLogin($user, $password, $redirect_url) { $originalUser = $user; // TODO: Add all the insane error handling - /* @var $loginResult IUser */ - $loginResult = $this->userManager->checkPassword($user, $password); - if ($loginResult === false) { + $loginResult = $this->userSession->login($user, $password); + if ($loginResult !== true) { $users = $this->userManager->getByEmail($user); // we only allow login by email if unique if (count($users) === 1) { $user = $users[0]->getUID(); - $loginResult = $this->userManager->checkPassword($user, $password); + $loginResult = $this->userSession->login($user, $password); } } - if ($loginResult === false) { + if ($loginResult !== true) { $this->session->set('loginMessages', [ ['invalidpassword'], [] ]); @@ -222,16 +222,17 @@ public function tryLogin($user, $password, $redirect_url) { } return new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args)); } + /* @var $userObject IUser */ + $userObject = $this->userSession->getUser(); // TODO: remove password checks from above and let the user session handle failures // requires https://github.com/owncloud/core/pull/24616 - $this->userSession->login($user, $password); - $this->userSession->createSessionToken($this->request, $loginResult->getUID(), $user, $password); + $this->userSession->createSessionToken($this->request, $userObject->getUID(), $user, $password); // User has successfully logged in, now remove the password reset link, when it is available - $this->config->deleteUserValue($loginResult->getUID(), 'owncloud', 'lostpassword'); + $this->config->deleteUserValue($userObject->getUID(), 'owncloud', 'lostpassword'); - if ($this->twoFactorManager->isTwoFactorAuthenticated($loginResult)) { - $this->twoFactorManager->prepareTwoFactorLogin($loginResult); + if ($this->twoFactorManager->isTwoFactorAuthenticated($userObject)) { + $this->twoFactorManager->prepareTwoFactorLogin($userObject); if (!is_null($redirect_url)) { return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge', [ 'redirect_url' => $redirect_url diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 2bbc29d33e9c..be3a960daac6 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -9,6 +9,7 @@ * @author Morris Jobke * @author Robin Appelman * @author Robin McCorkell + * @author Semih Serhat Karakaya * @author Thomas Müller * @author Vincent Petry * @@ -70,6 +71,7 @@ * - postCreateUser(\OC\User\User $user) * - preLogin(string $user, string $password) * - postLogin(\OC\User\User $user, string $password) + * - failedLogin(string $user) * - preRememberedLogin(string $uid) * - postRememberedLogin(\OC\User\User $user) * - logout() @@ -464,7 +466,7 @@ private function loginWithPassword($uid, $password) { $this->manager->emit('\OC\User', 'preLogin', [$uid, $password]); $user = $this->manager->checkPassword($uid, $password); if ($user === false) { - // Password check failed + $this->manager->emit('\OC\User', 'failedLogin', [$uid]); return false; } diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 442b339990d1..74ea7c4ae37b 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -1,6 +1,7 @@ + * @author Semih Serhat Karakaya * * @copyright Copyright (c) 2016, ownCloud, Inc. * @license AGPL-3.0 @@ -307,8 +308,8 @@ public function testLoginWithInvalidCredentials() { $password = 'secret'; $loginPageUrl = 'some url'; - $this->userManager->expects($this->once()) - ->method('checkPassword') + $this->userSession->expects($this->once()) + ->method('login') ->will($this->returnValue(false)); $this->urlGenerator->expects($this->once()) ->method('linkToRoute') @@ -328,12 +329,13 @@ public function testLoginWithValidCredentials() { $password = 'secret'; $indexPageUrl = 'some url'; - $this->userManager->expects($this->once()) - ->method('checkPassword') - ->will($this->returnValue($user)); $this->userSession->expects($this->once()) ->method('login') - ->with($user, $password); + ->with($user, $password) + ->will($this->returnValue(true)); + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); $this->userSession->expects($this->once()) ->method('createSessionToken') ->with($this->request, $user->getUID(), $user, $password); @@ -374,9 +376,12 @@ public function testLoginWithValidCredentialsAndRedirectUrl() { $originalUrl = 'another%20url'; $redirectUrl = 'http://localhost/another url'; - $this->userManager->expects($this->once()) - ->method('checkPassword') + $this->userSession->expects($this->once()) + ->method('login') ->with('Jane', $password) + ->will($this->returnValue(true)); + $this->userSession->expects($this->once()) + ->method('getUser') ->will($this->returnValue($user)); $this->userSession->expects($this->once()) ->method('createSessionToken') @@ -403,8 +408,11 @@ public function testLoginWithTwoFactorEnforced() { $password = 'secret'; $challengeUrl = 'challenge/url'; - $this->userManager->expects($this->once()) - ->method('checkPassword') + $this->userSession->expects($this->once()) + ->method('login') + ->will($this->returnValue(true)); + $this->userSession->expects($this->once()) + ->method('getUser') ->will($this->returnValue($user)); $this->userSession->expects($this->once()) ->method('login') @@ -435,12 +443,12 @@ public function testToNotLeakLoginName() { ->method('getUID') ->will($this->returnValue('john')); - $this->userManager->expects($this->exactly(2)) - ->method('checkPassword') + $this->userSession->expects($this->exactly(2)) + ->method('login') ->withConsecutive( ['john@doe.com', 'just wrong'], ['john', 'just wrong'] - ) + ) ->willReturn(false); $this->userManager->expects($this->once())