From 3a31b2875bc37114ac97f00e6cb6ea7b3ee53f60 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 20 Jul 2016 10:11:01 +0200 Subject: [PATCH] Add OCSShareAPIMiddleware * This will cleanup the locks after each request * Move check for enabled share api to the middleware --- apps/files_sharing/lib/API/Share20OCS.php | 99 +++++-------------- .../files_sharing/lib/AppInfo/Application.php | 9 ++ .../lib/Middleware/OCSShareAPIMiddleware.php | 53 ++++++++++ 3 files changed, 86 insertions(+), 75 deletions(-) create mode 100644 apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index a468fd3fc14..1a0e75f2749 100644 --- a/apps/files_sharing/lib/API/Share20OCS.php +++ b/apps/files_sharing/lib/API/Share20OCS.php @@ -63,6 +63,8 @@ class Share20OCS extends OCSController { private $currentUser; /** @var IL10N */ private $l; + /** @var \OCP\Files\Node */ + private $lockedNode; /** * Share20OCS constructor. @@ -189,13 +191,6 @@ class Share20OCS extends OCSController { * @throws OCSNotFoundException */ public function getShare($id) { - if (!$this->shareManager->shareApiEnabled()) { - return [ - 'statuscode' => 404, - 'message' => $this->l->t('Share API is disabled') - ]; - } - try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -220,17 +215,10 @@ class Share20OCS extends OCSController { * @NoAdminRequired * * @param string $id - * @return \OC\OCS\Result + * @return DataResponse * @throws OCSNotFoundException */ public function deleteShare($id) { - if (!$this->shareManager->shareApiEnabled()) { - return [ - 'statuscode' => 404, - 'message' => $this->l->t('Share API is disabled') - ]; - } - try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -238,20 +226,17 @@ class Share20OCS extends OCSController { } try { - $share->getNode()->lock(ILockingProvider::LOCK_SHARED); + $this->lock($share->getNode()); } catch (LockedException $e) { throw new OCSNotFoundException($this->l->t('could not delete share')); } if (!$this->canAccessShare($share, false)) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Could not delete share')); } $this->shareManager->deleteShare($share); - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); - return new DataResponse([]); } @@ -264,13 +249,6 @@ class Share20OCS extends OCSController { public function createShare() { $share = $this->shareManager->newShare(); - if (!$this->shareManager->shareApiEnabled()) { - return [ - 'statuscode' => 404, - 'message' => $this->l->t('Share API is disabled') - ]; - } - // Verify path $path = $this->request->getParam('path', null); if ($path === null) { @@ -287,7 +265,7 @@ class Share20OCS extends OCSController { $share->setNode($path); try { - $share->getNode()->lock(ILockingProvider::LOCK_SHARED); + $this->lock($share->getNode()); } catch (LockedException $e) { throw new OCSNotFoundException($this->l->t('Could not create share')); } @@ -301,7 +279,6 @@ class Share20OCS extends OCSController { } if ($permissions < 0 || $permissions > \OCP\Constants::PERMISSION_ALL) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('invalid permissions')); } @@ -329,20 +306,17 @@ class Share20OCS extends OCSController { if ($shareType === \OCP\Share::SHARE_TYPE_USER) { // Valid user is required to share if ($shareWith === null || !$this->userManager->userExists($shareWith)) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Please specify a valid user')); } $share->setSharedWith($shareWith); $share->setPermissions($permissions); } else if ($shareType === \OCP\Share::SHARE_TYPE_GROUP) { if (!$this->shareManager->allowGroupSharing()) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Group sharing is disabled by the administrator')); } // Valid group is required to share if ($shareWith === null || !$this->groupManager->groupExists($shareWith)) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Please specify a valid group')); } $share->setSharedWith($shareWith); @@ -350,7 +324,6 @@ class Share20OCS extends OCSController { } else if ($shareType === \OCP\Share::SHARE_TYPE_LINK) { //Can we even share links? if (!$this->shareManager->shareApiAllowLinks()) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Public link sharing is disabled by the administrator')); } @@ -360,7 +333,6 @@ class Share20OCS extends OCSController { */ $existingShares = $this->shareManager->getSharesBy($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_LINK, $path, false, 1, 0); if (!empty($existingShares)) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new DataResponse($this->formatShare($existingShares[0])); } @@ -368,7 +340,6 @@ class Share20OCS extends OCSController { if ($publicUpload === 'true') { // Check if public upload is allowed if (!$this->shareManager->shareApiLinkAllowPublicUpload()) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 403, 'message' => $this->l->t('Public upload disabled by the administrator') @@ -377,7 +348,6 @@ class Share20OCS extends OCSController { // Public upload can only be set for folders if ($path instanceof \OCP\Files\File) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders')); } @@ -406,14 +376,12 @@ class Share20OCS extends OCSController { $expireDate = $this->parseDate($expireDate); $share->setExpirationDate($expireDate); } catch (\Exception $e) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD')); } } } else if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE) { if (!$this->shareManager->outgoingServer2ServerSharesAllowed()) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 403, 'message' => $this->l->t('Sharing %s failed because the back end does not allow shares from type %s', [$path->getPath(), $shareType]) @@ -423,7 +391,6 @@ class Share20OCS extends OCSController { $share->setSharedWith($shareWith); $share->setPermissions($permissions); } else { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 400, $this->l->t('Unknown share type') @@ -437,13 +404,11 @@ class Share20OCS extends OCSController { $share = $this->shareManager->createShare($share); } catch (GenericShareException $e) { $code = $e->getCode() === 0 ? 403 : $e->getCode(); - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => $code, 'message' => $e->getHint() ]; }catch (\Exception $e) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 403, 'message' => $e->getMessage() @@ -452,8 +417,6 @@ class Share20OCS extends OCSController { $output = $this->formatShare($share); - $share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); - return ['data' => $output]; } @@ -532,10 +495,6 @@ class Share20OCS extends OCSController { * @throws OCSNotFoundException */ public function getShares() { - if (!$this->shareManager->shareApiEnabled()) { - return []; - } - $sharedWithMe = $this->request->getParam('shared_with_me', null); $reshares = $this->request->getParam('reshares', null); $subfiles = $this->request->getParam('subfiles'); @@ -555,17 +514,11 @@ class Share20OCS extends OCSController { if ($sharedWithMe === 'true') { $result = $this->getSharedWithMe($path); - if ($path !== null) { - $path->unlock(ILockingProvider::LOCK_SHARED); - } return $result; } if ($subfiles === 'true') { $result = $this->getSharesInDir($path); - if ($path !== null) { - $path->unlock(ILockingProvider::LOCK_SHARED); - } return $result; } @@ -595,10 +548,6 @@ class Share20OCS extends OCSController { } } - if ($path !== null) { - $path->unlock(ILockingProvider::LOCK_SHARED); - } - return new DataResponse($formatted); } @@ -610,23 +559,15 @@ class Share20OCS extends OCSController { * @throws OCSNotFoundException */ public function updateShare($id) { - if (!$this->shareManager->shareApiEnabled()) { - return [ - 'statuscode' => 404, - 'message' => $this->l->t('Share API is disabled') - ]; - } - try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); } - $share->getNode()->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $this->lock($share->getNode()); if (!$this->canAccessShare($share, false)) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); } @@ -640,7 +581,6 @@ class Share20OCS extends OCSController { */ if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 400, 'message' => 'Wrong or no update parameter given' @@ -666,7 +606,6 @@ class Share20OCS extends OCSController { \OCP\Constants::PERMISSION_CREATE, // hidden file list ]) ) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 400, 'message' => $this->l->t('Can\'t change permissions for public share links') @@ -680,7 +619,6 @@ class Share20OCS extends OCSController { $newPermissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) ) { if (!$this->shareManager->shareApiLinkAllowPublicUpload()) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 403, 'message' => $this->l->t('Public upload disabled by the administrator') @@ -688,7 +626,6 @@ class Share20OCS extends OCSController { } if (!($share->getNode() instanceof \OCP\Files\Folder)) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 400, 'message' => $this->l->t('Public upload is only possible for publicly shared folders') @@ -709,7 +646,6 @@ class Share20OCS extends OCSController { try { $expireDate = $this->parseDate($expireDate); } catch (\Exception $e) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 400, 'message' => $e->getMessage() @@ -727,7 +663,6 @@ class Share20OCS extends OCSController { } else { // For other shares only permissions is valid. if ($permissions === null) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 400, 'message' => $this->l->t('Wrong or no update parameter given') @@ -751,7 +686,6 @@ class Share20OCS extends OCSController { } if ($share->getPermissions() & ~$maxPermissions) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 404, 'message' => $this->l->t('Cannot increase permissions') @@ -764,15 +698,12 @@ class Share20OCS extends OCSController { try { $share = $this->shareManager->updateShare($share); } catch (\Exception $e) { - $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return [ 'statuscode' => 400, 'message' => $e->getMessage() ]; } - $share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); - return new DataResponse($this->formatShare($share)); } @@ -859,4 +790,22 @@ class Share20OCS extends OCSController { return $share; } + + /** + * Lock a Node + * @param \OCP\Files\Node $node + */ + private function lock(\OCP\Files\Node $node) { + $node->lock(ILockingProvider::LOCK_SHARED); + $this->lockedNode = $node; + } + + /** + * Cleanup the remaining locks + */ + public function cleanup() { + if ($this->lockedNode !== null) { + $this->lockedNode->unlock(ILockingProvider::LOCK_SHARED); + } + } } diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index d028cc917f8..b9598bd4a2b 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -29,6 +29,7 @@ namespace OCA\Files_Sharing\AppInfo; use OCA\FederatedFileSharing\DiscoveryManager; use OCA\Files_Sharing\API\Share20OCS; +use OCA\Files_Sharing\Middleware\OCSShareAPIMiddleware; use OCA\Files_Sharing\MountProvider; use OCP\AppFramework\App; use OC\AppFramework\Utility\SimpleContainer; @@ -123,8 +124,16 @@ class Application extends App { ); }); + $container->registerService('OCSShareAPIMiddleware', function (SimpleContainer $c) use ($server) { + return new OCSShareAPIMiddleware( + $server->getShareManager(), + $server->getL10N($c->query('AppName')) + ); + }); + // Execute middlewares $container->registerMiddleware('SharingCheckMiddleware'); + $container->registerMiddleWare('OCSShareAPIMiddleware'); $container->registerService('MountProvider', function (IContainer $c) { /** @var \OCP\IServerContainer $server */ diff --git a/apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php b/apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php new file mode 100644 index 00000000000..b42cf6b00ad --- /dev/null +++ b/apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php @@ -0,0 +1,53 @@ +shareManager = $shareManager; + $this->l = $l; + } + + /** + * @param \OCP\AppFramework\Controller $controller + * @param string $methodName + * + * @throws OCSNotFoundException + */ + public function beforeController($controller, $methodName) { + if ($controller instanceof Share20OCS) { + /** @var Share20OCS $controller */ + if (!$this->shareManager->shareApiEnabled()) { + throw new OCSNotFoundException($this->l->t('Share API is disabled')); + } + } + } + + /** + * @param \OCP\AppFramework\Controller $controller + * @param string $methodName + * @param Response $response + * @return Response + */ + public function afterController($controller, $methodName, Response $response) { + if ($controller instanceof Share20OCS) { + /** @var Share20OCS $controller */ + $controller->cleanup(); + } + + return $response; + } +} -- 2.39.5