From 8db21ad8c894332b85a37bef28818604a175db23 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sat, 18 Mar 2017 14:56:24 +0800 Subject: [PATCH 1/3] user_ldap: Add support for gidNumber This patch is based on the work of @dleeuw (https://github.com/dleeuw) (See https://github.com/nextcloud/server/issues/2640#issuecomment-269615883 for more details). The difference is user & group data will be written into cache to have better performance, and functions splited from primaryGroupID series to make them more readable. Fixed https://github.com/nextcloud/server/issues/2640 Signed-off-by: Xuanwo --- apps/user_ldap/lib/Configuration.php | 3 + apps/user_ldap/lib/Connection.php | 6 + apps/user_ldap/lib/Group_LDAP.php | 188 +++++++++++++++++++++++- apps/user_ldap/lib/Wizard.php | 5 +- apps/user_ldap/templates/settings.php | 3 +- apps/user_ldap/tests/Group_LDAPTest.php | 105 +++++++++++++ 6 files changed, 302 insertions(+), 8 deletions(-) diff --git a/apps/user_ldap/lib/Configuration.php b/apps/user_ldap/lib/Configuration.php index 65ee9c7080..0e08b05eb8 100644 --- a/apps/user_ldap/lib/Configuration.php +++ b/apps/user_ldap/lib/Configuration.php @@ -55,6 +55,7 @@ class Configuration { 'ldapIgnoreNamingRules' => null, 'ldapUserDisplayName' => null, 'ldapUserDisplayName2' => null, + 'ldapGidNumber' => null, 'ldapUserFilterObjectclass' => null, 'ldapUserFilterGroups' => null, 'ldapUserFilter' => null, @@ -430,6 +431,7 @@ class Configuration { 'ldap_group_filter_mode' => 0, 'ldap_groupfilter_objectclass' => '', 'ldap_groupfilter_groups' => '', + 'ldap_gid_number' => 'gidNumber', 'ldap_display_name' => 'displayName', 'ldap_user_display_name_2' => '', 'ldap_group_display_name' => 'cn', @@ -489,6 +491,7 @@ class Configuration { 'ldap_group_filter_mode' => 'ldapGroupFilterMode', 'ldap_groupfilter_objectclass' => 'ldapGroupFilterObjectclass', 'ldap_groupfilter_groups' => 'ldapGroupFilterGroups', + 'ldap_gid_number' => 'ldapGidNumber', 'ldap_display_name' => 'ldapUserDisplayName', 'ldap_user_display_name_2' => 'ldapUserDisplayName2', 'ldap_group_display_name' => 'ldapGroupDisplayName', diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 04f8c7401e..10fbea7174 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -12,6 +12,7 @@ * @author Robin Appelman * @author Robin McCorkell * @author Roger Szabo + * @author Xuanwo * * @license AGPL-3.0 * @@ -64,6 +65,11 @@ class Connection extends LDAPUtility { */ public $hasPrimaryGroups = true; + /** + * @var bool runtime flag that indicates whether supported POSIX gidNumber are available + */ + public $hasGidNumber = true; + //cache handler protected $cache; diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index d620a00f84..52e36bdf35 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -18,6 +18,7 @@ * @author Roeland Jago Douma * @author Thomas Müller * @author Vincent Petry + * @author Xuanwo * * @license AGPL-3.0 * @@ -229,9 +230,9 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { } } } - + $allMembers = array_merge($allMembers, $this->getDynamicGroupMembers($dnGroup)); - + $this->access->connection->writeToCache($cacheKey, $allMembers); return $allMembers; } @@ -263,7 +264,167 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { $allGroups = array_merge($allGroups, $subGroups); } } - return $allGroups; + return $allGroups; + } + + /** + * translates a gidNumber into an ownCloud internal name + * @param string $gid as given by gidNumber on POSIX LDAP + * @param string $dn a DN that belongs to the same domain as the group + * @return string|bool + */ + public function gidNumber2Name($gid, $dn) { + $cacheKey = 'gidNumberToName' . $gid; + $groupName = $this->access->connection->getFromCache($cacheKey); + if(!is_null($groupName) && isset($groupName)) { + return $groupName; + } + + //we need to get the DN from LDAP + $filter = $this->access->combineFilterWithAnd([ + $this->access->connection->ldapGroupFilter, + 'objectClass=posixGroup', + $this->access->connection->ldapGidNumber . '=' . $gid + ]); + $result = $this->access->searchGroups($filter, array('dn'), 1); + if(empty($result)) { + return false; + } + $dn = $result[0]['dn'][0]; + + //and now the group name + //NOTE once we have separate ownCloud group IDs and group names we can + //directly read the display name attribute instead of the DN + $name = $this->access->dn2groupname($dn); + + $this->access->connection->writeToCache($cacheKey, $name); + + return $name; + } + + /** + * returns the entry's gidNumber + * @param string $dn + * @param string $attribute + * @return string|bool + */ + private function getEntryGidNumber($dn, $attribute) { + $value = $this->access->readAttribute($dn, $attribute); + if(is_array($value) && !empty($value)) { + return $value[0]; + } + return false; + } + + /** + * returns the group's primary ID + * @param string $dn + * @return string|bool + */ + public function getGroupGidNumber($dn) { + return $this->getEntryGidNumber($dn, 'gidNumber'); + } + + /** + * returns the user's gidNumber + * @param string $dn + * @return string|bool + */ + public function getUserGidNumber($dn) { + $gidNumber = false; + if($this->access->connection->hasGidNumber) { + $gidNumber = $this->getEntryGidNumber($dn, 'gidNumber'); + if($gidNumber === false) { + $this->access->connection->hasGidNumber = false; + } + } + return $gidNumber; + } + + /** + * returns a filter for a "users has specific gid" search or count operation + * + * @param string $groupDN + * @param string $search + * @return string + * @throws \Exception + */ + private function prepareFilterForUsersHasGidNumber($groupDN, $search = '') { + $groupID = $this->getGroupGidNumber($groupDN); + if($groupID === false) { + throw new \Exception('Not a valid group'); + } + + $filterParts = []; + $filterParts[] = $this->access->getFilterForUserCount(); + if ($search !== '') { + $filterParts[] = $this->access->getFilterPartForUserSearch($search); + } + $filterParts[] = $this->access->connection->ldapGidNumber .'=' . $groupID; + + $filter = $this->access->combineFilterWithAnd($filterParts); + + return $filter; + } + + /** + * returns a list of users that have the given group as gid number + * + * @param string $groupDN + * @param string $search + * @param int $limit + * @param int $offset + * @return string[] + */ + public function getUsersInGidNumber($groupDN, $search = '', $limit = -1, $offset = 0) { + try { + $filter = $this->prepareFilterForUsersHasGidNumber($groupDN, $search); + $users = $this->access->fetchListOfUsers( + $filter, + [$this->access->connection->ldapUserDisplayName, 'dn'], + $limit, + $offset + ); + return $this->access->ownCloudUserNames($users); + } catch (\Exception $e) { + return []; + } + } + + /** + * returns the number of users that have the given group as gid number + * + * @param string $groupDN + * @param string $search + * @param int $limit + * @param int $offset + * @return int + */ + public function countUsersInGidNumber($groupDN, $search = '', $limit = -1, $offset = 0) { + try { + $filter = $this->prepareFilterForUsersHasGidNumber($groupDN, $search); + $users = $this->access->countUsers($filter, ['dn'], $limit, $offset); + return (int)$users; + } catch (\Exception $e) { + return 0; + } + } + + /** + * gets the gidNumber of a user + * @param string $dn + * @return string + */ + public function getUserGroupByGid($dn) { + $groupID = $this->getUserGidNumber($dn); + if($groupID !== false) { + $groupName = $this->gidNumber2Name($groupID, $dn); + if($groupName !== false) { + return $groupName; + } + } + + return false; } /** @@ -457,6 +618,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { $groups = []; $primaryGroup = $this->getUserPrimaryGroup($userDN); + $gidGroupName = $this->getUserGroupByGid($userDN); $dynamicGroupMemberURL = strtolower($this->access->connection->ldapDynamicGroupMemberURL); @@ -510,10 +672,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { } } } - + if($primaryGroup !== false) { $groups[] = $primaryGroup; } + if($gidGroupName !== false) { + $groups[] = $gidGroupName; + } $this->access->connection->writeToCache($cacheKey, $groups); return $groups; } @@ -547,6 +712,9 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { if($primaryGroup !== false) { $groups[] = $primaryGroup; } + if($gidGroupName !== false) { + $groups[] = $gidGroupName; + } $groups = array_unique($groups, SORT_LOCALE_STRING); $this->access->connection->writeToCache($cacheKey, $groups); @@ -641,6 +809,14 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { return array(); } + $posixGroupUsers = $this->getUsersInGidNumber($groupDN, $search, $limit, $offset); + $members = array_keys($this->_groupMembers($groupDN)); + if(!$members && empty($posixGroupUsers)) { + //in case users could not be retrieved, return empty result set + $this->access->connection->writeToCache($cacheKey, []); + return []; + } + $groupUsers = array(); $isMemberUid = (strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid'); $attrs = $this->access->userManager->getAttributes(true); @@ -677,6 +853,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { $this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers); $groupUsers = array_slice($groupUsers, $offset, $limit); + $groupUsers = array_unique(array_merge($groupUsers, $posixGroupUsers)); + natsort($groupUsers); + $this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers); + $groupUsers = array_slice($groupUsers, $offset, $limit); $this->access->connection->writeToCache($cacheKey, $groupUsers); diff --git a/apps/user_ldap/lib/Wizard.php b/apps/user_ldap/lib/Wizard.php index 2c388b1803..73fcd4f1e4 100644 --- a/apps/user_ldap/lib/Wizard.php +++ b/apps/user_ldap/lib/Wizard.php @@ -15,6 +15,7 @@ * @author Robin McCorkell * @author Stefan Weil * @author Victor Dubiniuk + * @author Xuanwo * * @license AGPL-3.0 * @@ -775,12 +776,12 @@ class Wizard extends LDAPUtility { /** * tries to detect the group member association attribute which is - * one of 'uniqueMember', 'memberUid', 'member' + * one of 'uniqueMember', 'memberUid', 'member', 'gidNumber' * @return string|false, string with the attribute name, false on error * @throws \Exception */ private function detectGroupMemberAssoc() { - $possibleAttrs = array('uniqueMember', 'memberUid', 'member'); + $possibleAttrs = array('uniqueMember', 'memberUid', 'member', 'gidNumber'); $filter = $this->configuration->ldapGroupFilter; if(empty($filter)) { return false; diff --git a/apps/user_ldap/templates/settings.php b/apps/user_ldap/templates/settings.php index e53456c703..0c56f2ee2b 100644 --- a/apps/user_ldap/templates/settings.php +++ b/apps/user_ldap/templates/settings.php @@ -97,8 +97,7 @@ style('user_ldap', 'settings');

-

-

+

t('(New password is sent as plain text to LDAP)'));?> diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index 906db6bb17..80989b6346 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -9,6 +9,7 @@ * @author Morris Jobke * @author Thomas Müller * @author Vincent Petry + * @author Xuanwo * * @license AGPL-3.0 * @@ -142,6 +143,107 @@ class Group_LDAPTest extends \Test\TestCase { $this->assertSame(2, $users); } + public function testGidNumber2NameSuccess() { + $access = $this->getAccessMock(); + $this->enableGroups($access); + + $userDN = 'cn=alice,cn=foo,dc=barfoo,dc=bar'; + + $access->expects($this->once()) + ->method('searchGroups') + ->will($this->returnValue([['dn' => ['cn=foo,dc=barfoo,dc=bar']]])); + + $access->expects($this->once()) + ->method('dn2groupname') + ->with('cn=foo,dc=barfoo,dc=bar') + ->will($this->returnValue('MyGroup')); + + $groupBackend = new GroupLDAP($access); + + $group = $groupBackend->gidNumber2Name('3117', $userDN); + + $this->assertSame('MyGroup', $group); + } + + public function testGidNumberID2NameNoGroup() { + $access = $this->getAccessMock(); + $this->enableGroups($access); + + $userDN = 'cn=alice,cn=foo,dc=barfoo,dc=bar'; + + $access->expects($this->once()) + ->method('searchGroups') + ->will($this->returnValue(array())); + + $access->expects($this->never()) + ->method('dn2groupname'); + + $groupBackend = new GroupLDAP($access); + + $group = $groupBackend->gidNumber2Name('3117', $userDN); + + $this->assertSame(false, $group); + } + + public function testGidNumberID2NameNoName() { + $access = $this->getAccessMock(); + $this->enableGroups($access); + + $userDN = 'cn=alice,cn=foo,dc=barfoo,dc=bar'; + + $access->expects($this->once()) + ->method('searchGroups') + ->will($this->returnValue([['dn' => ['cn=foo,dc=barfoo,dc=bar']]])); + + $access->expects($this->once()) + ->method('dn2groupname') + ->will($this->returnValue(false)); + + $groupBackend = new GroupLDAP($access); + + $group = $groupBackend->gidNumber2Name('3117', $userDN); + + $this->assertSame(false, $group); + } + + public function testGetEntryGidNumberValue() { + $access = $this->getAccessMock(); + $this->enableGroups($access); + + $dn = 'cn=foobar,cn=foo,dc=barfoo,dc=bar'; + $attr = 'gidNumber'; + + $access->expects($this->once()) + ->method('readAttribute') + ->with($dn, $attr) + ->will($this->returnValue(array('3117'))); + + $groupBackend = new GroupLDAP($access); + + $gid = $groupBackend->getGroupGidNumber($dn); + + $this->assertSame('3117', $gid); + } + + public function testGetEntryGidNumberNoValue() { + $access = $this->getAccessMock(); + $this->enableGroups($access); + + $dn = 'cn=foobar,cn=foo,dc=barfoo,dc=bar'; + $attr = 'gidNumber'; + + $access->expects($this->once()) + ->method('readAttribute') + ->with($dn, $attr) + ->will($this->returnValue(false)); + + $groupBackend = new GroupLDAP($access); + + $gid = $groupBackend->getGroupGidNumber($dn); + + $this->assertSame(false, $gid); + } + public function testPrimaryGroupID2NameSuccess() { $access = $this->getAccessMock(); $this->enableGroups($access); @@ -401,6 +503,7 @@ class Group_LDAPTest extends \Test\TestCase { $dn = 'cn=userX,dc=foobar'; $access->connection->hasPrimaryGroups = false; + $access->connection->hasGidNumber = false; $access->expects($this->any()) ->method('username2dn') @@ -441,6 +544,7 @@ class Group_LDAPTest extends \Test\TestCase { $dn = 'cn=userX,dc=foobar'; $access->connection->hasPrimaryGroups = false; + $access->connection->hasGidNumber = false; $access->expects($this->once()) ->method('username2dn') @@ -477,6 +581,7 @@ class Group_LDAPTest extends \Test\TestCase { $dn = 'cn=userX,dc=foobar'; $access->connection->hasPrimaryGroups = false; + $access->connection->hasGidNumber = false; $access->expects($this->exactly(2)) ->method('username2dn') From 685faad5ca437895e035cfe9583e1978e6ddedba Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 25 Apr 2017 13:03:08 +0200 Subject: [PATCH 2/3] fix method name due to changes in master Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Group_LDAP.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 17b64dbedc..5a209a3317 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -385,7 +385,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { $limit, $offset ); - return $this->access->ownCloudUserNames($users); + return $this->access->nextcloudUserNames($users); } catch (\Exception $e) { return []; } From 43f451e9e0e92c99285a471209e6645ff0b0eed9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 25 Apr 2017 15:07:05 +0200 Subject: [PATCH 3/3] Fix usersInGroup retrieval Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Group_LDAP.php | 16 ++--------- apps/user_ldap/tests/Group_LDAPTest.php | 37 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 5a209a3317..60ce664684 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -804,16 +804,9 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { } $primaryUsers = $this->getUsersInPrimaryGroup($groupDN, $search, $limit, $offset); - $members = array_keys($this->_groupMembers($groupDN)); - if(!$members && empty($primaryUsers)) { - //in case users could not be retrieved, return empty result set - $this->access->connection->writeToCache($cacheKey, array()); - return array(); - } - $posixGroupUsers = $this->getUsersInGidNumber($groupDN, $search, $limit, $offset); $members = array_keys($this->_groupMembers($groupDN)); - if(!$members && empty($posixGroupUsers)) { + if(!$members && empty($posixGroupUsers) && empty($primaryUsers)) { //in case users could not be retrieved, return empty result set $this->access->connection->writeToCache($cacheKey, []); return []; @@ -850,12 +843,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface { } } - $groupUsers = array_unique(array_merge($groupUsers, $primaryUsers)); - natsort($groupUsers); - $this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers); - $groupUsers = array_slice($groupUsers, $offset, $limit); - - $groupUsers = array_unique(array_merge($groupUsers, $posixGroupUsers)); + $groupUsers = array_unique(array_merge($groupUsers, $primaryUsers, $posixGroupUsers)); natsort($groupUsers); $this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers); $groupUsers = array_slice($groupUsers, $offset, $limit); diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index fc13b8a35e..9b5216742f 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -434,6 +434,43 @@ class Group_LDAPTest extends \Test\TestCase { $access = $this->getAccessMock(); $this->enableGroups($access); + $access->connection->expects($this->any()) + ->method('getFromCache') + ->will($this->returnValue(null)); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnCallback(function($dn, $attr) { + if($attr === 'primaryGroupToken') { + return array(1337); + } else if($attr === 'gidNumber') { + return [4211]; + } + return array(); + })); + + $access->expects($this->any()) + ->method('groupname2dn') + ->will($this->returnValue('cn=foobar,dc=foo,dc=bar')); + + $access->expects($this->exactly(2)) + ->method('nextcloudUserNames') + ->willReturnOnConsecutiveCalls(['lisa', 'bart', 'kira', 'brad'], ['walle', 'dino', 'xenia']); + + $groupBackend = new GroupLDAP($access); + $users = $groupBackend->usersInGroup('foobar'); + + $this->assertSame(7, count($users)); + } + + /** + * tests that a user listing is complete, if all it's members have the group + * as their primary. + */ + public function testUsersInGroupPrimaryAndUnixMembers() { + $access = $this->getAccessMock(); + $this->enableGroups($access); + $access->connection->expects($this->any()) ->method('getFromCache') ->will($this->returnValue(null));