From fc56e13fa936284d9f89f43e8b1c0bc9e7a7f71a Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 17 Oct 2022 10:29:42 +0200 Subject: [PATCH] If the passwd exop is not supported allow setting unicodePwd for AD * Do not rely on exop_passwd but check rootDSE for support and fallback to mod_replace Signed-off-by: Ferdinand Thiessen --- lib/LDAPConnect.php | 38 ++++++++++ lib/LDAPUserManager.php | 117 +++++++++++++++++++++---------- lib/Service/Configuration.php | 8 +++ lib/Settings/Admin.php | 2 + src/components/AdminSettings.vue | 9 +++ 5 files changed, 137 insertions(+), 37 deletions(-) diff --git a/lib/LDAPConnect.php b/lib/LDAPConnect.php index fbe96817..02375a0b 100644 --- a/lib/LDAPConnect.php +++ b/lib/LDAPConnect.php @@ -33,8 +33,11 @@ class LDAPConnect { private $ldapConfig; /** @var LoggerInterface */ private $logger; + /** @var bool|null */ + private $passwdSupport; public function __construct(Helper $ldapBackendHelper, LoggerInterface $logger) { + $this->passwdSupport = null; $this->logger = $logger; $ldapConfigPrefixes = $ldapBackendHelper->getServerConfigurationPrefixes(true); $prefix = array_shift($ldapConfigPrefixes); @@ -143,4 +146,39 @@ public function hasPasswordPolicy(): bool { $ppDN = $this->ldapConfig->ldapDefaultPPolicyDN; return !empty($ppDN); } + + /** + * checks whether the LDAP server supports the passwd exop + * + * @param \LDAP\Connection $connection LDAP connection to check + * @return boolean either the user can or cannot + */ + public function hasPasswdExopSupport($connection): bool { + // TODO: We should cache this by ldap prefix, but currently we have no access to it. + if (is_null($this->passwdSupport)) { + $ret = ldap_read($connection, '', '(objectclass=*)', ['supportedExtension']); + if ($ret === false) { + $this->passwdSupport = false; + $this->logger->debug( + 'Could not check passwd_exop support of LDAP host, host does not provide the supportedExtension entry.', + [ 'app' => Application::APP_ID ] + ); + return false; + } + + $ret = ldap_first_entry($connection, $ret); + if ($ret === false) { + $this->passwdSupport = false; + $this->logger->error( + 'Could not check passwd_exop support of LDAP host, host returned malformed data for the supported ldap extension entry.', + [ 'app' => Application::APP_ID ] + ); + return false; + } + + $values = ldap_get_values($connection, $ret, 'supportedExtension'); + $this->passwdSupport = ($values !== false) && in_array(LDAP_EXOP_MODIFY_PASSWD, $values); + } + return $this->passwdSupport; + } } diff --git a/lib/LDAPUserManager.php b/lib/LDAPUserManager.php index 0c982a21..125d29c8 100644 --- a/lib/LDAPUserManager.php +++ b/lib/LDAPUserManager.php @@ -84,7 +84,7 @@ public function __construct(IUserManager $userManager, IUserSession $userSession * @return int bitwise-or'ed actions */ public function respondToActions() { - $setPassword = function_exists('ldap_exop_passwd') && !$this->ldapConnect->hasPasswordPolicy() + $setPassword = $this->canSetPassword() && !$this->ldapConnect->hasPasswordPolicy() ? Backend::SET_PASSWORD : 0; @@ -140,7 +140,7 @@ public function setDisplayName($uid, $displayName) { * checks whether the user is allowed to change his avatar in Nextcloud * * @param string $uid the Nextcloud user name - * @return boolean either the user can or cannot + * @return bool either the user can or cannot */ public function canChangeAvatar($uid) { return $this->configuration->hasAvatarPermission(); @@ -222,7 +222,12 @@ public function createUser($uid, $password) { $displayNameAttribute = $this->ldapConnect->getDisplayNameAttribute(); } + if ($connection === false) { + throw new \Exception('Could not bind to LDAP server'); + } + [$newUserDN, $newUserEntry] = $this->buildNewEntry($uid, $password, $base); + $newUserDN = $this->ldapProvider->sanitizeDN([$newUserDN])[0]; $this->ensureAttribute($newUserEntry, $displayNameAttribute, $uid); @@ -244,27 +249,8 @@ public function createUser($uid, $password) { throw new \Exception('Cannot create user: ' . ldap_error($connection), ldap_errno($connection)); } - // Set password through ldap password exop, if supported if ($this->respondToActions() & Backend::SET_PASSWORD) { - try { - $ret = ldap_exop_passwd($connection, $newUserDN, '', $password); - if ($ret === false) { - $message = 'ldap_exop_passwd failed, falling back to ldap_mod_replace to to set password for new user'; - $this->logger->debug($message, ['app' => Application::APP_ID]); - - // Fallback to `userPassword` in case the server does not support exop_passwd - $ret = ldap_mod_replace($connection, $newUserDN, ['userPassword' => $password]); - if ($ret === false) { - $message = 'Failed to set password for new user {dn}'; - $this->logger->error($message, [ - 'app' => Application::APP_ID, - 'dn' => $newUserDN, - ]); - } - } - } catch (\Exception $e) { - $this->logger->error($e->getMessage(), ['exception' => $e, 'app' => Application::APP_ID]); - } + $this->handleSetPassword($newUserDN, $password, $connection); } return $ret ? $newUserDN : false; } @@ -350,6 +336,15 @@ public function deleteUser($uid): bool { return $res; } + /** + * checks whether the user is allowed to change their password in Nextcloud + * + * @return bool either the user can or cannot + */ + public function canSetPassword(): bool { + return $this->configuration->hasPasswordPermission(); + } + /** * Set password * @@ -360,27 +355,17 @@ public function deleteUser($uid): bool { * Change the password of a user */ public function setPassword($uid, $password) { - if (!function_exists('ldap_exop_passwd')) { - // since PHP 7.2 – respondToActions checked this already, this - // method should not be called. Double check due to public scope. - // This method can be removed when Nextcloud 16 compat is dropped. - return false; - } - try { - $cr = $this->ldapProvider->getLDAPConnection($uid); - $userDN = $this->getUserDN($uid); - return ldap_exop_passwd($cr, $userDN, '', $password) !== false; - } catch (\Exception $e) { - $this->logger->error($e->getMessage(), ['exception' => $e, 'app' => Application::APP_ID]); - } - return false; + $connection = $this->ldapProvider->getLDAPConnection($uid); + $userDN = $this->getUserDN($uid); + + return $this->handleSetPassword($userDN, $password, $connection); } /** * get the user's home directory * * @param string $uid the username - * @return boolean + * @return bool */ public function getHome($uid) { // Not implemented @@ -444,4 +429,62 @@ public function changeUserHook(IUser $user, string $feature, $attr1, $attr2): vo private function getUserDN($uid): string { return $this->ldapProvider->getUserDN($uid); } + + /** + * Handle setting user password password + * + * @param string $userDN The username + * @param string $password The new password + * @param \LDAP\Connection $connection The LDAP connection to use + * @return bool + * + * Change the password of a user + */ + private function handleSetPassword(string $userDN, string $password, \LDAP\Connection $connection): bool { + try { + $ret = false; + + // try ldap_exop_passwd first + if ($this->ldapConnect->hasPasswdExopSupport($connection)) { + if (ldap_exop_passwd($connection, $userDN, '', $password) === true) { + // `ldap_exop_passwd` is either FALSE or the password, in the later case return TRUE + return true; + } + + $message = 'Failed to set password for user {dn} using ldap_exop_passwd'; + $this->logger->error($message, [ + 'ldap_error' => ldap_error($connection), + 'app' => Application::APP_ID, + 'dn' => $userDN, + ]); + } else { + // Use ldap_mod_replace in case the server does not support exop_passwd + $entry = []; + if ($this->configuration->useUnicodePassword()) { + $entry['unicodePwd'] = iconv('UTF-8', 'UTF-16LE', '"' . $password . '"'); + } else { + $entry['userPassword'] = $password; + } + + if(ldap_mod_replace($connection, $userDN, $entry)) { + return true; + } + + $message = 'Failed to set password for user {dn} using ldap_mod_replace'; + $this->logger->error($message, [ + 'ldap_error' => ldap_error($connection), + 'app' => Application::APP_ID, + 'dn' => $userDN, + ]); + } + return false; + } catch (\Exception $e) { + $this->logger->error('Exception occured while setting the password of user {dn}', [ + 'app' => Application::APP_ID, + 'exception' => $e, + 'dn' => $userDN, + ]); + return false; + } + } } diff --git a/lib/Service/Configuration.php b/lib/Service/Configuration.php index 2bd7b9d8..883c6526 100644 --- a/lib/Service/Configuration.php +++ b/lib/Service/Configuration.php @@ -48,6 +48,14 @@ public function hasAvatarPermission(): bool { return $this->config->getAppValue('ldap_write_support', 'hasAvatarPermission', '1') === '1'; } + public function hasPasswordPermission(): bool { + return $this->config->getAppValue('ldap_write_support', 'hasPasswordPermission', '1') === '1'; + } + + public function useUnicodePassword(): bool { + return $this->config->getAppValue('ldap_write_support', 'useUnicodePassword', '0') === '1'; + } + public function getUserTemplate() { return $this->config->getAppValue( Application::APP_ID, diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index b08972e1..aa20538c 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -61,8 +61,10 @@ public function getForm() { 'createRequireActorFromLdap' => $this->config->isLdapActorRequired(), 'createPreventFallback' => $this->config->isPreventFallback(), 'hasAvatarPermission' => $this->config->hasAvatarPermission(), + 'hasPasswordPermission' => $this->config->hasPasswordPermission(), 'newUserRequireEmail' => $this->config->isRequireEmail(), 'newUserGenerateUserID' => $this->config->isGenerateUserId(), + 'useUnicodePassword' => $this->config->useUnicodePassword(), ] ); diff --git a/src/components/AdminSettings.vue b/src/components/AdminSettings.vue index a38b9cd0..cb73b740 100644 --- a/src/components/AdminSettings.vue +++ b/src/components/AdminSettings.vue @@ -44,6 +44,15 @@ @change.stop.prevent="toggleSwitch('hasAvatarPermission', !switches.hasAvatarPermission)"> {{ t('ldap_write_support', 'Allow users to set their avatar') }} + + {{ t('ldap_write_support', 'Allow users to set their password') }} + + + {{ t('ldap_write_support', 'Use the `unicodePwd` attribute for setting the user password') }} +

{{ t('ldap_write_support', 'User template') }}

{{ t('ldap_write_support', 'LDIF template for creating users. Following placeholders may be used') }}