From ebb565f7a87109c9cef7a0d7a79eae100ce11bef Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 12 Aug 2020 00:34:15 +0200 Subject: [PATCH] do not flip available state to unavailable, allow empty results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the detection relies that the first, requested result is not empty - it might be empty though – groups without members - protect switching from available to unavailable - switching the other way around was also not envisaged either Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Connection.php | 1 + apps/user_ldap/lib/Group_LDAP.php | 31 ++++++++++++++++--------- apps/user_ldap/tests/Group_LDAPTest.php | 22 ++++++++++++++++-- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 0cc93a08e6..f4ead2a383 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -67,6 +67,7 @@ use OCP\ILogger; * @property string[] ldapBaseGroups * @property string ldapGroupFilter * @property string ldapGroupDisplayName + * @property string ldapMatchingRuleInChainState */ 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 eab2752af5..54a8eaf08a 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -243,21 +243,27 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD ) { $attemptedLdapMatchingRuleInChain = true; // compatibility hack with servers supporting :1.2.840.113556.1.4.1941:, and others) - $filter = $this->access->combineFilterWithAnd([$this->access->connection->ldapUserFilter, 'memberof:1.2.840.113556.1.4.1941:=' . $dnGroup]); + $filter = $this->access->combineFilterWithAnd([ + $this->access->connection->ldapUserFilter, + $this->access->connection->ldapUserDisplayName . '=*', + 'memberof:1.2.840.113556.1.4.1941:=' . $dnGroup + ]); $memberRecords = $this->access->fetchListOfUsers( $filter, $this->access->userManager->getAttributes(true) ); - if (!empty($memberRecords)) { - if ($this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN) { - $this->access->connection->ldapMatchingRuleInChainState = Configuration::LDAP_SERVER_FEATURE_AVAILABLE; - $this->access->connection->saveConfiguration(); - } - return array_reduce($memberRecords, function ($carry, $record) { - $carry[] = $record['dn'][0]; - return $carry; - }, []); + $result = array_reduce($memberRecords, function ($carry, $record) { + $carry[] = $record['dn'][0]; + return $carry; + }, []); + if ($this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_AVAILABLE) { + return $result; + } elseif (!empty($memberRecords)) { + $this->access->connection->ldapMatchingRuleInChainState = Configuration::LDAP_SERVER_FEATURE_AVAILABLE; + $this->access->connection->saveConfiguration(); + return $result; } + // when feature availability is unknown, and the result is empty, continue and test with original approach } $seen[$dnGroup] = 1; @@ -272,7 +278,10 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD $allMembers += $this->getDynamicGroupMembers($dnGroup); $this->access->connection->writeToCache($cacheKey, $allMembers); - if (isset($attemptedLdapMatchingRuleInChain) && !empty($allMembers)) { + if (isset($attemptedLdapMatchingRuleInChain) + && $this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN + && !empty($allMembers) + ) { $this->access->connection->ldapMatchingRuleInChainState = Configuration::LDAP_SERVER_FEATURE_UNAVAILABLE; $this->access->connection->saveConfiguration(); } diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php index c03c1b01d3..1d6abeb7e8 100644 --- a/apps/user_ldap/tests/Group_LDAPTest.php +++ b/apps/user_ldap/tests/Group_LDAPTest.php @@ -129,6 +129,10 @@ class Group_LDAPTest extends TestCase { ->method('countUsers') ->willReturn(2); + $access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['displayName', 'mail']); + $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->countUsersInGroup('group'); @@ -169,6 +173,10 @@ class Group_LDAPTest extends TestCase { ->method('isDNPartOfBase') ->willReturn(true); + $access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['displayName', 'mail']); + $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->countUsersInGroup('group', '3'); @@ -543,7 +551,10 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('combineFilterWithAnd') ->willReturn('pseudo=filter'); - $access->userManager = $this->createMock(Manager::class); + + $access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['displayName', 'mail']); $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->usersInGroup('foobar'); @@ -584,7 +595,10 @@ class Group_LDAPTest extends TestCase { $access->expects($this->any()) ->method('combineFilterWithAnd') ->willReturn('pseudo=filter'); - $access->userManager = $this->createMock(Manager::class); + + $access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['displayName', 'mail']); $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->usersInGroup('foobar'); @@ -624,6 +638,10 @@ class Group_LDAPTest extends TestCase { ->method('isDNPartOfBase') ->willReturn(true); + $access->userManager->expects($this->any()) + ->method('getAttributes') + ->willReturn(['displayName', 'mail']); + $groupBackend = new GroupLDAP($access, $pluginManager); $users = $groupBackend->countUsersInGroup('foobar');