From 2c407b5634e0d886c78fd9036bfea0dbb50a587a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Tue, 23 Jul 2019 17:38:44 +0200
Subject: [PATCH] :sparkles: feat(login) Improve handling of ppolicy errors at
login
Handle ppolicy expired password, locked account and must change password
error codes.
In this first version after changing password you need to log out.
issue #6001
---
include/class_ldap.inc | 48 ++++++++++++++++++++++++++---
include/class_userinfo.inc | 19 +++++++++---
include/login/class_LoginMethod.inc | 8 +----
3 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/include/class_ldap.inc b/include/class_ldap.inc
index 07f07b0f1..df4418e12 100644
--- a/include/class_ldap.inc
+++ b/include/class_ldap.inc
@@ -121,6 +121,16 @@ class LDAP
return ldap_escape_f($dn);
}
+ /*!
+ * \brief Error text that must be returned for invalid user or password
+ *
+ * This is useful to make sure the same error text is shown whether a user exists or not, when the password is not correct.
+ */
+ static function invalidCredentialsError (): string
+ {
+ return _(ldap_err2str(49));
+ }
+
/*!
* \brief Create a connection to LDAP server
*
@@ -151,11 +161,41 @@ class LDAP
if (@ldap_parse_result($this->cid, $result, $errcode, $matcheddn, $errmsg, $referrals, $ctrls)) {
if (isset($ctrls[LDAP_CONTROL_PASSWORDPOLICYRESPONSE]['value']['error'])) {
$this->hascon = FALSE;
- $this->error = $ctrls[LDAP_CONTROL_PASSWORDPOLICYRESPONSE]['value']['error'];
+ switch ($ctrls[LDAP_CONTROL_PASSWORDPOLICYRESPONSE]['value']['error']) {
+ case 0:
+ /* passwordExpired - password has expired and must be reset */
+ $this->error = _('It seems your user password has expired. Please use <a href="recovery.php">password recovery</a> to change it.');
+ break;
+ case 1:
+ /* accountLocked */
+ $this->error = _('Account locked. Please contact your system administrator!');
+ break;
+ case 2:
+ /* changeAfterReset - password must be changed before the user will be allowed to perform any other operation */
+ $this->error = 'changeAfterReset';
+ break;
+ case 3:
+ /* passwordModNotAllowed */
+ case 4:
+ /* mustSupplyOldPassword */
+ case 5:
+ /* insufficientPasswordQuality */
+ case 6:
+ /* passwordTooShort */
+ case 7:
+ /* passwordTooYoung */
+ case 8:
+ /* passwordInHistory */
+ default:
+ $this->error = sprintf(_('Unexpected ppolicy error "%s", please contact the administrator'), $ctrls[LDAP_CONTROL_PASSWORDPOLICYRESPONSE]['value']['error']);
+ break;
+ }
// Note: Also available: expire, grace
} else {
$this->hascon = ($errcode == 0);
- if (empty($errmsg)) {
+ if ($errcode == 49) {
+ $this->error = static::invalidCredentialsError();
+ } elseif (empty($errmsg)) {
$this->error = ldap_err2str($errcode);
} else {
$this->error = $errmsg;
@@ -171,10 +211,10 @@ class LDAP
} else {
if ($this->reconnect) {
if ($this->error != 'Success') {
- $this->error = 'Could not rebind to ' . $this->binddn;
+ $this->error = static::invalidCredentialsError();
}
} else {
- $this->error = 'Could not bind to ' . $this->binddn;
+ $this->error = static::invalidCredentialsError();
}
}
} else {
diff --git a/include/class_userinfo.inc b/include/class_userinfo.inc
index 619954477..61a781246 100644
--- a/include/class_userinfo.inc
+++ b/include/class_userinfo.inc
@@ -62,6 +62,9 @@ class userinfo
/*! \brief Current management base */
protected $currentBase;
+ /*! \brief Password change should be forced */
+ protected $forcePasswordChange = FALSE;
+
/* get acl's an put them into the userinfo object
attr subtreeACL (userdn:components, userdn:component1#sub1#sub2,component2,...) */
function __construct ($userdn)
@@ -903,6 +906,10 @@ class userinfo
{
global $config;
+ if ($this->forcePasswordChange) {
+ return POSIX_FORCE_PASSWORD_CHANGE;
+ }
+
// Skip this for the admin account, we do not want to lock him out.
if ($this->is_user_admin()) {
return 0;
@@ -1209,7 +1216,7 @@ class userinfo
$ui = static::getLdapUser($username);
if ($ui === FALSE) {
- throw new LoginFailureException('User not found');
+ throw new LoginFailureException(ldap::invalidCredentialsError());
} elseif (is_string($ui)) {
msg_dialog::display(_('Internal error'), $ui, FATAL_ERROR_DIALOG);
exit();
@@ -1222,14 +1229,18 @@ class userinfo
);
$ldap = new ldapMultiplexer($ldapObj);
if (!$ldap->success()) {
- throw new LoginFailureException($ldap->get_error());
+ if ($ldap->get_error() == 'changeAfterReset') {
+ $ui->forcePasswordChange = TRUE;
+ } else {
+ throw new LoginFailureException($ldap->get_error());
+ }
}
- if (class_available('ppolicyAccount')) {
+ if (class_available('ppolicyAccount') && !function_exists('ldap_bind_ext')) {
$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.'));
+ $ui->forcePasswordChange = TRUE;
}
}
diff --git a/include/login/class_LoginMethod.inc b/include/login/class_LoginMethod.inc
index 92797de64..5f5e8f68a 100644
--- a/include/login/class_LoginMethod.inc
+++ b/include/login/class_LoginMethod.inc
@@ -97,12 +97,6 @@ class LoginMethod
/* Login as user, initialize user ACL's */
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'].']: '.$e->getMessage());
@@ -110,7 +104,7 @@ class LoginMethod
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.');
+ $message = $e->getMessage();
$smarty->assign('focusfield', 'password');
return FALSE;
}
--
GitLab