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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable changing passwords in user_ldap #1715

Closed
Closed
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
5 changes: 5 additions & 0 deletions apps/user_ldap/css/settings.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
width: 85%;
}

.inlinetable {
display: inline-table;
vertical-align: bottom;
}

.tablerow {
display: table-row;
white-space: nowrap;
Expand Down
15 changes: 15 additions & 0 deletions apps/user_ldap/js/wizard/wizardTabAdvanced.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ OCA = OCA || {};
$element: $('#ldap_paging_size'),
setMethod: 'setPagingSize'
},
ldap_turn_on_pwd_change: {
$element: $('#ldap_turn_on_pwd_change'),
setMethod: 'setPasswordChangeEnabled'
},

//Special Attributes
ldap_quota_attr: {
Expand Down Expand Up @@ -288,6 +292,17 @@ OCA = OCA || {};
setPagingSize: function(size) {
this.setElementValue(this.managedItems.ldap_paging_size.$element, size);
},

/**
* sets whether the password changes per user should be enabled
*
* @param {string} doPasswordChange contains an int
*/
setPasswordChangeEnabled: function(doPasswordChange) {
this.setElementValue(
this.managedItems.ldap_turn_on_pwd_change.$element, doPasswordChange
);
},

/**
* sets the email attribute
Expand Down
26 changes: 26 additions & 0 deletions apps/user_ldap/lib/Access.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@

namespace OCA\User_LDAP;

use OC\HintException;
use OCA\User_LDAP\Exceptions\ConstraintViolationException;
use OCA\User_LDAP\User\IUserTools;
use OCA\User_LDAP\User\Manager;
use OCA\User_LDAP\User\OfflineUser;
Expand Down Expand Up @@ -221,6 +223,30 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
\OCP\Util::writeLog('user_ldap', 'Requested attribute '.$attr.' not found for '.$dn, \OCP\Util::DEBUG);
return false;
}

/**
* Set password for an LDAP user identified by a DN
* @param string $userDN the user in question
* @param string $password the new password
* @return bool
*/
public function setPassword($userDN, $password) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing tests. Will add myself so we get this done.

if(intval($this->connection->turnOnPasswordChange) !== 1) {
throw new \Exception('LDAP password changes are disabled.');
}
$cr = $this->connection->getConnectionResource();
if(!$this->ldap->isResource($cr)) {
//LDAP not available
\OCP\Util::writeLog('user_ldap', 'LDAP resource not available.', \OCP\Util::DEBUG);
return false;
}

try {
return $this->ldap->modReplace($cr, $userDN, $password);
} catch(ConstraintViolationException $e) {
throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ').$e->getMessage(), $e->getCode());
}
}

/**
* checks whether the given attributes value is probably a DN
Expand Down
4 changes: 4 additions & 0 deletions apps/user_ldap/lib/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* @author Lukas Reschke <lukas@statuscode.ch>
* @author Morris Jobke <hey@morrisjobke.de>
* @author Robin McCorkell <robin@mccorkell.me.uk>
* @author Roger Szabo <roger.szabo@web.de>
*
* @license AGPL-3.0
*
Expand Down Expand Up @@ -90,6 +91,7 @@ class Configuration {
'lastJpegPhotoLookup' => null,
'ldapNestedGroups' => false,
'ldapPagingSize' => null,
'turnOnPasswordChange' => false,
'ldapDynamicGroupMemberURL' => null,
);

Expand Down Expand Up @@ -449,6 +451,7 @@ public function getDefaults() {
'last_jpegPhoto_lookup' => 0,
'ldap_nested_groups' => 0,
'ldap_paging_size' => 500,
'ldap_turn_on_pwd_change' => 0,
'ldap_experienced_admin' => 0,
'ldap_dynamic_group_member_url' => '',
);
Expand Down Expand Up @@ -505,6 +508,7 @@ public function getConfigTranslationArray() {
'last_jpegPhoto_lookup' => 'lastJpegPhotoLookup',
'ldap_nested_groups' => 'ldapNestedGroups',
'ldap_paging_size' => 'ldapPagingSize',
'ldap_turn_on_pwd_change' => 'turnOnPasswordChange',
'ldap_experienced_admin' => 'ldapExperiencedAdmin',
'ldap_dynamic_group_member_url' => 'ldapDynamicGroupMemberURL',
);
Expand Down
26 changes: 26 additions & 0 deletions apps/user_ldap/lib/Exceptions/ConstraintViolationException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php
/**
* @copyright Copyright (c) 2016 Roger Szabo <roger.szabo@web.de>
*
* @author Roger Szabo <roger.szabo@web.de>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OCA\User_LDAP\Exceptions;

class ConstraintViolationException extends \Exception {}
9 changes: 9 additions & 0 deletions apps/user_ldap/lib/ILDAPWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,15 @@ public function read($link, $baseDN, $filter, $attr);
* @return resource|false an LDAP search result resource, false on error
*/
public function search($link, $baseDN, $filter, $attr, $attrsOnly = 0, $limit = 0);

/**
* Replace the value of a userPassword by $password
* @param resource $link LDAP link resource
* @param string $userDN the DN of the user whose password is to be replaced
* @param string $password the new value for the userPassword
* @return bool true on success, false otherwise
*/
public function modReplace($link, $userDN, $password);

/**
* Sets the value of the specified option to be $value
Expand Down
15 changes: 15 additions & 0 deletions apps/user_ldap/lib/LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* @author Lukas Reschke <lukas@statuscode.ch>
* @author Morris Jobke <hey@morrisjobke.de>
* @author Robin McCorkell <robin@mccorkell.me.uk>
* @author Roger Szabo <roger.szabo@web.de>
*
* @license AGPL-3.0
*
Expand All @@ -29,6 +30,7 @@
namespace OCA\User_LDAP;

use OC\ServerNotAvailableException;
use OCA\User_LDAP\Exceptions\ConstraintViolationException;

class LDAP implements ILDAPWrapper {
protected $curFunc = '';
Expand Down Expand Up @@ -192,6 +194,16 @@ public function search($link, $baseDN, $filter, $attr, $attrsOnly = 0, $limit =
return $this->invokeLDAPMethod('search', $link, $baseDN, $filter, $attr, $attrsOnly, $limit);
}

/**
* @param LDAP $link
* @param string $userDN
* @param string $password
* @return bool
*/
public function modReplace($link, $userDN, $password) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add this method to the ILDAPWrapper interface, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok~

return $this->invokeLDAPMethod('mod_replace', $link, $userDN, array('userPassword' => $password));
}

/**
* @param LDAP $link
* @param string $option
Expand Down Expand Up @@ -288,6 +300,9 @@ private function postFunctionCall() {
throw new \Exception('LDAP authentication method rejected', $errorCode);
} else if ($errorCode === 1) {
throw new \Exception('LDAP Operations error', $errorCode);
} else if ($errorCode === 19) {
ldap_get_option($this->curArgs[0], LDAP_OPT_ERROR_STRING, $extended_error);
throw new ConstraintViolationException(!empty($extended_error)?$extended_error:$errorMsg, $errorCode);
} else {
\OCP\Util::writeLog('user_ldap',
'LDAP error '.$errorMsg.' (' .
Expand Down
32 changes: 27 additions & 5 deletions apps/user_ldap/lib/User_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

namespace OCA\User_LDAP;

use OC\User\Backend;
use OC\User\NoUserException;
use OCA\User_LDAP\Exceptions\NotOnLDAP;
use OCA\User_LDAP\User\OfflineUser;
Expand Down Expand Up @@ -174,6 +175,26 @@ public function checkPassword($uid, $password) {
return false;
}

/**
* Set password
* @param string $uid The username
* @param string $password The new password
* @return bool
*/
public function setPassword($uid, $password) {
$user = $this->access->userManager->get($uid);

if(!$user instanceof User) {
throw new \Exception('LDAP setPassword: Could not get user object for uid ' . $uid .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not covered by tests. Will add.

'. Maybe the LDAP entry has no set display name attribute?');
}
if($user->getUsername() !== false) {
return $this->access->setPassword($user->getDN(), $password);
}

return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not covered by tests. Will add.

}

/**
* Get a list of all users
*
Expand Down Expand Up @@ -449,11 +470,12 @@ public function getDisplayNames($search = '', $limit = null, $offset = null) {
* compared with OC_USER_BACKEND_CREATE_USER etc.
*/
public function implementsActions($actions) {
return (bool)((\OC\User\Backend::CHECK_PASSWORD
| \OC\User\Backend::GET_HOME
| \OC\User\Backend::GET_DISPLAYNAME
| \OC\User\Backend::PROVIDE_AVATAR
| \OC\User\Backend::COUNT_USERS)
return (bool)((Backend::CHECK_PASSWORD
| Backend::GET_HOME
| Backend::GET_DISPLAYNAME
| Backend::PROVIDE_AVATAR
| Backend::COUNT_USERS
| ((intval($this->access->connection->turnOnPasswordChange) === 1)?(Backend::SET_PASSWORD):0))
& $actions);
}

Expand Down
11 changes: 11 additions & 0 deletions apps/user_ldap/lib/User_Proxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,17 @@ public function getDisplayNames($search = '', $limit = null, $offset = null) {
public function deleteUser($uid) {
return $this->handleRequest($uid, 'deleteUser', array($uid));
}

/**
* Set password
* @param string $uid The username
* @param string $password The new password
* @return bool
*
*/
public function setPassword($uid, $password) {
return $this->handleRequest($uid, 'setPassword', array($uid, $password));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not covered by tests. Will add.

}

/**
* @return bool
Expand Down
2 changes: 2 additions & 0 deletions apps/user_ldap/templates/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@
<p><label for="ldap_dynamic_group_member_url"><?php p($l->t('Dynamic Group Member URL'));?></label><input type="text" id="ldap_dynamic_group_member_url" name="ldap_dynamic_group_member_url" title="<?php p($l->t('The LDAP attribute that on group objects contains an LDAP search URL that determines what objects belong to the group. (An empty setting disables dynamic group membership functionality.)'));?>" data-default="<?php p($_['ldap_dynamic_group_member_url_default']); ?>" /></p>
<p><label for="ldap_nested_groups"><?php p($l->t('Nested Groups'));?></label><input type="checkbox" id="ldap_nested_groups" name="ldap_nested_groups" value="1" data-default="<?php p($_['ldap_nested_groups_default']); ?>" title="<?php p($l->t('When switched on, groups that contain groups are supported. (Only works if the group member attribute contains DNs.)'));?>" /></p>
<p><label for="ldap_paging_size"><?php p($l->t('Paging chunksize'));?></label><input type="number" id="ldap_paging_size" name="ldap_paging_size" title="<?php p($l->t('Chunksize used for paged LDAP searches that may return bulky results like user or group enumeration. (Setting it 0 disables paged LDAP searches in those situations.)'));?>" data-default="<?php p($_['ldap_paging_size_default']); ?>" /></p>
<p><label for="ldap_turn_on_pwd_change"><?php p($l->t('Enable LDAP password changes per user'));?></label><span class="inlinetable"><span class="tablerow left"><input type="checkbox" id="ldap_turn_on_pwd_change" name="ldap_turn_on_pwd_change" value="1" data-default="<?php p($_['ldap_turn_on_pwd_change_default']); ?>" title="<?php p($l->t('Allow LDAP users to change their password and allow Super Administrators and Group Administrators to change the password of their LDAP users. Only works when access control policies are configured accordingly on the LDAP server. As passwords are sent in plaintext to the LDAP server, transport encryption must be used and password hashing should be configured on the LDAP server.'));?>" /><span class="tablecell"><?php p($l->t('(New password is sent as plain text to LDAP)'));?></span></span>
</span><br/></p>
</div>
<h3><?php p($l->t('Special Attributes'));?></h3>
<div>
Expand Down
101 changes: 100 additions & 1 deletion apps/user_ldap/tests/User_LDAPTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* @author Morris Jobke <hey@morrisjobke.de>
* @author Robin McCorkell <robin@mccorkell.me.uk>
* @author Thomas M眉ller <thomas.mueller@tmit.eu>
* @author Roger Szabo <roger.szabo@web.de>
*
* @license AGPL-3.0
*
Expand Down Expand Up @@ -36,7 +37,7 @@
use OCA\User_LDAP\LogWrapper;
use OCA\User_LDAP\User\Manager;
use OCA\User_LDAP\User\OfflineUser;
use OCA\User_LDAP\User\User;
use OC\HintException;
use OCA\User_LDAP\User_LDAP as UserLDAP;
use OCP\IAvatarManager;
use OCP\IConfig;
Expand Down Expand Up @@ -958,5 +959,103 @@ public function testLoginName2UserNameOfflineUser() {
// and once again to verify that caching works
$backend->loginName2UserName($loginName);
}

/**
* Prepares the Access mock for setPassword tests
* @param \OCA\User_LDAP\Access|\PHPUnit_Framework_MockObject_MockObject $access mock
* @return void
*/
private function prepareAccessForSetPassword(&$access, $enablePasswordChange = true) {
$access->connection->expects($this->any())
->method('__get')
->will($this->returnCallback(function($name) use (&$enablePasswordChange) {
if($name === 'ldapLoginFilter') {
return '%uid';
}
if($name === 'turnOnPasswordChange') {
return $enablePasswordChange?1:0;
}
return null;
}));

$access->connection->expects($this->any())
->method('getFromCache')
->will($this->returnCallback(function($uid) {
if($uid === 'userExists'.'roland') {
return true;
}
return null;
}));

$access->expects($this->any())
->method('fetchListOfUsers')
->will($this->returnCallback(function($filter) {
if($filter === 'roland') {
return array(array('dn' => ['dnOfRoland,dc=test']));
}
return array();
}));

$access->expects($this->any())
->method('fetchUsersByLoginName')
->will($this->returnCallback(function($uid) {
if($uid === 'roland') {
return array(array('dn' => ['dnOfRoland,dc=test']));
}
return array();
}));

$access->expects($this->any())
->method('dn2username')
->with($this->equalTo('dnOfRoland,dc=test'))
->will($this->returnValue('roland'));

$access->expects($this->any())
->method('stringResemblesDN')
->with($this->equalTo('dnOfRoland,dc=test'))
->will($this->returnValue(true));

$access->expects($this->any())
->method('setPassword')
->will($this->returnCallback(function($uid, $password) {
if(strlen($password) <= 5) {
throw new HintException('Password fails quality checking policy', '', 19);
}
return true;
}));
}

/**
* @expectedException \OC\HintException
* @expectedExceptionMessage Password fails quality checking policy
*/
public function testSetPasswordInvalid() {
$access = $this->getAccessMock();

$this->prepareAccessForSetPassword($access);
$backend = new UserLDAP($access, $this->createMock(IConfig::class));
\OC_User::useBackend($backend);

$this->assertTrue(\OC_User::setPassword('roland', 'dt'));
}

public function testSetPasswordValid() {
$access = $this->getAccessMock();

$this->prepareAccessForSetPassword($access);
$backend = new UserLDAP($access, $this->createMock(IConfig::class));
\OC_User::useBackend($backend);

$this->assertTrue(\OC_User::setPassword('roland', 'dt12234$'));
}

public function testSetPasswordValidDisabled() {
$access = $this->getAccessMock();

$this->prepareAccessForSetPassword($access, false);
$backend = new UserLDAP($access, $this->createMock(IConfig::class));
\OC_User::useBackend($backend);

$this->assertFalse(\OC_User::setPassword('roland', 'dt12234$'));
}
}