From 28127553591205c3ecd37233509d6ce8bacaed0f Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 19 Aug 2016 12:01:13 +0200 Subject: [PATCH] Move federated sharing routes to the federatedfilesharingapp * Move routes to app * Move routes over to the AppFramework * Fix tests There is plenty of stuff to fix properly here. But out of scope for now. --- apps/federatedfilesharing/appinfo/routes.php | 11 +- .../lib/AppInfo/Application.php | 34 ++++ .../RequestHandlerController.php} | 169 ++++++++++-------- .../RequestHandlerControllerTest.php} | 40 ++--- ocs/routes.php | 73 -------- 5 files changed, 162 insertions(+), 165 deletions(-) rename apps/federatedfilesharing/lib/{RequestHandler.php => Controller/RequestHandlerController.php} (82%) rename apps/federatedfilesharing/tests/{RequestHandlerTest.php => Controller/RequestHandlerControllerTest.php} (91%) diff --git a/apps/federatedfilesharing/appinfo/routes.php b/apps/federatedfilesharing/appinfo/routes.php index a4f56e372c9..9caaa939348 100644 --- a/apps/federatedfilesharing/appinfo/routes.php +++ b/apps/federatedfilesharing/appinfo/routes.php @@ -26,5 +26,14 @@ return [ 'routes' => [ ['name' => 'MountPublicLink#createFederatedShare', 'url' => '/createFederatedShare', 'verb' => 'POST'], ['name' => 'MountPublicLink#askForFederatedShare', 'url' => '/askForFederatedShare', 'verb' => 'POST'], - ] + ], + 'ocs' => [ + ['root' => '/cloud', 'name' => 'RequestHandler#createShare', 'url' => '/shares', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'RequestHandler#reShare', 'url' => '/shares/{id}/reshare', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'RequestHandler#updatePermissions', 'url' => '/shares/{id}/permissions', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'RequestHandler#acceptShare', 'url' => '/shares/{id}/accept', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'RequestHandler#declineShare', 'url' => '/shares/{id}/decline', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'RequestHandler#unshare', 'url' => '/shares/{id}/unshare', 'verb' => 'POST'], + ['root' => '/cloud', 'name' => 'RequestHandler#revoke', 'url' => '/shares/{id}/revoke', 'verb' => 'POST'], + ], ]; diff --git a/apps/federatedfilesharing/lib/AppInfo/Application.php b/apps/federatedfilesharing/lib/AppInfo/Application.php index b767a322505..b470bb3e584 100644 --- a/apps/federatedfilesharing/lib/AppInfo/Application.php +++ b/apps/federatedfilesharing/lib/AppInfo/Application.php @@ -24,7 +24,12 @@ namespace OCA\FederatedFileSharing\AppInfo; +use OC\AppFramework\Utility\SimpleContainer; +use OCA\FederatedFileSharing\AddressHandler; +use OCA\FederatedFileSharing\Controller\RequestHandlerController; use OCA\FederatedFileSharing\FederatedShareProvider; +use OCA\FederatedFileSharing\Notifications; +use OCA\FederatedFileSharing\RequestHandler; use OCP\AppFramework\App; class Application extends App { @@ -34,6 +39,35 @@ class Application extends App { public function __construct() { parent::__construct('federatedfilesharing'); + + $container = $this->getContainer(); + $server = $container->getServer(); + + $container->registerService('RequestHandlerController', function(SimpleContainer $c) use ($server) { + $addressHandler = new AddressHandler( + $server->getURLGenerator(), + $server->getL10N('federatedfilesharing') + ); + $notification = new Notifications( + $addressHandler, + $server->getHTTPClientService(), + new \OCA\FederatedFileSharing\DiscoveryManager( + $server->getMemCacheFactory(), + $server->getHTTPClientService() + ), + \OC::$server->getJobList() + ); + return new RequestHandlerController( + $c->query('AppName'), + $server->getRequest(), + $this->getFederatedShareProvider(), + $server->getDatabaseConnection(), + $server->getShareManager(), + $notification, + $addressHandler, + $server->getUserManager() + ); + }); } /** diff --git a/apps/federatedfilesharing/lib/RequestHandler.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php similarity index 82% rename from apps/federatedfilesharing/lib/RequestHandler.php rename to apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index f531c7bcb4a..9a41962ee3a 100644 --- a/apps/federatedfilesharing/lib/RequestHandler.php +++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php @@ -24,25 +24,28 @@ * */ -namespace OCA\FederatedFileSharing; +namespace OCA\FederatedFileSharing\Controller; +use OCA\FederatedFileSharing\DiscoveryManager; use OCA\Files_Sharing\Activity; +use OCA\FederatedFileSharing\AddressHandler; +use OCA\FederatedFileSharing\FederatedShareProvider; +use OCA\FederatedFileSharing\Notifications; use OCP\AppFramework\Http; +use OCP\AppFramework\OCS\OCSBadRequestException; +use OCP\AppFramework\OCS\OCSException; +use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\AppFramework\OCS\OCSNotFoundException; +use OCP\AppFramework\OCSController; use OCP\Constants; use OCP\Files\NotFoundException; use OCP\IDBConnection; use OCP\IRequest; use OCP\IUserManager; use OCP\Share; +use OCP\Share\IShare; -/** - * Class RequestHandler - * - * handles OCS Request to the federated share API - * - * @package OCA\FederatedFileSharing\API - */ -class RequestHandler { +class RequestHandlerController extends OCSController { /** @var FederatedShareProvider */ private $federatedShareProvider; @@ -53,9 +56,6 @@ class RequestHandler { /** @var Share\IManager */ private $shareManager; - /** @var IRequest */ - private $request; - /** @var Notifications */ private $notifications; @@ -71,41 +71,47 @@ class RequestHandler { /** * Server2Server constructor. * + * @param string $appName + * @param IRequest $request * @param FederatedShareProvider $federatedShareProvider * @param IDBConnection $connection * @param Share\IManager $shareManager - * @param IRequest $request * @param Notifications $notifications * @param AddressHandler $addressHandler * @param IUserManager $userManager */ - public function __construct(FederatedShareProvider $federatedShareProvider, + public function __construct($appName, + IRequest $request, + FederatedShareProvider $federatedShareProvider, IDBConnection $connection, Share\IManager $shareManager, - IRequest $request, Notifications $notifications, AddressHandler $addressHandler, IUserManager $userManager ) { + parent::__construct($appName, $request); + $this->federatedShareProvider = $federatedShareProvider; $this->connection = $connection; $this->shareManager = $shareManager; - $this->request = $request; $this->notifications = $notifications; $this->addressHandler = $addressHandler; $this->userManager = $userManager; } /** + * @NoCSRFRequired + * @PublicPage + * * create a new share * - * @param array $params - * @return \OC_OCS_Result + * @return Http\DataResponse + * @throws OCSException */ - public function createShare($params) { + public function createShare() { if (!$this->isS2SEnabled(true)) { - return new \OC_OCS_Result(null, 503, 'Server does not support federated cloud sharing'); + throw new OCSException('Server does not support federated cloud sharing', 503); } $remote = isset($_POST['remote']) ? $_POST['remote'] : null; @@ -121,7 +127,7 @@ class RequestHandler { if ($remote && $token && $name && $owner && $remoteId && $shareWith) { if(!\OCP\Util::isValidFileName($name)) { - return new \OC_OCS_Result(null, 400, 'The mountpoint name contains invalid characters.'); + throw new OCSException('The mountpoint name contains invalid characters.', 400); } // FIXME this should be a method in the user management instead @@ -134,7 +140,7 @@ class RequestHandler { \OCP\Util::writeLog('files_sharing', 'shareWith after, ' . $shareWith, \OCP\Util::DEBUG); if (!\OCP\User::userExists($shareWith)) { - return new \OC_OCS_Result(null, 400, 'User does not exists'); + throw new OCSException('User does not exists', 400); } \OC_Util::setupFS($shareWith); @@ -192,25 +198,30 @@ class RequestHandler { $notificationManager->notify($notification); - return new \OC_OCS_Result(); + return new Http\DataResponse(); } catch (\Exception $e) { \OCP\Util::writeLog('files_sharing', 'server can not add remote share, ' . $e->getMessage(), \OCP\Util::ERROR); - return new \OC_OCS_Result(null, 500, 'internal server error, was not able to add share from ' . $remote); + throw new OCSException('internal server error, was not able to add share from ' . $remote, 500); } } - return new \OC_OCS_Result(null, 400, 'server can not add remote share, missing parameter'); + throw new OCSException('server can not add remote share, missing parameter', 400); } /** + * @NoCSRFRequired + * @PublicPage + * * create re-share on behalf of another user * - * @param $params - * @return \OC_OCS_Result + * @param int $id + * @return Http\DataResponse + * @throws OCSBadRequestException + * @throws OCSForbiddenException + * @throws OCSNotFoundException */ - public function reShare($params) { + public function reShare($id) { - $id = isset($params['id']) ? (int)$params['id'] : null; $token = $this->request->getParam('token', null); $shareWith = $this->request->getParam('shareWith', null); $permission = (int)$this->request->getParam('permission', null); @@ -222,13 +233,13 @@ class RequestHandler { $permission === null || $remoteId === null ) { - return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST); + throw new OCSBadRequestException(); } try { $share = $this->federatedShareProvider->getShareById($id); } catch (Share\Exceptions\ShareNotFound $e) { - return new \OC_OCS_Result(null, Http::STATUS_NOT_FOUND); + throw new OCSNotFoundException(); } // don't allow to share a file back to the owner @@ -236,7 +247,7 @@ class RequestHandler { $owner = $share->getShareOwner(); $currentServer = $this->addressHandler->generateRemoteURL(); if ($this->addressHandler->compareAddresses($user, $remote,$owner , $currentServer)) { - return new \OC_OCS_Result(null, Http::STATUS_FORBIDDEN); + throw new OCSForbiddenException(); } if ($this->verifyShare($share, $token)) { @@ -250,37 +261,42 @@ class RequestHandler { try { $result = $this->federatedShareProvider->create($share); $this->federatedShareProvider->storeRemoteId((int)$result->getId(), $remoteId); - return new \OC_OCS_Result(['token' => $result->getToken(), 'remoteId' => $result->getId()]); + return new Http\DataResponse([ + 'token' => $result->getToken(), + 'remoteId' => $result->getId() + ]); } catch (\Exception $e) { - return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST); + throw new OCSBadRequestException(); } } else { - return new \OC_OCS_Result(null, Http::STATUS_FORBIDDEN); + throw new OCSForbiddenException(); } } - return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST); - + throw new OCSBadRequestException(); } /** + * @NoCSRFRequired + * @PublicPage + * * accept server-to-server share * - * @param array $params - * @return \OC_OCS_Result + * @param int $id + * @return Http\DataResponse + * @throws OCSException */ - public function acceptShare($params) { + public function acceptShare($id) { if (!$this->isS2SEnabled()) { - return new \OC_OCS_Result(null, 503, 'Server does not support federated cloud sharing'); + throw new OCSException('Server does not support federated cloud sharing', 503); } - $id = $params['id']; $token = isset($_POST['token']) ? $_POST['token'] : null; try { $share = $this->federatedShareProvider->getShareById($id); } catch (Share\Exceptions\ShareNotFound $e) { - return new \OC_OCS_Result(); + return new Http\DataResponse(); } if ($this->verifyShare($share, $token)) { @@ -292,7 +308,7 @@ class RequestHandler { } } - return new \OC_OCS_Result(); + return new Http\DataResponse(); } protected function executeAcceptShare(Share\IShare $share) { @@ -309,24 +325,27 @@ class RequestHandler { } /** + * @NoCSRFRequired + * @PublicPage + * * decline server-to-server share * - * @param array $params - * @return \OC_OCS_Result + * @param int $id + * @return Http\DataResponse + * @throws OCSException */ - public function declineShare($params) { + public function declineShare($id) { if (!$this->isS2SEnabled()) { - return new \OC_OCS_Result(null, 503, 'Server does not support federated cloud sharing'); + throw new OCSException('Server does not support federated cloud sharing', 503); } - $id = (int)$params['id']; $token = isset($_POST['token']) ? $_POST['token'] : null; try { $share = $this->federatedShareProvider->getShareById($id); } catch (Share\Exceptions\ShareNotFound $e) { - return new \OC_OCS_Result(); + return new Http\DataResponse(); } if($this->verifyShare($share, $token)) { @@ -338,7 +357,7 @@ class RequestHandler { $this->executeDeclineShare($share); } - return new \OC_OCS_Result(); + return new Http\DataResponse(); } /** @@ -376,18 +395,21 @@ class RequestHandler { } /** + * @NoCSRFRequired + * @PublicPage + * * remove server-to-server share if it was unshared by the owner * - * @param array $params - * @return \OC_OCS_Result + * @param int $id + * @return Http\DataResponse + * @throws OCSException */ - public function unshare($params) { + public function unshare($id) { if (!$this->isS2SEnabled()) { - return new \OC_OCS_Result(null, 503, 'Server does not support federated cloud sharing'); + throw new OCSException('Server does not support federated cloud sharing', 503); } - $id = $params['id']; $token = isset($_POST['token']) ? $_POST['token'] : null; $query = \OCP\DB::prepare('SELECT * FROM `*PREFIX*share_external` WHERE `remote_id` = ? AND `share_token` = ?'); @@ -423,7 +445,7 @@ class RequestHandler { '', '', $user, Activity::TYPE_REMOTE_SHARE, Activity::PRIORITY_MEDIUM); } - return new \OC_OCS_Result(); + return new Http\DataResponse(); } private function cleanupRemote($remote) { @@ -434,24 +456,26 @@ class RequestHandler { /** + * @NoCSRFRequired + * @PublicPage + * * federated share was revoked, either by the owner or the re-sharer * - * @param $params - * @return \OC_OCS_Result + * @param int $id + * @return Http\DataResponse + * @throws OCSBadRequestException */ - public function revoke($params) { - $id = (int)$params['id']; + public function revoke($id) { $token = $this->request->getParam('token'); $share = $this->federatedShareProvider->getShareById($id); if ($this->verifyShare($share, $token)) { $this->federatedShareProvider->removeShareFromTable($share); - return new \OC_OCS_Result(); + return new Http\DataResponse(); } - return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST); - + throw new OCSBadRequestException(); } /** @@ -537,20 +561,23 @@ class RequestHandler { } /** + * @NoCSRFRequired + * @PublicPage + * * update share information to keep federated re-shares in sync * - * @param array $params - * @return \OC_OCS_Result + * @param int $id + * @return Http\DataResponse + * @throws OCSBadRequestException */ - public function updatePermissions($params) { - $id = (int)$params['id']; + public function updatePermissions($id) { $token = $this->request->getParam('token', null); $permissions = $this->request->getParam('permissions', null); try { $share = $this->federatedShareProvider->getShareById($id); } catch (Share\Exceptions\ShareNotFound $e) { - return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST); + throw new OCSBadRequestException(); } $validPermission = ctype_digit($permissions); @@ -558,10 +585,10 @@ class RequestHandler { if ($validPermission && $validToken) { $this->updatePermissionsInDatabase($share, (int)$permissions); } else { - return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST); + throw new OCSBadRequestException(); } - return new \OC_OCS_Result(); + return new Http\DataResponse(); } /** diff --git a/apps/federatedfilesharing/tests/RequestHandlerTest.php b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php similarity index 91% rename from apps/federatedfilesharing/tests/RequestHandlerTest.php rename to apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php index 8f9f1384184..8e1000fb500 100644 --- a/apps/federatedfilesharing/tests/RequestHandlerTest.php +++ b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php @@ -30,7 +30,7 @@ namespace OCA\FederatedFileSharing\Tests; use OC\Files\Filesystem; use OCA\FederatedFileSharing\DiscoveryManager; use OCA\FederatedFileSharing\FederatedShareProvider; -use OCA\FederatedFileSharing\RequestHandler; +use OCA\FederatedFileSharing\Controller\RequestHandlerController; use OCP\IUserManager; use OCP\Share\IShare; @@ -40,7 +40,7 @@ use OCP\Share\IShare; * @package OCA\FederatedFileSharing\Tests * @group DB */ -class RequestHandlerTest extends TestCase { +class RequestHandlerControllerTest extends TestCase { const TEST_FOLDER_NAME = '/folder_share_api_test'; @@ -50,23 +50,23 @@ class RequestHandlerTest extends TestCase { private $connection; /** - * @var RequestHandler + * @var RequestHandlerController */ private $s2s; - /** @var \OCA\FederatedFileSharing\FederatedShareProvider | PHPUnit_Framework_MockObject_MockObject */ + /** @var \OCA\FederatedFileSharing\FederatedShareProvider|\PHPUnit_Framework_MockObject_MockObject */ private $federatedShareProvider; - /** @var \OCA\FederatedFileSharing\Notifications | PHPUnit_Framework_MockObject_MockObject */ + /** @var \OCA\FederatedFileSharing\Notifications|\PHPUnit_Framework_MockObject_MockObject */ private $notifications; - /** @var \OCA\FederatedFileSharing\AddressHandler | PHPUnit_Framework_MockObject_MockObject */ + /** @var \OCA\FederatedFileSharing\AddressHandler|\PHPUnit_Framework_MockObject_MockObject */ private $addressHandler; - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ private $userManager; - /** @var IShare | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IShare|\PHPUnit_Framework_MockObject_MockObject */ private $share; protected function setUp() { @@ -77,12 +77,12 @@ class RequestHandlerTest extends TestCase { $config = $this->getMockBuilder('\OCP\IConfig') ->disableOriginalConstructor()->getMock(); - $clientService = $this->getMock('\OCP\Http\Client\IClientService'); + $clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService')->getMock(); $httpHelperMock = $this->getMockBuilder('\OC\HTTPHelper') ->setConstructorArgs([$config, $clientService]) ->getMock(); $httpHelperMock->expects($this->any())->method('post')->with($this->anything())->will($this->returnValue(true)); - $this->share = $this->getMock('\OCP\Share\IShare'); + $this->share = $this->getMockBuilder('\OCP\Share\IShare')->getMock(); $this->federatedShareProvider = $this->getMockBuilder('OCA\FederatedFileSharing\FederatedShareProvider') ->disableOriginalConstructor()->getMock(); $this->federatedShareProvider->expects($this->any()) @@ -96,15 +96,16 @@ class RequestHandlerTest extends TestCase { ->disableOriginalConstructor()->getMock(); $this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler') ->disableOriginalConstructor()->getMock(); - $this->userManager = $this->getMock('OCP\IUserManager'); + $this->userManager = $this->getMockBuilder('OCP\IUserManager')->getMock(); $this->registerHttpHelper($httpHelperMock); - $this->s2s = new RequestHandler( + $this->s2s = new RequestHandlerController( + 'federatedfilesharing', + \OC::$server->getRequest(), $this->federatedShareProvider, \OC::$server->getDatabaseConnection(), \OC::$server->getShareManager(), - \OC::$server->getRequest(), $this->notifications, $this->addressHandler, $this->userManager @@ -127,7 +128,7 @@ class RequestHandlerTest extends TestCase { /** * Register an http helper mock for testing purposes. - * @param $httpHelper http helper mock + * @param \OC\HTTPHelper $httpHelper helper mock */ private function registerHttpHelper($httpHelper) { $this->oldHttpHelper = \OC::$server->query('HTTPHelper'); @@ -158,9 +159,7 @@ class RequestHandlerTest extends TestCase { $_POST['shareWith'] = self::TEST_FILES_SHARING_API_USER2; $_POST['remoteId'] = 1; - $result = $this->s2s->createShare(null); - - $this->assertTrue($result->succeeded()); + $this->s2s->createShare(null); $query = \OCP\DB::prepare('SELECT * FROM `*PREFIX*share_external` WHERE `remote_id` = ?'); $result = $query->execute(array('1')); @@ -178,13 +177,14 @@ class RequestHandlerTest extends TestCase { function testDeclineShare() { - $this->s2s = $this->getMockBuilder('\OCA\FederatedFileSharing\RequestHandler') + $this->s2s = $this->getMockBuilder('\OCA\FederatedFileSharing\Controller\RequestHandlerController') ->setConstructorArgs( [ + 'federatedfilessharing', + \OC::$server->getRequest(), $this->federatedShareProvider, \OC::$server->getDatabaseConnection(), \OC::$server->getShareManager(), - \OC::$server->getRequest(), $this->notifications, $this->addressHandler, $this->userManager @@ -197,7 +197,7 @@ class RequestHandlerTest extends TestCase { $_POST['token'] = 'token'; - $this->s2s->declineShare(array('id' => 42)); + $this->s2s->declineShare(42); } diff --git a/ocs/routes.php b/ocs/routes.php index d14f32e045c..3085cd9db65 100644 --- a/ocs/routes.php +++ b/ocs/routes.php @@ -75,76 +75,3 @@ API::register( 'core', API::USER_AUTH ); - -// Server-to-Server Sharing -if (\OC::$server->getAppManager()->isEnabledForUser('files_sharing')) { - $federatedSharingApp = new \OCA\FederatedFileSharing\AppInfo\Application(); - $addressHandler = new \OCA\FederatedFileSharing\AddressHandler( - \OC::$server->getURLGenerator(), - \OC::$server->getL10N('federatedfilesharing') - ); - $notification = new \OCA\FederatedFileSharing\Notifications( - $addressHandler, - \OC::$server->getHTTPClientService(), - new \OCA\FederatedFileSharing\DiscoveryManager(\OC::$server->getMemCacheFactory(), \OC::$server->getHTTPClientService()), - \OC::$server->getJobList() - ); - $s2s = new OCA\FederatedFileSharing\RequestHandler( - $federatedSharingApp->getFederatedShareProvider(), - \OC::$server->getDatabaseConnection(), - \OC::$server->getShareManager(), - \OC::$server->getRequest(), - $notification, - $addressHandler, - \OC::$server->getUserManager() - ); - API::register('post', - '/cloud/shares', - array($s2s, 'createShare'), - 'files_sharing', - API::GUEST_AUTH - ); - - API::register('post', - '/cloud/shares/{id}/reshare', - array($s2s, 'reShare'), - 'files_sharing', - API::GUEST_AUTH - ); - - API::register('post', - '/cloud/shares/{id}/permissions', - array($s2s, 'updatePermissions'), - 'files_sharing', - API::GUEST_AUTH - ); - - - API::register('post', - '/cloud/shares/{id}/accept', - array($s2s, 'acceptShare'), - 'files_sharing', - API::GUEST_AUTH - ); - - API::register('post', - '/cloud/shares/{id}/decline', - array($s2s, 'declineShare'), - 'files_sharing', - API::GUEST_AUTH - ); - - API::register('post', - '/cloud/shares/{id}/unshare', - array($s2s, 'unshare'), - 'files_sharing', - API::GUEST_AUTH - ); - - API::register('post', - '/cloud/shares/{id}/revoke', - array($s2s, 'revoke'), - 'files_sharing', - API::GUEST_AUTH - ); -}