aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--.drone.yml2
-rw-r--r--apps/user_ldap/lib/Connection.php3
-rw-r--r--apps/user_ldap/lib/Group_LDAP.php140
-rw-r--r--apps/user_ldap/tests/Group_LDAPTest.php57
-rw-r--r--build/integration/features/bootstrap/LDAPContext.php8
-rw-r--r--build/integration/ldap_features/ldap-openldap.feature64
-rw-r--r--build/integration/ldap_features/openldap-numerical-id.feature36
7 files changed, 230 insertions, 80 deletions
diff --git a/.drone.yml b/.drone.yml
index e3cfc2d7be5..f821333ee91 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 ba393dffc12..4335f8e4397 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 2240c2ad229..1658807c0dd 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;
}
@@ -212,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;
@@ -226,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);
@@ -251,30 +250,69 @@ 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, &$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();
+ return [];
+ }
+
+ $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;
+ }
+ 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;
}
- $groups = $this->access->groupsMatchFilter($groups);
- $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 = [];
+ 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 $allGroups;
+
+ return $recordMode ? $seen : array_keys($seen);
}
/**
@@ -737,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));
}
/**
@@ -811,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, []);
@@ -886,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 ec8d888e2a0..cb5760e3171 100644
--- a/apps/user_ldap/tests/Group_LDAPTest.php
+++ b/apps/user_ldap/tests/Group_LDAPTest.php
@@ -98,16 +98,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 +143,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 +156,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())
@@ -625,7 +636,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));
@@ -659,14 +670,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 +692,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 +731,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 +1016,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 +1054,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/features/bootstrap/LDAPContext.php b/build/integration/features/bootstrap/LDAPContext.php
index ee7acab6f5f..2ad737bf8b8 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"');
+ }
}
diff --git a/build/integration/ldap_features/ldap-openldap.feature b/build/integration/ldap_features/ldap-openldap.feature
index 4b0b02c5b4f..6c5ed8b462b 100644
--- a/build/integration/ldap_features/ldap-openldap.feature
+++ b/build/integration/ldap_features/ldap-openldap.feature
@@ -102,3 +102,67 @@ 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))) |
+ | ldapExpertUsernameAttr | uid |
+ | ldapGroupMemberAssocAttr | member |
+ 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))) |
+ | ldapExpertUsernameAttr | uid |
+ | ldapGroupMemberAssocAttr | member |
+ 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))) |
+ | ldapExpertUsernameAttr | uid |
+ | ldapGroupMemberAssocAttr | member |
+ 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 2d87ba33e6e..4112df0ae1a 100644
--- a/build/integration/ldap_features/openldap-numerical-id.feature
+++ b/build/integration/ldap_features/openldap-numerical-id.feature
@@ -29,3 +29,39 @@ 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))) |
+ | ldapGroupMemberAssocAttr | member |
+ 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 |
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-28 11:44:53 +0000