Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions core/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* @author Christoph Wurst <christoph@owncloud.com>
* @author Joas Schilling <coding@schilljs.com>
* @author Lukas Reschke <lukas@statuscode.ch>
* @author Semih Serhat Karakaya <karakayasemi@itu.edu.tr>
* @author Thomas Müller <thomas.mueller@tmit.eu>
*
* @copyright Copyright (c) 2017, ownCloud GmbH
Expand Down Expand Up @@ -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'], []
]);
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion lib/private/User/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* @author Morris Jobke <hey@morrisjobke.de>
* @author Robin Appelman <icewind@owncloud.com>
* @author Robin McCorkell <robin@mccorkell.me.uk>
* @author Semih Serhat Karakaya <karakayasemi@itu.edu.tr>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Vincent Petry <pvince81@owncloud.com>
*
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;
}

Expand Down
34 changes: 21 additions & 13 deletions tests/Core/Controller/LoginControllerTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
/**
* @author Lukas Reschke <lukas@owncloud.com>
* @author Semih Serhat Karakaya <karakayasemi@itu.edu.tr>
*
* @copyright Copyright (c) 2016, ownCloud, Inc.
* @license AGPL-3.0
Expand Down Expand Up @@ -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')
Expand All @@ -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);
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -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())
Expand Down