diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 2c6955b0db..b4880c2f46 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -124,7 +124,7 @@ class UsersController extends OCSController { * @param int $offset * @return DataResponse */ - public function getUsers(string $search = '', $limit = null, $offset = null): DataResponse { + public function getUsers(string $search = '', $limit = null, $offset = 0): DataResponse { $user = $this->userSession->getUser(); $users = []; @@ -139,16 +139,10 @@ class UsersController extends OCSController { $subAdminOfGroups[$key] = $group->getGID(); } - if($offset === null) { - $offset = 0; - } - $users = []; foreach ($subAdminOfGroups as $group) { - $users = array_merge($users, $this->groupManager->displayNamesInGroup($group, $search)); + $users = array_merge($users, $this->groupManager->displayNamesInGroup($group, $search, $limit, $offset)); } - - $users = array_slice($users, $offset, $limit); } $users = array_keys($users); @@ -162,14 +156,8 @@ class UsersController extends OCSController { * @NoAdminRequired * * returns a list of users and their data - * - * @param string $search - * @param int $offset - * @return DataResponse */ - public function getUsersDetails(string $search = '', $offset = null): DataResponse { - // Limit set to 25 for performance issues - $limit = 25; + public function getUsersDetails(string $search = '', $limit = null, $offset = 0): DataResponse { $user = $this->userSession->getUser(); $users = []; @@ -184,22 +172,20 @@ class UsersController extends OCSController { $subAdminOfGroups[$key] = $group->getGID(); } - if($offset === null) { - $offset = 0; - } - $users = []; foreach ($subAdminOfGroups as $group) { - $users = array_merge($users, $this->groupManager->displayNamesInGroup($group, $search)); + $users = array_merge($users, $this->groupManager->displayNamesInGroup($group, $search, $limit, $offset)); } - - $users = array_slice($users, $offset, $limit); } $users = array_keys($users); $usersDetails = []; foreach ($users as $key => $userId) { - $usersDetails[$userId] = $this->getUserData($userId, false); + $userData = $this->getUserData($userId); + // Do not insert empty entry + if(!empty($userData)) { + $usersDetails[$userId] = $userData; + } } return new DataResponse([ @@ -281,6 +267,10 @@ class UsersController extends OCSController { */ public function getUser(string $userId): DataResponse { $data = $this->getUserData($userId); + // getUserData returns empty array if not enough permissions + if(empty($data)) { + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); + } return new DataResponse($data); } @@ -315,7 +305,7 @@ class UsersController extends OCSController { * @return array * @throws OCSException */ - protected function getUserData(string $userId, bool $throw = true): array { + protected function getUserData(string $userId): array { $currentLoggedInUser = $this->userSession->getUser(); $data = []; @@ -326,17 +316,13 @@ class UsersController extends OCSController { throw new OCSException('The requested user could not be found', \OCP\API::RESPOND_NOT_FOUND); } - // Admin? Or SubAdmin? - if($this->groupManager->isAdmin($currentLoggedInUser->getUID()) + // Should be at least Admin Or SubAdmin! + if( $this->groupManager->isAdmin($currentLoggedInUser->getUID()) || $this->groupManager->getSubAdmin()->isUserAccessible($currentLoggedInUser, $targetUserObject)) { - - $data['enabled'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'enabled', 'true'); - + $data['enabled'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'enabled', 'true'); } else { // Check they are looking up themselves - if($currentLoggedInUser->getUID() !== $targetUserObject->getUID() && $throw) { - throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); - } else { + if($currentLoggedInUser->getUID() !== $targetUserObject->getUID()) { return $data; } } @@ -870,7 +856,7 @@ class UsersController extends OCSController { if(!$groups) { throw new OCSException('Unknown error occurred', 102); } else { - return $groups; + return new DataResponse($groups);; } } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 216ca76a0f..83be63c49a 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -139,7 +139,7 @@ class UsersControllerTest extends TestCase { $this->userManager ->expects($this->once()) ->method('search') - ->with('MyCustomSearch', null, null) + ->with('MyCustomSearch') ->will($this->returnValue(['Admin' => [], 'Foo' => [], 'Bar' => []])); $expected = ['users' => [ @@ -662,6 +662,9 @@ class UsersControllerTest extends TestCase { $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); + $subAdminManager = $this->getMockBuilder('OC\SubAdmin') + ->disableOriginalConstructor() + ->getMock(); $loggedInUser ->expects($this->once()) ->method('getUID') @@ -671,15 +674,15 @@ class UsersControllerTest extends TestCase { ->getMock(); $targetUser->expects($this->once()) ->method('getEMailAddress') - ->willReturn('demo@owncloud.org'); + ->willReturn('demo@nextcloud.com'); $this->userSession ->expects($this->once()) ->method('getUser') ->will($this->returnValue($loggedInUser)); $this->userManager - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('get') - ->with('UserToGet') + ->with('UID') ->will($this->returnValue($targetUser)); $this->groupManager ->expects($this->once()) @@ -690,6 +693,14 @@ class UsersControllerTest extends TestCase { ->expects($this->any()) ->method('getUserGroups') ->willReturn([$group, $group, $group]); + $this->groupManager + ->expects($this->once()) + ->method('getSubAdmin') + ->will($this->returnValue($subAdminManager)); + $subAdminManager + ->expects($this->once()) + ->method('getSubAdminsGroups') + ->willReturn([$group]); $group->expects($this->at(0)) ->method('getDisplayName') ->willReturn('group0'); @@ -699,6 +710,9 @@ class UsersControllerTest extends TestCase { $group->expects($this->at(2)) ->method('getDisplayName') ->willReturn('group2'); + $group->expects($this->at(3)) + ->method('getGID') + ->willReturn('group3'); $this->accountManager->expects($this->any())->method('getUser') ->with($targetUser) ->willReturn( @@ -713,7 +727,7 @@ class UsersControllerTest extends TestCase { ->expects($this->at(0)) ->method('getUserValue') ->with('UID', 'core', 'enabled', 'true') - ->will($this->returnValue('true')); + ->will($this->returnValue('true')); $this->config ->expects($this->at(1)) ->method('getUserValue') @@ -729,15 +743,31 @@ class UsersControllerTest extends TestCase { ->method('getDisplayName') ->will($this->returnValue('Demo User')); $targetUser - ->expects($this->exactly(4)) + ->expects($this->once()) + ->method('getHome') + ->will($this->returnValue('/var/www/newtcloud/data/UID')); + $targetUser + ->expects($this->once()) + ->method('getLastLogin') + ->will($this->returnValue(1521191471)); + $targetUser + ->expects($this->once()) + ->method('getBackendClassName') + ->will($this->returnValue('Database')); + $targetUser + ->expects($this->exactly(5)) ->method('getUID') ->will($this->returnValue('UID')); $expected = [ 'id' => 'UID', 'enabled' => 'true', + 'storageLocation' => '/var/www/newtcloud/data/UID', + 'lastLogin' => 1521191471000, + 'backend' => 'Database', + 'subadmins' => ['group3'], 'quota' => ['DummyValue'], - 'email' => 'demo@owncloud.org', + 'email' => 'demo@nextcloud.com', 'displayname' => 'Demo User', 'phone' => 'phone', 'address' => 'address', @@ -746,7 +776,7 @@ class UsersControllerTest extends TestCase { 'groups' => ['group0', 'group1', 'group2'], 'language' => 'de', ]; - $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UserToGet'])); + $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID'])); } public function testGetUserDataAsSubAdminAndUserIsAccessible() { @@ -763,15 +793,15 @@ class UsersControllerTest extends TestCase { $targetUser ->expects($this->once()) ->method('getEMailAddress') - ->willReturn('demo@owncloud.org'); + ->willReturn('demo@nextcloud.com'); $this->userSession ->expects($this->once()) ->method('getUser') ->will($this->returnValue($loggedInUser)); $this->userManager - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('get') - ->with('UserToGet') + ->with('UID') ->will($this->returnValue($targetUser)); $this->groupManager ->expects($this->once()) @@ -790,8 +820,12 @@ class UsersControllerTest extends TestCase { ->method('isUserAccessible') ->with($loggedInUser, $targetUser) ->will($this->returnValue(true)); - $this->groupManager + $subAdminManager ->expects($this->once()) + ->method('getSubAdminsGroups') + ->willReturn([]); + $this->groupManager + ->expects($this->exactly(2)) ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); $this->config @@ -814,7 +848,19 @@ class UsersControllerTest extends TestCase { ->method('getDisplayName') ->will($this->returnValue('Demo User')); $targetUser - ->expects($this->exactly(4)) + ->expects($this->once()) + ->method('getHome') + ->will($this->returnValue('/var/www/newtcloud/data/UID')); + $targetUser + ->expects($this->once()) + ->method('getLastLogin') + ->will($this->returnValue(1521191471)); + $targetUser + ->expects($this->once()) + ->method('getBackendClassName') + ->will($this->returnValue('Database')); + $targetUser + ->expects($this->exactly(5)) ->method('getUID') ->will($this->returnValue('UID')); $this->accountManager->expects($this->any())->method('getUser') @@ -831,8 +877,12 @@ class UsersControllerTest extends TestCase { $expected = [ 'id' => 'UID', 'enabled' => 'true', + 'storageLocation' => '/var/www/newtcloud/data/UID', + 'lastLogin' => 1521191471000, + 'backend' => 'Database', + 'subadmins' => [], 'quota' => ['DummyValue'], - 'email' => 'demo@owncloud.org', + 'email' => 'demo@nextcloud.com', 'displayname' => 'Demo User', 'phone' => 'phone', 'address' => 'address', @@ -841,7 +891,7 @@ class UsersControllerTest extends TestCase { 'groups' => [], 'language' => 'da', ]; - $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UserToGet'])); + $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID'])); } @@ -887,7 +937,7 @@ class UsersControllerTest extends TestCase { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $this->invokePrivate($this->api, 'getUserData', ['UserToGet']); + $this->invokePrivate($this->api, 'getUser', ['UserToGet']); } public function testGetUserDataAsSubAdminSelfLookup() { @@ -906,9 +956,9 @@ class UsersControllerTest extends TestCase { ->method('getUser') ->will($this->returnValue($loggedInUser)); $this->userManager - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('get') - ->with('subadmin') + ->with('UID') ->will($this->returnValue($targetUser)); $this->groupManager ->expects($this->once()) @@ -923,8 +973,12 @@ class UsersControllerTest extends TestCase { ->method('isUserAccessible') ->with($loggedInUser, $targetUser) ->will($this->returnValue(false)); - $this->groupManager + $subAdminManager ->expects($this->once()) + ->method('getSubAdminsGroups') + ->willReturn([]); + $this->groupManager + ->expects($this->exactly(2)) ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); $this->groupManager @@ -943,11 +997,23 @@ class UsersControllerTest extends TestCase { $targetUser ->expects($this->once()) ->method('getEMailAddress') - ->will($this->returnValue('subadmin@owncloud.org')); + ->will($this->returnValue('subadmin@nextcloud.com')); $targetUser - ->expects($this->exactly(4)) + ->expects($this->exactly(5)) ->method('getUID') ->will($this->returnValue('UID')); + $targetUser + ->expects($this->once()) + ->method('getHome') + ->will($this->returnValue('/var/www/newtcloud/data/UID')); + $targetUser + ->expects($this->once()) + ->method('getLastLogin') + ->will($this->returnValue(1521191471)); + $targetUser + ->expects($this->once()) + ->method('getBackendClassName') + ->will($this->returnValue('Database')); $this->config ->expects($this->at(0)) ->method('getUserValue') @@ -966,8 +1032,12 @@ class UsersControllerTest extends TestCase { $expected = [ 'id' => 'UID', + 'storageLocation' => '/var/www/newtcloud/data/UID', + 'lastLogin' => 1521191471000, + 'backend' => 'Database', + 'subadmins' => [], 'quota' => ['DummyValue'], - 'email' => 'subadmin@owncloud.org', + 'email' => 'subadmin@nextcloud.com', 'displayname' => 'Subadmin User', 'phone' => 'phone', 'address' => 'address', @@ -976,7 +1046,7 @@ class UsersControllerTest extends TestCase { 'groups' => [], 'language' => 'ru', ]; - $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['subadmin'])); + $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID'])); } public function testEditUserRegularUserSelfEditChangeDisplayName() { @@ -1034,13 +1104,13 @@ class UsersControllerTest extends TestCase { $targetUser ->expects($this->once()) ->method('setEMailAddress') - ->with('demo@owncloud.org'); + ->with('demo@nextcloud.com'); $targetUser ->expects($this->any()) ->method('getUID') ->will($this->returnValue('UID')); - $this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@owncloud.org')->getData()); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@nextcloud.com')->getData()); } @@ -2896,7 +2966,7 @@ class UsersControllerTest extends TestCase { 'id' => 'UID', 'enabled' => 'true', 'quota' => ['DummyValue'], - 'email' => 'demo@owncloud.org', + 'email' => 'demo@nextcloud.com', 'displayname' => 'Demo User', 'phone' => 'phone', 'address' => 'address', @@ -2909,7 +2979,7 @@ class UsersControllerTest extends TestCase { 'id' => 'UID', 'enabled' => 'true', 'quota' => ['DummyValue'], - 'email' => 'demo@owncloud.org', + 'email' => 'demo@nextcloud.com', 'phone' => 'phone', 'address' => 'address', 'website' => 'website', @@ -2956,7 +3026,7 @@ class UsersControllerTest extends TestCase { 'id' => 'UID', 'enabled' => 'true', 'quota' => ['DummyValue'], - 'email' => 'demo@owncloud.org', + 'email' => 'demo@nextcloud.com', 'phone' => 'phone', 'address' => 'address', 'website' => 'website',