From 43f451e9e0e92c99285a471209e6645ff0b0eed9 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Tue, 25 Apr 2017 15:07:05 +0200
Subject: [PATCH] Fix usersInGroup retrieval

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 apps/user_ldap/lib/Group_LDAP.php       | 16 ++---------
 apps/user_ldap/tests/Group_LDAPTest.php | 37 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
index 5a209a3317..60ce664684 100644
--- a/apps/user_ldap/lib/Group_LDAP.php
+++ b/apps/user_ldap/lib/Group_LDAP.php
@@ -804,16 +804,9 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface {
 		}
 
 		$primaryUsers = $this->getUsersInPrimaryGroup($groupDN, $search, $limit, $offset);
-		$members = array_keys($this->_groupMembers($groupDN));
-		if(!$members && empty($primaryUsers)) {
-			//in case users could not be retrieved, return empty result set
-			$this->access->connection->writeToCache($cacheKey, array());
-			return array();
-		}
-
 		$posixGroupUsers = $this->getUsersInGidNumber($groupDN, $search, $limit, $offset);
 		$members = array_keys($this->_groupMembers($groupDN));
-		if(!$members && empty($posixGroupUsers)) {
+		if(!$members && empty($posixGroupUsers) && empty($primaryUsers)) {
 			//in case users could not be retrieved, return empty result set
 			$this->access->connection->writeToCache($cacheKey, []);
 			return [];
@@ -850,12 +843,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface {
 			}
 		}
 
-		$groupUsers = array_unique(array_merge($groupUsers, $primaryUsers));
-		natsort($groupUsers);
-		$this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers);
-		$groupUsers = array_slice($groupUsers, $offset, $limit);
-
-		$groupUsers = array_unique(array_merge($groupUsers, $posixGroupUsers));
+		$groupUsers = array_unique(array_merge($groupUsers, $primaryUsers, $posixGroupUsers));
 		natsort($groupUsers);
 		$this->access->connection->writeToCache('usersInGroup-'.$gid.'-'.$search, $groupUsers);
 		$groupUsers = array_slice($groupUsers, $offset, $limit);
diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php
index fc13b8a35e..9b5216742f 100644
--- a/apps/user_ldap/tests/Group_LDAPTest.php
+++ b/apps/user_ldap/tests/Group_LDAPTest.php
@@ -434,6 +434,43 @@ class Group_LDAPTest extends \Test\TestCase {
 		$access = $this->getAccessMock();
 		$this->enableGroups($access);
 
+		$access->connection->expects($this->any())
+			->method('getFromCache')
+			->will($this->returnValue(null));
+
+		$access->expects($this->any())
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn, $attr) {
+				if($attr === 'primaryGroupToken') {
+					return array(1337);
+				} else if($attr === 'gidNumber') {
+					return [4211];
+				}
+				return array();
+			}));
+
+		$access->expects($this->any())
+			->method('groupname2dn')
+			->will($this->returnValue('cn=foobar,dc=foo,dc=bar'));
+
+		$access->expects($this->exactly(2))
+			->method('nextcloudUserNames')
+			->willReturnOnConsecutiveCalls(['lisa', 'bart', 'kira', 'brad'], ['walle', 'dino', 'xenia']);
+
+		$groupBackend = new GroupLDAP($access);
+		$users = $groupBackend->usersInGroup('foobar');
+
+		$this->assertSame(7, count($users));
+	}
+
+	/**
+	 * tests that a user listing is complete, if all it's members have the group
+	 * as their primary.
+	 */
+	public function testUsersInGroupPrimaryAndUnixMembers() {
+		$access = $this->getAccessMock();
+		$this->enableGroups($access);
+
 		$access->connection->expects($this->any())
 			->method('getFromCache')
 			->will($this->returnValue(null));