From c92d7429d7ff9a76a6dd62607e55ef680a43a679 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Sat, 21 Jul 2018 13:05:13 +0200 Subject: [PATCH] Implements handling for deactivated users Signed-off-by: Michael Weimann --- core/Controller/LoginController.php | 109 +++++++++++++++++++++------- core/templates/login.php | 12 ++- 2 files changed, 93 insertions(+), 28 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 7bf2555819..5bd06ac7e6 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -58,6 +58,10 @@ use OC\Hooks\PublicEmitter; use OCP\Util; class LoginController extends Controller { + + const LOGIN_MSG_INVALIDPASSWORD = 'invalidpassword'; + const LOGIN_MSG_USERDISABLED = 'userdisabled'; + /** @var IUserManager */ private $userManager; /** @var IConfig */ @@ -171,19 +175,7 @@ class LoginController extends Controller { $parameters['redirect_url'] = $redirect_url; } - $parameters['canResetPassword'] = true; - $parameters['resetPasswordLink'] = $this->config->getSystemValue('lost_password_link', ''); - if (!$parameters['resetPasswordLink']) { - if ($user !== null && $user !== '') { - $userObj = $this->userManager->get($user); - if ($userObj instanceof IUser) { - $parameters['canResetPassword'] = $userObj->canChangePassword(); - } - } - } elseif ($parameters['resetPasswordLink'] === 'disabled') { - $parameters['canResetPassword'] = false; - } - + $parameters = $this->setPasswordResetParameters($user, $parameters); $parameters['alt_login'] = OC_App::getAlternativeLogIns(); if ($user !== null && $user !== '') { @@ -209,6 +201,40 @@ class LoginController extends Controller { ); } + /** + * Sets the password reset params. + * + * Users may not change their passwords if: + * - The account is disabled + * - The backend doesn't support password resets + * - The password reset function is disabled + * + * @param string $user + * @param array $parameters + * @return array + */ + private function setPasswordResetParameters( + string $user = null, array $parameters): array { + if ($user !== null && $user !== '') { + $userObj = $this->userManager->get($user); + } else { + $userObj = null; + } + + $parameters['resetPasswordLink'] = $this->config + ->getSystemValue('lost_password_link', ''); + + if (!$parameters['resetPasswordLink'] && $userObj !== null) { + $parameters['canResetPassword'] = $userObj->canChangePassword(); + } else if ($userObj !== null && $userObj->isEnabled() === false) { + $parameters['canResetPassword'] = false; + } else { + $parameters['canResetPassword'] = true; + } + + return $parameters; + } + /** * @param string $redirectUrl * @return RedirectResponse @@ -256,6 +282,17 @@ class LoginController extends Controller { } $originalUser = $user; + + $userObj = $this->userManager->get($user); + + if ($userObj !== null && $userObj->isEnabled() === false) { + $this->logger->warning('Login failed: \''. $user . '\' disabled' . + ' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', + ['app' => 'core']); + return $this->createLoginFailedResponse($user, $originalUser, + $redirect_url, self::LOGIN_MSG_USERDISABLED); + } + // TODO: Add all the insane error handling /* @var $loginResult IUser */ $loginResult = $this->userManager->checkPasswordNoLogging($user, $password); @@ -270,20 +307,15 @@ class LoginController extends Controller { } } } + if ($loginResult === false) { - $this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']); - // Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name - $args = !is_null($user) ? ['user' => $originalUser] : []; - if (!is_null($redirect_url)) { - $args['redirect_url'] = $redirect_url; - } - $response = new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args)); - $response->throttle(['user' => $user]); - $this->session->set('loginMessages', [ - ['invalidpassword'], [] - ]); - return $response; + $this->logger->warning('Login failed: \''. $user . + '\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', + ['app' => 'core']); + return $this->createLoginFailedResponse($user, $originalUser, + $redirect_url, self::LOGIN_MSG_INVALIDPASSWORD); } + // TODO: remove password checks from above and let the user session handle failures // requires https://github.com/owncloud/core/pull/24616 $this->userSession->completeLogin($loginResult, ['loginName' => $user, 'password' => $password]); @@ -330,6 +362,33 @@ class LoginController extends Controller { return $this->generateRedirect($redirect_url); } + /** + * Creates a login failed response. + * + * @param string $user + * @param string $originalUser + * @param string $redirect_url + * @param string $loginMessage + * @return RedirectResponse + */ + private function createLoginFailedResponse( + $user, $originalUser, $redirect_url, string $loginMessage) { + // Read current user and append if possible we need to + // return the unmodified user otherwise we will leak the login name + $args = !is_null($user) ? ['user' => $originalUser] : []; + if (!is_null($redirect_url)) { + $args['redirect_url'] = $redirect_url; + } + $response = new RedirectResponse( + $this->urlGenerator->linkToRoute('core.login.showLoginForm', $args) + ); + $response->throttle(['user' => $user]); + $this->session->set('loginMessages', [ + [$loginMessage], [] + ]); + return $response; + } + /** * @NoAdminRequired * @UseSession diff --git a/core/templates/login.php b/core/templates/login.php index 8fc68edb92..989ea1eaad 100644 --- a/core/templates/login.php +++ b/core/templates/login.php @@ -2,6 +2,8 @@ @@ -34,7 +36,7 @@ script('core', 'merged-login');
-

+

t('Username or email')); ?>

-

+

- +

t('Wrong password.')); ?>

+ +

+ getL10N('lib')->t('User disabled')); ?> +

5000) { ?>