From 7b723813cef60e744ab14ab418c82e5ec67a9f2e Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 15 Jul 2022 17:11:54 +0200 Subject: Multiple fixes - Fix tests - Use non deprecated event stuff - Add a bit of type hinting to the new stuff - More safe handling of instanceOfStorage (share might not be the first wrapper) - Fix resharing Signed-off-by: Carl Schwan --- apps/files_sharing/lib/AppInfo/Application.php | 53 ++++++++--------- .../lib/Controller/PublicPreviewController.php | 2 +- .../lib/Controller/ShareAPIController.php | 67 +++++++++++++--------- apps/files_sharing/lib/MountProvider.php | 5 +- 4 files changed, 70 insertions(+), 57 deletions(-) (limited to 'apps/files_sharing/lib') diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index ae039520c5b..63fdced9011 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -50,6 +50,7 @@ use OCA\Files_Sharing\Notification\Listener; use OCA\Files_Sharing\Notification\Notifier; use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCA\Files\Event\LoadSidebar; +use OCP\Files\Event\BeforeDirectGetEvent; use OCA\Files_Sharing\ShareBackend\File; use OCA\Files_Sharing\ShareBackend\Folder; use OCA\Files_Sharing\ViewOnly; @@ -62,6 +63,8 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\GenericEvent; use OCP\Federation\ICloudIdManager; use OCP\Files\Config\IMountProviderCollection; +use OCP\Files\Events\BeforeDirectFileDownloadEvent; +use OCP\Files\Events\BeforeZipCreatedEvent; use OCP\Files\IRootFolder; use OCP\Group\Events\UserAddedEvent; use OCP\IDBConnection; @@ -157,59 +160,53 @@ class Application extends App implements IBootstrap { public function registerDownloadEvents( IEventDispatcher $dispatcher, - ?IUserSession $userSession, + IUserSession $userSession, IRootFolder $rootFolder ): void { $dispatcher->addListener( - 'file.beforeGetDirect', - function (GenericEvent $event) use ($userSession, $rootFolder) { - $pathsToCheck = [$event->getArgument('path')]; - $event->setArgument('run', true); - + BeforeDirectFileDownloadEvent::class, + function (BeforeDirectFileDownloadEvent $event) use ($userSession, $rootFolder): void { + $pathsToCheck = [$event->getPath()]; // Check only for user/group shares. Don't restrict e.g. share links - if ($userSession && $userSession->isLoggedIn()) { - $uid = $userSession->getUser()->getUID(); + $user = $userSession->getUser(); + if ($user) { $viewOnlyHandler = new ViewOnly( - $rootFolder->getUserFolder($uid) + $rootFolder->getUserFolder($user->getUID()) ); if (!$viewOnlyHandler->check($pathsToCheck)) { - $event->setArgument('run', false); - $event->setArgument('errorMessage', 'Access to this resource or one of its sub-items has been denied.'); + $event->setSuccessful(false); + $event->setErrorMessage('Access to this resource or one of its sub-items has been denied.'); } } } ); $dispatcher->addListener( - 'file.beforeCreateZip', - function (GenericEvent $event) use ($userSession, $rootFolder) { - $dir = $event->getArgument('dir'); - $files = $event->getArgument('files'); + BeforeZipCreatedEvent::class, + function (BeforeZipCreatedEvent $event) use ($userSession, $rootFolder): void { + $dir = $event->getDirectory(); + $files = $event->getFiles(); $pathsToCheck = []; - if (\is_array($files)) { - foreach ($files as $file) { - $pathsToCheck[] = $dir . '/' . $file; - } - } elseif (\is_string($files)) { - $pathsToCheck[] = $dir . '/' . $files; + foreach ($files as $file) { + $pathsToCheck[] = $dir . '/' . $file; } // Check only for user/group shares. Don't restrict e.g. share links - if ($userSession && $userSession->isLoggedIn()) { - $uid = $userSession->getUser()->getUID(); + $user = $userSession->getUser(); + if ($user) { $viewOnlyHandler = new ViewOnly( - $rootFolder->getUserFolder($uid) + $rootFolder->getUserFolder($user->getUID()) ); if (!$viewOnlyHandler->check($pathsToCheck)) { - $event->setArgument('errorMessage', 'Access to this resource or one of its sub-items has been denied.'); - $event->setArgument('run', false); + $event->setErrorMessage('Access to this resource or one of its sub-items has been denied.'); + $event->setSuccessful(false); } else { - $event->setArgument('run', true); + $event->setSuccessful(true); } } else { - $event->setArgument('run', true); + $event->setSuccessful(true); } } ); diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php index 4a16afa7ac0..98c4d8cafb4 100644 --- a/apps/files_sharing/lib/Controller/PublicPreviewController.php +++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php @@ -136,7 +136,7 @@ class PublicPreviewController extends PublicShareController { * @param $token * @return DataResponse|FileDisplayResponse */ - public function directLink($token) { + public function directLink(string $token) { // No token no image if ($token === '') { return new DataResponse([], Http::STATUS_BAD_REQUEST); diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index c72fd8ba8af..59089390667 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -45,6 +45,7 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Controller; use OC\Files\FileInfo; +use OC\Files\Storage\Wrapper\Wrapper; use OCA\Files_Sharing\Exceptions\SharingRightsException; use OCA\Files_Sharing\External\Storage; use OCA\Files_Sharing\SharedStorage; @@ -524,14 +525,7 @@ class ShareAPIController extends OCSController { $permissions &= ~($permissions & ~$node->getPermissions()); } - if ($share->getNode()->getStorage()->instanceOfStorage(SharedStorage::class)) { - /** @var \OCA\Files_Sharing\SharedStorage $storage */ - $inheritedAttributes = $share->getNode()->getStorage()->getShare()->getAttributes(); - if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) { - $share->setHideDownload(true); - } - } - + $this->checkInheritedAttributes($share); if ($shareType === IShare::TYPE_USER) { // Valid user is required to share @@ -1110,6 +1104,25 @@ class ShareAPIController extends OCSController { $share->setNote($note); } + $userFolder = $this->rootFolder->getUserFolder($this->currentUser); + + // get the node with the point of view of the current user + $nodes = $userFolder->getById($share->getNode()->getId()); + if (count($nodes) > 0) { + $node = $nodes[0]; + $storage = $node->getStorage(); + if ($storage && $storage->instanceOfStorage(SharedStorage::class)) { + /** @var \OCA\Files_Sharing\SharedStorage $storage */ + $inheritedAttributes = $storage->getShare()->getAttributes(); + if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) { + if ($hideDownload === 'false') { + throw new OCSBadRequestException($this->l->t('Cannot increase permissions')); + } + $share->setHideDownload(true); + } + } + } + /** * expirationdate, password and publicUpload only make sense for link shares */ @@ -1135,24 +1148,6 @@ class ShareAPIController extends OCSController { $share->setHideDownload(false); } - $userFolder = $this->rootFolder->getUserFolder($this->currentUser); - // get the node with the point of view of the current user - $nodes = $userFolder->getById($share->getNode()->getId()); - if (count($nodes) > 0) { - $node = $nodes[0]; - $storage = $node->getStorage(); - if ($storage->instanceOfStorage(SharedStorage::class)) { - /** @var \OCA\Files_Sharing\SharedStorage $storage */ - $inheritedAttributes = $storage->getShare()->getAttributes(); - if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) { - if ($hideDownload === 'false') { - throw new OCSBadRequestException($this->l->t('Cannot increate permissions')); - } - $share->setHideDownload(true); - } - } - } - $newPermissions = null; if ($publicUpload === 'true') { $newPermissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE; @@ -1905,4 +1900,24 @@ class ShareAPIController extends OCSController { return $share; } + + private function checkInheritedAttributes(IShare $share): void { + if ($share->getNode()->getStorage()->instanceOfStorage(SharedStorage::class)) { + $storage = $share->getNode()->getStorage(); + if ($storage instanceof Wrapper) { + $storage = $storage->getInstanceOfStorage(SharedStorage::class); + if ($storage === null) { + throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null'); + } + } else { + throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper'); + } + /** @var \OCA\Files_Sharing\SharedStorage $storage */ + $inheritedAttributes = $storage->getShare()->getAttributes(); + if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) { + $share->setHideDownload(true); + } + } + + } } diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 30e398d663b..954c9cf70e6 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -242,8 +242,9 @@ class MountProvider implements IMountProvider { $superPermissions |= $share->getPermissions(); // update share permission attributes - if ($share->getAttributes() !== null) { - foreach ($share->getAttributes()->toArray() as $attribute) { + $attributes = $share->getAttributes(); + if ($attributes !== null) { + foreach ($attributes->toArray() as $attribute) { if ($superAttributes->getAttribute($attribute['scope'], $attribute['key']) === true) { // if super share attribute is already enabled, it is most permissive continue; -- cgit v1.2.3