Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not rely on exop_passwd #536

Merged
merged 1 commit into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions lib/LDAPConnect.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
come-nc marked this conversation as resolved.
Show resolved Hide resolved
$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;
}
}
117 changes: 80 additions & 37 deletions lib/LDAPUserManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand All @@ -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;
}
Expand Down Expand Up @@ -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
*
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
}
}
8 changes: 8 additions & 0 deletions lib/Service/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions lib/Settings/Admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
]
);

Expand Down
9 changes: 9 additions & 0 deletions src/components/AdminSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@
@change.stop.prevent="toggleSwitch('hasAvatarPermission', !switches.hasAvatarPermission)">
{{ t('ldap_write_support', 'Allow users to set their avatar') }}
</NcActionCheckbox>
<NcActionCheckbox :checked="switches.hasPasswordPermission"
@change.stop.prevent="toggleSwitch('hasPasswordPermission', !switches.hasPasswordPermission)">
{{ t('ldap_write_support', 'Allow users to set their password') }}
</NcActionCheckbox>
<NcActionCheckbox :checked="switches.useUnicodePassword"
:title="t('ldap_write_support', 'If the server does not support the modify password extended operation use the `unicodePwd` instead of the `userPassword` attribute for setting the password')"
@change.stop.prevent="toggleSwitch('useUnicodePassword', !switches.useUnicodePassword)">
{{ t('ldap_write_support', 'Use the `unicodePwd` attribute for setting the user password') }}
</NcActionCheckbox>
</ul>
<h3>{{ t('ldap_write_support', 'User template') }}</h3>
<p>{{ t('ldap_write_support', 'LDIF template for creating users. Following placeholders may be used') }}</p>
Expand Down
Loading