diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index a7a90c7583..24695f64fa 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -382,7 +382,12 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { if (is_array($groupDNs)) { $groupDNs = $this->access->groupsMatchFilter($groupDNs); foreach ($groupDNs as $dn) { - $groups[] = $this->access->dn2groupname($dn); + $groupName = $this->access->dn2groupname($dn); + if(is_string($groupName)) { + // be sure to never return false if the dn could not be + // resolved to a name, for whatever reason. + $groups[] = $groupName; + } } } if($primaryGroup !== false) { diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index b201220d72..44237b5239 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -365,10 +365,21 @@ class Access extends LDAPUtility implements user\IUserTools { continue; } + // Check the base DN first. If this is not met already, we don't + // need to ask the server at all. + if(!$this->isDNPartOfBase($dn, $this->connection->ldapBaseGroups)) { + $this->connection->writeToCache($cacheKey, false); + continue; + } + $result = $this->readAttribute($dn, 'cn', $this->connection->ldapGroupFilter); if(is_array($result)) { + $this->connection->writeToCache($cacheKey, true); $validGroupDNs[] = $dn; + } else { + $this->connection->writeToCache($cacheKey, false); } + } return $validGroupDNs; } diff --git a/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php b/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php index 6560153bb6..92035d94b4 100644 --- a/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php +++ b/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php @@ -43,6 +43,7 @@ class IntegrationTestAccessGroupsMatchFilter { public function init() { require('setup-scripts/createExplicitUsers.php'); require('setup-scripts/createExplicitGroups.php'); + require('setup-scripts/createExplicitGroupsDifferentOU.php'); $this->initLDAPWrapper(); $this->initConnection(); @@ -55,7 +56,7 @@ class IntegrationTestAccessGroupsMatchFilter { * If a test failed, the script is exited with return code 1. */ public function run() { - $cases = ['case1', 'case2']; + $cases = ['case1', 'case2', 'case3']; foreach ($cases as $case) { print("running $case " . PHP_EOL); @@ -106,6 +107,30 @@ class IntegrationTestAccessGroupsMatchFilter { return $status; } + /** + * Tests whether a filter for limited groups is effective when more existing + * groups were passed for validation. + * + * @return bool + */ + private function case3() { + $this->connection->setConfiguration(['ldapGroupFilter' => '(objectclass=groupOfNames)']); + + $dns = [ + 'cn=RedGroup,ou=Groups,' . $this->base, + 'cn=PurpleGroup,ou=Groups,' . $this->base, + 'cn=SquaredCircleGroup,ou=SpecialGroups,' . $this->base + ]; + $result = $this->access->groupsMatchFilter($dns); + + $status = + count($result) === 2 + && in_array('cn=RedGroup,ou=Groups,' . $this->base, $result) + && in_array('cn=PurpleGroup,ou=Groups,' . $this->base, $result); + + return $status; + } + /** * initializes the Access test instance */ @@ -129,6 +154,7 @@ class IntegrationTestAccessGroupsMatchFilter { 'ldapHost' => $this->server['host'], 'ldapPort' => $this->server['port'], 'ldapBase' => $this->base, + 'ldapBaseGroups' => 'ou=Groups,' . $this->base, 'ldapAgentName' => $this->server['dn'], 'ldapAgentPassword' => $this->server['pwd'], 'ldapUserFilter' => 'objectclass=inetOrgPerson', diff --git a/apps/user_ldap/tests/integration/setup-scripts/createExplicitGroupsDifferentOU.php b/apps/user_ldap/tests/integration/setup-scripts/createExplicitGroupsDifferentOU.php new file mode 100644 index 0000000000..361881969c --- /dev/null +++ b/apps/user_ldap/tests/integration/setup-scripts/createExplicitGroupsDifferentOU.php @@ -0,0 +1,52 @@ +