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 2240c2ad22..b16cb95302 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -265,7 +265,6 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD 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 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $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 b16cb95302..427245d4a0 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -252,28 +252,33 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @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 427245d4a0..2e3bc0b4a5 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 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $this->cachedGroupMembers = new CappedMemoryCache(); $this->cachedGroupsByMember = new CappedMemoryCache(); + $this->cachedNestedGroups = new CappedMemoryCache(); $this->groupPluginManager = $groupPluginManager; } @@ -257,8 +263,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD 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 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $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 ec8d888e2a..0c5a06144a 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -625,7 +625,7 @@ class Group_LDAPTest extends TestCase { ->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 | 138 ++++++++++-------- apps/user_ldap/tests/Group_LDAPTest.php | 56 ++++--- .../ldap_features/ldap-openldap.feature | 58 ++++++++ .../openldap-numerical-id.feature | 35 +++++ 6 files changed, 206 insertions(+), 86 deletions(-) diff --git a/.drone.yml b/.drone.yml index e3cfc2d7be..f821333ee9 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 ba393dffc1..4335f8e439 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -62,6 +62,9 @@ use OCP\ILogger; * @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 2e3bc0b4a5..1658807c0d 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -218,12 +218,12 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD */ 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 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD 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,42 +250,71 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD * @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; - } - $seen[$group] = 1; - // Resolve nested groups - 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); + $fetcher = function($groupDN) { + if (isset($this->cachedNestedGroups[$groupDN])) { + $nestedGroups = $this->cachedNestedGroups[$groupDN]; + } else { + $nestedGroups = $this->access->readAttribute($groupDN, 'memberOf'); + if (!is_array($nestedGroups)) { + $nestedGroups = []; } + $this->cachedNestedGroups[$groupDN] = $nestedGroups; } - // Get unique group DN's from those we have visited in the loop - $groups = array_keys($seen); - } + return $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; + } + return $transformed; + }, []); + } + return $list; + } + + $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); + } + /** * translates a gidNumber into an ownCloud internal name * @param string $gid as given by gidNumber on POSIX LDAP @@ -753,34 +775,28 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD */ 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 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $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 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD 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 0c5a06144a..870dddf1bd 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\Connection; 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 @@ class Group_LDAPTest extends TestCase { 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 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('fetchListOfUsers') - ->will($this->returnValue(array())); + ->will($this->returnValue([])); $access->expects($this->any()) ->method('readAttribute') @@ -145,7 +157,7 @@ class Group_LDAPTest extends TestCase { 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 @@ class Group_LDAPTest extends TestCase { $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 @@ class Group_LDAPTest extends TestCase { $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 @@ class Group_LDAPTest extends TestCase { ->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 @@ class Group_LDAPTest extends TestCase { $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 @@ class Group_LDAPTest extends TestCase { $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 4b0b02c5b4..2e1f637a50 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 2d87ba33e6..4959c7328e 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 ee7acab6f5..2ad737bf8b 100644 --- a/build/integration/features/bootstrap/LDAPContext.php +++ b/build/integration/features/bootstrap/LDAPContext.php @@ -27,6 +27,7 @@ use PHPUnit\Framework\Assert; class LDAPContext implements Context { use BasicStructure; + use CommandLine; protected $configID; @@ -37,6 +38,8 @@ class LDAPContext implements Context { 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 @@ class LDAPContext implements Context { $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 2e1f637a50..6c5ed8b462 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 4959c7328e..4112df0ae1 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 870dddf1bd..cb5760e317 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\Connection; 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; /**