fix nested group retrieval also for 2 other cases

and also consolidate logic in one method

Signed-off-by: Arthur Schiwon <[email protected]>

Author: blizzz

apps/user_ldap/lib/Connection.php

@@ -62,6 +62,9 @@
  * @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;

apps/user_ldap/lib/Group_LDAP.php

@@ -218,12 +218,12 @@ public function getDynamicGroupMembers($dnGroup) {
 	 */
 	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;
@@ -232,19 +232,12 @@ private function _groupMembers($dnGroup, &$seen = null) {
 			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);
@@ -257,42 +250,69 @@ private function _groupMembers($dnGroup, &$seen = null) {
 	 * @param string $DN
 	 * @param array|null &$seen
 	 * @return array
+	 * @throws \OC\ServerNotAvailableException
 	 */
 	private function _getGroupDNsFromMemberOf($DN) {
 		$groups = $this->access->readAttribute($DN, 'memberOf');
 		if (!is_array($groups)) {
-			return array();
+			return [];
 		}
-		$nestedGroups = (int) $this->access->connection->ldapNestedGroups;
-		if ($nestedGroups === 1) {
-			$seen = array();
-			while ($group = array_pop($groups)) {
-				if ($group === $DN || array_key_exists($group, $seen)) {
-					// Prevent loops
-					continue;
-				}
-				$seen[$group] = 1;
 
-				// Resolve nested groups
-				if (isset($cachedNestedGroups[$group])) {
-					$nestedGroups = $cachedNestedGroups[$group];
-				} else {
-					$nestedGroups = $this->access->readAttribute($group, 'memberOf');
-					if (!is_array($nestedGroups)) {
-						$nestedGroups = [];
-					}
-					$cachedNestedGroups[$group] = $nestedGroups;
-				}
-				foreach ($nestedGroups as $nestedGroup) {
-					array_push($groups, $nestedGroup);
+		$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;
 			}
-			// Get unique group DN's from those we have visited in the loop
-			$groups = array_keys($seen);
-		}
+			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 {
+		$nestedRecords = (int) $this->access->connection->ldapNestedGroups;
+		if ($nestedRecords !== 1) {
+			return $list;
+		}
+		$seen = [];
+		$recordMode = false;
+		while ($record = array_pop($list)) {
+			if(is_string($record)) {
+				$recordDN = $record;
+			} else if(is_array($record) && isset($record['dn'][0])) {
+				$recordDN = $record['dn'][0];
+				$recordMode = true;
+			}
+			if(!isset($recordDN)) {
+				throw new \InvalidArgumentException('Supplied list is incompatible');
+			}
+			if ($recordDN === $dn || array_key_exists($recordDN, $seen)) {
+				// Prevent loops
+				continue;
+			}
+			$fetched = $fetcher($record, $seen);
+			$list = array_merge($list, $fetched);
+			$seen[$recordDN] = $record;
+		}
+		// Get unique group DN's from those we have visited in the loop
+		if($recordMode) {
+			return($seen);
+		}
+		return array_keys($seen);
+	}
+
 	/**
 	 * translates a gidNumber into an ownCloud internal name
 	 * @param string $gid as given by gidNumber on POSIX LDAP
@@ -753,34 +773,28 @@ public function getUserGroups($uid) {
 	 */
 	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($allGroups, array_flip($visibleGroups));
 	}
 
 	/**
@@ -827,7 +841,7 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
 
 		$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, []);