Merge pull request #15606 from owncloud/fix-group-list-and-count-primary-group-only

Fixes returns of group memberships and counting if all members have the ...

2 files changed, 78 insertions, 9 deletions

diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php
index 8f56e01bf3d..4c5c01743aa 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/apps/user_ldap/group_ldap.php
@@ -292,12 +292,13 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 	public function getUsersInPrimaryGroup($groupDN, $search = '', $limit = -1, $offset = 0) {
 		try {
 			$filter = $this->prepareFilterForUsersInPrimaryGroup($groupDN, $search);
-			return $this->access->fetchListOfUsers(
+			$users = $this->access->fetchListOfUsers(
 				$filter,
 				array($this->access->connection->ldapUserDisplayName, 'dn'),
 				$limit,
 				$offset
 			);
+			return $this->access->ownCloudUserNames($users);
 		} catch (\Exception $e) {
 			return array();
 		}
@@ -476,8 +477,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 			return array();
 		}
 
+		$primaryUsers = $this->getUsersInPrimaryGroup($groupDN, $search, $limit, $offset);
 		$members = array_keys($this->_groupMembers($groupDN));
-		if(!$members) {
+		if(!$members && empty($primaryUsers)) {
 			//in case users could not be retrieved, return empty result set
 			$this->access->connection->writeToCache($cacheKey, array());
 			return array();
@@ -514,13 +516,11 @@ 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);
 
-		//and get users that have the group as primary
-		$primaryUsers = $this->getUsersInPrimaryGroup($groupDN, $search, $limit, $offset);
-		$groupUsers = array_unique(array_merge($groupUsers, $primaryUsers));
 
 		$this->access->connection->writeToCache($cacheKey, $groupUsers);
 
@@ -551,16 +551,15 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 		}
 
 		$members = array_keys($this->_groupMembers($groupDN));
-		if(!$members) {
+		$primaryUserCount = $this->countUsersInPrimaryGroup($groupDN, '');
+		if(!$members && $primaryUserCount === 0) {
 			//in case users could not be retrieved, return empty result set
 			$this->access->connection->writeToCache($cacheKey, false);
 			return false;
 		}
 
 		if(empty($search)) {
-			$primaryUsers = $this->countUsersInPrimaryGroup($groupDN, '');
-			$groupUsers = count($members) + $primaryUsers;
-
+			$groupUsers = count($members) + $primaryUserCount;
 			$this->access->connection->writeToCache($cacheKey, $groupUsers);
 			return $groupUsers;
 		}
diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php
index 5bcd5953075..d91f1503abd 100644
--- a/apps/user_ldap/tests/group_ldap.php
+++ b/apps/user_ldap/tests/group_ldap.php
@@ -313,4 +313,74 @@ class Test_Group_Ldap extends \Test\TestCase {
 		$this->assertSame(2, count($groups));
 	}
 
+	/**
+	 * tests that a user listing is complete, if all it's members have the group
+	 * as their primary.
+	 */
+	public function  testUsersInGroupPrimaryMembersOnly() {
+		$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);
+				}
+				return array();
+			}));
+
+		$access->expects($this->any())
+			->method('groupname2dn')
+			->will($this->returnValue('cn=foobar,dc=foo,dc=bar'));
+
+		$access->expects($this->once())
+			->method('ownCloudUserNames')
+			->will($this->returnValue(array('lisa', 'bart', 'kira', 'brad')));
+
+		$groupBackend = new GroupLDAP($access);
+		$users = $groupBackend->usersInGroup('foobar');
+
+		$this->assertSame(4, count($users));
+	}
+
+	/**
+	 * tests that a user counting is complete, if all it's members have the group
+	 * as their primary.
+	 */
+	public function  testCountUsersInGroupPrimaryMembersOnly() {
+		$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);
+				}
+				return array();
+			}));
+
+		$access->expects($this->any())
+			->method('groupname2dn')
+			->will($this->returnValue('cn=foobar,dc=foo,dc=bar'));
+
+		$access->expects($this->once())
+			->method('countUsers')
+			->will($this->returnValue(4));
+
+		$groupBackend = new GroupLDAP($access);
+		$users = $groupBackend->countUsersInGroup('foobar');
+
+		$this->assertSame(4, $users);
+	}
+
 }