Skip to content

Commit 8fca3a7

Browse files
committed
Do not rely on exop_passwd but check rootDSE for support and fallback to mod_replace
This should fix AD support Signed-off-by: Ferdinand Thiessen <[email protected]>
1 parent d0e5420 commit 8fca3a7

File tree

5 files changed

+80
-32
lines changed

5 files changed

+80
-32
lines changed

lib/LDAPConnect.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,11 @@ class LDAPConnect {
3333
private $ldapConfig;
3434
/** @var LoggerInterface */
3535
private $logger;
36+
/** @var bool|null */
37+
private $passwdSupport;
3638

3739
public function __construct(Helper $ldapBackendHelper, LoggerInterface $logger) {
40+
$this->passwdSupport = null;
3841
$this->logger = $logger;
3942
$ldapConfigPrefixes = $ldapBackendHelper->getServerConfigurationPrefixes(true);
4043
$prefix = array_shift($ldapConfigPrefixes);
@@ -145,4 +148,29 @@ public function hasPasswordPolicy(): bool {
145148
$ppDN = $this->ldapConfig->ldapDefaultPPolicyDN;
146149
return !empty($ppDN);
147150
}
151+
152+
/**
153+
* checks whether the LDAP server supports the passwd exop
154+
*
155+
* @param LDAP\Connection $connection LDAP connection to check
156+
* @return boolean either the user can or cannot
157+
*/
158+
public function hasPasswdExopSupport():bool {
159+
if (is_null($this->passwdSupport)) {
160+
$ret = ldap_read($connection, '', '(objectclass=*)', ['supportedExtension']);
161+
if ($ret === false) {
162+
$this->passwdSupport = false;
163+
return false;
164+
}
165+
$ret = ldap_first_entry($connection, $ret);
166+
if ($ret === false) {
167+
$this->passwdSupport = false;
168+
return false;
169+
}
170+
171+
$values = ldap_get_values($connection, $ret, 'supportedExtension');
172+
$this->passwdSupport = ($values !== false) && in_array(LDAP_EXOP_MODIFY_PASSWD, $values['supportedExtension']);
173+
}
174+
return $this->passwdSupport;
175+
}
148176
}

lib/LDAPUserManager.php

Lines changed: 43 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public function __construct(IUserManager $userManager, IUserSession $userSession
8787
* compared with OC_USER_BACKEND_CREATE_USER etc.
8888
*/
8989
public function respondToActions() {
90-
$setPassword = function_exists('ldap_exop_passwd') && !$this->ldapConnect->hasPasswordPolicy()
90+
$setPassword = $this->canSetPassword() && !$this->ldapConnect->hasPasswordPolicy()
9191
? Backend::SET_PASSWORD
9292
: 0;
9393

@@ -247,29 +247,8 @@ public function createUser($username, $password) {
247247
throw new \Exception('Cannot create user: ' . ldap_error($connection), ldap_errno($connection));
248248
}
249249

250-
// Set password through ldap password exop, if supported
251250
if ($this->respondToActions() & Backend::SET_PASSWORD) {
252-
try {
253-
$ret = ldap_exop_passwd($connection, $newUserDN, '', $password);
254-
if ($ret === false) {
255-
$message = 'ldap_exop_passwd failed, falling back to ldap_mod_replace to to set password for new user';
256-
$this->logger->log(ILogger::DEBUG, $message, [
257-
'app' => Application::APP_ID,
258-
]);
259-
260-
// Fallback to `userPassword` in case the server does not support exop_passwd
261-
$ret = ldap_mod_replace($connection, $newUserDN, ['userPassword' => $password]);
262-
if ($ret === false) {
263-
$message = 'Failed to set password for new user {dn}';
264-
$this->logger->log(ILogger::ERROR, $message, [
265-
'app' => Application::APP_ID,
266-
'dn' => $newUserDN,
267-
]);
268-
}
269-
}
270-
} catch (\Exception $e) {
271-
$this->logger->logException($e, ['app' => Application::APP_ID]);
272-
}
251+
$this->setPassword($username, $password, $connection);
273252
}
274253
return $ret ? $newUserDN : false;
275254
}
@@ -355,30 +334,62 @@ public function deleteUser($uid): bool {
355334
return $res;
356335
}
357336

337+
/**
338+
* checks whether the user is allowed to change their password in Nextcloud
339+
*
340+
* @return boolean either the user can or cannot
341+
*/
342+
public function canSetPassword() {
343+
return $this->configuration->hasPasswordPermission();
344+
}
345+
358346
/**
359347
* Set password
360348
*
361349
* @param string $uid The username
362350
* @param string $password The new password
351+
* @param ?LDAP\Connection $connection LDAP connection or null to create one
363352
* @return bool
364353
*
365354
* Change the password of a user
366355
*/
367-
public function setPassword($uid, $password) {
368-
if (!function_exists('ldap_exop_passwd')) {
369-
// since PHP 7.2 – respondToActions checked this already, this
370-
// method should not be called. Double check due to public scope.
371-
// This method can be removed when Nextcloud 16 compat is dropped.
372-
return false;
356+
public function setPassword($uid, $password, $connection = null) {
357+
if (is_null($connection)) {
358+
$connection = $this->ldapProvider->getLDAPConnection($uid);
373359
}
360+
374361
try {
375-
$cr = $this->ldapProvider->getLDAPConnection($uid);
376362
$userDN = $this->getUserDN($uid);
377-
return ldap_exop_passwd($cr, $userDN, '', $password);
363+
$ret = false;
364+
365+
// try ldap_exop_passwd first
366+
if ($ldapConnect->hasPasswdExopSupport($connection)) {
367+
$ret = ldap_exop_passwd($connection, $userDN, '', $password);
368+
if ($ret === false) {
369+
$message = 'ldap_exop_passwd failed, falling back to ldap_mod_replace to to set password for new user';
370+
$this->logger->log(ILogger::ERROR, $message, [
371+
'app' => Application::APP_ID,
372+
'ldap_error' => ldap_error($connection),
373+
]);
374+
}
375+
}
376+
377+
// Fallback to `userPassword` in case the server does not support exop_passwd
378+
if ($ret === false) {
379+
$ret = ldap_mod_replace($connection, $userDN, ['userPassword' => $password]);
380+
if ($ret === false) {
381+
$message = 'Failed to set password for user {dn}';
382+
$this->logger->log(ILogger::ERROR, $message, [
383+
'app' => Application::APP_ID,
384+
'dn' => $userDN,
385+
]);
386+
}
387+
}
388+
return $ret;
378389
} catch (\Exception $e) {
379390
$this->logger->logException($e, ['app' => Application::APP_ID]);
391+
return false;
380392
}
381-
return false;
382393
}
383394

384395
/**

lib/Service/Configuration.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ public function hasAvatarPermission(): bool {
4848
return $this->config->getAppValue('ldap_write_support', 'hasAvatarPermission', '1') === '1';
4949
}
5050

51+
public function hasPasswordPermission(): bool {
52+
return $this->config->getAppValue('ldap_write_support', 'hasPasswordPermission', '1') === '1';
53+
}
54+
5155
public function getUserTemplate() {
5256
return $this->config->getAppValue(
5357
Application::APP_ID,

lib/Settings/Admin.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public function getForm() {
6161
'createRequireActorFromLdap' => $this->config->isLdapActorRequired(),
6262
'createPreventFallback' => $this->config->isPreventFallback(),
6363
'hasAvatarPermission' => $this->config->hasAvatarPermission(),
64+
'hasPasswordPermission' => $this->config->hasPasswordPermission(),
6465
'newUserRequireEmail' => $this->config->isRequireEmail(),
6566
'newUserGenerateUserID' => $this->config->isGenerateUserId(),
6667
]

src/components/AdminSettings.vue

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@
4444
@change.stop.prevent="toggleSwitch('hasAvatarPermission', !switches.hasAvatarPermission)">
4545
{{ t('ldap_write_support', 'Allow users to set their avatar') }}
4646
</ActionCheckbox>
47+
<ActionCheckbox :checked="switches.hasPasswordPermission"
48+
@change.stop.prevent="toggleSwitch('hasPasswordPermission', !switches.hasPasswordPermission)">
49+
{{ t('ldap_write_support', 'Allow users to set their password') }}
50+
</ActionCheckbox>
4751
</ul>
4852
<h3>{{ t('ldap_write_support', 'User template') }}</h3>
4953
<p>{{ t('ldap_write_support', 'LDIF template for creating users. Following placeholders may be used') }}</p>

0 commit comments

Comments
 (0)