Merge pull request #25845 from nextcloud/backport/25757/stable19

[stable19] fix detecting cyclic group memberships

apps/user_ldap/lib/Group_LDAP.php

@@ -43,6 +43,7 @@
 
 namespace OCA\User_LDAP;
 
+use Closure;
 use OC\Cache\CappedMemoryCache;
 use OCP\Group\Backend\IGetDisplayNameBackend;
 use OCP\GroupInterface;
@@ -223,6 +224,9 @@ public function getDynamicGroupMembers($dnGroup) {
 	private function _groupMembers($dnGroup, &$seen = null) {
 		if ($seen === null) {
 			$seen = [];
+			// the root entry has to be marked as processed to avoind infinit loops,
+			// but not included in the results laters on
+			$excludeFromResult = $dnGroup;
 		}
 		$allMembers = [];
 		if (array_key_exists($dnGroup, $seen)) {
@@ -269,13 +273,19 @@ private function _groupMembers($dnGroup, &$seen = null) {
 		$seen[$dnGroup] = 1;
 		$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
 		if (is_array($members)) {
-			$fetcher = function ($memberDN, &$seen) {
+			$fetcher = function ($memberDN) use (&$seen) {
 				return $this->_groupMembers($memberDN, $seen);
 			};
-			$allMembers = $this->walkNestedGroups($dnGroup, $fetcher, $members);
+			$allMembers = $this->walkNestedGroups($dnGroup, $fetcher, $members, $seen);
 		}
 
 		$allMembers += $this->getDynamicGroupMembers($dnGroup);
+		if (isset($excludeFromResult)) {
+			$index = array_search($excludeFromResult, $allMembers, true);
+			if ($index !== false) {
+				unset($allMembers[$index]);
+			}
+		}
 
 		$this->access->connection->writeToCache($cacheKey, $allMembers);
 		if (isset($attemptedLdapMatchingRuleInChain)
@@ -317,13 +327,7 @@ private function _getGroupDNsFromMemberOf($DN) {
 		return $this->filterValidGroups($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 {
+	private function walkNestedGroups(string $dn, Closure $fetcher, array $list, array &$seen = []): 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.
@@ -342,19 +346,21 @@ private function walkNestedGroups(string $dn, \Closure $fetcher, array $list): a
 			return $list;
 		}
 
-		$seen = [];
 		while ($record = array_shift($list)) {
-			$recordDN = $recordMode ? $record['dn'][0] : $record;
+			$recordDN = $record['dn'][0] ?? $record;
 			if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
 				// Prevent loops
 				continue;
 			}
-			$fetched = $fetcher($record, $seen);
+			$fetched = $fetcher($record);
 			$list = array_merge($list, $fetched);
-			$seen[$recordDN] = $record;
+			if (!isset($seen[$recordDN]) || is_bool($seen[$recordDN]) && is_array($record)) {
+				$seen[$recordDN] = $record;
+			}
 		}
 
-		return $recordMode ? $seen : array_keys($seen);
+		// on record mode, filter out intermediate state
+		return $recordMode ? array_filter($seen, 'is_array') : array_keys($seen);
 	}
 
 	/**
@@ -850,13 +856,13 @@ private function getGroupsByMember($dn, &$seen = null) {
 		$groups = $this->access->fetchListOfGroups($filter,
 			[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
 		if (is_array($groups)) {
-			$fetcher = function ($dn, &$seen) {
+			$fetcher = function ($dn) use (&$seen) {
 				if (is_array($dn) && isset($dn['dn'][0])) {
 					$dn = $dn['dn'][0];
 				}
 				return $this->getGroupsByMember($dn, $seen);
 			};
-			$allGroups = $this->walkNestedGroups($dn, $fetcher, $groups);
+			$allGroups = $this->walkNestedGroups($dn, $fetcher, $groups, $seen);
 		}
 		$visibleGroups = $this->filterValidGroups($allGroups);
 		return array_intersect_key($allGroups, $visibleGroups);