From a98fb01af7149d84a12e1d7ff2fdb1a1ef8d707a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Mon, 22 Jul 2019 17:03:39 +0200
Subject: [PATCH] :ambulance: fix(login) Fix ppolicy force password change
check
userinfo::loginUser now throws exceptions on failure, webservice code
will need to be adapted to take them into account as well
issue #6001
---
include/class_exceptions.inc | 8 ++++++++
include/class_userinfo.inc | 23 ++++++++++++-----------
include/login/class_LoginMethod.inc | 27 ++++++++++++---------------
3 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/include/class_exceptions.inc b/include/class_exceptions.inc
index 1dc822e6c..ecaa716f1 100644
--- a/include/class_exceptions.inc
+++ b/include/class_exceptions.inc
@@ -78,3 +78,11 @@ class UnknownClassException extends FusionDirectoryException
class LDAPFailureException extends FusionDirectoryException
{
}
+
+class LoginFailureException extends FusionDirectoryException
+{
+}
+
+class LoginFailurePpolicyException extends LoginFailureException
+{
+}
diff --git a/include/class_userinfo.inc b/include/class_userinfo.inc
index 101473a8d..619954477 100644
--- a/include/class_userinfo.inc
+++ b/include/class_userinfo.inc
@@ -30,7 +30,6 @@ define('POSIX_ACCOUNT_EXPIRED', 1);
define('POSIX_WARN_ABOUT_EXPIRATION', 2);
define('POSIX_FORCE_PASSWORD_CHANGE', 4);
define('POSIX_DISALLOW_PASSWORD_CHANGE', 8);
-define('PPOLICY_ACCOUNT_EXPIRED', 16);
/*!
* \brief Class userinfo
@@ -912,12 +911,6 @@ class userinfo
$ldap = $config->get_ldap_link();
if (class_available('ppolicyAccount')) {
- $ldap->cd($config->current['BASE']);
- $ldap->search('(objectClass=*)', [], 'one');
- if (!$ldap->success()) {
- return PPOLICY_ACCOUNT_EXPIRED;
- }
-
try {
list($policy, $attrs) = user::fetchPpolicy($this->dn);
if (
@@ -1209,17 +1202,17 @@ class userinfo
*
* \return TRUE on SUCCESS, NULL or FALSE on error
*/
- public static function loginUser (string $username, string $password)
+ public static function loginUser (string $username, string $password): userinfo
{
global $config;
$ui = static::getLdapUser($username);
if ($ui === FALSE) {
- return NULL;
+ throw new LoginFailureException('User not found');
} elseif (is_string($ui)) {
msg_dialog::display(_('Internal error'), $ui, FATAL_ERROR_DIALOG);
- return NULL;
+ exit();
}
/* password check, bind as user with supplied password */
@@ -1229,7 +1222,15 @@ class userinfo
);
$ldap = new ldapMultiplexer($ldapObj);
if (!$ldap->success()) {
- return NULL;
+ throw new LoginFailureException($ldap->get_error());
+ }
+
+ if (class_available('ppolicyAccount')) {
+ $ldap->cd($config->current['BASE']);
+ $ldap->search('(objectClass=*)', [], 'one');
+ if (!$ldap->success()) {
+ throw new LoginFailurePpolicyException(_('It seems your user password has expired. Please use <a href="recovery.php">password recovery</a> to change it.'));
+ }
}
/* Username is set, load subtreeACL's now */
diff --git a/include/login/class_LoginMethod.inc b/include/login/class_LoginMethod.inc
index 18a86edec..92797de64 100644
--- a/include/login/class_LoginMethod.inc
+++ b/include/login/class_LoginMethod.inc
@@ -95,13 +95,21 @@ class LoginMethod
{
global $ui, $config, $message, $smarty;
/* Login as user, initialize user ACL's */
- $ui = userinfo::loginUser(static::$username, static::$password);
- if ($ui === NULL) {
+ try {
+ $ui = userinfo::loginUser(static::$username, static::$password);
+ } catch (LoginFailurePpolicyException $e) {
+ msg_dialog::display(_('Authentication error'), $e->getMessage(), ERROR_DIALOG);
+ logging::log('security', 'login', '', [], 'Account for user "'.static::$username.'" has expired (ppolicy)');
+ $message = _('Password expired');
+ $smarty->assign('focusfield', 'username');
+ return FALSE;
+ } catch (LoginFailureException $e) {
if (isset($_SERVER['REMOTE_ADDR'])) {
- logging::log('security', 'login', '', [], 'Authentication failed for user "'.static::$username.'" [from '.$_SERVER['REMOTE_ADDR'].']');
+ logging::log('security', 'login', '', [], 'Authentication failed for user "'.static::$username.'" [from '.$_SERVER['REMOTE_ADDR'].']: '.$e->getMessage());
} else {
- logging::log('security', 'login', '', [], 'Authentication failed for user "'.static::$username.'"');
+ logging::log('security', 'login', '', [], 'Authentication failed for user "'.static::$username.'": '.$e->getMessage());
}
+ /* Show the same message whether the user exists or not to avoid information leak */
$message = _('Please check the username/password combination.');
$smarty->assign('focusfield', 'password');
return FALSE;
@@ -133,17 +141,6 @@ class LoginMethod
/* Check account expiration */
$expired = $ui->expired_status();
- if ($expired == PPOLICY_ACCOUNT_EXPIRED) {
- msg_dialog::display(
- _('Authentication error'),
- _('It seems your user password has expired. Please use <a href="recovery.php">password recovery</a> to change it.'),
- ERROR_DIALOG
- );
- logging::log('security', 'login', '', [], 'Account for user "'.static::$username.'" has expired (ppolicy)');
- $message = _('Password expired');
- $smarty->assign('focusfield', 'username');
- return FALSE;
- }
if ($expired == POSIX_ACCOUNT_EXPIRED) {
logging::log('security', 'login', '', [], 'Account for user "'.static::$username.'" has expired');
--
GitLab