From 698297feb3cb8e78c0e4adb6756e0ea98da49677 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Apr 2014 18:56:14 +0200 Subject: [PATCH 1/6] add optional countUsersInGroup method to group backends --- lib/private/group/backend.php | 2 ++ lib/private/group/database.php | 14 ++++++++++++++ lib/private/group/dummy.php | 10 ++++++++++ lib/private/group/group.php | 21 +++++++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/lib/private/group/backend.php b/lib/private/group/backend.php index 2e17b5d0b7f..b0ed0d90d76 100644 --- a/lib/private/group/backend.php +++ b/lib/private/group/backend.php @@ -34,6 +34,7 @@ define('OC_GROUP_BACKEND_DELETE_GROUP', 0x00000010); define('OC_GROUP_BACKEND_ADD_TO_GROUP', 0x00000100); define('OC_GROUP_BACKEND_REMOVE_FROM_GOUP', 0x00001000); define('OC_GROUP_BACKEND_GET_DISPLAYNAME', 0x00010000); +define('OC_GROUP_BACKEND_COUNT_USERS', 0x00100000); /** * Abstract base class for user management @@ -45,6 +46,7 @@ abstract class OC_Group_Backend implements OC_Group_Interface { OC_GROUP_BACKEND_ADD_TO_GROUP => 'addToGroup', OC_GROUP_BACKEND_REMOVE_FROM_GOUP => 'removeFromGroup', OC_GROUP_BACKEND_GET_DISPLAYNAME => 'displayNamesInGroup', + OC_GROUP_BACKEND_COUNT_USERS => 'countUsersInGroup', ); /** diff --git a/lib/private/group/database.php b/lib/private/group/database.php index d0974685ff6..3815dcff2e5 100644 --- a/lib/private/group/database.php +++ b/lib/private/group/database.php @@ -211,6 +211,20 @@ class OC_Group_Database extends OC_Group_Backend { return $users; } + /** + * @brief get the number of all users matching the search string in a group + * @param string $gid + * @param string $search + * @param int $limit + * @param int $offset + * @return int | false + */ + public function countUsersInGroup($gid, $search = '') { + $stmt = OC_DB::prepare('SELECT COUNT(`uid`) AS `count` FROM `*PREFIX*group_user` WHERE `gid` = ? AND `uid` LIKE ?'); + $result = $stmt->execute(array($gid, $search.'%')); + return $result->fetchOne(); + } + /** * @brief get a list of all display names in a group * @param string $gid diff --git a/lib/private/group/dummy.php b/lib/private/group/dummy.php index da26e1b910e..94cbb607ad1 100644 --- a/lib/private/group/dummy.php +++ b/lib/private/group/dummy.php @@ -157,4 +157,14 @@ class OC_Group_Dummy extends OC_Group_Backend { } } + /** + * @brief get the number of all users in a group + * @returns int | bool + */ + public function countUsersInGroup($gid, $search = '', $limit = -1, $offset = 0) { + if(isset($this->groups[$gid])) { + return count($this->groups[$gid]); + } + } + } diff --git a/lib/private/group/group.php b/lib/private/group/group.php index 8d2aa87a788..9965d938ebb 100644 --- a/lib/private/group/group.php +++ b/lib/private/group/group.php @@ -186,6 +186,27 @@ class Group { return array_values($users); } + /** + * returns the number of users matching the search string + * + * @param string $search + * @return int | bool + */ + public function count($search) { + $users = false; + foreach ($this->backends as $backend) { + if(method_exists($backend, 'countUsersInGroup')) { + if($users === false) { + //we could directly add to a bool variable, but this would + //be ugly + $users = 0; + } + $users += $backend->countUsersInGroup($this->gid, $search); + } + } + return $users; + } + /** * search for users in the group by displayname * From 96cb75f5cf8c14f389213e1745cc3a98f8d1cbd4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Apr 2014 18:56:34 +0200 Subject: [PATCH 2/6] implement countUsersInGroup in LDAP group backend --- apps/user_ldap/group_ldap.php | 82 +++++++++++++++++++++++++++++++++- apps/user_ldap/group_proxy.php | 11 +++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 4f2424d9531..820d2c4636e 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -276,6 +276,83 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { return $groupUsers; } + /** + * @brief returns the number of users in a group, who match the search term + * @param string the internal group name + * @param string optional, a search string + * @returns int | bool + */ + public function countUsersInGroup($gid, $search = '') { + $cachekey = 'countUsersInGroup-'.$gid.'-'.$search; + if(!$this->enabled || !$this->groupExists($gid)) { + return false; + } + $groupUsers = $this->access->connection->getFromCache($cachekey); + if(!is_null($groupUsers)) { + return $groupUsers; + } + + $groupDN = $this->access->groupname2dn($gid); + if(!$groupDN) { + // group couldn't be found, return empty resultset + $this->access->connection->writeToCache($cachekey, false); + return false; + } + + $members = array_keys($this->_groupMembers($groupDN)); + if(!$members) { + //in case users could not be retrieved, return empty resultset + $this->access->connection->writeToCache($cachekey, false); + return false; + } + + if(empty($search)) { + $groupUsers = count($members); + $this->access->connection->writeToCache($cachekey, $groupUsers); + return $groupUsers; + } + $isMemberUid = + (strtolower($this->access->connection->ldapGroupMemberAssocAttr) + === 'memberuid'); + + //we need to apply the search filter + //alternatives that need to be checked: + //a) get all users by search filter and array_intersect them + //b) a, but only when less than 1k 10k ?k users like it is + //c) put all DNs|uids in a LDAP filter, combine with the search string + // and let it count. + //For now this is not important, because the only use of this method + //does not supply a search string + foreach($members as $member) { + if($isMemberUid) { + //we got uids, need to get their DNs to 'tranlsate' them to usernames + $filter = $this->access->combineFilterWithAnd(array( + \OCP\Util::mb_str_replace('%uid', $member, + $this->access->connection->ldapLoginFilter, 'UTF-8'), + $this->access->getFilterPartForUserSearch($search) + )); + $ldap_users = $this->access->fetchListOfUsers($filter, 'dn'); + if(count($ldap_users) < 1) { + continue; + } + $groupUsers[] = $this->access->dn2username($ldap_users[0]); + } else { + //we need to apply the search filter now + if(!$this->access->readAttribute($member, + $this->access->connection->ldapUserDisplayName, + $this->access->getFilterPartForUserSearch($search))) { + continue; + } + // dn2username will also check if the users belong to the allowed base + if($ocname = $this->access->dn2username($member)) { + $groupUsers[] = $ocname; + } + } + } + + return $groupUsers; + } + /** * @brief get a list of all display names in a group * @returns array with display names (value) and user ids(key) @@ -418,6 +495,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { * compared with OC_USER_BACKEND_CREATE_USER etc. */ public function implementsActions($actions) { - return (bool)(OC_GROUP_BACKEND_GET_DISPLAYNAME & $actions); + return (bool)(( + OC_GROUP_BACKEND_GET_DISPLAYNAME + | OC_GROUP_BACKEND_COUNT_USERS + ) & $actions); } } diff --git a/apps/user_ldap/group_proxy.php b/apps/user_ldap/group_proxy.php index 4404bd7fe3a..c0009736239 100644 --- a/apps/user_ldap/group_proxy.php +++ b/apps/user_ldap/group_proxy.php @@ -144,6 +144,17 @@ class Group_Proxy extends lib\Proxy implements \OCP\GroupInterface { return $users; } + /** + * @brief returns the number of users in a group, who match the search term + * @param string the internal group name + * @param string optional, a search string + * @returns int | bool + */ + public function countUsersInGroup($gid, $search = '') { + return $this->handleRequest( + $gid, 'countUsersInGroup', array($gid, $search)); + } + /** * @brief get a list of all display names in a group * @returns array with display names (value) and user ids(key) From 5f8d9b3a4e259d1f9c02b482bab37321c0d3b3dc Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Apr 2014 12:31:11 +0200 Subject: [PATCH 3/6] ask implementsAction instead of checking method_exists for easier testing --- lib/private/group/group.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/group/group.php b/lib/private/group/group.php index 9965d938ebb..a2b8a0dcbea 100644 --- a/lib/private/group/group.php +++ b/lib/private/group/group.php @@ -195,7 +195,7 @@ class Group { public function count($search) { $users = false; foreach ($this->backends as $backend) { - if(method_exists($backend, 'countUsersInGroup')) { + if($backend->implementsActions(OC_GROUP_BACKEND_COUNT_USERS)) { if($users === false) { //we could directly add to a bool variable, but this would //be ugly From 9597f4190460c54066586c0eb42ed9a8532ad90a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Apr 2014 12:32:30 +0200 Subject: [PATCH 4/6] add group tests --- tests/lib/group/group.php | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/lib/group/group.php b/tests/lib/group/group.php index 3982c00e45f..147532f9947 100644 --- a/tests/lib/group/group.php +++ b/tests/lib/group/group.php @@ -299,6 +299,68 @@ class Group extends \PHPUnit_Framework_TestCase { $this->assertEquals('user1', $user1->getUID()); } + public function testCountUsers() { + $backend1 = $this->getMock('OC_Group_Database'); + $userManager = $this->getUserManager(); + $group = new \OC\Group\Group('group1', array($backend1), $userManager); + + $backend1->expects($this->once()) + ->method('countUsersInGroup') + ->with('group1', '2') + ->will($this->returnValue(3)); + + $backend1->expects($this->any()) + ->method('implementsActions') + ->will($this->returnValue(true)); + + $users = $group->count('2'); + + $this->assertSame(3, $users); + } + + public function testCountUsersMultipleBackends() { + $backend1 = $this->getMock('OC_Group_Database'); + $backend2 = $this->getMock('OC_Group_Database'); + $userManager = $this->getUserManager(); + $group = new \OC\Group\Group('group1', array($backend1, $backend2), $userManager); + + $backend1->expects($this->once()) + ->method('countUsersInGroup') + ->with('group1', '2') + ->will($this->returnValue(3)); + $backend1->expects($this->any()) + ->method('implementsActions') + ->will($this->returnValue(true)); + + $backend2->expects($this->once()) + ->method('countUsersInGroup') + ->with('group1', '2') + ->will($this->returnValue(4)); + $backend2->expects($this->any()) + ->method('implementsActions') + ->will($this->returnValue(true)); + + $users = $group->count('2'); + + $this->assertSame(7, $users); + } + + public function testCountUsersNoMethod() { + $backend1 = $this->getMock('OC_Group_Database'); + $userManager = $this->getUserManager(); + $group = new \OC\Group\Group('group1', array($backend1), $userManager); + + $backend1->expects($this->never()) + ->method('countUsersInGroup'); + $backend1->expects($this->any()) + ->method('implementsActions') + ->will($this->returnValue(false)); + + $users = $group->count('2'); + + $this->assertSame(false, $users); + } + public function testDelete() { $backend = $this->getMock('OC_Group_Database'); $userManager = $this->getUserManager(); From 142fc5f3af6e81f5c777caa9f8d42fad36866de2 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Apr 2014 12:53:59 +0200 Subject: [PATCH 5/6] fix return value when a search string was passed to return integer instead of array --- apps/user_ldap/group_ldap.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 820d2c4636e..40d9dec1410 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -323,6 +323,7 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { // and let it count. //For now this is not important, because the only use of this method //does not supply a search string + $groupUsers = array(); foreach($members as $member) { if($isMemberUid) { //we got uids, need to get their DNs to 'tranlsate' them to usernames @@ -350,7 +351,7 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { } } - return $groupUsers; + return count($groupUsers); } /** From 240732162509d0caf71ef5f1414b2595bbe47fe9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Apr 2014 12:57:06 +0200 Subject: [PATCH 6/6] add tests for LDAP --- apps/user_ldap/tests/group_ldap.php | 115 ++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 apps/user_ldap/tests/group_ldap.php diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php new file mode 100644 index 00000000000..ecbd42319e3 --- /dev/null +++ b/apps/user_ldap/tests/group_ldap.php @@ -0,0 +1,115 @@ + +* +* This library is free software; you can redistribute it and/or +* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE +* License as published by the Free Software Foundation; either +* version 3 of the License, or any later version. +* +* This library is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU AFFERO GENERAL PUBLIC LICENSE for more details. +* +* You should have received a copy of the GNU Affero General Public +* License along with this library. If not, see . +* +*/ + +namespace OCA\user_ldap\tests; + +namespace OCA\user_ldap\tests; + +use \OCA\user_ldap\GROUP_LDAP as GroupLDAP; +use \OCA\user_ldap\lib\Access; +use \OCA\user_ldap\lib\Connection; +use \OCA\user_ldap\lib\ILDAPWrapper; + +class Test_Group_Ldap extends \PHPUnit_Framework_TestCase { + private function getAccessMock() { + static $conMethods; + static $accMethods; + + if(is_null($conMethods) || is_null($accMethods)) { + $conMethods = get_class_methods('\OCA\user_ldap\lib\Connection'); + $accMethods = get_class_methods('\OCA\user_ldap\lib\Access'); + } + $lw = $this->getMock('\OCA\user_ldap\lib\ILDAPWrapper'); + $connector = $this->getMock('\OCA\user_ldap\lib\Connection', + $conMethods, + array($lw, null, null)); + $access = $this->getMock('\OCA\user_ldap\lib\Access', + $accMethods, + array($connector, $lw)); + + return $access; + } + + private function enableGroups($access) { + $access->connection->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function($name) { +// if($name === 'ldapLoginFilter') { +// return '%uid'; +// } + return 1; + })); + } + + public function testCountEmptySearchString() { + $access = $this->getAccessMock(); + + $this->enableGroups($access); + + $access->expects($this->any()) + ->method('groupname2dn') + ->will($this->returnValue('cn=group,dc=foo,dc=bar')); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnValue(array('u11', 'u22', 'u33', 'u34'))); + + $groupBackend = new GroupLDAP($access); + $users = $groupBackend->countUsersInGroup('group'); + + $this->assertSame(4, $users); + } + + public function testCountWithSearchString() { + $access = $this->getAccessMock(); + + $this->enableGroups($access); + + $access->expects($this->any()) + ->method('groupname2dn') + ->will($this->returnValue('cn=group,dc=foo,dc=bar')); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnCallback(function($name) { + //the search operation will call readAttribute, thus we need + //to anaylze the "dn". All other times we just need to return + //something that is neither null or false, but once an array + //with the users in the group – so we do so all other times for + //simplicicity. + if(strpos($name, 'u') === 0) { + return strpos($name, '3'); + } + return array('u11', 'u22', 'u33', 'u34'); + })); + + $access->expects($this->any()) + ->method('dn2username') + ->will($this->returnValue('foobar')); + + $groupBackend = new GroupLDAP($access); + $users = $groupBackend->countUsersInGroup('group', '3'); + + $this->assertSame(2, $users); + } + +} \ No newline at end of file