From 05eddb4445e5b7c5f4bb9be39ed8c3918c831706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be> Date: Mon, 17 Sep 2018 16:10:29 +0200 Subject: [PATCH] :sparkles: feat(users) Make sure accounts with empty password are still lockable issue #3710 --- .../class_password-methods-crypt.inc | 2 +- .../class_password-methods-empty.inc | 13 ++- .../class_password-methods.inc | 91 ++++++++++--------- plugins/admin/users/class_userManagement.inc | 6 +- .../generic/class_UserPasswordAttribute.inc | 2 +- 5 files changed, 65 insertions(+), 49 deletions(-) diff --git a/include/password-methods/class_password-methods-crypt.inc b/include/password-methods/class_password-methods-crypt.inc index 0e44832dd..5b98a605c 100644 --- a/include/password-methods/class_password-methods-crypt.inc +++ b/include/password-methods/class_password-methods-crypt.inc @@ -151,7 +151,7 @@ class passwordMethodCrypt extends passwordMethod * * \param string $password_hash */ - static function _extract_method($classname, $password_hash) + static function _extract_method($password_hash) { if (!preg_match('/^{crypt}/i', $password_hash)) { return ""; diff --git a/include/password-methods/class_password-methods-empty.inc b/include/password-methods/class_password-methods-empty.inc index 14f43e4e0..38361352c 100644 --- a/include/password-methods/class_password-methods-empty.inc +++ b/include/password-methods/class_password-methods-empty.inc @@ -30,10 +30,12 @@ */ class passwordMethodEmpty extends passwordMethod { - protected $lockable = FALSE; + protected $lockable = TRUE; public $hash = 'empty'; + const LOCKVALUE = '{CRYPT}!'; + /*! * \brief passwordMethodClear Constructor */ @@ -78,5 +80,14 @@ class passwordMethodEmpty extends passwordMethod { return FALSE; } + + static function _extract_method($password_hash) + { + if (empty($password_hash) || ($password_hash == static::LOCKVALUE)) { + return static::get_hash_name(); + } + + return ''; + } } ?> diff --git a/include/password-methods/class_password-methods.inc b/include/password-methods/class_password-methods.inc index 0582f523f..342e717b7 100644 --- a/include/password-methods/class_password-methods.inc +++ b/include/password-methods/class_password-methods.inc @@ -147,56 +147,60 @@ class passwordMethod } } - /* We can only lock/unlock non-empty passwords */ - if (!empty($pwd)) { - /* Check if this entry is already locked. */ - if (!preg_match("/^[^\}]*+\}!/", $pwd)) { - if ($mode == 'UNLOCK') { - return TRUE; - } - } elseif ($mode == 'LOCK') { + /* Check if this entry is already locked. */ + if (!preg_match("/^[^\}]*+\}!/", $pwd)) { + if ($mode == 'UNLOCK') { return TRUE; } + } elseif ($mode == 'LOCK') { + return TRUE; + } - // (Un)lock the samba account - $modify = lock_samba_account($mode, $attrs); + // (Un)lock the samba account + $modify = lock_samba_account($mode, $attrs); - // (Un)lock SSH keys - lock_ssh_account($mode, $attrs, $modify); + // (Un)lock SSH keys + lock_ssh_account($mode, $attrs, $modify); - // Call pre hooks - $userClass = new user($dn); - $errors = $userClass->callHook('PRE'.$mode, array(), $ret); - if (!empty($errors)) { - msg_dialog::displayChecks($errors); - return FALSE; - } + // Call pre hooks + $userClass = new user($dn); + $errors = $userClass->callHook('PRE'.$mode, array(), $ret); + if (!empty($errors)) { + msg_dialog::displayChecks($errors); + return FALSE; + } - // (Un)lock the account by modifying the password hash. - if ($mode == 'LOCK') { - /* Lock entry */ + // (Un)lock the account by modifying the password hash. + if ($mode == 'LOCK') { + /* Lock entry */ + if (empty($pwd)) { + $pwd = passwordMethodEmpty::LOCKVALUE; + } else { $pwd = preg_replace("/(^[^\}]+\})(.*$)/", "\\1!\\2", $pwd); + } + } else { + /* Unlock entry */ + if ($pwd == passwordMethodEmpty::LOCKVALUE) { + $pwd = array(); } else { - /* Unlock entry */ $pwd = preg_replace("/(^[^\}]+\})!(.*$)/", "\\1\\2", $pwd); } - $modify['userPassword'] = $pwd; - $ldap->cd($dn); - $ldap->modify($modify); - - // Call the password post-lock hook, if defined. - if ($ldap->success()) { - $userClass = new user($dn); - $errors = $userClass->callHook('POST'.$mode, array(), $ret); - if (!empty($errors)) { - msg_dialog::displayChecks($errors); - } - } else { - msg_dialog::display(_('LDAP error'), msgPool::ldaperror($ldap->get_error(), $dn, LDAP_MOD), LDAP_ERROR); + } + $modify['userPassword'] = $pwd; + $ldap->cd($dn); + $ldap->modify($modify); + + // Call the password post-lock hook, if defined. + if ($ldap->success()) { + $userClass = new user($dn); + $errors = $userClass->callHook('POST'.$mode, array(), $ret); + if (!empty($errors)) { + msg_dialog::displayChecks($errors); } - return $ldap->success(); + } else { + msg_dialog::display(_('LDAP error'), msgPool::ldaperror($ldap->get_error(), $dn, LDAP_MOD), LDAP_ERROR); } - return FALSE; + return $ldap->success(); } @@ -299,14 +303,15 @@ class passwordMethod { $methods = passwordMethod::get_available_methods(); - if (empty($password_hash) && passwordMethodEmpty::is_available()) { + if (isset($methods['class']['passwordMethodEmpty']) && (passwordMethodEmpty::_extract_method($password_hash) != '')) { + /* Test empty method first as it gets priority */ $method = new passwordMethodEmpty($dn); return $method; } foreach ($methods['class'] as $class) { - $method = $class::_extract_method($class, $password_hash); - if ($method != "") { + $method = $class::_extract_method($password_hash); + if ($method != '') { $test = new $class($dn); $test->set_hash($method); return $test; @@ -324,9 +329,9 @@ class passwordMethod * * \param string $password_hash */ - static function _extract_method($classname, $password_hash) + static function _extract_method($password_hash) { - $hash = $classname::get_hash_name(); + $hash = static::get_hash_name(); if (preg_match("/^\{$hash\}/i", $password_hash)) { return $hash; } diff --git a/plugins/admin/users/class_userManagement.inc b/plugins/admin/users/class_userManagement.inc index 2ad4f6b44..02ad2d1ee 100644 --- a/plugins/admin/users/class_userManagement.inc +++ b/plugins/admin/users/class_userManagement.inc @@ -37,6 +37,9 @@ class LockAction extends Action { if (isset($entry['userPassword']) && preg_match('/^\{[^\}]/', $entry['userPassword'])) { return (preg_match('/^[^\}]*+\}!/', $entry['userPassword']) === 1); + } elseif ((strtolower($entry->type) == 'user') && !isset($entry['userPassword'])) { + /* Empty lockable password */ + return FALSE; } return NULL; } @@ -170,9 +173,6 @@ class userManagement extends management foreach ($allowed as $dn) { // We can't lock empty passwords. $entry = $this->listing->getEntry($dn); - if (!isset($entry['userPassword'])) { - continue; - } // Detect the password method and try to lock/unlock. $pwd = $entry['userPassword']; diff --git a/plugins/personal/generic/class_UserPasswordAttribute.inc b/plugins/personal/generic/class_UserPasswordAttribute.inc index a89e924fd..5b6e3ce80 100644 --- a/plugins/personal/generic/class_UserPasswordAttribute.inc +++ b/plugins/personal/generic/class_UserPasswordAttribute.inc @@ -198,7 +198,7 @@ class UserPasswordAttribute extends CompositeAttribute if ($this->plugin->is_template && ($values[0] == '%askme%')) { return '%askme%'; } - if (!$this->plugin->is_template && $this->needPassword[$values[0]] && ($values[1] == '')) { + if (!$this->plugin->is_template && ($this->needPassword[$values[0]] || ($values[0] == 'empty')) && ($values[1] == '')) { return $values[3]; } $temp = passwordMethod::get_available_methods(); -- GitLab