Merge pull request #24402 from nextcloud/fix/24252/ldap-ingroup-memberid

LDAP: fix inGroup for memberUid type of group memberships

1 files changed, 22 insertions, 11 deletions

diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
index 5444f0815e3..be2fbecad85 100644
--- a/apps/user_ldap/lib/Group_LDAP.php
+++ b/apps/user_ldap/lib/Group_LDAP.php
@@ -55,7 +55,7 @@ use OCP\ILogger;
 class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend {
 	protected $enabled = false;
 
-	/** @var string[] $cachedGroupMembers array of users with gid as key */
+	/** @var string[][] $cachedGroupMembers array of users with gid as key */
 	protected $cachedGroupMembers;
 	/** @var string[] $cachedGroupsByMember array of groups with uid as key */
 	protected $cachedGroupsByMember;
@@ -136,17 +136,13 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
 
 		//usually, LDAP attributes are said to be case insensitive. But there are exceptions of course.
 		$members = $this->_groupMembers($groupDN);
-		if (!is_array($members) || count($members) === 0) {
-			$this->access->connection->writeToCache($cacheKey, false);
-			return false;
-		}
 
 		//extra work if we don't get back user DNs
 		switch ($this->ldapGroupMemberAssocAttr) {
 			case 'memberuid':
 			case 'zimbramailforwardingaddress':
 				$requestAttributes = $this->access->userManager->getAttributes(true);
-				$dns = [];
+				$users = [];
 				$filterParts = [];
 				$bytes = 0;
 				foreach ($members as $mid) {
@@ -160,22 +156,37 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
 					if ($bytes >= 9000000) {
 						// AD has a default input buffer of 10 MB, we do not want
 						// to take even the chance to exceed it
+						// so we fetch results with the filterParts we collected so far
 						$filter = $this->access->combineFilterWithOr($filterParts);
-						$users = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts));
+						$search = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts));
 						$bytes = 0;
 						$filterParts = [];
-						$dns = array_merge($dns, $users);
+						$users = array_merge($users, $search);
 					}
 				}
+
 				if (count($filterParts) > 0) {
+					// if there are filterParts left we need to add their result
 					$filter = $this->access->combineFilterWithOr($filterParts);
-					$users = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts));
-					$dns = array_merge($dns, $users);
+					$search = $this->access->fetchListOfUsers($filter, $requestAttributes, count($filterParts));
+					$users = array_merge($users, $search);
 				}
-				$members = $dns;
+
+				// now we cleanup the users array to get only dns
+				$dns = [];
+				foreach ($users as $record) {
+					$dns[$record['dn'][0]] = 1;
+				}
+				$members = array_keys($dns);
+
 				break;
 		}
 
+		if (count($members) === 0) {
+			$this->access->connection->writeToCache($cacheKey, false);
+			return false;
+		}
+
 		$isInGroup = in_array($userDN, $members);
 		$this->access->connection->writeToCache($cacheKey, $isInGroup);
 		$this->access->connection->writeToCache($cacheKeyMembers, $members);