From ce625bbad52bd7d416d5fdaac6bef399feaf0c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 16 Mar 2018 09:18:53 +0100 Subject: [PATCH 1/5] Add userdetails to ocs api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/provisioning_api/appinfo/routes.php | 1 + .../lib/Controller/UsersController.php | 73 ++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index aa5a30199a..791267a97a 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -41,6 +41,7 @@ return [ // Users ['root' => '/cloud', 'name' => 'Users#getUsers', 'url' => '/users', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Users#getUsersDetails', 'url' => '/users/details', 'verb' => 'GET'], ['root' => '/cloud', 'name' => 'Users#addUser', 'url' => '/users', 'verb' => 'POST'], ['root' => '/cloud', 'name' => 'Users#getUser', 'url' => '/users/{userId}', 'verb' => 'GET'], ['root' => '/cloud', 'name' => 'Users#getCurrentUser', 'url' => '/user', 'verb' => 'GET'], diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 087e65a1bc..17c04c708f 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -158,6 +158,55 @@ 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; + $user = $this->userSession->getUser(); + $users = []; + + // Admin? Or SubAdmin? + $uid = $user->getUID(); + $subAdminManager = $this->groupManager->getSubAdmin(); + if($this->groupManager->isAdmin($uid)){ + $users = $this->userManager->search($search, $limit, $offset); + } else if ($subAdminManager->isSubAdmin($user)) { + $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($user); + foreach ($subAdminOfGroups as $key => $group) { + $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_slice($users, $offset, $limit); + } + + $users = array_keys($users); + $usersDetails = []; + foreach ($users as $key => $userId) { + $usersDetails[$userId] = $this->getUserData($userId); + } + + return new DataResponse([ + 'users' => $usersDetails + ]); + } + /** * @PasswordConfirmationRequired * @NoAdminRequired @@ -281,6 +330,10 @@ class UsersController extends OCSController { if($this->groupManager->isAdmin($currentLoggedInUser->getUID()) || $this->groupManager->getSubAdmin()->isUserAccessible($currentLoggedInUser, $targetUserObject)) { $data['enabled'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'enabled', 'true'); + $data['storageLocation'] = $targetUserObject->getHome(); + $data['lastLogin'] = $targetUserObject->getLastLogin() * 1000; + $data['backend'] = $targetUserObject->getBackendClassName(); + $data['subadmins'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); } else { // Check they are looking up themselves if($currentLoggedInUser->getUID() !== $targetUserObject->getUID()) { @@ -288,6 +341,7 @@ class UsersController extends OCSController { } } + // Get groups data $userAccount = $this->accountManager->getUser($targetUserObject); $groups = $this->groupManager->getUserGroups($targetUserObject); $gids = []; @@ -779,10 +833,10 @@ class UsersController extends OCSController { * Get the groups a user is a subadmin of * * @param string $userId - * @return DataResponse + * @return array * @throws OCSException */ - public function getUserSubAdminGroups(string $userId): DataResponse { + protected function getUserSubAdminGroupsData(string $userId): array { $user = $this->userManager->get($userId); // Check if the user exists if($user === null) { @@ -796,10 +850,23 @@ class UsersController extends OCSController { $groups[] = $group->getGID(); } + return $groups; + } + + /** + * Get the groups a user is a subadmin of + * + * @param string $userId + * @return DataResponse + * @throws OCSException + */ + public function getUserSubAdminGroups(string $userId): DataResponse { + $groups = $this->getUserSubAdminGroupsData($userId); + if(!$groups) { throw new OCSException('Unknown error occurred', 102); } else { - return new DataResponse($groups); + return $groups; } } From aa1e8713fbdd8332537a6a9b51c1954ef81790b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 16 Mar 2018 09:38:09 +0100 Subject: [PATCH 2/5] Fixed throw error on unreachable users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- .../lib/Controller/UsersController.php | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 17c04c708f..2c6955b0db 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -199,7 +199,7 @@ class UsersController extends OCSController { $users = array_keys($users); $usersDetails = []; foreach ($users as $key => $userId) { - $usersDetails[$userId] = $this->getUserData($userId); + $usersDetails[$userId] = $this->getUserData($userId, false); } return new DataResponse([ @@ -315,7 +315,7 @@ class UsersController extends OCSController { * @return array * @throws OCSException */ - protected function getUserData(string $userId): array { + protected function getUserData(string $userId, bool $throw = true): array { $currentLoggedInUser = $this->userSession->getUser(); $data = []; @@ -329,15 +329,15 @@ class UsersController extends OCSController { // 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['storageLocation'] = $targetUserObject->getHome(); - $data['lastLogin'] = $targetUserObject->getLastLogin() * 1000; - $data['backend'] = $targetUserObject->getBackendClassName(); - $data['subadmins'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); + } else { // Check they are looking up themselves - if($currentLoggedInUser->getUID() !== $targetUserObject->getUID()) { + if($currentLoggedInUser->getUID() !== $targetUserObject->getUID() && $throw) { throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); + } else { + return $data; } } @@ -351,6 +351,10 @@ class UsersController extends OCSController { // Find the data $data['id'] = $targetUserObject->getUID(); + $data['storageLocation'] = $targetUserObject->getHome(); + $data['lastLogin'] = $targetUserObject->getLastLogin() * 1000; + $data['backend'] = $targetUserObject->getBackendClassName(); + $data['subadmins'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); $data['quota'] = $this->fillStorageInfo($targetUserObject->getUID()); $data[AccountManager::PROPERTY_EMAIL] = $targetUserObject->getEMailAddress(); $data[AccountManager::PROPERTY_DISPLAYNAME] = $targetUserObject->getDisplayName(); From f5137fef6c79509473ddbd0bc9ac1e4e9c6d840d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 16 Mar 2018 11:44:20 +0100 Subject: [PATCH 3/5] Fixed tests and improved code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- .../lib/Controller/UsersController.php | 52 +++----- .../tests/Controller/UsersControllerTest.php | 126 ++++++++++++++---- 2 files changed, 117 insertions(+), 61 deletions(-) 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', From 2493e9426db1528437c6055a54fdf0ed17914690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 16 Mar 2018 15:32:36 +0100 Subject: [PATCH 4/5] Check if groups is array, return empty do not throw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/provisioning_api/lib/Controller/UsersController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index b4880c2f46..1db4c96b75 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -853,10 +853,10 @@ class UsersController extends OCSController { public function getUserSubAdminGroups(string $userId): DataResponse { $groups = $this->getUserSubAdminGroupsData($userId); - if(!$groups) { + if(is_array($groups)) { throw new OCSException('Unknown error occurred', 102); } else { - return new DataResponse($groups);; + return new DataResponse($groups); } } From 24659342f5e12903a252b50778179b1b558832ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 16 Mar 2018 15:37:05 +0100 Subject: [PATCH 5/5] fixup! Check if groups is array, return empty do not throw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- .../lib/Controller/UsersController.php | 9 ++--- .../tests/Controller/UsersControllerTest.php | 33 ++----------------- 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 1db4c96b75..c09f1617f4 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -340,7 +340,7 @@ class UsersController extends OCSController { $data['storageLocation'] = $targetUserObject->getHome(); $data['lastLogin'] = $targetUserObject->getLastLogin() * 1000; $data['backend'] = $targetUserObject->getBackendClassName(); - $data['subadmins'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); + $data['subadmin'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); $data['quota'] = $this->fillStorageInfo($targetUserObject->getUID()); $data[AccountManager::PROPERTY_EMAIL] = $targetUserObject->getEMailAddress(); $data[AccountManager::PROPERTY_DISPLAYNAME] = $targetUserObject->getDisplayName(); @@ -852,12 +852,7 @@ class UsersController extends OCSController { */ public function getUserSubAdminGroups(string $userId): DataResponse { $groups = $this->getUserSubAdminGroupsData($userId); - - if(is_array($groups)) { - throw new OCSException('Unknown error occurred', 102); - } else { - return new DataResponse($groups); - } + return new DataResponse($groups); } /** diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 83be63c49a..38e3598813 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -765,7 +765,7 @@ class UsersControllerTest extends TestCase { 'storageLocation' => '/var/www/newtcloud/data/UID', 'lastLogin' => 1521191471000, 'backend' => 'Database', - 'subadmins' => ['group3'], + 'subadmin' => ['group3'], 'quota' => ['DummyValue'], 'email' => 'demo@nextcloud.com', 'displayname' => 'Demo User', @@ -880,7 +880,7 @@ class UsersControllerTest extends TestCase { 'storageLocation' => '/var/www/newtcloud/data/UID', 'lastLogin' => 1521191471000, 'backend' => 'Database', - 'subadmins' => [], + 'subadmin' => [], 'quota' => ['DummyValue'], 'email' => 'demo@nextcloud.com', 'displayname' => 'Demo User', @@ -1035,7 +1035,7 @@ class UsersControllerTest extends TestCase { 'storageLocation' => '/var/www/newtcloud/data/UID', 'lastLogin' => 1521191471000, 'backend' => 'Database', - 'subadmins' => [], + 'subadmin' => [], 'quota' => ['DummyValue'], 'email' => 'subadmin@nextcloud.com', 'displayname' => 'Subadmin User', @@ -2853,33 +2853,6 @@ class UsersControllerTest extends TestCase { $this->assertEquals(['TargetGroup'], $this->api->getUserSubAdminGroups('RequestedUser')->getData()); } - /** - * @expectedException \OCP\AppFramework\OCS\OCSException - * @expectedExceptionCode 102 - * @expectedExceptionMessage Unknown error occurred - */ - public function testGetUserSubAdminGroupsWithoutGroups() { - $targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); - $this->userManager - ->expects($this->once()) - ->method('get') - ->with('RequestedUser') - ->will($this->returnValue($targetUser)); - $subAdminManager = $this->getMockBuilder('OC\SubAdmin') - ->disableOriginalConstructor()->getMock(); - $subAdminManager - ->expects($this->once()) - ->method('getSubAdminsGroups') - ->with($targetUser) - ->will($this->returnValue([])); - $this->groupManager - ->expects($this->once()) - ->method('getSubAdmin') - ->will($this->returnValue($subAdminManager)); - - $this->api->getUserSubAdminGroups('RequestedUser'); - } - public function testEnableUser() { $targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $targetUser->expects($this->once())