consolidate groupsMatchFilter in groupsExist

- less duplication
- profiting of the same cache entry

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

2 files changed, 25 insertions, 44 deletions

diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index 1b3a97cef95..c087211cec7 100644
--- a/apps/user_ldap/lib/Access.php
+++ b/apps/user_ldap/lib/Access.php
@@ -471,45 +471,6 @@ class Access extends LDAPUtility {
 	}
 
 	/**
-	 * accepts an array of group DNs and tests whether they match the user
-	 * filter by doing read operations against the group entries. Returns an
-	 * array of DNs that match the filter.
-	 *
-	 * @param string[] $groupDNs
-	 * @return string[]
-	 * @throws ServerNotAvailableException
-	 */
-	public function groupsMatchFilter($groupDNs) {
-		$validGroupDNs = [];
-		foreach ($groupDNs as $dn) {
-			$cacheKey = 'groupsMatchFilter-'.$dn;
-			$groupMatchFilter = $this->connection->getFromCache($cacheKey);
-			if (!is_null($groupMatchFilter)) {
-				if ($groupMatchFilter) {
-					$validGroupDNs[] = $dn;
-				}
-				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, '', $this->connection->ldapGroupFilter);
-			if (is_array($result)) {
-				$this->connection->writeToCache($cacheKey, true);
-				$validGroupDNs[] = $dn;
-			} else {
-				$this->connection->writeToCache($cacheKey, false);
-			}
-		}
-		return $validGroupDNs;
-	}
-
-	/**
 	 * returns the internal Nextcloud name for the given LDAP DN of the user, false on DN outside of search DN or failure
 	 *
 	 * @param string $dn the dn of the user object
diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
index 85d9e38e03e..95bcbf50f8c 100644
--- a/apps/user_ldap/lib/Group_LDAP.php
+++ b/apps/user_ldap/lib/Group_LDAP.php
@@ -274,7 +274,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		};
 
 		$groups = $this->walkNestedGroups($DN, $fetcher, $groups);
-		return $this->access->groupsMatchFilter($groups);
+		return $this->filterValidGroups($groups);
 	}
 
 	/**
@@ -791,7 +791,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		$seen[$dn] = true;
 		$filter = $this->access->connection->ldapGroupMemberAssocAttr.'='.$dn;
 		$groups = $this->access->fetchListOfGroups($filter,
-			[$this->access->connection->ldapGroupDisplayName, 'dn']);
+			[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
 		if (is_array($groups)) {
 			$fetcher = function ($dn, &$seen) {
 				if (is_array($dn) && isset($dn['dn'][0])) {
@@ -801,8 +801,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			};
 			$allGroups = $this->walkNestedGroups($dn, $fetcher, $groups);
 		}
-		$visibleGroups = $this->access->groupsMatchFilter(array_keys($allGroups));
-		return array_intersect_key($allGroups, array_flip($visibleGroups));
+		$visibleGroups = $this->filterValidGroups($allGroups);
+		return array_intersect_key($allGroups, $visibleGroups);
 	}
 
 	/**
@@ -1117,8 +1117,13 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 			return false;
 		}
 
+		if(!$this->access->isDNPartOfBase($dn, $this->access->connection->ldapBaseGroups)) {
+			$this->access->connection->writeToCache('groupExists'.$gid, false);
+			return false;
+		}
+
 		//if group really still exists, we will be able to read its objectclass
-		if (!is_array($this->access->readAttribute($dn, ''))) {
+		if (!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapGroupFilter))) {
 			$this->access->connection->writeToCache('groupExists'.$gid, false);
 			return false;
 		}
@@ -1127,6 +1132,21 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD
 		return true;
 	}
 
+	protected function filterValidGroups (array $listOfGroups): array {
+		$validGroupDNs = [];
+		foreach($listOfGroups as $key => $item) {
+			$dn = is_string($item) ? $item : $item['dn'][0];
+			$gid = $this->access->dn2groupname($dn);
+			if(!$gid) {
+				continue;
+			}
+			if($this->groupExists($gid)) {
+				$validGroupDNs[$key] = $item;
+			}
+		}
+		return $validGroupDNs;
+	}
+
 	/**
 	 * Check if backend implements actions
 	 * @param int $actions bitwise-or'ed actions