From deba0f992210ad2a3a837d8ad45d3b7fc63ecc79 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 12 Aug 2016 15:15:24 +0200 Subject: [PATCH 1/8] Move OCS Middleware before security middleware This is required to be able to catch the NotLoggedIn exceptions etc in the OCSMiddleware and convert them to proper OCS Responses. --- lib/private/AppFramework/DependencyInjection/DIContainer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 66ca59d26e..b60ce64324 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -396,6 +396,7 @@ class DIContainer extends SimpleContainer implements IAppContainer { $this->registerService('MiddlewareDispatcher', function($c) use (&$middleWares) { $dispatcher = new MiddlewareDispatcher(); $dispatcher->registerMiddleware($c['CORSMiddleware']); + $dispatcher->registerMiddleware($c['OCSMiddleware']); $dispatcher->registerMiddleware($c['SecurityMiddleware']); $dispatcher->registerMiddleWare($c['TwoFactorMiddleware']); @@ -404,7 +405,6 @@ class DIContainer extends SimpleContainer implements IAppContainer { } $dispatcher->registerMiddleware($c['SessionMiddleware']); - $dispatcher->registerMiddleware($c['OCSMiddleware']); return $dispatcher; }); From e3b0e50dda14946b561f618c6185b636dbf66aa9 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 11 Aug 2016 09:44:12 +0200 Subject: [PATCH 2/8] Extend OCSMiddleware * Always set 401 (v1.php and v2.php) * Set proper error codes for v2.php * Proper OCS output on unhandled exceptions --- .../AppFramework/Middleware/OCSMiddleware.php | 62 ++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/lib/private/AppFramework/Middleware/OCSMiddleware.php b/lib/private/AppFramework/Middleware/OCSMiddleware.php index e07d100d8a..68445bbcc5 100644 --- a/lib/private/AppFramework/Middleware/OCSMiddleware.php +++ b/lib/private/AppFramework/Middleware/OCSMiddleware.php @@ -23,8 +23,14 @@ namespace OC\AppFramework\Middleware; use OC\AppFramework\Http; +use OCP\API; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\OCSResponse; +use OCP\AppFramework\Http\Response; use OCP\AppFramework\OCS\OCSException; +use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\IRequest; use OCP\AppFramework\Middleware; @@ -54,12 +60,35 @@ class OCSMiddleware extends Middleware { $code = $exception->getCode(); if ($code === 0) { - $code = Http::STATUS_INTERNAL_SERVER_ERROR; + $code = API::RESPOND_UNKNOWN_ERROR; } + + // Build the response $response = new OCSResponse($format, $code, $exception->getMessage()); + // Forbidden always sets 401 (even on v1.php) + if ($exception instanceof OCSForbiddenException || $code === API::RESPOND_UNAUTHORISED) { + $response->setStatus(Http::STATUS_UNAUTHORIZED); + } + + // On v2.php we set actual HTTP error codes if (substr_compare($this->request->getScriptName(), '/ocs/v2.php', -strlen('/ocs/v2.php')) === 0) { - $response->setStatus($code); + if ($code === API::RESPOND_NOT_FOUND) { + $response->setStatus(Http::STATUS_NOT_FOUND); + } else if ($code === API::RESPOND_SERVER_ERROR) { + $response->setStatus(Http::STATUS_INTERNAL_SERVER_ERROR); + } else if ($code === API::RESPOND_UNKNOWN_ERROR) { + $response->setStatus(Http::STATUS_INTERNAL_SERVER_ERROR); + } else if ($code === API::RESPOND_UNAUTHORISED) { + // Already set + } + // 4xx and 5xx codes are forwarded as is. + else if ($code >= 400 && $code < 600) { + $response->setStatus($code); + } else { + // All other codes get a bad request + $response->setStatus(Http::STATUS_BAD_REQUEST); + } } return $response; } @@ -67,6 +96,35 @@ class OCSMiddleware extends Middleware { throw $exception; } + /** + * @param \OCP\AppFramework\Controller $controller + * @param string $methodName + * @param Response $response + * @return \OCP\AppFramework\Http\Response + */ + public function afterController($controller, $methodName, Response $response) { + /* + * If a different middleware has detected that a request unauthorized or forbidden + * we need to catch the response and convert it to a proper OCS response. + */ + if ($controller instanceof OCSController && !($response instanceof OCSResponse)) { + if ($response->getStatus() === Http::STATUS_UNAUTHORIZED || + $response->getStatus() === Http::STATUS_FORBIDDEN) { + $format = $this->getFormat($controller); + + $message = ''; + if ($response instanceof JSONResponse) { + /** @var DataResponse $response */ + $message = $response->getData()['message']; + } + $response = new OCSResponse($format, \OCP\API::RESPOND_UNAUTHORISED, $message); + $response->setStatus(Http::STATUS_UNAUTHORIZED); + } + } + + return $response; + } + /** * @param \OCP\AppFramework\Controller $controller * @return string From a0b22227fc13e5df0abab79184e376768e64cf0a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 12 Aug 2016 15:42:26 +0200 Subject: [PATCH 3/8] Add tests --- .../Middleware/OCSMiddlewareTest.php | 76 +++++++++++++++++-- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/tests/lib/AppFramework/Middleware/OCSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/OCSMiddlewareTest.php index 7d8cadc677..b2295fdc26 100644 --- a/tests/lib/AppFramework/Middleware/OCSMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/OCSMiddlewareTest.php @@ -27,14 +27,14 @@ use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; -use OC\AppFramework\Http\Request; +use OCP\IRequest; use OC\AppFramework\Middleware\OCSMiddleware; class OCSMiddlewareTest extends \Test\TestCase { /** - * @var Request + * @var IRequest */ private $request; @@ -101,8 +101,18 @@ class OCSMiddlewareTest extends \Test\TestCase { $this->assertInstanceOf('OCP\AppFramework\Http\OCSResponse', $result); $this->assertSame($message, $this->invokePrivate($result, 'message')); - $this->assertSame($code, $this->invokePrivate($result, 'statuscode')); - $this->assertSame(200, $result->getStatus()); + + if ($exception->getCode() === 0) { + $this->assertSame(\OCP\API::RESPOND_UNKNOWN_ERROR, $this->invokePrivate($result, 'statuscode')); + } else { + $this->assertSame($code, $this->invokePrivate($result, 'statuscode')); + } + + if ($exception instanceof OCSForbiddenException) { + $this->assertSame(Http::STATUS_UNAUTHORIZED, $result->getStatus()); + } else { + $this->assertSame(200, $result->getStatus()); + } } catch (\Exception $e) { $this->assertTrue($forward); $this->assertEquals($exception, $e); @@ -131,7 +141,11 @@ class OCSMiddlewareTest extends \Test\TestCase { $this->assertInstanceOf('OCP\AppFramework\Http\OCSResponse', $result); $this->assertSame($message, $this->invokePrivate($result, 'message')); - $this->assertSame($code, $this->invokePrivate($result, 'statuscode')); + if ($exception->getCode() === 0) { + $this->assertSame(\OCP\API::RESPOND_UNKNOWN_ERROR, $this->invokePrivate($result, 'statuscode')); + } else { + $this->assertSame($code, $this->invokePrivate($result, 'statuscode')); + } $this->assertSame($code, $result->getStatus()); } catch (\Exception $e) { $this->assertTrue($forward); @@ -161,7 +175,11 @@ class OCSMiddlewareTest extends \Test\TestCase { $this->assertInstanceOf('OCP\AppFramework\Http\OCSResponse', $result); $this->assertSame($message, $this->invokePrivate($result, 'message')); - $this->assertSame($code, $this->invokePrivate($result, 'statuscode')); + if ($exception->getCode() === 0) { + $this->assertSame(\OCP\API::RESPOND_UNKNOWN_ERROR, $this->invokePrivate($result, 'statuscode')); + } else { + $this->assertSame($code, $this->invokePrivate($result, 'statuscode')); + } $this->assertSame($code, $result->getStatus()); } catch (\Exception $e) { $this->assertTrue($forward); @@ -169,4 +187,50 @@ class OCSMiddlewareTest extends \Test\TestCase { } } + public function dataAfterController() { + $OCSController = $this->getMockBuilder('OCP\AppFramework\OCSController') + ->disableOriginalConstructor() + ->getMock(); + $controller = $this->getMockBuilder('OCP\AppFramework\Controller') + ->disableOriginalConstructor() + ->getMock(); + + return [ + [$OCSController, new Http\Response(), false], + [$OCSController, new Http\JSONResponse(), false], + [$OCSController, new Http\JSONResponse(['message' => 'foo']), false], + [$OCSController, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_UNAUTHORIZED), true], + [$OCSController, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_FORBIDDEN), true], + + [$controller, new Http\Response(), false], + [$controller, new Http\JSONResponse(), false], + [$controller, new Http\JSONResponse(['message' => 'foo']), false], + [$controller, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_UNAUTHORIZED), false], + [$controller, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_FORBIDDEN), false], + + ]; + } + + /** + * @dataProvider dataAfterController + * + * @param Controller $controller + * @param Http\Response $response + * @param bool $converted + */ + public function testAfterController($controller, $response, $converted) { + $OCSMiddleware = new OCSMiddleware($this->request); + $newResponse = $OCSMiddleware->afterController($controller, 'foo', $response); + + if ($converted === false) { + $this->assertSame($response, $newResponse); + } else { + $this->assertInstanceOf('\OCP\AppFramework\Http\OCSResponse', $newResponse); + /** @var Http\OCSResponse $newResponse */ + $this->assertSame($response->getData()['message'], $this->invokePrivate($newResponse, 'message')); + $this->assertSame(\OCP\API::RESPOND_UNAUTHORISED, $this->invokePrivate($newResponse, 'statuscode')); + $this->assertSame(Http::STATUS_UNAUTHORIZED, $newResponse->getStatus()); + } + } + } From 0fdeefe47c82b18eb6adf1bd66ec2471b4d76c25 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 11 Aug 2016 09:45:15 +0200 Subject: [PATCH 4/8] Add ProvisioningAPI middleware The provisioning API has 3 access levels: * Admin * SubAdmin * User This middleware adds a check for the SubAdmin part. --- .../lib/AppInfo/Application.php | 28 ++++++++ .../Exceptions/NotSubAdminException.php | 11 ++++ .../Middleware/ProvisioningApiMiddleware.php | 64 +++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 apps/provisioning_api/lib/AppInfo/Application.php create mode 100644 apps/provisioning_api/lib/Middleware/Exceptions/NotSubAdminException.php create mode 100644 apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php diff --git a/apps/provisioning_api/lib/AppInfo/Application.php b/apps/provisioning_api/lib/AppInfo/Application.php new file mode 100644 index 0000000000..2d6a82e2ff --- /dev/null +++ b/apps/provisioning_api/lib/AppInfo/Application.php @@ -0,0 +1,28 @@ +getContainer(); + $server = $container->getServer(); + + $container->registerService('ProvisioningApiMiddleware', function(SimpleContainer $c) use ($server) { + $user = $server->getUserManager()->get($c['UserId']); + $isAdmin = $user !== null ? $server->getGroupManager()->isAdmin($user->getUID()) : false; + $isSubAdmin = $user !== null ? $server->getGroupManager()->getSubAdmin()->isSubAdmin($user) : false; + return new ProvisioningApiMiddleware( + $c['ControllerMethodReflector'], + $isAdmin, + $isSubAdmin + ); + }); + $container->registerMiddleWare('ProvisioningApiMiddleware'); + } +} diff --git a/apps/provisioning_api/lib/Middleware/Exceptions/NotSubAdminException.php b/apps/provisioning_api/lib/Middleware/Exceptions/NotSubAdminException.php new file mode 100644 index 0000000000..007ea04db4 --- /dev/null +++ b/apps/provisioning_api/lib/Middleware/Exceptions/NotSubAdminException.php @@ -0,0 +1,11 @@ +reflector = $reflector; + $this->isAdmin = $isAdmin; + $this->isSubAdmin = $isSubAdmin; + } + + /** + * @param \OCP\AppFramework\Controller $controller + * @param string $methodName + * + * @throws NotSubAdminException + */ + public function beforeController($controller, $methodName) { + if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin) { + throw new NotSubAdminException(); + } + } + + /** + * @param \OCP\AppFramework\Controller $controller + * @param string $methodName + * @param \Exception $exception + * @throws \Exception + * @return Response + */ + public function afterException($controller, $methodName, \Exception $exception) { + if ($exception instanceof NotSubAdminException) { + throw new OCSException($exception->getMessage(), \OCP\API::RESPOND_UNAUTHORISED); + } + + throw $exception; + } +} \ No newline at end of file From 432e7c93c6dad564abbaec1e3d374f73653d7ba6 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 11 Aug 2016 09:46:25 +0200 Subject: [PATCH 5/8] Move Groups over to OCSController * Take advantage of the AppFramework * Fix tests --- apps/provisioning_api/appinfo/routes.php | 27 ++- .../GroupsController.php} | 128 ++++++------ .../GroupsControllerTest.php} | 197 ++++++------------ 3 files changed, 138 insertions(+), 214 deletions(-) rename apps/provisioning_api/lib/{Groups.php => Controller/GroupsController.php} (52%) rename apps/provisioning_api/tests/{GroupsTest.php => Controller/GroupsControllerTest.php} (59%) diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index 907e16ebf3..dbc0321efb 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -26,13 +26,22 @@ * */ -namespace OCA\Provisioning_API\AppInfo; - use OCA\Provisioning_API\Apps; -use OCA\Provisioning_API\Groups; use OCA\Provisioning_API\Users; use OCP\API; +$app = new \OCA\Provisioning_API\AppInfo\Application(); +$app->registerRoutes($this, [ + 'ocs' => [ + // Groups + ['root' => '/cloud', 'name' => 'Groups#getGroups', 'url' => '/groups', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Groups#getGroup', 'url' => '/groups/{groupId}', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Groups#addGroup', 'url' => '/groups', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'Groups#deleteGroup', 'url' => '/groups/{groupId}', 'verb' => 'DELETE'], + ['root' => '/cloud', 'name' => 'Groups#getSubAdminsOfGroup', 'url' => '/groups/{groupId}/subadmins', 'verb' => 'GET'], + ], +]); + // Users $users = new Users( \OC::$server->getUserManager(), @@ -55,18 +64,6 @@ API::register('post', '/cloud/users/{userid}/subadmins', [$users, 'addSubAdmin'] API::register('delete', '/cloud/users/{userid}/subadmins', [$users, 'removeSubAdmin'], 'provisioning_api', API::ADMIN_AUTH); API::register('get', '/cloud/users/{userid}/subadmins', [$users, 'getUserSubAdminGroups'], 'provisioning_api', API::ADMIN_AUTH); -// Groups -$groups = new Groups( - \OC::$server->getGroupManager(), - \OC::$server->getUserSession(), - \OC::$server->getRequest() -); -API::register('get', '/cloud/groups', [$groups, 'getGroups'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('post', '/cloud/groups', [$groups, 'addGroup'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('get', '/cloud/groups/{groupid}', [$groups, 'getGroup'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('delete', '/cloud/groups/{groupid}', [$groups, 'deleteGroup'], 'provisioning_api', API::ADMIN_AUTH); -API::register('get', '/cloud/groups/{groupid}/subadmins', [$groups, 'getSubAdminsOfGroup'], 'provisioning_api', API::ADMIN_AUTH); - // Apps $apps = new Apps( \OC::$server->getAppManager(), diff --git a/apps/provisioning_api/lib/Groups.php b/apps/provisioning_api/lib/Controller/GroupsController.php similarity index 52% rename from apps/provisioning_api/lib/Groups.php rename to apps/provisioning_api/lib/Controller/GroupsController.php index 18302595ae..d36d0de899 100644 --- a/apps/provisioning_api/lib/Groups.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -23,46 +23,54 @@ * */ -namespace OCA\Provisioning_API; +namespace OCA\Provisioning_API\Controller; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSException; +use OCP\AppFramework\OCSController; use OCP\IGroup; +use OCP\IGroupManager; +use OCP\IRequest; +use OCP\IUserSession; use OCP\IUser; -class Groups{ - /** @var \OCP\IGroupManager */ +class GroupsController extends OCSController { + + /** @var IGroupManager */ private $groupManager; - /** @var \OCP\IUserSession */ + /** @var IUserSession */ private $userSession; - /** @var \OCP\IRequest */ - private $request; - /** - * @param \OCP\IGroupManager $groupManager - * @param \OCP\IUserSession $userSession - * @param \OCP\IRequest $request + * @param string $appName + * @param IRequest $request + * @param IGroupManager $groupManager + * @param IUserSession $userSession */ - public function __construct(\OCP\IGroupManager $groupManager, - \OCP\IUserSession $userSession, - \OCP\IRequest $request) { + public function __construct( + $appName, + IRequest $request, + IGroupManager $groupManager, + IUserSession $userSession) { + parent::__construct($appName, $request); + $this->groupManager = $groupManager; $this->userSession = $userSession; - $this->request = $request; } /** * returns a list of groups * - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string $search + * @param int $limit + * @param int $offset + * @return DataResponse */ - public function getGroups($parameters) { - $search = $this->request->getParam('search', ''); - $limit = $this->request->getParam('limit'); - $offset = $this->request->getParam('offset'); - + public function getGroups($search = '', $limit = null, $offset = null) { if ($limit !== null) { $limit = (int)$limit; } @@ -76,27 +84,24 @@ class Groups{ return $group->getGID(); }, $groups); - return new \OC\OCS\Result(['groups' => $groups]); + return new DataResponse(['groups' => $groups]); } /** * returns an array of users in the group specified * - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string $groupId + * @return DataResponse + * @throws OCSException */ - public function getGroup($parameters) { - // Check if user is logged in + public function getGroup($groupId) { $user = $this->userSession->getUser(); - if ($user === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - - $groupId = $parameters['groupid']; // Check the group exists if(!$this->groupManager->groupExists($groupId)) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_NOT_FOUND, 'The requested group could not be found'); + throw new OCSException('The requested group could not be found', \OCP\API::RESPOND_NOT_FOUND); } $isSubadminOfGroup = false; @@ -114,59 +119,62 @@ class Groups{ return $user->getUID(); }, $users); $users = array_values($users); - return new \OC\OCS\Result(['users' => $users]); + return new DataResponse(['users' => $users]); } else { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED, 'User does not have access to specified group'); + throw new OCSException('User does not have access to specified group', \OCP\API::RESPOND_UNAUTHORISED); } } /** * creates a new group * - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string $groupid + * @return DataResponse + * @throws OCSException */ - public function addGroup($parameters) { + public function addGroup($groupid) { // Validate name - $groupId = $this->request->getParam('groupid', ''); - if(empty($groupId)){ + if(empty($groupid)){ \OCP\Util::writeLog('provisioning_api', 'Group name not supplied', \OCP\Util::ERROR); - return new \OC\OCS\Result(null, 101, 'Invalid group name'); + throw new OCSException('Invalid group name', 101); } // Check if it exists - if($this->groupManager->groupExists($groupId)){ - return new \OC\OCS\Result(null, 102); + if($this->groupManager->groupExists($groupid)){ + throw new OCSException('', 102); } - $this->groupManager->createGroup($groupId); - return new \OC\OCS\Result(null, 100); + $this->groupManager->createGroup($groupid); + return new DataResponse(); } /** - * @param array $parameters - * @return \OC\OCS\Result + * @param string $groupId + * @return DataResponse + * @throws OCSException */ - public function deleteGroup($parameters) { + public function deleteGroup($groupId) { // Check it exists - if(!$this->groupManager->groupExists($parameters['groupid'])){ - return new \OC\OCS\Result(null, 101); - } else if($parameters['groupid'] === 'admin' || !$this->groupManager->get($parameters['groupid'])->delete()){ + if(!$this->groupManager->groupExists($groupId)){ + throw new OCSException('', 101); + } else if($groupId === 'admin' || !$this->groupManager->get($groupId)->delete()){ // Cannot delete admin group - return new \OC\OCS\Result(null, 102); - } else { - return new \OC\OCS\Result(null, 100); + throw new OCSException('', 102); } + + return new DataResponse(null, 100); } /** - * @param array $parameters - * @return \OC\OCS\Result + * @param string $groupId + * @return DataResponse + * @throws OCSException */ - public function getSubAdminsOfGroup($parameters) { - $group = $parameters['groupid']; + public function getSubAdminsOfGroup($groupId) { // Check group exists - $targetGroup = $this->groupManager->get($group); + $targetGroup = $this->groupManager->get($groupId); if($targetGroup === null) { - return new \OC\OCS\Result(null, 101, 'Group does not exist'); + throw new OCSException('Group does not exist', 101); } $subadmins = $this->groupManager->getSubAdmin()->getGroupsSubAdmins($targetGroup); @@ -176,7 +184,7 @@ class Groups{ $uids[] = $user->getUID(); } - return new \OC\OCS\Result($uids); + return new DataResponse($uids); } } diff --git a/apps/provisioning_api/tests/GroupsTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php similarity index 59% rename from apps/provisioning_api/tests/GroupsTest.php rename to apps/provisioning_api/tests/Controller/GroupsControllerTest.php index 2fa19c4f8b..25059e8542 100644 --- a/apps/provisioning_api/tests/GroupsTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -24,24 +24,20 @@ * */ -namespace OCA\Provisioning_API\Tests; +namespace OCA\Provisioning_API\Tests\Controller; -use OCA\Provisioning_API\Groups; -use OCP\API; +use OCA\Provisioning_API\Controller\GroupsController; use OCP\IGroupManager; use OCP\IUserSession; -use OCP\IRequest; -class GroupsTest extends \Test\TestCase { +class GroupsControllerTest extends \Test\TestCase { /** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */ protected $groupManager; /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ protected $userSession; - /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ - protected $request; /** @var \OC\SubAdmin|\PHPUnit_Framework_MockObject_MockObject */ protected $subAdminManager; - /** @var Groups */ + /** @var GroupsController */ protected $api; protected function setUp() { @@ -61,13 +57,14 @@ class GroupsTest extends \Test\TestCase { $this->userSession = $this->getMockBuilder('OCP\IUserSession') ->disableOriginalConstructor() ->getMock(); - $this->request = $this->getMockBuilder('OCP\IRequest') + $request = $this->getMockBuilder('OCP\IRequest') ->disableOriginalConstructor() ->getMock(); - $this->api = new Groups( + $this->api = new GroupsController( + 'provisioning_api', + $request, $this->groupManager, - $this->userSession, - $this->request + $this->userSession ); } @@ -148,15 +145,6 @@ class GroupsTest extends \Test\TestCase { * @param int|null $offset */ public function testGetGroups($search, $limit, $offset) { - $this->request - ->expects($this->exactly(3)) - ->method('getParam') - ->will($this->returnValueMap([ - ['search', '', $search], - ['limit', null, $limit], - ['offset', null, $offset], - ])); - $groups = [$this->createGroup('group1'), $this->createGroup('group2')]; $search = $search === null ? '' : $search; @@ -167,19 +155,8 @@ class GroupsTest extends \Test\TestCase { ->with($search, $limit, $offset) ->willReturn($groups); - $result = $this->api->getGroups([]); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); - $this->assertEquals(['group1', 'group2'], $result->getData()['groups']); - } - - public function testGetGroupAsUser() { - $result = $this->api->getGroup([]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(API::RESPOND_UNAUTHORISED, $result->getStatusCode()); - + $result = $this->api->getGroups($search, $limit, $offset); + $this->assertEquals(['groups' => ['group1', 'group2']], $result->getData()); } public function testGetGroupAsSubadmin() { @@ -201,17 +178,15 @@ class GroupsTest extends \Test\TestCase { $this->createUser('user2') ]); - $result = $this->api->getGroup([ - 'groupid' => 'group', - ]); + $result = $this->api->getGroup('group'); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); - $this->assertEquals(1, sizeof($result->getData()), 'Asserting the result data array only has the "users" key'); - $this->assertArrayHasKey('users', $result->getData()); - $this->assertEquals(['user1', 'user2'], $result->getData()['users']); + $this->assertEquals(['users' => ['user1', 'user2']], $result->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 997 + */ public function testGetGroupAsIrrelevantSubadmin() { $group = $this->createGroup('group'); $otherGroup = $this->createGroup('otherGroup'); @@ -226,13 +201,7 @@ class GroupsTest extends \Test\TestCase { ->with('group') ->willReturn(true); - $result = $this->api->getGroup([ - 'groupid' => 'group', - ]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(API::RESPOND_UNAUTHORISED, $result->getStatusCode()); + $this->api->getGroup('group'); } public function testGetGroupAsAdmin() { @@ -254,39 +223,29 @@ class GroupsTest extends \Test\TestCase { $this->createUser('user2') ]); - $result = $this->api->getGroup([ - 'groupid' => 'group', - ]); + $result = $this->api->getGroup('group'); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); - $this->assertEquals(1, sizeof($result->getData()), 'Asserting the result data array only has the "users" key'); - $this->assertArrayHasKey('users', $result->getData()); - $this->assertEquals(['user1', 'user2'], $result->getData()['users']); + $this->assertEquals(['users' => ['user1', 'user2']], $result->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 998 + * @expectedExceptionMessage The requested group could not be found + */ public function testGetGroupNonExisting() { $this->asUser(); - $result = $this->api->getGroup([ - 'groupid' => $this->getUniqueID() - ]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(API::RESPOND_NOT_FOUND, $result->getStatusCode()); - $this->assertEquals('The requested group could not be found', $result->getMeta()['message']); + $this->api->getGroup($this->getUniqueID()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + * @expectedExceptionMessage Group does not exist + */ public function testGetSubAdminsOfGroupsNotExists() { - $result = $this->api->getSubAdminsOfGroup([ - 'groupid' => 'NonExistingGroup', - ]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(101, $result->getStatusCode()); - $this->assertEquals('Group does not exist', $result->getMeta()['message']); + $this->api->getSubAdminsOfGroup('NonExistingGroup'); } public function testGetSubAdminsOfGroup() { @@ -305,12 +264,7 @@ class GroupsTest extends \Test\TestCase { $this->createUser('SubAdmin2'), ]); - $result = $this->api->getSubAdminsOfGroup([ - 'groupid' => 'GroupWithSubAdmins', - ]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); + $result = $this->api->getSubAdminsOfGroup('GroupWithSubAdmins'); $this->assertEquals(['SubAdmin1', 'SubAdmin2'], $result->getData()); } @@ -328,53 +282,33 @@ class GroupsTest extends \Test\TestCase { ->willReturn([ ]); - $result = $this->api->getSubAdminsOfGroup([ - 'groupid' => 'GroupWithOutSubAdmins', - ]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); + $result = $this->api->getSubAdminsOfGroup('GroupWithOutSubAdmins'); $this->assertEquals([], $result->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + * @expectedExceptionMessage Invalid group name + */ public function testAddGroupEmptyGroup() { - $this->request - ->method('getParam') - ->with('groupid') - ->willReturn(''); - - $result = $this->api->addGroup([]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(101, $result->getStatusCode()); - $this->assertEquals('Invalid group name', $result->getMeta()['message']); + $this->api->addGroup(''); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + */ public function testAddGroupExistingGroup() { - $this->request - ->method('getParam') - ->with('groupid') - ->willReturn('ExistingGroup'); - $this->groupManager ->method('groupExists') ->with('ExistingGroup') ->willReturn(true); - $result = $this->api->addGroup([]); - - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(102, $result->getStatusCode()); + $this->api->addGroup('ExistingGroup'); } public function testAddGroup() { - $this->request - ->method('getParam') - ->with('groupid') - ->willReturn('NewGroup'); - $this->groupManager ->method('groupExists') ->with('NewGroup') @@ -385,17 +319,10 @@ class GroupsTest extends \Test\TestCase { ->method('createGroup') ->with('NewGroup'); - $result = $this->api->addGroup([]); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); + $this->api->addGroup('NewGroup'); } public function testAddGroupWithSpecialChar() { - $this->request - ->method('getParam') - ->with('groupid') - ->willReturn('Iñtërnâtiônàlizætiøn'); - $this->groupManager ->method('groupExists') ->with('Iñtërnâtiônàlizætiøn') @@ -406,32 +333,28 @@ class GroupsTest extends \Test\TestCase { ->method('createGroup') ->with('Iñtërnâtiônàlizætiøn'); - $result = $this->api->addGroup([]); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); + $this->api->addGroup('Iñtërnâtiônàlizætiøn'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testDeleteGroupNonExisting() { - $result = $this->api->deleteGroup([ - 'groupid' => 'NonExistingGroup' - ]); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(101, $result->getStatusCode()); + $this->api->deleteGroup('NonExistingGroup'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + */ public function testDeleteAdminGroup() { $this->groupManager ->method('groupExists') ->with('admin') ->willReturn('true'); - $result = $this->api->deleteGroup([ - 'groupid' => 'admin' - ]); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(102, $result->getStatusCode()); + $this->api->deleteGroup('admin'); } public function testDeleteGroup() { @@ -450,10 +373,6 @@ class GroupsTest extends \Test\TestCase { ->method('delete') ->willReturn(true); - $result = $this->api->deleteGroup([ - 'groupid' => 'ExistingGroup', - ]); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); + $this->api->deleteGroup('ExistingGroup'); } } From 8f4adebab7cf3133bb33b8081fbdf2c6e2e8e549 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 11 Aug 2016 19:49:45 +0200 Subject: [PATCH 6/8] Move Users to OCSController --- apps/provisioning_api/appinfo/routes.php | 38 +- .../UsersController.php} | 381 +++++----- .../UsersControllerTest.php} | 683 +++++++----------- 3 files changed, 476 insertions(+), 626 deletions(-) rename apps/provisioning_api/lib/{Users.php => Controller/UsersController.php} (56%) rename apps/provisioning_api/tests/{UsersTest.php => Controller/UsersControllerTest.php} (76%) diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index dbc0321efb..473ae2ff71 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -39,31 +39,25 @@ $app->registerRoutes($this, [ ['root' => '/cloud', 'name' => 'Groups#addGroup', 'url' => '/groups', 'verb' => 'POST'], ['root' => '/cloud', 'name' => 'Groups#deleteGroup', 'url' => '/groups/{groupId}', 'verb' => 'DELETE'], ['root' => '/cloud', 'name' => 'Groups#getSubAdminsOfGroup', 'url' => '/groups/{groupId}/subadmins', 'verb' => 'GET'], + + //Users + ['root' => '/cloud', 'name' => 'Users#getUsers', 'url' => '/users', '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#editUser', 'url' => '/users/{userId}', 'verb' => 'PUT'], + ['root' => '/cloud', 'name' => 'Users#deleteUser', 'url' => '/users/{userId}', 'verb' => 'DELETE'], + ['root' => '/cloud', 'name' => 'Users#enableUser', 'url' => '/users/{userId}/enable', 'verb' => 'PUT'], + ['root' => '/cloud', 'name' => 'Users#disableUser', 'url' => '/users/{userId}/disable', 'verb' => 'PUT'], + ['root' => '/cloud', 'name' => 'Users#getUsersGroups', 'url' => '/users/{userId}/groups', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Users#addToGroup', 'url' => '/users/{userId}/groups', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'Users#removeFromGroup', 'url' => '/users/{userId}/groups', 'verb' => 'DELETE'], + ['root' => '/cloud', 'name' => 'Users#getUserSubAdminGroups', 'url' => '/users/{userId}/subadmins', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Users#addSubAdmin', 'url' => '/users/{userId}/subadmins', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'Users#removeSubAdmin', 'url' => '/users/{userId}/subadmins', 'verb' => 'DELETE'], + ], ]); -// Users -$users = new Users( - \OC::$server->getUserManager(), - \OC::$server->getConfig(), - \OC::$server->getGroupManager(), - \OC::$server->getUserSession(), - \OC::$server->getLogger() -); -API::register('get', '/cloud/users', [$users, 'getUsers'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('post', '/cloud/users', [$users, 'addUser'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('get', '/cloud/users/{userid}', [$users, 'getUser'], 'provisioning_api', API::USER_AUTH); -API::register('put', '/cloud/users/{userid}', [$users, 'editUser'], 'provisioning_api', API::USER_AUTH); -API::register('delete', '/cloud/users/{userid}', [$users, 'deleteUser'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('put', '/cloud/users/{userid}/enable', [$users, 'enableUser'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('put', '/cloud/users/{userid}/disable', [$users, 'disableUser'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('get', '/cloud/users/{userid}/groups', [$users, 'getUsersGroups'], 'provisioning_api', API::USER_AUTH); -API::register('post', '/cloud/users/{userid}/groups', [$users, 'addToGroup'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('delete', '/cloud/users/{userid}/groups', [$users, 'removeFromGroup'], 'provisioning_api', API::SUBADMIN_AUTH); -API::register('post', '/cloud/users/{userid}/subadmins', [$users, 'addSubAdmin'], 'provisioning_api', API::ADMIN_AUTH); -API::register('delete', '/cloud/users/{userid}/subadmins', [$users, 'removeSubAdmin'], 'provisioning_api', API::ADMIN_AUTH); -API::register('get', '/cloud/users/{userid}/subadmins', [$users, 'getUserSubAdminGroups'], 'provisioning_api', API::ADMIN_AUTH); - // Apps $apps = new Apps( \OC::$server->getAppManager(), diff --git a/apps/provisioning_api/lib/Users.php b/apps/provisioning_api/lib/Controller/UsersController.php similarity index 56% rename from apps/provisioning_api/lib/Users.php rename to apps/provisioning_api/lib/Controller/UsersController.php index 0e5a8043e8..29d449e97d 100644 --- a/apps/provisioning_api/lib/Users.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -27,17 +27,23 @@ * */ -namespace OCA\Provisioning_API; +namespace OCA\Provisioning_API\Controller; use \OC_Helper; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSException; +use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\AppFramework\OCS\OCSNotFoundException; +use OCP\AppFramework\OCSController; use OCP\Files\NotFoundException; use OCP\IConfig; use OCP\IGroupManager; use OCP\ILogger; +use OCP\IRequest; use OCP\IUserManager; use OCP\IUserSession; -class Users { +class UsersController extends OCSController { /** @var IUserManager */ private $userManager; @@ -51,17 +57,23 @@ class Users { private $logger; /** + * @param string $appName + * @param IRequest $request * @param IUserManager $userManager * @param IConfig $config * @param IGroupManager $groupManager * @param IUserSession $userSession * @param ILogger $logger */ - public function __construct(IUserManager $userManager, + public function __construct($appName, + IRequest $request, + IUserManager $userManager, IConfig $config, IGroupManager $groupManager, IUserSession $userSession, ILogger $logger) { + parent::__construct($appName, $request); + $this->userManager = $userManager; $this->config = $config; $this->groupManager = $groupManager; @@ -70,20 +82,17 @@ class Users { } /** + * @NoAdminRequired + * * returns a list of users * - * @return \OC\OCS\Result + * @param string $search + * @param int $limit + * @param int $offset + * @return DataResponse */ - public function getUsers() { - $search = !empty($_GET['search']) ? $_GET['search'] : ''; - $limit = !empty($_GET['limit']) ? $_GET['limit'] : null; - $offset = !empty($_GET['offset']) ? $_GET['offset'] : null; - - // Check if user is logged in + public function getUsers($search = '', $limit = null, $offset = null) { $user = $this->userSession->getUser(); - if ($user === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } // Admin? Or SubAdmin? $uid = $user->getUID(); @@ -106,89 +115,85 @@ class Users { } $users = array_slice($users, $offset, $limit); - } else { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); } + $users = array_keys($users); - return new \OC\OCS\Result([ + return new DataResponse([ 'users' => $users ]); } /** - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string $userid + * @param string $password + * @param array $groups + * @return DataResponse + * @throws OCSException */ - public function addUser() { - $userId = isset($_POST['userid']) ? $_POST['userid'] : null; - $password = isset($_POST['password']) ? $_POST['password'] : null; - $groups = isset($_POST['groups']) ? $_POST['groups'] : null; + public function addUser($userid, $password, $groups = null) { $user = $this->userSession->getUser(); $isAdmin = $this->groupManager->isAdmin($user->getUID()); $subAdminManager = $this->groupManager->getSubAdmin(); - if (!$isAdmin && !$subAdminManager->isSubAdmin($user)) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - - if($this->userManager->userExists($userId)) { + if($this->userManager->userExists($userid)) { $this->logger->error('Failed addUser attempt: User already exists.', ['app' => 'ocs_api']); - return new \OC\OCS\Result(null, 102, 'User already exists'); + throw new OCSException('User already exists', 102); } if(is_array($groups)) { foreach ($groups as $group) { - if(!$this->groupManager->groupExists($group)){ - return new \OC\OCS\Result(null, 104, 'group '.$group.' does not exist'); + if(!$this->groupManager->groupExists($group)) { + throw new OCSException('group '.$group.' does not exist', 104); } if(!$isAdmin && !$subAdminManager->isSubAdminofGroup($user, $this->groupManager->get($group))) { - return new \OC\OCS\Result(null, 105, 'insufficient privileges for group '. $group); + throw new OCSException('insufficient privileges for group '. $group, 105); } } } else { if(!$isAdmin) { - return new \OC\OCS\Result(null, 106, 'no group specified (required for subadmins)'); + throw new OCSException('no group specified (required for subadmins)', 106); } } try { - $newUser = $this->userManager->createUser($userId, $password); - $this->logger->info('Successful addUser call with userid: '.$userId, ['app' => 'ocs_api']); + $newUser = $this->userManager->createUser($userid, $password); + $this->logger->info('Successful addUser call with userid: '.$userid, ['app' => 'ocs_api']); if (is_array($groups)) { foreach ($groups as $group) { $this->groupManager->get($group)->addUser($newUser); - $this->logger->info('Added userid '.$userId.' to group '.$group, ['app' => 'ocs_api']); + $this->logger->info('Added userid '.$userid.' to group '.$group, ['app' => 'ocs_api']); } } - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } catch (\Exception $e) { $this->logger->error('Failed addUser attempt with exception: '.$e->getMessage(), ['app' => 'ocs_api']); - return new \OC\OCS\Result(null, 101, 'Bad request'); + throw new OCSException('Bad request', 101); } } /** + * @NoAdminRequired + * @NoSubAdminRequired + * * gets user info * - * @param array $parameters - * @return \OC\OCS\Result + * @param string $userId + * @return DataResponse + * @throws OCSException */ - public function getUser($parameters) { - $userId = $parameters['userid']; - - // Check if user is logged in + public function getUser($userId) { $currentLoggedInUser = $this->userSession->getUser(); - if ($currentLoggedInUser === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } $data = []; // Check if the target user exists $targetUserObject = $this->userManager->get($userId); if($targetUserObject === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_NOT_FOUND, 'The requested user could not be found'); + throw new OCSException('The requested user could not be found', \OCP\API::RESPOND_NOT_FOUND); } // Admin? Or SubAdmin? @@ -198,7 +203,7 @@ class Users { } else { // Check they are looking up themselves if($currentLoggedInUser->getUID() !== $userId) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } } @@ -207,32 +212,32 @@ class Users { $data['email'] = $targetUserObject->getEMailAddress(); $data['displayname'] = $targetUserObject->getDisplayName(); - return new \OC\OCS\Result($data); + return new DataResponse($data); } - /** + /** + * @NoAdminRequired + * @NoSubAdminRequired + * * edit users * - * @param array $parameters - * @return \OC\OCS\Result + * @param string $userId + * @param string $key + * @param string $value + * @return DataResponse + * @throws OCSException + * @throws OCSForbiddenException */ - public function editUser($parameters) { - /** @var string $targetUserId */ - $targetUserId = $parameters['userid']; - - // Check if user is logged in + public function editUser($userId, $key, $value) { $currentLoggedInUser = $this->userSession->getUser(); - if ($currentLoggedInUser === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - $targetUser = $this->userManager->get($targetUserId); + $targetUser = $this->userManager->get($userId); if($targetUser === null) { - return new \OC\OCS\Result(null, 997); + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } $permittedFields = []; - if($targetUserId === $currentLoggedInUser->getUID()) { + if($userId === $currentLoggedInUser->getUID()) { // Editing self (display, email) $permittedFields[] = 'display'; $permittedFields[] = 'email'; @@ -253,20 +258,20 @@ class Users { $permittedFields[] = 'email'; } else { // No rights - return new \OC\OCS\Result(null, 997); + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } } // Check if permitted to edit this field - if(!in_array($parameters['_put']['key'], $permittedFields)) { - return new \OC\OCS\Result(null, 997); + if(!in_array($key, $permittedFields)) { + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } // Process the edit - switch($parameters['_put']['key']) { + switch($key) { case 'display': - $targetUser->setDisplayName($parameters['_put']['value']); + $targetUser->setDisplayName($value); break; case 'quota': - $quota = $parameters['_put']['value']; + $quota = $value; if($quota !== 'none' and $quota !== 'default') { if (is_numeric($quota)) { $quota = floatval($quota); @@ -274,7 +279,7 @@ class Users { $quota = \OCP\Util::computerFileSize($quota); } if ($quota === false) { - return new \OC\OCS\Result(null, 103, "Invalid quota value {$parameters['_put']['value']}"); + throw new OCSException('Invalid quota value '.$value, 103); } if($quota === 0) { $quota = 'default'; @@ -287,115 +292,118 @@ class Users { $targetUser->setQuota($quota); break; case 'password': - $targetUser->setPassword($parameters['_put']['value']); + $targetUser->setPassword($value); break; case 'email': - if(filter_var($parameters['_put']['value'], FILTER_VALIDATE_EMAIL)) { - $targetUser->setEMailAddress($parameters['_put']['value']); + if(filter_var($value, FILTER_VALIDATE_EMAIL)) { + $targetUser->setEMailAddress($value); } else { - return new \OC\OCS\Result(null, 102); + throw new OCSException('', 102); } break; default: - return new \OC\OCS\Result(null, 103); + throw new OCSException('', 103); } - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } /** - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string $userId + * @return DataResponse + * @throws OCSException + * @throws OCSForbiddenException */ - public function deleteUser($parameters) { - // Check if user is logged in + public function deleteUser($userId) { $currentLoggedInUser = $this->userSession->getUser(); - if ($currentLoggedInUser === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - $targetUser = $this->userManager->get($parameters['userid']); + $targetUser = $this->userManager->get($userId); if($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) { - return new \OC\OCS\Result(null, 101); + throw new OCSException('', 101); } // If not permitted $subAdminManager = $this->groupManager->getSubAdmin(); if(!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { - return new \OC\OCS\Result(null, 997); + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } // Go ahead with the delete if($targetUser->delete()) { - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } else { - return new \OC\OCS\Result(null, 101); + throw new OCSException('', 101); } } /** - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string $userId + * @return DataResponse */ - public function disableUser($parameters) { - return $this->setEnabled($parameters, false); + public function disableUser($userId) { + return $this->setEnabled($userId, false); } /** - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string $userId + * @return DataResponse */ - public function enableUser($parameters) { - return $this->setEnabled($parameters, true); + public function enableUser($userId) { + return $this->setEnabled($userId, true); } /** - * @param array $parameters + * @param string $userId * @param bool $value - * @return \OC\OCS\Result + * @return DataResponse + * @throws OCSException + * @throws OCSForbiddenException */ - private function setEnabled($parameters, $value) { - // Check if user is logged in + private function setEnabled($userId, $value) { $currentLoggedInUser = $this->userSession->getUser(); - if ($currentLoggedInUser === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - $targetUser = $this->userManager->get($parameters['userid']); + $targetUser = $this->userManager->get($userId); if($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) { - return new \OC\OCS\Result(null, 101); + throw new OCSException('', 101); } // If not permitted $subAdminManager = $this->groupManager->getSubAdmin(); if(!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { - return new \OC\OCS\Result(null, 997); + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } // enable/disable the user now $targetUser->setEnabled($value); - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } /** - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * @NoSubAdminRequired + * + * @param string $userId + * @return DataResponse + * @throws OCSForbiddenException + * @throws OCSNotFoundException */ - public function getUsersGroups($parameters) { - // Check if user is logged in + public function getUsersGroups($userId) { $loggedInUser = $this->userSession->getUser(); - if ($loggedInUser === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - $targetUser = $this->userManager->get($parameters['userid']); + $targetUser = $this->userManager->get($userId); if($targetUser === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_NOT_FOUND); + throw new OCSException('', \OCP\API::RESPOND_NOT_FOUND); } if($targetUser->getUID() === $loggedInUser->getUID() || $this->groupManager->isAdmin($loggedInUser->getUID())) { // Self lookup or admin lookup - return new \OC\OCS\Result([ + return new DataResponse([ 'groups' => $this->groupManager->getUserGroupIds($targetUser) ]); } else { @@ -412,87 +420,75 @@ class Users { $getSubAdminsGroups, $this->groupManager->getUserGroupIds($targetUser) ); - return new \OC\OCS\Result(array('groups' => $groups)); + return new DataResponse(['groups' => $groups]); } else { // Not permitted - return new \OC\OCS\Result(null, 997); + throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); } } } /** - * @param array $parameters - * @return \OC\OCS\Result + * @param string $userId + * @param string $groupid + * @return DataResponse + * @throws OCSException */ - public function addToGroup($parameters) { - // Check if user is logged in - $user = $this->userSession->getUser(); - if ($user === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); + public function addToGroup($userId, $groupid = '') { + if($groupid === '') { + throw new OCSException('', 101); } - // Check they're an admin - if(!$this->groupManager->isAdmin($user->getUID())) { - // This user doesn't have rights to add a user to this group - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - - $groupId = !empty($_POST['groupid']) ? $_POST['groupid'] : null; - if($groupId === null) { - return new \OC\OCS\Result(null, 101); - } - - $group = $this->groupManager->get($groupId); - $targetUser = $this->userManager->get($parameters['userid']); + $group = $this->groupManager->get($groupid); + $targetUser = $this->userManager->get($userId); if($group === null) { - return new \OC\OCS\Result(null, 102); + throw new OCSException('', 102); } if($targetUser === null) { - return new \OC\OCS\Result(null, 103); + throw new OCSException('', 103); } // Add user to group $group->addUser($targetUser); - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } /** - * @param array $parameters - * @return \OC\OCS\Result + * @NoAdminRequired + * + * @param string userId + * @param string $groupid + * @return DataResponse + * @throws OCSException */ - public function removeFromGroup($parameters) { - // Check if user is logged in + public function removeFromGroup($userId, $groupid) { $loggedInUser = $this->userSession->getUser(); - if ($loggedInUser === null) { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED); + + if($groupid === null) { + throw new OCSException('', 101); } - $group = !empty($parameters['_delete']['groupid']) ? $parameters['_delete']['groupid'] : null; + $group = $this->groupManager->get($groupid); if($group === null) { - return new \OC\OCS\Result(null, 101); + throw new OCSException('', 102); } - $group = $this->groupManager->get($group); - if($group === null) { - return new \OC\OCS\Result(null, 102); - } - - $targetUser = $this->userManager->get($parameters['userid']); + $targetUser = $this->userManager->get($userId); if($targetUser === null) { - return new \OC\OCS\Result(null, 103); + throw new OCSException('', 103); } // If they're not an admin, check they are a subadmin of the group in question $subAdminManager = $this->groupManager->getSubAdmin(); if(!$this->groupManager->isAdmin($loggedInUser->getUID()) && !$subAdminManager->isSubAdminofGroup($loggedInUser, $group)) { - return new \OC\OCS\Result(null, 104); + throw new OCSException('', 104); } // Check they aren't removing themselves from 'admin' or their 'subadmin; group - if($parameters['userid'] === $loggedInUser->getUID()) { + if($userId === $loggedInUser->getUID()) { if($this->groupManager->isAdmin($loggedInUser->getUID())) { if($group->getGID() === 'admin') { - return new \OC\OCS\Result(null, 105, 'Cannot remove yourself from the admin group'); + throw new OCSException('Cannot remove yourself from the admin group', 105); } } else { // Not an admin, check they are not removing themself from their subadmin group @@ -502,96 +498,101 @@ class Users { } if(in_array($group->getGID(), $subAdminGroups, true)) { - return new \OC\OCS\Result(null, 105, 'Cannot remove yourself from this group as you are a SubAdmin'); + throw new OCSException('Cannot remove yourself from this group as you are a SubAdmin', 105); } } } // Remove user from group $group->removeUser($targetUser); - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } /** * Creates a subadmin * - * @param array $parameters - * @return \OC\OCS\Result + * @param string $userId + * @param string $groupid + * @return DataResponse + * @throws OCSException */ - public function addSubAdmin($parameters) { - $group = $this->groupManager->get($_POST['groupid']); - $user = $this->userManager->get($parameters['userid']); + public function addSubAdmin($userId, $groupid) { + $group = $this->groupManager->get($groupid); + $user = $this->userManager->get($userId); // Check if the user exists if($user === null) { - return new \OC\OCS\Result(null, 101, 'User does not exist'); + throw new OCSException('User does not exist', 101); } // Check if group exists if($group === null) { - return new \OC\OCS\Result(null, 102, 'Group:'.$_POST['groupid'].' does not exist'); + throw new OCSException('Group:'.$groupid.' does not exist', 102); } // Check if trying to make subadmin of admin group - if(strtolower($_POST['groupid']) === 'admin') { - return new \OC\OCS\Result(null, 103, 'Cannot create subadmins for admin group'); + if(strtolower($groupid) === 'admin') { + throw new OCSException('Cannot create subadmins for admin group', 103); } $subAdminManager = $this->groupManager->getSubAdmin(); // We cannot be subadmin twice if ($subAdminManager->isSubAdminofGroup($user, $group)) { - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } // Go if($subAdminManager->createSubAdmin($user, $group)) { - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } else { - return new \OC\OCS\Result(null, 103, 'Unknown error occurred'); + throw new OCSException('Unknown error occurred', 103); } } /** * Removes a subadmin from a group * - * @param array $parameters - * @return \OC\OCS\Result + * @param string $userId + * @param string $groupid + * @return DataResponse + * @throws OCSException */ - public function removeSubAdmin($parameters) { - $group = $this->groupManager->get($parameters['_delete']['groupid']); - $user = $this->userManager->get($parameters['userid']); + public function removeSubAdmin($userId, $groupid) { + $group = $this->groupManager->get($groupid); + $user = $this->userManager->get($userId); $subAdminManager = $this->groupManager->getSubAdmin(); // Check if the user exists if($user === null) { - return new \OC\OCS\Result(null, 101, 'User does not exist'); + throw new OCSException('User does not exist', 101); } // Check if the group exists if($group === null) { - return new \OC\OCS\Result(null, 101, 'Group does not exist'); + throw new OCSException('Group does not exist', 101); } // Check if they are a subadmin of this said group if(!$subAdminManager->isSubAdminofGroup($user, $group)) { - return new \OC\OCS\Result(null, 102, 'User is not a subadmin of this group'); + throw new OCSException('User is not a subadmin of this group', 102); } // Go if($subAdminManager->deleteSubAdmin($user, $group)) { - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } else { - return new \OC\OCS\Result(null, 103, 'Unknown error occurred'); + throw new OCSException('Unknown error occurred', 103); } } /** * Get the groups a user is a subadmin of * - * @param array $parameters - * @return \OC\OCS\Result + * @param string $userId + * @return DataResponse + * @throws OCSException */ - public function getUserSubAdminGroups($parameters) { - $user = $this->userManager->get($parameters['userid']); + public function getUserSubAdminGroups($userId) { + $user = $this->userManager->get($userId); // Check if the user exists if($user === null) { - return new \OC\OCS\Result(null, 101, 'User does not exist'); + throw new OCSException('User does not exist', 101); } // Get the subadmin groups @@ -601,9 +602,9 @@ class Users { } if(!$groups) { - return new \OC\OCS\Result(null, 102, 'Unknown error occurred'); + throw new OCSException('Unknown error occurred', 102); } else { - return new \OC\OCS\Result($groups); + return new DataResponse($groups); } } diff --git a/apps/provisioning_api/tests/UsersTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php similarity index 76% rename from apps/provisioning_api/tests/UsersTest.php rename to apps/provisioning_api/tests/Controller/UsersControllerTest.php index e67d603e48..e04ee86fea 100644 --- a/apps/provisioning_api/tests/UsersTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -27,10 +27,9 @@ * */ -namespace OCA\Provisioning_API\Tests; +namespace OCA\Provisioning_API\Tests\Controller; -use OCA\Provisioning_API\Users; -use OCP\API; +use OCA\Provisioning_API\Controller\UsersController; use OCP\IUserManager; use OCP\IConfig; use OCP\IUserSession; @@ -38,7 +37,7 @@ use PHPUnit_Framework_MockObject_MockObject; use Test\TestCase as OriginalTest; use OCP\ILogger; -class UsersTest extends OriginalTest { +class UsersControllerTest extends OriginalTest { /** @var IUserManager | PHPUnit_Framework_MockObject_MockObject */ protected $userManager; @@ -50,12 +49,10 @@ class UsersTest extends OriginalTest { protected $userSession; /** @var ILogger | PHPUnit_Framework_MockObject_MockObject */ protected $logger; - /** @var Users | PHPUnit_Framework_MockObject_MockObject */ + /** @var UsersController | PHPUnit_Framework_MockObject_MockObject */ protected $api; protected function tearDown() { - $_GET = null; - $_POST = null; parent::tearDown(); } @@ -77,8 +74,13 @@ class UsersTest extends OriginalTest { $this->logger = $this->getMockBuilder('OCP\ILogger') ->disableOriginalConstructor() ->getMock(); - $this->api = $this->getMockBuilder('OCA\Provisioning_API\Users') + $request = $this->getMockBuilder('OCP\IRequest') + ->disableOriginalConstructor() + ->getMock(); + $this->api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController') ->setConstructorArgs([ + 'provisioning_api', + $request, $this->userManager, $this->config, $this->groupManager, @@ -89,19 +91,7 @@ class UsersTest extends OriginalTest { ->getMock(); } - public function testGetUsersNotLoggedIn() { - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue(null)); - - $expected = new \OC\OCS\Result(null, API::RESPOND_UNAUTHORISED); - $this->assertEquals($expected, $this->api->getUsers()); - } - public function testGetUsersAsAdmin() { - $_GET['search'] = 'MyCustomSearch'; - $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() ->getMock(); @@ -123,19 +113,16 @@ class UsersTest extends OriginalTest { ->with('MyCustomSearch', null, null) ->will($this->returnValue(['Admin' => [], 'Foo' => [], 'Bar' => []])); - $expected = new \OC\OCS\Result([ - 'users' => [ + $expected = ['users' => [ 'Admin', 'Foo', 'Bar', ], - ]); - $this->assertEquals($expected, $this->api->getUsers()); + ]; + $this->assertEquals($expected, $this->api->getUsers('MyCustomSearch')->getData()); } public function testGetUsersAsSubAdmin() { - $_GET['search'] = 'MyCustomSearch'; - $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() ->getMock(); @@ -186,51 +173,20 @@ class UsersTest extends OriginalTest { ->method('displayNamesInGroup') ->will($this->onConsecutiveCalls(['AnotherUserInTheFirstGroup' => []], ['UserInTheSecondGroup' => []])); - $expected = new \OC\OCS\Result([ + $expected = [ 'users' => [ 'AnotherUserInTheFirstGroup', 'UserInTheSecondGroup', ], - ]); - $this->assertEquals($expected, $this->api->getUsers()); - } - - public function testGetUsersAsRegularUser() { - $_GET['search'] = 'MyCustomSearch'; - - $loggedInUser = $this->getMockBuilder('OCP\IUser') - ->disableOriginalConstructor() - ->getMock(); - $loggedInUser - ->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('regularUser')); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue($loggedInUser)); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->will($this->returnValue(false)); - $subAdminManager = $this->getMockBuilder('OC\SubAdmin') - ->disableOriginalConstructor()->getMock(); - $subAdminManager - ->expects($this->once()) - ->method('isSubAdmin') - ->with($loggedInUser) - ->will($this->returnValue(false)); - $this->groupManager - ->expects($this->once()) - ->method('getSubAdmin') - ->will($this->returnValue($subAdminManager)); - - $expected = new \OC\OCS\Result(null, API::RESPOND_UNAUTHORISED); - $this->assertEquals($expected, $this->api->getUsers()); + ]; + $this->assertEquals($expected, $this->api->getUsers('MyCustomSearch')->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + */ public function testAddUserAlreadyExisting() { - $_POST['userid'] = 'AlreadyExistingUser'; $this->userManager ->expects($this->once()) ->method('userExists') @@ -257,13 +213,15 @@ class UsersTest extends OriginalTest { ->with('adminUser') ->willReturn(true); - $expected = new \OC\OCS\Result(null, 102, 'User already exists'); - $this->assertEquals($expected, $this->api->addUser()); + $this->api->addUser('AlreadyExistingUser', null, null); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 104 + * @expectedExceptionMessage group NonExistingGroup does not exist + */ public function testAddUserNonExistingGroup() { - $_POST['userid'] = 'NewUser'; - $_POST['groups'] = ['NonExistingGroup']; $this->userManager ->expects($this->once()) ->method('userExists') @@ -291,13 +249,15 @@ class UsersTest extends OriginalTest { ->with('NonExistingGroup') ->willReturn(false); - $expected = new \OC\OCS\Result(null, 104, 'group NonExistingGroup does not exist'); - $this->assertEquals($expected, $this->api->addUser()); + $this->api->addUser('NewUser', 'pass', ['NonExistingGroup']); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 104 + * @expectedExceptionMessage group NonExistingGroup does not exist + */ public function testAddUserExistingGroupNonExistingGroup() { - $_POST['userid'] = 'NewUser'; - $_POST['groups'] = ['ExistingGroup', 'NonExistingGroup']; $this->userManager ->expects($this->once()) ->method('userExists') @@ -331,13 +291,10 @@ class UsersTest extends OriginalTest { ['NonExistingGroup', false] ])); - $expected = new \OC\OCS\Result(null, 104, 'group NonExistingGroup does not exist'); - $this->assertEquals($expected, $this->api->addUser()); + $this->api->addUser('NewUser', 'pass', ['ExistingGroup', 'NonExistingGroup']); } public function testAddUserSuccessful() { - $_POST['userid'] = 'NewUser'; - $_POST['password'] = 'PasswordOfTheNewUser'; $this->userManager ->expects($this->once()) ->method('userExists') @@ -368,14 +325,10 @@ class UsersTest extends OriginalTest { ->with('adminUser') ->willReturn(true); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->addUser()); + $this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser')->getData()); } public function testAddUserExistingGroup() { - $_POST['userid'] = 'NewUser'; - $_POST['password'] = 'PasswordOfTheNewUser'; - $_POST['groups'] = ['ExistingGroup']; $this->userManager ->expects($this->once()) ->method('userExists') @@ -430,13 +383,15 @@ class UsersTest extends OriginalTest { ['Added userid NewUser to group ExistingGroup', ['app' => 'ocs_api']] ); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->addUser()); + $this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', ['ExistingGroup'])->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + * @expectedExceptionMessage Bad request + */ public function testAddUserUnsuccessful() { - $_POST['userid'] = 'NewUser'; - $_POST['password'] = 'PasswordOfTheNewUser'; $this->userManager ->expects($this->once()) ->method('userExists') @@ -468,49 +423,15 @@ class UsersTest extends OriginalTest { ->with('adminUser') ->willReturn(true); - $expected = new \OC\OCS\Result(null, 101, 'Bad request'); - $this->assertEquals($expected, $this->api->addUser()); - } - - public function testAddUserAsRegularUser() { - $_POST['userid'] = 'NewUser'; - $_POST['password'] = 'PasswordOfTheNewUser'; - $loggedInUser = $this->getMockBuilder('OCP\IUser') - ->disableOriginalConstructor() - ->getMock(); - $loggedInUser - ->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('regularUser')); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue($loggedInUser)); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('regularUser') - ->willReturn(false); - $subAdminManager = $this->getMockBuilder('OC\SubAdmin') - ->disableOriginalConstructor()->getMock(); - $subAdminManager - ->expects($this->once()) - ->method('isSubAdmin') - ->with($loggedInUser) - ->willReturn(false); - $this->groupManager - ->expects($this->once()) - ->method('getSubAdmin') - ->with() - ->willReturn($subAdminManager); - - $expected = new \OC\OCS\Result(null, API::RESPOND_UNAUTHORISED); - $this->assertEquals($expected, $this->api->addUser()); + $this->api->addUser('NewUser', 'PasswordOfTheNewUser'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 106 + * @expectedExceptionMessage no group specified (required for subadmins) + */ public function testAddUserAsSubAdminNoGroup() { - $_POST['userid'] = 'NewUser'; - $_POST['password'] = 'PasswordOfTheNewUser'; $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() ->getMock(); @@ -529,25 +450,21 @@ class UsersTest extends OriginalTest { ->willReturn(false); $subAdminManager = $this->getMockBuilder('OC\SubAdmin') ->disableOriginalConstructor()->getMock(); - $subAdminManager - ->expects($this->once()) - ->method('isSubAdmin') - ->with($loggedInUser) - ->willReturn(true); $this->groupManager ->expects($this->once()) ->method('getSubAdmin') ->with() ->willReturn($subAdminManager); - $expected = new \OC\OCS\Result(null, 106, 'no group specified (required for subadmins)'); - $this->assertEquals($expected, $this->api->addUser()); + $this->api->addUser('NewUser', 'PasswordOfTheNewUser', null); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 105 + * @expectedExceptionMessage insufficient privileges for group ExistingGroup + */ public function testAddUserAsSubAdminValidGroupNotSubAdmin() { - $_POST['userid'] = 'NewUser'; - $_POST['password'] = 'PasswordOfTheNewUser'; - $_POST['groups'] = ['ExistingGroup']; $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() ->getMock(); @@ -572,11 +489,6 @@ class UsersTest extends OriginalTest { ->willReturn($existingGroup); $subAdminManager = $this->getMockBuilder('OC\SubAdmin') ->disableOriginalConstructor()->getMock(); - $subAdminManager - ->expects($this->once()) - ->method('isSubAdmin') - ->with($loggedInUser) - ->willReturn(true); $subAdminManager ->expects($this->once()) ->method('isSubAdminOfGroup') @@ -593,14 +505,10 @@ class UsersTest extends OriginalTest { ->with('ExistingGroup') ->willReturn(true); - $expected = new \OC\OCS\Result(null, 105, 'insufficient privileges for group ExistingGroup'); - $this->assertEquals($expected, $this->api->addUser()); + $this->api->addUser('NewUser', 'PasswordOfTheNewUser', ['ExistingGroup'])->getData(); } public function testAddUserAsSubAdminExistingGroups() { - $_POST['userid'] = 'NewUser'; - $_POST['password'] = 'PasswordOfTheNewUser'; - $_POST['groups'] = ['ExistingGroup1', 'ExistingGroup2']; $this->userManager ->expects($this->once()) ->method('userExists') @@ -679,11 +587,6 @@ class UsersTest extends OriginalTest { ->expects($this->once()) ->method('getSubAdmin') ->willReturn($subAdminManager); - $subAdminManager - ->expects($this->once()) - ->method('isSubAdmin') - ->with($loggedInUser) - ->willReturn(true); $subAdminManager ->expects($this->exactly(2)) ->method('isSubAdminOfGroup') @@ -693,22 +596,14 @@ class UsersTest extends OriginalTest { ) ->willReturn(true); - - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->addUser()); - } - - - public function testGetUserNotLoggedIn() { - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue(null)); - - $expected = new \OC\OCS\Result(null, API::RESPOND_UNAUTHORISED); - $this->assertEquals($expected, $this->api->getUser(['userid' => 'UserToGet'])); + $this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', ['ExistingGroup1', 'ExistingGroup2'])->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 998 + * @expectedExceptionMessage The requested user could not be found + */ public function testGetUserTargetDoesNotExist() { $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() @@ -723,8 +618,7 @@ class UsersTest extends OriginalTest { ->with('UserToGet') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, API::RESPOND_NOT_FOUND, 'The requested user could not be found'); - $this->assertEquals($expected, $this->api->getUser(['userid' => 'UserToGet'])); + $this->api->getUser('UserToGet'); } public function testGetUserAsAdmin() { @@ -770,15 +664,13 @@ class UsersTest extends OriginalTest { ->method('getDisplayName') ->will($this->returnValue('Demo User')); - $expected = new \OC\OCS\Result( - [ - 'enabled' => 'true', - 'quota' => ['DummyValue'], - 'email' => 'demo@owncloud.org', - 'displayname' => 'Demo User', - ] - ); - $this->assertEquals($expected, $this->api->getUser(['userid' => 'UserToGet'])); + $expected = [ + 'enabled' => 'true', + 'quota' => ['DummyValue'], + 'email' => 'demo@owncloud.org', + 'displayname' => 'Demo User', + ]; + $this->assertEquals($expected, $this->api->getUser('UserToGet')->getData()); } public function testGetUserAsSubAdminAndUserIsAccessible() { @@ -837,17 +729,20 @@ class UsersTest extends OriginalTest { ->method('getDisplayName') ->will($this->returnValue('Demo User')); - $expected = new \OC\OCS\Result( - [ - 'enabled' => 'true', - 'quota' => ['DummyValue'], - 'email' => 'demo@owncloud.org', - 'displayname' => 'Demo User', - ] - ); - $this->assertEquals($expected, $this->api->getUser(['userid' => 'UserToGet'])); + $expected = [ + 'enabled' => 'true', + 'quota' => ['DummyValue'], + 'email' => 'demo@owncloud.org', + 'displayname' => 'Demo User', + ]; + $this->assertEquals($expected, $this->api->getUser('UserToGet')->getData()); } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 997 + */ public function testGetUserAsSubAdminAndUserIsNotAccessible() { $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() @@ -886,8 +781,7 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, API::RESPOND_UNAUTHORISED); - $this->assertEquals($expected, $this->api->getUser(['userid' => 'UserToGet'])); + $this->api->getUser('UserToGet'); } public function testGetUserAsSubAdminSelfLookup() { @@ -941,22 +835,12 @@ class UsersTest extends OriginalTest { ->method('getEMailAddress') ->will($this->returnValue('subadmin@owncloud.org')); - $expected = new \OC\OCS\Result([ + $expected = [ 'quota' => ['DummyValue'], 'email' => 'subadmin@owncloud.org', 'displayname' => 'Subadmin User', - ]); - $this->assertEquals($expected, $this->api->getUser(['userid' => 'subadmin'])); - } - - public function testEditUserNotLoggedIn() { - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue(null)); - - $expected = new \OC\OCS\Result(null, API::RESPOND_UNAUTHORISED); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit'])); + ]; + $this->assertEquals($expected, $this->api->getUser('subadmin')->getData()); } public function testEditUserRegularUserSelfEditChangeDisplayName() { @@ -984,8 +868,7 @@ class UsersTest extends OriginalTest { ->method('setDisplayName') ->with('NewDisplayName'); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'display', 'value' => 'NewDisplayName']])); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'display', 'NewDisplayName')->getData()); } public function testEditUserRegularUserSelfEditChangeEmailValid() { @@ -1013,10 +896,14 @@ class UsersTest extends OriginalTest { ->method('setEMailAddress') ->with('demo@owncloud.org'); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'email', 'value' => 'demo@owncloud.org']])); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@owncloud.org')->getData()); } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + */ public function testEditUserRegularUserSelfEditChangeEmailInvalid() { $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() @@ -1038,8 +925,7 @@ class UsersTest extends OriginalTest { ->with('UserToEdit') ->will($this->returnValue($targetUser)); - $expected = new \OC\OCS\Result(null, 102); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'email', 'value' => 'demo.org']])); + $this->api->editUser('UserToEdit', 'email', 'demo.org'); } public function testEditUserRegularUserSelfEditChangePassword() { @@ -1067,10 +953,14 @@ class UsersTest extends OriginalTest { ->method('setPassword') ->with('NewPassword'); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'password', 'value' => 'NewPassword']])); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'password', 'NewPassword')->getData()); } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 997 + */ public function testEditUserRegularUserSelfEditChangeQuota() { $loggedInUser = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() @@ -1092,8 +982,7 @@ class UsersTest extends OriginalTest { ->with('UserToEdit') ->will($this->returnValue($targetUser)); - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => 'NewQuota']])); + $this->api->editUser('UserToEdit', 'quota', 'NewQuota'); } public function testEditUserAdminUserSelfEditChangeValidQuota() { @@ -1121,10 +1010,15 @@ class UsersTest extends OriginalTest { ->with('UserToEdit') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => '3042824']])); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData()); } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 103 + * @expectedExceptionMessage Invalid quota value ABC + */ public function testEditUserAdminUserSelfEditChangeInvalidQuota() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1147,8 +1041,7 @@ class UsersTest extends OriginalTest { ->with('UserToEdit') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 103, 'Invalid quota value ABC'); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => 'ABC']])); + $this->api->editUser('UserToEdit', 'quota', 'ABC'); } public function testEditUserAdminUserEditChangeValidQuota() { @@ -1183,8 +1076,7 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => '3042824']])); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData()); } public function testEditUserSubadminUserAccessible() { @@ -1219,10 +1111,13 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => '3042824']])); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 997 + */ public function testEditUserSubadminUserInaccessible() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1252,20 +1147,13 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'quota', 'value' => '3042824']])); - } - - public function testDeleteUserNotLoggedIn() { - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue(null)); - - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); + $this->api->editUser('UserToEdit', 'quota', 'value'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testDeleteUserNotExistingUser() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1282,10 +1170,13 @@ class UsersTest extends OriginalTest { ->with('UserToDelete') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 101); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); + $this->api->deleteUser('UserToDelete'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testDeleteUserSelf() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1307,8 +1198,7 @@ class UsersTest extends OriginalTest { ->with('UserToDelete') ->will($this->returnValue($targetUser)); - $expected = new \OC\OCS\Result(null, 101); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); + $this->api->deleteUser('UserToDelete'); } public function testDeleteSuccessfulUserAsAdmin() { @@ -1341,10 +1231,13 @@ class UsersTest extends OriginalTest { ->method('delete') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); + $this->assertEquals([], $this->api->deleteUser('UserToDelete')->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testDeleteUnsuccessfulUserAsAdmin() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1375,8 +1268,7 @@ class UsersTest extends OriginalTest { ->method('delete') ->will($this->returnValue(false)); - $expected = new \OC\OCS\Result(null, 101); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); + $this->api->deleteUser('UserToDelete'); } public function testDeleteSuccessfulUserAsSubadmin() { @@ -1420,10 +1312,13 @@ class UsersTest extends OriginalTest { ->method('delete') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); + $this->assertEquals([], $this->api->deleteUser('UserToDelete')->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testDeleteUnsuccessfulUserAsSubadmin() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1465,10 +1360,13 @@ class UsersTest extends OriginalTest { ->method('delete') ->will($this->returnValue(false)); - $expected = new \OC\OCS\Result(null, 101); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); + $this->api->deleteUser('UserToDelete'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 997 + */ public function testDeleteUserAsSubAdminAndUserIsNotAccessible() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1506,20 +1404,13 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->deleteUser(['userid' => 'UserToDelete'])); - } - - public function testGetUsersGroupsNotLoggedIn() { - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue(null)); - - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->getUsersGroups(['userid' => 'UserToLookup'])); + $this->api->deleteUser('UserToDelete'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 998 + */ public function testGetUsersGroupsTargetUserNotExisting() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $this->userSession @@ -1527,8 +1418,7 @@ class UsersTest extends OriginalTest { ->method('getUser') ->will($this->returnValue($loggedInUser)); - $expected = new \OC\OCS\Result(null, 998); - $this->assertEquals($expected, $this->api->getUsersGroups(['userid' => 'UserToLookup'])); + $this->api->getUsersGroups('UserToLookup'); } public function testGetUsersGroupsSelfTargetted() { @@ -1557,8 +1447,7 @@ class UsersTest extends OriginalTest { ->with($targetUser) ->will($this->returnValue(['DummyValue'])); - $expected = new \OC\OCS\Result(['groups' => ['DummyValue']]); - $this->assertEquals($expected, $this->api->getUsersGroups(['userid' => 'UserToLookup'])); + $this->assertEquals(['groups' => ['DummyValue']], $this->api->getUsersGroups('UserToLookup')->getData()); } public function testGetUsersGroupsForAdminUser() { @@ -1592,8 +1481,7 @@ class UsersTest extends OriginalTest { ->with('admin') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(['groups' => ['DummyValue']]); - $this->assertEquals($expected, $this->api->getUsersGroups(['userid' => 'UserToLookup'])); + $this->assertEquals(['groups' => ['DummyValue']], $this->api->getUsersGroups('UserToLookup')->getData()); } public function testGetUsersGroupsForSubAdminUserAndUserIsAccessible() { @@ -1653,11 +1541,13 @@ class UsersTest extends OriginalTest { ->with($targetUser) ->will($this->returnValue(['Group1'])); - $expected = new \OC\OCS\Result(['groups' => ['Group1']]); - $this->assertEquals($expected, $this->api->getUsersGroups(['userid' => 'UserToLookup'])); + $this->assertEquals(['groups' => ['Group1']], $this->api->getUsersGroups('UserToLookup')->getData()); } - + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 997 + */ public function testGetUsersGroupsForSubAdminUserAndUserIsInaccessible() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1700,137 +1590,64 @@ class UsersTest extends OriginalTest { ->with($targetUser) ->will($this->returnValue(['Group1'])); - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->getUsersGroups(['userid' => 'UserToLookup'])); - } - - public function testAddToGroupNotLoggedIn() { - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue(null)); - - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->addToGroup([])); + $this->api->getUsersGroups('UserToLookup'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + */ public function testAddToGroupWithTargetGroupNotExisting() { - $_POST['groupid'] = 'GroupToAddTo'; - - $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); - $loggedInUser - ->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('admin')); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue($loggedInUser)); $this->groupManager ->expects($this->once()) ->method('get') ->with('GroupToAddTo') ->will($this->returnValue(null)); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('admin') - ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 102); - $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); + $this->api->addToGroup('TargetUser', 'GroupToAddTo'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testAddToGroupWithNoGroupSpecified() { - $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); - $loggedInUser - ->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('admin')); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue($loggedInUser)); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('admin') - ->will($this->returnValue(true)); - - $expected = new \OC\OCS\Result(null, 101); - $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); + $this->api->addToGroup('TargetUser'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 103 + */ public function testAddToGroupWithTargetUserNotExisting() { - $_POST['groupid'] = 'GroupToAddTo'; - - $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); - $loggedInUser - ->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('admin')); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue($loggedInUser)); $this->groupManager ->expects($this->once()) ->method('get') ->with('GroupToAddTo') ->will($this->returnValue($targetGroup)); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('admin') - ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 103); - $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); - } - - public function testAddToGroupWithoutPermission() { - $_POST['groupid'] = 'GroupToAddTo'; - - $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); - $loggedInUser - ->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('admin')); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue($loggedInUser)); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('admin') - ->will($this->returnValue(false)); - - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); - } - - public function testRemoveFromGroupWithoutLogIn() { - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue(null)); - - $expected = new \OC\OCS\Result(null, 997); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'TargetUser', '_delete' => ['groupid' => 'TargetGroup']])); + $this->api->addToGroup('TargetUser', 'GroupToAddTo'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testRemoveFromGroupWithNoTargetGroup() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $this->userSession ->expects($this->once()) ->method('getUser') ->will($this->returnValue($loggedInUser)); - $expected = new \OC\OCS\Result(null, 101); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'TargetUser', '_delete' => []])); + + $this->api->removeFromGroup('TargetUser', null); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + */ public function testRemoveFromGroupWithNotExistingTargetGroup() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $this->userSession @@ -1843,10 +1660,13 @@ class UsersTest extends OriginalTest { ->with('TargetGroup') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 102); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'TargetUser', '_delete' => ['groupid' => 'TargetGroup']])); + $this->api->removeFromGroup('TargetUser', 'TargetGroup'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 103 + */ public function testRemoveFromGroupWithNotExistingTargetUser() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); @@ -1865,10 +1685,13 @@ class UsersTest extends OriginalTest { ->with('TargetUser') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 103); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'TargetUser', '_delete' => ['groupid' => 'TargetGroup']])); + $this->api->removeFromGroup('TargetUser', 'TargetGroup'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 104 + */ public function testRemoveFromGroupWithoutPermission() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1903,10 +1726,14 @@ class UsersTest extends OriginalTest { ->with('unauthorizedUser') ->will($this->returnValue(false)); - $expected = new \OC\OCS\Result(null, 104); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'TargetUser', '_delete' => ['groupid' => 'TargetGroup']])); + $this->api->removeFromGroup('TargetUser', 'TargetGroup'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 105 + * @expectedExceptionMessage Cannot remove yourself from the admin group + */ public function testRemoveFromGroupAsAdminFromAdmin() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1945,10 +1772,14 @@ class UsersTest extends OriginalTest { ->with('admin') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 105, 'Cannot remove yourself from the admin group'); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'admin', '_delete' => ['groupid' => 'admin']])); + $this->api->removeFromGroup('admin', 'admin'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 105 + * @expectedExceptionMessage Cannot remove yourself from this group as you are a SubAdmin + */ public function testRemoveFromGroupAsSubAdminFromSubAdmin() { $loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $loggedInUser @@ -1997,8 +1828,7 @@ class UsersTest extends OriginalTest { ->with('subadmin') ->will($this->returnValue(false)); - $expected = new \OC\OCS\Result(null, 105, 'Cannot remove yourself from this group as you are a SubAdmin'); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'subadmin', '_delete' => ['groupid' => 'subadmin']])); + $this->api->removeFromGroup('subadmin', 'subadmin'); } public function testRemoveFromGroupSuccessful() { @@ -2039,10 +1869,14 @@ class UsersTest extends OriginalTest { ->method('removeUser') ->with($targetUser); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'AnotherUser', '_delete' => ['groupid' => 'admin']])); + $this->assertEquals([], $this->api->removeFromGroup('AnotherUser', 'admin')->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + * @expectedExceptionMessage User does not exist + */ public function testAddSubAdminWithNotExistingTargetUser() { $this->userManager ->expects($this->once()) @@ -2050,12 +1884,15 @@ class UsersTest extends OriginalTest { ->with('NotExistingUser') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 101, 'User does not exist'); - $this->assertEquals($expected, $this->api->addSubAdmin(['userid' => 'NotExistingUser'])); + $this->api->addSubAdmin('NotExistingUser', null); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + * @expectedExceptionMessage Group:NotExistingGroup does not exist + */ public function testAddSubAdminWithNotExistingTargetGroup() { - $_POST['groupid'] = 'NotExistingGroup'; $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $this->userManager @@ -2069,13 +1906,15 @@ class UsersTest extends OriginalTest { ->with('NotExistingGroup') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 102, 'Group:NotExistingGroup does not exist'); - $this->assertEquals($expected, $this->api->addSubAdmin(['userid' => 'ExistingUser'])); + $this->api->addSubAdmin('ExistingUser', 'NotExistingGroup'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 103 + * @expectedExceptionMessage Cannot create subadmins for admin group + */ public function testAddSubAdminToAdminGroup() { - $_POST['groupid'] = 'ADmiN'; - $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); $this->userManager @@ -2089,13 +1928,10 @@ class UsersTest extends OriginalTest { ->with('ADmiN') ->will($this->returnValue($targetGroup)); - $expected = new \OC\OCS\Result(null, 103, 'Cannot create subadmins for admin group'); - $this->assertEquals($expected, $this->api->addSubAdmin(['userid' => 'ExistingUser'])); + $this->api->addSubAdmin('ExistingUser', 'ADmiN'); } public function testAddSubAdminTwice() { - $_POST['groupid'] = 'TargetGroup'; - $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); $this->userManager @@ -2120,13 +1956,10 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->addSubAdmin(['userid' => 'ExistingUser'])); + $this->assertEquals([], $this->api->addSubAdmin('ExistingUser', 'TargetGroup')->getData()); } public function testAddSubAdminSuccessful() { - $_POST['groupid'] = 'TargetGroup'; - $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); $this->userManager @@ -2156,13 +1989,15 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->addSubAdmin(['userid' => 'ExistingUser'])); + $this->assertEquals([], $this->api->addSubAdmin('ExistingUser', 'TargetGroup')->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 103 + * @expectedExceptionMessage Unknown error occurred + */ public function testAddSubAdminUnsuccessful() { - $_POST['groupid'] = 'TargetGroup'; - $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); $this->userManager @@ -2192,10 +2027,14 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 103, 'Unknown error occurred'); - $this->assertEquals($expected, $this->api->addSubAdmin(['userid' => 'ExistingUser'])); + $this->api->addSubAdmin('ExistingUser', 'TargetGroup'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + * @expectedExceptionMessage User does not exist + */ public function testRemoveSubAdminNotExistingTargetUser() { $this->userManager ->expects($this->once()) @@ -2203,10 +2042,14 @@ class UsersTest extends OriginalTest { ->with('NotExistingUser') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 101, 'User does not exist'); - $this->assertEquals($expected, $this->api->removeSubAdmin(['userid' => 'NotExistingUser', '_delete' => ['groupid' => 'GroupToDeleteFrom']])); + $this->api->removeSubAdmin('NotExistingUser', 'GroupToDeleteFrom'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + * @expectedExceptionMessage Group does not exist + */ public function testRemoveSubAdminNotExistingTargetGroup() { $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $this->userManager @@ -2220,10 +2063,15 @@ class UsersTest extends OriginalTest { ->with('GroupToDeleteFrom') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 101, 'Group does not exist'); - $this->assertEquals($expected, $this->api->removeSubAdmin(['userid' => 'ExistingUser', '_delete' => ['groupid' => 'GroupToDeleteFrom']])); + $this->api->removeSubAdmin('ExistingUser', 'GroupToDeleteFrom'); } + + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 102 + * @expectedExceptionMessage User is not a subadmin of this group + */ public function testRemoveSubAdminFromNotASubadmin() { $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); @@ -2249,8 +2097,7 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 102, 'User is not a subadmin of this group'); - $this->assertEquals($expected, $this->api->removeSubAdmin(['userid' => 'ExistingUser', '_delete' => ['groupid' => 'GroupToDeleteFrom']])); + $this->api->removeSubAdmin('ExistingUser', 'GroupToDeleteFrom'); } public function testRemoveSubAdminSuccessful() { @@ -2283,10 +2130,14 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->removeSubAdmin(['userid' => 'ExistingUser', '_delete' => ['groupid' => 'GroupToDeleteFrom']])); + $this->assertEquals([], $this->api->removeSubAdmin('ExistingUser', 'GroupToDeleteFrom')->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 103 + * @expectedExceptionMessage Unknown error occurred + */ public function testRemoveSubAdminUnsuccessful() { $targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock(); $targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock(); @@ -2317,10 +2168,14 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 103, 'Unknown error occurred'); - $this->assertEquals($expected, $this->api->removeSubAdmin(['userid' => 'ExistingUser', '_delete' => ['groupid' => 'GroupToDeleteFrom']])); + $this->api->removeSubAdmin('ExistingUser', 'GroupToDeleteFrom'); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + * @expectedExceptionMessage User does not exist + */ public function testGetUserSubAdminGroupsNotExistingTargetUser() { $this->userManager ->expects($this->once()) @@ -2328,8 +2183,7 @@ class UsersTest extends OriginalTest { ->with('RequestedUser') ->will($this->returnValue(null)); - $expected = new \OC\OCS\Result(null, 101, 'User does not exist'); - $this->assertEquals($expected, $this->api->getUserSubAdminGroups(['userid' => 'RequestedUser'])); + $this->api->getUserSubAdminGroups('RequestedUser'); } public function testGetUserSubAdminGroupsWithGroups() { @@ -2356,10 +2210,14 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(['TargetGroup'], 100); - $this->assertEquals($expected, $this->api->getUserSubAdminGroups(['userid' => 'RequestedUser'])); + $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('\OCP\IUser')->disableOriginalConstructor()->getMock(); $this->userManager @@ -2379,8 +2237,7 @@ class UsersTest extends OriginalTest { ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); - $expected = new \OC\OCS\Result(null, 102, 'Unknown error occurred'); - $this->assertEquals($expected, $this->api->getUserSubAdminGroups(['userid' => 'RequestedUser'])); + $this->api->getUserSubAdminGroups('RequestedUser'); } public function testEnableUser() { @@ -2407,8 +2264,7 @@ class UsersTest extends OriginalTest { ->method('isAdmin') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->enableUser(['userid' => 'RequestedUser'])); + $this->assertEquals([], $this->api->enableUser('RequestedUser')->getData()); } public function testDisableUser() { @@ -2435,7 +2291,6 @@ class UsersTest extends OriginalTest { ->method('isAdmin') ->will($this->returnValue(true)); - $expected = new \OC\OCS\Result(null, 100); - $this->assertEquals($expected, $this->api->disableUser(['userid' => 'RequestedUser'])); + $this->assertEquals([], $this->api->disableUser('RequestedUser')->getData()); } } From 092b767ef998a6afe2e01eb34aef1f8d21f6ec69 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 12 Aug 2016 10:27:08 +0200 Subject: [PATCH 7/8] Move Apps to OCSController --- apps/provisioning_api/appinfo/routes.php | 16 ++--- .../AppsController.php} | 72 +++++++++++-------- .../AppsControllerTest.php} | 54 +++++++------- 3 files changed, 78 insertions(+), 64 deletions(-) rename apps/provisioning_api/lib/{Apps.php => Controller/AppsController.php} (54%) rename apps/provisioning_api/tests/{AppsTest.php => Controller/AppsControllerTest.php} (72%) diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index 473ae2ff71..a7366a32a0 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -33,6 +33,12 @@ use OCP\API; $app = new \OCA\Provisioning_API\AppInfo\Application(); $app->registerRoutes($this, [ 'ocs' => [ + // Apps + ['root' => '/cloud', 'name' => 'Apps#getApps', 'url' => '/apps', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Apps#getAppInfo', 'url' => '/apps/{app}', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Apps#enable', 'url' => '/apps/{app}', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'Apps#disable', 'url' => '/apps/{app}', 'verb' => 'DELETE'], + // Groups ['root' => '/cloud', 'name' => 'Groups#getGroups', 'url' => '/groups', 'verb' => 'GET'], ['root' => '/cloud', 'name' => 'Groups#getGroup', 'url' => '/groups/{groupId}', 'verb' => 'GET'], @@ -57,13 +63,3 @@ $app->registerRoutes($this, [ ], ]); - -// Apps -$apps = new Apps( - \OC::$server->getAppManager(), - \OC::$server->getOcsClient() -); -API::register('get', '/cloud/apps', [$apps, 'getApps'], 'provisioning_api', API::ADMIN_AUTH); -API::register('get', '/cloud/apps/{appid}', [$apps, 'getAppInfo'], 'provisioning_api', API::ADMIN_AUTH); -API::register('post', '/cloud/apps/{appid}', [$apps, 'enable'], 'provisioning_api', API::ADMIN_AUTH); -API::register('delete', '/cloud/apps/{appid}', [$apps, 'disable'], 'provisioning_api', API::ADMIN_AUTH); diff --git a/apps/provisioning_api/lib/Apps.php b/apps/provisioning_api/lib/Controller/AppsController.php similarity index 54% rename from apps/provisioning_api/lib/Apps.php rename to apps/provisioning_api/lib/Controller/AppsController.php index f880e41905..3821fc343a 100644 --- a/apps/provisioning_api/lib/Apps.php +++ b/apps/provisioning_api/lib/Controller/AppsController.php @@ -23,89 +23,101 @@ * */ -namespace OCA\Provisioning_API; +namespace OCA\Provisioning_API\Controller; use OC\OCSClient; use \OC_App; +use OCP\App\IAppManager; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSException; +use OCP\AppFramework\OCS\OCSNotFoundException; +use OCP\AppFramework\OCSController; +use OCP\IRequest; -class Apps { +class AppsController extends OCSController { /** @var \OCP\App\IAppManager */ private $appManager; /** @var OCSClient */ private $ocsClient; /** - * @param \OCP\App\IAppManager $appManager + * @param string $appName + * @param IRequest $request + * @param IAppManager $appManager + * @param OCSClient $ocsClient */ - public function __construct(\OCP\App\IAppManager $appManager, - OCSClient $ocsClient) { + public function __construct( + $appName, + IRequest $request, + IAppManager $appManager, + OCSClient $ocsClient + ) { + parent::__construct($appName, $request); + $this->appManager = $appManager; $this->ocsClient = $ocsClient; } /** - * @param array $parameters - * @return \OC\OCS\Result + * @param string $filter + * @return DataResponse + * @throws OCSException */ - public function getApps($parameters) { + public function getApps($filter = null) { $apps = OC_App::listAllApps(false, true, $this->ocsClient); $list = []; foreach($apps as $app) { $list[] = $app['id']; } - $filter = isset($_GET['filter']) ? $_GET['filter'] : false; if($filter){ switch($filter){ case 'enabled': - return new \OC\OCS\Result(array('apps' => \OC_App::getEnabledApps())); + return new DataResponse(['apps' => \OC_App::getEnabledApps()]); break; case 'disabled': $enabled = OC_App::getEnabledApps(); - return new \OC\OCS\Result(array('apps' => array_diff($list, $enabled))); + return new DataResponse(['apps' => array_diff($list, $enabled)]); break; default: // Invalid filter variable - return new \OC\OCS\Result(null, 101); - break; + throw new OCSException('', 101); } } else { - return new \OC\OCS\Result(array('apps' => $list)); + return new DataResponse(['apps' => $list]); } } /** - * @param array $parameters - * @return \OC\OCS\Result + * @param string $app + * @return DataResponse + * @throws OCSNotFoundException */ - public function getAppInfo($parameters) { - $app = $parameters['appid']; + public function getAppInfo($app) { $info = \OCP\App::getAppInfo($app); if(!is_null($info)) { - return new \OC\OCS\Result(OC_App::getAppInfo($app)); + return new DataResponse(OC_App::getAppInfo($app)); } else { - return new \OC\OCS\Result(null, \OCP\API::RESPOND_NOT_FOUND, 'The request app was not found'); + throw new OCSException('The request app was not found', \OCP\API::RESPOND_NOT_FOUND); } } /** - * @param array $parameters - * @return \OC\OCS\Result + * @param string $app + * @return DataResponse */ - public function enable($parameters) { - $app = $parameters['appid']; + public function enable($app) { $this->appManager->enableApp($app); - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } /** - * @param array $parameters - * @return \OC\OCS\Result + * @param string $app + * @return DataResponse */ - public function disable($parameters) { - $app = $parameters['appid']; + public function disable($app) { $this->appManager->disableApp($app); - return new \OC\OCS\Result(null, 100); + return new DataResponse(); } } diff --git a/apps/provisioning_api/tests/AppsTest.php b/apps/provisioning_api/tests/Controller/AppsControllerTest.php similarity index 72% rename from apps/provisioning_api/tests/AppsTest.php rename to apps/provisioning_api/tests/Controller/AppsControllerTest.php index 35808b1581..9ac4a8290e 100644 --- a/apps/provisioning_api/tests/AppsTest.php +++ b/apps/provisioning_api/tests/Controller/AppsControllerTest.php @@ -25,11 +25,11 @@ * */ -namespace OCA\Provisioning_API\Tests; +namespace OCA\Provisioning_API\Tests\Controller; use OC\OCSClient; -use OCA\Provisioning_API\Apps; +use OCA\Provisioning_API\Controller\AppsController; use OCP\API; use OCP\App\IAppManager; use OCP\IUserSession; @@ -41,10 +41,10 @@ use OCP\IUserSession; * * @package OCA\Provisioning_API\Tests */ -class AppsTest extends TestCase { +class AppsControllerTest extends \OCA\Provisioning_API\Tests\TestCase { /** @var IAppManager */ private $appManager; - /** @var Apps */ + /** @var AppsController */ private $api; /** @var IUserSession */ private $userSession; @@ -61,20 +61,30 @@ class AppsTest extends TestCase { ->disableOriginalConstructor() ->getMock(); - $this->api = new Apps($this->appManager, $this->ocsClient); + $request = $this->getMockBuilder('OCP\IRequest') + ->disableOriginalConstructor() + ->getMock(); + + $this->api = new AppsController( + 'provisioning_api', + $request, + $this->appManager, + $this->ocsClient + ); } public function testGetAppInfo() { - $result = $this->api->getAppInfo(['appid' => 'provisioning_api']); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertTrue($result->succeeded()); + $result = $this->api->getAppInfo('provisioning_api'); + $expected = \OC_App::getAppInfo('provisioning_api'); + $this->assertEquals($expected, $result->getData()); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 998 + */ public function testGetAppInfoOnBadAppID() { - $result = $this->api->getAppInfo(['appid' => 'not_provisioning_api']); - $this->assertInstanceOf('\OC\OCS\Result', $result); - $this->assertFalse($result->succeeded()); - $this->assertEquals(API::RESPOND_NOT_FOUND, $result->getStatusCode()); + $this->api->getAppInfo('not_provisioning_api'); } public function testGetApps() { @@ -86,17 +96,14 @@ class AppsTest extends TestCase { $this->groupManager->get('admin')->addUser($user); $this->userSession->setUser($user); - $result = $this->api->getApps([]); + $result = $this->api->getApps(); - $this->assertTrue($result->succeeded()); $data = $result->getData(); $this->assertEquals(count(\OC_App::listAllApps(false, true, $this->ocsClient)), count($data['apps'])); } public function testGetAppsEnabled() { - $_GET['filter'] = 'enabled'; - $result = $this->api->getApps(['filter' => 'enabled']); - $this->assertTrue($result->succeeded()); + $result = $this->api->getApps('enabled'); $data = $result->getData(); $this->assertEquals(count(\OC_App::getEnabledApps()), count($data['apps'])); } @@ -106,9 +113,7 @@ class AppsTest extends TestCase { ->expects($this->any()) ->method($this->anything()) ->will($this->returnValue(null)); - $_GET['filter'] = 'disabled'; - $result = $this->api->getApps(['filter' => 'disabled']); - $this->assertTrue($result->succeeded()); + $result = $this->api->getApps('disabled'); $data = $result->getData(); $apps = \OC_App::listAllApps(false, true, $this->ocsClient); $list = array(); @@ -119,10 +124,11 @@ class AppsTest extends TestCase { $this->assertEquals(count($disabled), count($data['apps'])); } + /** + * @expectedException \OCP\AppFramework\OCS\OCSException + * @expectedExceptionCode 101 + */ public function testGetAppsInvalidFilter() { - $_GET['filter'] = 'foo'; - $result = $this->api->getApps([]); - $this->assertFalse($result->succeeded()); - $this->assertEquals(101, $result->getStatusCode()); + $this->api->getApps('foo'); } } From 77542817d1bfb260c4bff74f77b9507b699f1d76 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 12 Aug 2016 10:46:33 +0200 Subject: [PATCH 8/8] Fix intergration tests * Set OCS-APIREQUEST: true * PUT requires a body --- .../features/bootstrap/BasicStructure.php | 7 ++ .../features/bootstrap/Provisioning.php | 79 ++++++++++++++++--- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 0b0e5998c4..e9e20c047a 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -165,6 +165,13 @@ trait BasicStructure { $options['body'] = $fd; } + // TODO: Fix this hack! + if ($verb === 'PUT' && $body === null) { + $options['body'] = [ + 'foo' => 'bar', + ]; + } + try { $this->response = $client->send($client->createRequest($verb, $fullUrl, $options)); } catch (\GuzzleHttp\Exception\ClientException $ex) { diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php index bc3fc9a4c4..dbdfafcecd 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -96,9 +96,12 @@ trait Provisioning { } $options['body'] = [ - 'userid' => $user, - 'password' => '123456' - ]; + 'userid' => $user, + 'password' => '123456' + ]; + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->send($client->createRequest("POST", $fullUrl, $options)); if ($this->currentServer === 'LOCAL'){ @@ -111,6 +114,9 @@ trait Provisioning { $options2 = [ 'auth' => [$user, '123456'], ]; + $options2['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $url = $fullUrl.'/'.$user; $client->send($client->createRequest('GET', $url, $options2)); } @@ -152,6 +158,9 @@ trait Provisioning { $client = new Client(); $options = []; $options['auth'] = $this->adminUser; + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true' + ]; $this->response = $client->get($fullUrl, $options); } @@ -168,6 +177,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); $respondedArray = $this->getArrayOfGroupsResponded($this->response); @@ -183,6 +195,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); $respondedArray = $this->getArrayOfGroupsResponded($this->response); @@ -223,6 +238,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); $groups = array($group); @@ -244,8 +262,11 @@ trait Provisioning { } $options['body'] = [ - 'groupid' => $group, - ]; + 'groupid' => $group, + ]; + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->send($client->createRequest("POST", $fullUrl, $options)); if ($this->currentServer === 'LOCAL'){ @@ -265,6 +286,13 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; + // TODO: fix hack + $options['body'] = [ + 'foo' => 'bar' + ]; $this->response = $client->send($client->createRequest("PUT", $fullUrl, $options)); } @@ -280,6 +308,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->send($client->createRequest("DELETE", $fullUrl, $options)); } @@ -295,6 +326,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->send($client->createRequest("DELETE", $fullUrl, $options)); } @@ -323,10 +357,13 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $options['body'] = [ - 'groupid' => $group, - ]; + 'groupid' => $group, + ]; $this->response = $client->send($client->createRequest("POST", $fullUrl, $options)); } @@ -337,6 +374,9 @@ trait Provisioning { $client = new Client(); $options = []; $options['auth'] = $this->adminUser; + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); } @@ -394,6 +434,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); $respondedArray = $this->getArrayOfSubadminsResponded($this->response); @@ -415,8 +458,11 @@ trait Provisioning { $options['auth'] = $this->adminUser; } $options['body'] = [ - 'groupid' => $group - ]; + 'groupid' => $group + ]; + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->send($client->createRequest("POST", $fullUrl, $options)); PHPUnit_Framework_Assert::assertEquals(200, $this->response->getStatusCode()); } @@ -433,6 +479,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); $respondedArray = $this->getArrayOfSubadminsResponded($this->response); @@ -561,6 +610,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); $respondedArray = $this->getArrayOfAppsResponded($this->response); @@ -579,6 +631,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); $respondedArray = $this->getArrayOfAppsResponded($this->response); @@ -597,6 +652,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); PHPUnit_Framework_Assert::assertEquals("false", $this->response->xml()->data[0]->enabled); @@ -613,6 +671,9 @@ trait Provisioning { if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; } + $options['headers'] = [ + 'OCS-APIREQUEST' => 'true', + ]; $this->response = $client->get($fullUrl, $options); PHPUnit_Framework_Assert::assertEquals("true", $this->response->xml()->data[0]->enabled);