From 278b13155a66253c9472138bff89bc24ebd69634 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 23 Mar 2022 16:29:10 +0100 Subject: [PATCH] take permissions from multiple paths into account for share permissions Signed-off-by: Robin Appelman --- .../lib/Controller/ShareAPIController.php | 33 +++++++++----- .../Controller/ShareAPIControllerTest.php | 44 +++++++++++++++++++ 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 6375ee3d703..a29f8f6a2b2 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -44,6 +44,7 @@ declare(strict_types=1); */ namespace OCA\Files_Sharing\Controller; +use OC\Files\FileInfo; use OCA\Files_Sharing\Exceptions\SharingRightsException; use OCA\Files_Sharing\External\Storage; use OCA\Files\Helper; @@ -469,12 +470,22 @@ class ShareAPIController extends OCSController { $userFolder = $this->rootFolder->getUserFolder($this->currentUser); try { - $path = $userFolder->get($path); + /** @var \OC\Files\Node\Node $node */ + $node = $userFolder->get($path); } catch (NotFoundException $e) { throw new OCSNotFoundException($this->l->t('Wrong path, file/folder does not exist')); } - $share->setNode($path); + // a user can have access to a file through different paths, with differing permissions + // combine all permissions to determine if the user can share this file + $nodes = $userFolder->getById($node->getId()); + foreach ($nodes as $nodeById) { + /** @var FileInfo $fileInfo */ + $fileInfo = $node->getFileInfo(); + $fileInfo['permissions'] |= $nodeById->getPermissions(); + } + + $share->setNode($node); try { $this->lock($share->getNode()); @@ -489,7 +500,7 @@ class ShareAPIController extends OCSController { // Shares always require read permissions $permissions |= Constants::PERMISSION_READ; - if ($path instanceof \OCP\Files\File) { + if ($node instanceof \OCP\Files\File) { // Single file shares should never have delete or create permissions $permissions &= ~Constants::PERMISSION_DELETE; $permissions &= ~Constants::PERMISSION_CREATE; @@ -500,8 +511,8 @@ class ShareAPIController extends OCSController { * We check the permissions via webdav. But the permissions of the mount point * do not equal the share permissions. Here we fix that for federated mounts. */ - if ($path->getStorage()->instanceOfStorage(Storage::class)) { - $permissions &= ~($permissions & ~$path->getPermissions()); + if ($node->getStorage()->instanceOfStorage(Storage::class)) { + $permissions &= ~($permissions & ~$node->getPermissions()); } if ($shareType === IShare::TYPE_USER) { @@ -537,7 +548,7 @@ class ShareAPIController extends OCSController { } // Public upload can only be set for folders - if ($path instanceof \OCP\Files\File) { + if ($node instanceof \OCP\Files\File) { throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders')); } @@ -573,7 +584,7 @@ class ShareAPIController extends OCSController { if ($sendPasswordByTalk === 'true') { if (!$this->appManager->isEnabledForUser('spreed')) { - throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$path->getPath()])); + throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$node->getPath()])); } $share->setSendPasswordByTalk(true); @@ -590,7 +601,7 @@ class ShareAPIController extends OCSController { } } elseif ($shareType === IShare::TYPE_REMOTE) { if (!$this->shareManager->outgoingServer2ServerSharesAllowed()) { - throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType])); + throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$node->getPath(), $shareType])); } if ($shareWith === null) { @@ -609,7 +620,7 @@ class ShareAPIController extends OCSController { } } elseif ($shareType === IShare::TYPE_REMOTE_GROUP) { if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) { - throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType])); + throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$node->getPath(), $shareType])); } if ($shareWith === null) { @@ -643,13 +654,13 @@ class ShareAPIController extends OCSController { try { $this->getRoomShareHelper()->createShare($share, $shareWith, $permissions, $expireDate); } catch (QueryException $e) { - throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$path->getPath()])); + throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()])); } } elseif ($shareType === IShare::TYPE_DECK) { try { $this->getDeckShareHelper()->createShare($share, $shareWith, $permissions, $expireDate); } catch (QueryException $e) { - throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$path->getPath()])); + throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()])); } } else { throw new OCSBadRequestException($this->l->t('Unknown share type')); diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 324862a746d..2da8d600e24 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -1619,6 +1619,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $path->expects($this->once()) ->method('lock') @@ -1651,6 +1653,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $path->expects($this->once()) ->method('lock') @@ -1683,6 +1687,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $path->expects($this->once()) ->method('lock') ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); @@ -1733,6 +1739,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $this->userManager->method('userExists')->with('validUser')->willReturn(true); @@ -1787,6 +1795,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $path->expects($this->once()) ->method('lock') @@ -1844,6 +1854,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $this->groupManager->method('groupExists')->with('validGroup')->willReturn(true); @@ -1896,6 +1908,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $this->groupManager->method('groupExists')->with('validGroup')->willReturn(true); @@ -1926,6 +1940,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); @@ -1945,6 +1961,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -1965,6 +1983,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -1984,6 +2004,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -2018,6 +2040,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -2052,6 +2076,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -2094,6 +2120,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getPath')->willReturn('valid-path'); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -2127,6 +2155,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -2168,6 +2198,8 @@ class ShareAPIControllerTest extends TestCase { $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); + $this->rootFolder->method('getById') + ->willReturn([]); $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare()); $this->shareManager->method('shareApiAllowLinks')->willReturn(true); @@ -2216,6 +2248,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $this->userManager->method('userExists')->with('validUser')->willReturn(true); @@ -2286,6 +2320,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $this->userManager->method('userExists')->with('validUser')->willReturn(true); @@ -2338,6 +2374,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $path->expects($this->once()) ->method('lock') @@ -2421,6 +2459,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $path->expects($this->once()) ->method('lock') @@ -2461,6 +2501,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $path->expects($this->once()) ->method('lock') @@ -2541,6 +2583,8 @@ class ShareAPIControllerTest extends TestCase { ->method('get') ->with('valid-path') ->willReturn($path); + $userFolder->method('getById') + ->willReturn([]); $this->userManager->method('userExists')->with('validUser')->willReturn(true); -- 2.39.5