diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 8f56e01bf3..4c5c01743a 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 5bcd595307..d91f1503ab 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); + } + }