Skip to content

Commit 2812826

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 2812826

File tree

4 files changed

+72
-32
lines changed

4 files changed

+72
-32
lines changed

lib/LDAPUserManager.php

Lines changed: 63 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(null) && !$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,82 @@ 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+
* @param string $uid the Nextcloud user name
341+
* @return boolean either the user can or cannot
342+
*/
343+
public function canSetPassword($uid) {
344+
return $this->configuration->hasPasswordPermission();
345+
}
346+
347+
/**
348+
* checks whether the LDAP server supports the passwd exop
349+
*
350+
* @param LDAP\Connection $connection LDAP connection to check
351+
* @return boolean either the user can or cannot
352+
*/
353+
public function canUseExopPasswd($connection) {
354+
$ret = ldap_read($connection, '', '(objectclass=*)', ['supportedExtension']);
355+
if ($ret === false) {
356+
return false;
357+
}
358+
$ret = ldap_first_entry($connection, $ret);
359+
if ($ret === false) {
360+
return false;
361+
}
362+
363+
$values = ldap_get_values($connection, $ret, 'supportedExtension');
364+
return ($values !== false) && in_array('1.3.6.1.4.1.4203.1.11.1', $values['supportedExtension']);
365+
}
366+
358367
/**
359368
* Set password
360369
*
361370
* @param string $uid The username
362371
* @param string $password The new password
372+
* @param LDAP\Connection $connection LDAP connection or null to create one
363373
* @return bool
364374
*
365375
* Change the password of a user
366376
*/
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;
377+
public function setPassword($uid, $password, $connection = null) {
378+
if (is_null($connection)) {
379+
$connection = $this->ldapProvider->getLDAPConnection($uid);
373380
}
381+
374382
try {
375-
$cr = $this->ldapProvider->getLDAPConnection($uid);
376383
$userDN = $this->getUserDN($uid);
377-
return ldap_exop_passwd($cr, $userDN, '', $password);
384+
$ret = false;
385+
386+
// try ldap_exop_passwd first
387+
if ($this->canUseExopPasswd($connection)) {
388+
$ret = ldap_exop_passwd($connection, $userDN, '', $password);
389+
if ($ret === false) {
390+
$message = 'ldap_exop_passwd failed, falling back to ldap_mod_replace to to set password for new user';
391+
$this->logger->log(ILogger::DEBUG, $message, [
392+
'app' => Application::APP_ID,
393+
]);
394+
}
395+
}
396+
397+
// Fallback to `userPassword` in case the server does not support exop_passwd
398+
if ($ret === false) {
399+
$ret = ldap_mod_replace($connection, $userDN, ['userPassword' => $password]);
400+
if ($ret === false) {
401+
$message = 'Failed to set password for user {dn}';
402+
$this->logger->log(ILogger::ERROR, $message, [
403+
'app' => Application::APP_ID,
404+
'dn' => $userDN,
405+
]);
406+
}
407+
}
408+
return $ret;
378409
} catch (\Exception $e) {
379410
$this->logger->logException($e, ['app' => Application::APP_ID]);
411+
return false;
380412
}
381-
return false;
382413
}
383414

384415
/**

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)