From c2d8a36d9a824791234cc4093b79c8a66ee55cbb Mon Sep 17 00:00:00 2001 From: Roland Tapken Date: Wed, 7 Feb 2018 14:08:08 +0100 Subject: [PATCH 1/8] user_ldap: Filter groups after nexted groups Currently groupsMatchFilter is called before nested groups are resolved. This basicly breaks this feature since it is not possible to inherit membership in a group from another group. Minimal example: Group filter: (&(objectClass=group),(cn=nextcloud)) Nested groups: enabled cn=nextcloud,ou=Nextcloud,ou=groups,dn=company,dn=local objectClass: group cn=IT,ou=groups,dn=company,dn=local objectClass: group memberOf: cn=nextcloud,ou=Nextcloud,ou=groups,dn=company,dn=local cn=John Doe,ou=users,dn=company,dn=local objectClass: person memberOf: cn=IT,ou=groups,dn=company,dn=local Since 'cn=IT,ou=groups,dn=company,dn=local' doesn't match the group filter, John wouldn't be a member of group 'nextcloud'. This patch fixes this by filtering the groups after all nested groups have been collected. If nested groups is disabled the result will be the same as without this patch. Signed-off-by: Roland Tapken --- apps/user_ldap/lib/Group_LDAP.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 2240c2ad229ba..b16cb95302108 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -265,7 +265,6 @@ private function _getGroupDNsFromMemberOf($DN, &$seen = null) { if (!is_array($groups)) { return array(); } - $groups = $this->access->groupsMatchFilter($groups); $allGroups = $groups; $nestedGroups = $this->access->connection->ldapNestedGroups; if ((int)$nestedGroups === 1) { @@ -274,7 +273,7 @@ private function _getGroupDNsFromMemberOf($DN, &$seen = null) { $allGroups = array_merge($allGroups, $subGroups); } } - return $allGroups; + return $this->access->groupsMatchFilter($allGroups); } /** From afb182650e02ee0dd9ff18e8669d9077cdddef73 Mon Sep 17 00:00:00 2001 From: Roland Tapken Date: Wed, 7 Feb 2018 15:49:40 +0100 Subject: [PATCH 2/8] user_ldap: really resolve nested groups The previous patch fixed the problem only for one level of indirection because groupsMatchFilter() had been applied on each recursive call (and thus there would be no second level if the first level fails the check). This new implementation replaces the recursive call with a stack that iterates all nested groups before filtering with groupsMatchFilter(). Signed-off-by: Roland Tapken --- apps/user_ldap/lib/Group_LDAP.php | 33 ++++++++++++++++++------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index b16cb95302108..427245d4a0676 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -252,28 +252,33 @@ private function _groupMembers($dnGroup, &$seen = null) { * @param array|null &$seen * @return array */ - private function _getGroupDNsFromMemberOf($DN, &$seen = null) { - if ($seen === null) { - $seen = array(); - } - if (array_key_exists($DN, $seen)) { - // avoid loops - return array(); - } - $seen[$DN] = 1; + private function _getGroupDNsFromMemberOf($DN) { $groups = $this->access->readAttribute($DN, 'memberOf'); if (!is_array($groups)) { return array(); } - $allGroups = $groups; $nestedGroups = $this->access->connection->ldapNestedGroups; if ((int)$nestedGroups === 1) { - foreach ($groups as $group) { - $subGroups = $this->_getGroupDNsFromMemberOf($group, $seen); - $allGroups = array_merge($allGroups, $subGroups); + $seen = array(); + while ($group = array_pop($groups)) { + if ($group === $DN || array_key_exists($group, $seen)) { + // Prevent loops + continue; + } + $seen[$group] = 1; + + // Resolve nested groups + $nestedGroups = $this->access->readAttribute($group, 'memberOf'); + if (is_array($nestedGroups)) { + foreach ($nestedGroups as $nestedGroup) { + array_push($groups, $nestedGroup); + } + } } + // Get unique group DN's from those we have visited in the loop + $groups = array_keys($seen); } - return $this->access->groupsMatchFilter($allGroups); + return $this->access->groupsMatchFilter($groups); } /** From e7c506cff100b588e1943bd7dbaef2ef687a1bfb Mon Sep 17 00:00:00 2001 From: Roland Tapken Date: Wed, 7 Feb 2018 16:08:25 +0100 Subject: [PATCH 3/8] Reduce queries to LDAP by caching nested groups Nested groups are now cached in a CappedMemoryCache object to reduce queries to the LDAP backend. Signed-off-by: Roland Tapken --- apps/user_ldap/lib/Group_LDAP.php | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 427245d4a0676..2e3bc0b4a5cab 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -58,6 +58,11 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD */ protected $cachedGroupsByMember; + /** + * @var string[] $cachedNestedGroups array of groups with gid (DN) as key + */ + protected $cachedNestedGroups; + /** @var GroupPluginManager */ protected $groupPluginManager; @@ -71,6 +76,7 @@ public function __construct(Access $access, GroupPluginManager $groupPluginManag $this->cachedGroupMembers = new CappedMemoryCache(); $this->cachedGroupsByMember = new CappedMemoryCache(); + $this->cachedNestedGroups = new CappedMemoryCache(); $this->groupPluginManager = $groupPluginManager; } @@ -257,8 +263,8 @@ private function _getGroupDNsFromMemberOf($DN) { if (!is_array($groups)) { return array(); } - $nestedGroups = $this->access->connection->ldapNestedGroups; - if ((int)$nestedGroups === 1) { + $nestedGroups = (int) $this->access->connection->ldapNestedGroups; + if ($nestedGroups === 1) { $seen = array(); while ($group = array_pop($groups)) { if ($group === $DN || array_key_exists($group, $seen)) { @@ -268,11 +274,17 @@ private function _getGroupDNsFromMemberOf($DN) { $seen[$group] = 1; // Resolve nested groups - $nestedGroups = $this->access->readAttribute($group, 'memberOf'); - if (is_array($nestedGroups)) { - foreach ($nestedGroups as $nestedGroup) { - array_push($groups, $nestedGroup); + if (isset($cachedNestedGroups[$group])) { + $nestedGroups = $cachedNestedGroups[$group]; + } else { + $nestedGroups = $this->access->readAttribute($group, 'memberOf'); + if (!is_array($nestedGroups)) { + $nestedGroups = []; } + $cachedNestedGroups[$group] = $nestedGroups; + } + foreach ($nestedGroups as $nestedGroup) { + array_push($groups, $nestedGroup); } } // Get unique group DN's from those we have visited in the loop From 459b8a4845686522476241f3287fc140b8288090 Mon Sep 17 00:00:00 2001 From: Roland Tapken Date: Thu, 8 Feb 2018 13:14:57 +0100 Subject: [PATCH 4/8] Fixed unit test: groupsMatchFilter will not be called multiple times anymore. Signed-off-by: Roland Tapken --- apps/user_ldap/tests/Group_LDAPTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index ec8d888e2a02c..0c5a06144a0bb 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -625,7 +625,7 @@ public function testGetUserGroupsMemberOf() { ->method('dn2groupname') ->will($this->returnArgument(0)); - $access->expects($this->exactly(3)) + $access->expects($this->exactly(1)) ->method('groupsMatchFilter') ->will($this->returnArgument(0)); From 5dd2207c958ff70d4b0c8801cc29c3295f76f725 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 2 Mar 2019 00:36:08 +0100 Subject: [PATCH 5/8] fix nested group retrieval also for 2 other cases and also consolidate logic in one method Signed-off-by: Arthur Schiwon --- .drone.yml | 2 +- apps/user_ldap/lib/Connection.php | 3 + apps/user_ldap/lib/Group_LDAP.php | 136 ++++++++++-------- apps/user_ldap/tests/Group_LDAPTest.php | 56 ++++---- .../ldap_features/ldap-openldap.feature | 58 ++++++++ .../openldap-numerical-id.feature | 35 +++++ 6 files changed, 205 insertions(+), 85 deletions(-) diff --git a/.drone.yml b/.drone.yml index e3cfc2d7be566..f821333ee9122 100644 --- a/.drone.yml +++ b/.drone.yml @@ -1049,7 +1049,7 @@ services: matrix: TESTS: acceptance openldap: - image: nextcloudci/openldap:openldap-6 + image: nextcloudci/openldap:openldap-7 environment: - SLAPD_DOMAIN=nextcloud.ci - SLAPD_ORGANIZATION=Nextcloud diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index ba393dffc122a..4335f8e4397fd 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -62,6 +62,9 @@ * @property string ldapEmailAttribute * @property string ldapExtStorageHomeAttribute * @property string homeFolderNamingRule + * @property bool|string ldapNestedGroups + * @property string[] ldapBaseGroups + * @property string ldapGroupFilter */ class Connection extends LDAPUtility { private $ldapConnectionRes = null; diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 2e3bc0b4a5cab..1658807c0dd96 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -218,12 +218,12 @@ public function getDynamicGroupMembers($dnGroup) { */ private function _groupMembers($dnGroup, &$seen = null) { if ($seen === null) { - $seen = array(); + $seen = []; } - $allMembers = array(); + $allMembers = []; if (array_key_exists($dnGroup, $seen)) { // avoid loops - return array(); + return []; } // used extensively in cron job, caching makes sense for nested groups $cacheKey = '_groupMembers'.$dnGroup; @@ -232,19 +232,12 @@ private function _groupMembers($dnGroup, &$seen = null) { return $groupMembers; } $seen[$dnGroup] = 1; - $members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr, - $this->access->connection->ldapGroupFilter); + $members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr); if (is_array($members)) { - foreach ($members as $member) { - $allMembers[$member] = 1; - $nestedGroups = $this->access->connection->ldapNestedGroups; - if (!empty($nestedGroups)) { - $subMembers = $this->_groupMembers($member, $seen); - if ($subMembers) { - $allMembers += $subMembers; - } - } - } + $fetcher = function($memberDN, &$seen) { + return $this->_groupMembers($memberDN, $seen); + }; + $allMembers = $this->walkNestedGroups($dnGroup, $fetcher, $members); } $allMembers += $this->getDynamicGroupMembers($dnGroup); @@ -257,40 +250,69 @@ private function _groupMembers($dnGroup, &$seen = null) { * @param string $DN * @param array|null &$seen * @return array + * @throws \OC\ServerNotAvailableException */ private function _getGroupDNsFromMemberOf($DN) { $groups = $this->access->readAttribute($DN, 'memberOf'); if (!is_array($groups)) { - return array(); + return []; } - $nestedGroups = (int) $this->access->connection->ldapNestedGroups; - if ($nestedGroups === 1) { - $seen = array(); - while ($group = array_pop($groups)) { - if ($group === $DN || array_key_exists($group, $seen)) { - // Prevent loops - continue; + + $fetcher = function($groupDN) { + if (isset($this->cachedNestedGroups[$groupDN])) { + $nestedGroups = $this->cachedNestedGroups[$groupDN]; + } else { + $nestedGroups = $this->access->readAttribute($groupDN, 'memberOf'); + if (!is_array($nestedGroups)) { + $nestedGroups = []; } - $seen[$group] = 1; + $this->cachedNestedGroups[$groupDN] = $nestedGroups; + } + return $nestedGroups; + }; - // Resolve nested groups - if (isset($cachedNestedGroups[$group])) { - $nestedGroups = $cachedNestedGroups[$group]; - } else { - $nestedGroups = $this->access->readAttribute($group, 'memberOf'); - if (!is_array($nestedGroups)) { - $nestedGroups = []; + $groups = $this->walkNestedGroups($DN, $fetcher, $groups); + return $this->access->groupsMatchFilter($groups); + } + + /** + * @param string $dn + * @param \Closure $fetcher args: string $dn, array $seen, returns: string[] of dns + * @param array $list + * @return array + */ + private function walkNestedGroups(string $dn, \Closure $fetcher, array $list): array { + $nesting = (int) $this->access->connection->ldapNestedGroups; + // depending on the input, we either have a list of DNs or a list of LDAP records + // also, the output expects either DNs or records. Testing the first element should suffice. + $recordMode = is_array($list) && isset($list[0]) && is_array($list[0]) && isset($list[0]['dn'][0]); + + if ($nesting !== 1) { + if($recordMode) { + // the keys are numeric, but should hold the DN + return array_reduce($list, function ($transformed, $record) use ($dn) { + if($record['dn'][0] != $dn) { + $transformed[$record['dn'][0]] = $record; } - $cachedNestedGroups[$group] = $nestedGroups; - } - foreach ($nestedGroups as $nestedGroup) { - array_push($groups, $nestedGroup); - } + return $transformed; + }, []); } - // Get unique group DN's from those we have visited in the loop - $groups = array_keys($seen); + return $list; } - return $this->access->groupsMatchFilter($groups); + + $seen = []; + while ($record = array_pop($list)) { + $recordDN = $recordMode ? $record['dn'][0] : $record; + if ($recordDN === $dn || array_key_exists($recordDN, $seen)) { + // Prevent loops + continue; + } + $fetched = $fetcher($record, $seen); + $list = array_merge($list, $fetched); + $seen[$recordDN] = $record; + } + + return $recordMode ? $seen : array_keys($seen); } /** @@ -753,34 +775,28 @@ public function getUserGroups($uid) { */ private function getGroupsByMember($dn, &$seen = null) { if ($seen === null) { - $seen = array(); + $seen = []; } - $allGroups = array(); if (array_key_exists($dn, $seen)) { // avoid loops - return array(); + return []; } + $allGroups = []; $seen[$dn] = true; - $filter = $this->access->combineFilterWithAnd(array( - $this->access->connection->ldapGroupFilter, - $this->access->connection->ldapGroupMemberAssocAttr.'='.$dn - )); + $filter = $this->access->connection->ldapGroupMemberAssocAttr.'='.$dn; $groups = $this->access->fetchListOfGroups($filter, - array($this->access->connection->ldapGroupDisplayName, 'dn')); + [$this->access->connection->ldapGroupDisplayName, 'dn']); if (is_array($groups)) { - foreach ($groups as $groupobj) { - $groupDN = $groupobj['dn'][0]; - $allGroups[$groupDN] = $groupobj; - $nestedGroups = $this->access->connection->ldapNestedGroups; - if (!empty($nestedGroups)) { - $supergroups = $this->getGroupsByMember($groupDN, $seen); - if (is_array($supergroups) && (count($supergroups)>0)) { - $allGroups = array_merge($allGroups, $supergroups); - } + $fetcher = function ($dn, &$seen) { + if(is_array($dn) && isset($dn['dn'][0])) { + $dn = $dn['dn'][0]; } - } + return $this->getGroupsByMember($dn, $seen); + }; + $allGroups = $this->walkNestedGroups($dn, $fetcher, $groups); } - return $allGroups; + $visibleGroups = $this->access->groupsMatchFilter(array_keys($allGroups)); + return array_intersect_key($allGroups, array_flip($visibleGroups)); } /** @@ -827,7 +843,7 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { $primaryUsers = $this->getUsersInPrimaryGroup($groupDN, $search, $limit, $offset); $posixGroupUsers = $this->getUsersInGidNumber($groupDN, $search, $limit, $offset); - $members = array_keys($this->_groupMembers($groupDN)); + $members = $this->_groupMembers($groupDN); if(!$members && empty($posixGroupUsers) && empty($primaryUsers)) { //in case users could not be retrieved, return empty result set $this->access->connection->writeToCache($cacheKey, []); @@ -902,7 +918,7 @@ public function countUsersInGroup($gid, $search = '') { return false; } - $members = array_keys($this->_groupMembers($groupDN)); + $members = $this->_groupMembers($groupDN); $primaryUserCount = $this->countUsersInPrimaryGroup($groupDN, ''); if(!$members && $primaryUserCount === 0) { //in case users could not be retrieved, return empty result set diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index 0c5a06144a0bb..870dddf1bd8b0 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -39,6 +39,7 @@ use OCA\User_LDAP\Group_LDAP as GroupLDAP; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\User\Manager; +use function SebastianBergmann\GlobalState\functions; use Test\TestCase; /** @@ -98,16 +99,27 @@ private function enableGroups($access) { public function testCountEmptySearchString() { $access = $this->getAccessMock(); $pluginManager = $this->getPluginManagerMock(); + $groupDN = 'cn=group,dc=foo,dc=bar'; $this->enableGroups($access); $access->expects($this->any()) ->method('groupname2dn') - ->will($this->returnValue('cn=group,dc=foo,dc=bar')); + ->will($this->returnValue($groupDN)); $access->expects($this->any()) ->method('readAttribute') - ->will($this->returnValue(array('u11', 'u22', 'u33', 'u34'))); + ->willReturnCallback(function($dn) use ($groupDN) { + if($dn === $groupDN) { + return [ + 'uid=u11,ou=users,dc=foo,dc=bar', + 'uid=u22,ou=users,dc=foo,dc=bar', + 'uid=u33,ou=users,dc=foo,dc=bar', + 'uid=u34,ou=users,dc=foo,dc=bar' + ]; + } + return []; + }); // for primary groups $access->expects($this->once()) @@ -132,7 +144,7 @@ public function testCountWithSearchString() { $access->expects($this->any()) ->method('fetchListOfUsers') - ->will($this->returnValue(array())); + ->will($this->returnValue([])); $access->expects($this->any()) ->method('readAttribute') @@ -145,7 +157,7 @@ public function testCountWithSearchString() { if(strpos($name, 'u') === 0) { return strpos($name, '3'); } - return array('u11', 'u22', 'u33', 'u34'); + return ['u11', 'u22', 'u33', 'u34']; })); $access->expects($this->any()) @@ -659,14 +671,15 @@ public function testGetUserGroupsMemberOfDisabled() { $access->expects($this->once()) ->method('username2dn') ->will($this->returnValue($dn)); - $access->expects($this->never()) ->method('readAttribute') ->with($dn, 'memberOf'); - $access->expects($this->once()) ->method('nextcloudGroupNames') ->will($this->returnValue([])); + $access->expects($this->any()) + ->method('groupsMatchFilter') + ->willReturnArgument(0); $groupBackend = new GroupLDAP($access, $pluginManager); $groupBackend->getUserGroups('userX'); @@ -680,12 +693,15 @@ public function testGetGroupsByMember() { $access->connection->expects($this->any()) ->method('__get') ->will($this->returnCallback(function($name) { - if($name === 'useMemberOfToDetectMembership') { - return 0; - } else if($name === 'ldapDynamicGroupMemberURL') { - return ''; - } else if($name === 'ldapNestedGroups') { - return false; + switch($name) { + case 'useMemberOfToDetectMembership': + return 0; + case 'ldapDynamicGroupMemberURL': + return ''; + case 'ldapNestedGroups': + return false; + case 'ldapGroupMemberAssocAttr': + return 'member'; } return 1; })); @@ -716,10 +732,12 @@ public function testGetGroupsByMember() { ->method('nextcloudGroupNames') ->with([$group1, $group2]) ->will($this->returnValue(['group1', 'group2'])); - $access->expects($this->once()) ->method('fetchListOfGroups') ->will($this->returnValue([$group1, $group2])); + $access->expects($this->any()) + ->method('groupsMatchFilter') + ->willReturnArgument(0); $groupBackend = new GroupLDAP($access, $pluginManager); $groups = $groupBackend->getUserGroups('userX'); @@ -999,14 +1017,6 @@ public function groupMemberProvider() { $groups1, ['cn=Birds,' . $base => $groups1] ], - [ #2 – test uids with nested groups - 'cn=Birds,' . $base, - $expGroups2, - [ - 'cn=Birds,' . $base => $groups1, - '8427' => $groups2Nested, // simplified - nested groups would work with DNs - ], - ], ]; } @@ -1045,9 +1055,7 @@ public function testGroupMembers($groupDN, $expectedMembers, $groupsInfo = null) $ldap = new GroupLDAP($access, $pluginManager); $resultingMembers = $this->invokePrivate($ldap, '_groupMembers', [$groupDN]); - $expected = array_keys(array_flip($expectedMembers)); - - $this->assertEquals($expected, array_keys($resultingMembers), '', 0.0, 10, true); + $this->assertEquals($expectedMembers, $resultingMembers, '', 0.0, 10, true); } } diff --git a/build/integration/ldap_features/ldap-openldap.feature b/build/integration/ldap_features/ldap-openldap.feature index 4b0b02c5b4fe5..2e1f637a50a4c 100644 --- a/build/integration/ldap_features/ldap-openldap.feature +++ b/build/integration/ldap_features/ldap-openldap.feature @@ -102,3 +102,61 @@ Feature: LDAP | ldapHost | foo.bar | | ldapPort | 2456 | Then Expect ServerException on failed web login as "alice" + + Scenario: Test LDAP group membership with intermediate groups not matching filter + Given modify LDAP configuration + | ldapBaseGroups | ou=OtherGroups,dc=nextcloud,dc=ci | + | ldapGroupFilter | (&(cn=Gardeners)(objectclass=groupOfNames)) | + | ldapNestedGroups | 1 | + | useMemberOfToDetectMembership | 1 | + | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + And As an "admin" + # for population + And sending "GET" to "/cloud/groups" + And sending "GET" to "/cloud/groups/Gardeners/users" + Then the OCS status code should be "200" + And the "users" result should match + | alice | 0 | + | clara | 1 | + | elisa | 1 | + | gustaf | 1 | + | jesper | 1 | + + Scenario: Test LDAP group membership with intermediate groups not matching filter and without memberof + Given modify LDAP configuration + | ldapBaseGroups | ou=OtherGroups,dc=nextcloud,dc=ci | + | ldapGroupFilter | (&(cn=Gardeners)(objectclass=groupOfNames)) | + | ldapNestedGroups | 1 | + | useMemberOfToDetectMembership | 0 | + | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + And As an "admin" + # for population + And sending "GET" to "/cloud/groups" + And sending "GET" to "/cloud/groups/Gardeners/users" + Then the OCS status code should be "200" + And the "users" result should match + | alice | 0 | + | clara | 1 | + | elisa | 1 | + | gustaf | 1 | + | jesper | 1 | + + Scenario: Test LDAP group membership with intermediate groups not matching filter, numeric group ids + Given modify LDAP configuration + | ldapBaseGroups | ou=NumericGroups,dc=nextcloud,dc=ci | + | ldapGroupFilter | (&(cn=2000)(objectclass=groupOfNames)) | + | ldapNestedGroups | 1 | + | useMemberOfToDetectMembership | 1 | + | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + And As an "admin" + # for population + And sending "GET" to "/cloud/groups" + And sending "GET" to "/cloud/groups/2000/users" + Then the OCS status code should be "200" + And the "users" result should match + | alice | 0 | + | clara | 1 | + | elisa | 1 | + | gustaf | 1 | + | jesper | 1 | + diff --git a/build/integration/ldap_features/openldap-numerical-id.feature b/build/integration/ldap_features/openldap-numerical-id.feature index 2d87ba33e6ef3..4959c7328e651 100644 --- a/build/integration/ldap_features/openldap-numerical-id.feature +++ b/build/integration/ldap_features/openldap-numerical-id.feature @@ -29,3 +29,38 @@ Scenario: Test by logging in And Logging in using web as "92379" And Sending a "GET" to "/remote.php/webdav/welcome.txt" with requesttoken Then the HTTP status code should be "200" + +Scenario: Test LDAP group retrieval with numeric group ids and nesting + # Nesting does not play a role here really + Given modify LDAP configuration + | ldapBaseGroups | ou=NumericGroups,dc=nextcloud,dc=ci | + | ldapGroupFilter | (objectclass=groupOfNames) | + | ldapNestedGroups | 1 | + | useMemberOfToDetectMembership | 1 | + And As an "admin" + And sending "GET" to "/cloud/groups" + Then the OCS status code should be "200" + And the "groups" result should match + | 2000 | 1 | + | 3000 | 1 | + | 3001 | 1 | + | 3002 | 1 | + +Scenario: Test LDAP group membership with intermediate groups not matching filter, numeric group ids + Given modify LDAP configuration + | ldapBaseGroups | ou=NumericGroups,dc=nextcloud,dc=ci | + | ldapGroupFilter | (&(cn=2000)(objectclass=groupOfNames)) | + | ldapNestedGroups | 1 | + | useMemberOfToDetectMembership | 1 | + | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + And As an "admin" + # for population + And sending "GET" to "/cloud/groups" + And sending "GET" to "/cloud/groups/2000/users" + Then the OCS status code should be "200" + And the "users" result should match + | 92379 | 0 | + | 54172 | 1 | + | 50194 | 1 | + | 59376 | 1 | + | 59463 | 1 | From dfc700724248006048fa83839b5ae7f2df53168b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 5 Mar 2019 12:53:13 +0100 Subject: [PATCH 6/8] with LDAP server set offline, config cannot be controlled via ocs anymore Signed-off-by: Arthur Schiwon --- build/integration/features/bootstrap/LDAPContext.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/build/integration/features/bootstrap/LDAPContext.php b/build/integration/features/bootstrap/LDAPContext.php index ee7acab6f5f7a..2ad737bf8b864 100644 --- a/build/integration/features/bootstrap/LDAPContext.php +++ b/build/integration/features/bootstrap/LDAPContext.php @@ -27,6 +27,7 @@ class LDAPContext implements Context { use BasicStructure; + use CommandLine; protected $configID; @@ -37,6 +38,8 @@ public function teardown() { if($this->configID === null) { return; } + $this->disableLDAPConfiguration(); # via occ in case of big config issues + $this->asAn('admin'); $this->sendingTo('DELETE', $this->apiUrl . '/' . $this->configID); } @@ -196,4 +199,9 @@ public function theRecordFieldsShouldMatch(TableNode $expectations) { $backend = (string)simplexml_load_string($this->response->getBody())->data[0]->backend; Assert::assertEquals('LDAP', $backend); } + + public function disableLDAPConfiguration() { + $configKey = $this->configID . 'ldap_configuration_active'; + $this->invokingTheCommand('config:app:set user_ldap ' . $configKey . ' --value="0"'); + } } From 987db8e6c56ace702b7bfbfd76b661ae79df5c45 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 6 Mar 2019 00:09:23 +0100 Subject: [PATCH 7/8] add missing config bits to integration tests Signed-off-by: Arthur Schiwon --- build/integration/ldap_features/ldap-openldap.feature | 6 ++++++ .../integration/ldap_features/openldap-numerical-id.feature | 1 + 2 files changed, 7 insertions(+) diff --git a/build/integration/ldap_features/ldap-openldap.feature b/build/integration/ldap_features/ldap-openldap.feature index 2e1f637a50a4c..6c5ed8b462baa 100644 --- a/build/integration/ldap_features/ldap-openldap.feature +++ b/build/integration/ldap_features/ldap-openldap.feature @@ -110,6 +110,8 @@ Feature: LDAP | ldapNestedGroups | 1 | | useMemberOfToDetectMembership | 1 | | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + | ldapExpertUsernameAttr | uid | + | ldapGroupMemberAssocAttr | member | And As an "admin" # for population And sending "GET" to "/cloud/groups" @@ -129,6 +131,8 @@ Feature: LDAP | ldapNestedGroups | 1 | | useMemberOfToDetectMembership | 0 | | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + | ldapExpertUsernameAttr | uid | + | ldapGroupMemberAssocAttr | member | And As an "admin" # for population And sending "GET" to "/cloud/groups" @@ -148,6 +152,8 @@ Feature: LDAP | ldapNestedGroups | 1 | | useMemberOfToDetectMembership | 1 | | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + | ldapExpertUsernameAttr | uid | + | ldapGroupMemberAssocAttr | member | And As an "admin" # for population And sending "GET" to "/cloud/groups" diff --git a/build/integration/ldap_features/openldap-numerical-id.feature b/build/integration/ldap_features/openldap-numerical-id.feature index 4959c7328e651..4112df0ae1a6e 100644 --- a/build/integration/ldap_features/openldap-numerical-id.feature +++ b/build/integration/ldap_features/openldap-numerical-id.feature @@ -53,6 +53,7 @@ Scenario: Test LDAP group membership with intermediate groups not matching filte | ldapNestedGroups | 1 | | useMemberOfToDetectMembership | 1 | | ldapUserFilter | (&(objectclass=inetorgperson)(!(uid=alice))) | + | ldapGroupMemberAssocAttr | member | And As an "admin" # for population And sending "GET" to "/cloud/groups" From e36cede9947e47dac15e9b1d5643dd613085c1c3 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 6 Mar 2019 00:34:29 +0100 Subject: [PATCH 8/8] remove unused use statement Signed-off-by: Arthur Schiwon --- apps/user_ldap/tests/Group_LDAPTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index 870dddf1bd8b0..cb5760e3171dc 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -39,7 +39,6 @@ use OCA\User_LDAP\Group_LDAP as GroupLDAP; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\User\Manager; -use function SebastianBergmann\GlobalState\functions; use Test\TestCase; /**