diff --git a/apps/files_sharing/lib/Listener/RestrictInteractionListener.php b/apps/files_sharing/lib/Listener/RestrictInteractionListener.php index 6ab8f38f6e4bc..32b452d04f5c1 100644 --- a/apps/files_sharing/lib/Listener/RestrictInteractionListener.php +++ b/apps/files_sharing/lib/Listener/RestrictInteractionListener.php @@ -51,7 +51,9 @@ public function __construct( public function handle(Event $event): void { foreach ($event->resources as $resource) { if ($resource instanceof NodeResource && $event->action instanceof ShareAction) { - if (!$resource->getNode()->isShareable()) { + // A user can reach the same node through different paths with differing permissions, + // so the merged permissions decide whether they are allowed to share it. + if (($resource->getNodePermissions() & Constants::PERMISSION_SHARE) !== Constants::PERMISSION_SHARE) { throw new InteractionRestrictedException('Node is not shareable.', $this->l10n->t('You are not allowed to share "%s".', [$resource->getNode()->getName()])); } diff --git a/apps/files_sharing/tests/Listener/RestrictInteractionListenerTest.php b/apps/files_sharing/tests/Listener/RestrictInteractionListenerTest.php index 3680fcb0a2d27..a1503240deccf 100644 --- a/apps/files_sharing/tests/Listener/RestrictInteractionListenerTest.php +++ b/apps/files_sharing/tests/Listener/RestrictInteractionListenerTest.php @@ -78,6 +78,28 @@ public function testNodeResourceShareActionMissingSharePermission(): void { } } + public function testNodeResourceShareActionSharePermissionOnOtherPath(): void { + $userFolder = Server::get(IRootFolder::class)->getUserFolder($this->user->getUID()); + + $fileNode = $userFolder->newFile('foo.txt', 'bar'); + $fileNode->getStorage()->getCache()->update($fileNode->getId(), ['permissions' => Constants::PERMISSION_ALL & ~Constants::PERMISSION_SHARE]); + $fileNode = $userFolder->getFirstNodeById($fileNode->getId()); + $this->assertNotNull($fileNode); + + $folderNode = $userFolder->newFolder('foo'); + $folderNode->getStorage()->getCache()->update($folderNode->getId(), ['permissions' => Constants::PERMISSION_ALL & ~Constants::PERMISSION_SHARE]); + $folderNode = $userFolder->getFirstNodeById($folderNode->getId()); + $this->assertNotNull($folderNode); + + // The node found first has no share permission, but another path to the same file id has, + // which is reflected by the merged permissions of the resource. + foreach ([$fileNode, $folderNode] as $node) { + $resource = new NodeResource($node->getId(), $this->user->getUID(), $node, Constants::PERMISSION_ALL); + $event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [$resource], new ShareAction(), []); + $this->assertFalse($event->isInteractionRestricted()); + } + } + public function testNodeResourceShareActionNotHomeFolder(): void { $userFolder = Server::get(IRootFolder::class)->getUserFolder($this->user->getUID()); diff --git a/build/integration/sharing_features/sharing-v1-part4.feature b/build/integration/sharing_features/sharing-v1-part4.feature index 4409ebb1116be..08ab1a1daed3c 100644 --- a/build/integration/sharing_features/sharing-v1-part4.feature +++ b/build/integration/sharing_features/sharing-v1-part4.feature @@ -537,3 +537,21 @@ Scenario: User added/removed to group share with marking When User "user0" moves file "/textfile0 (2).txt" to "/target.txt" Then Share mounts for "user0" match | /user0/files/target.txt/ | + + # The initiator reaches the folder through a read-only path as well, which is found first + # by id. The share permission of the other path still allows creating the share. + Scenario: Creating a link share of a folder that is also reachable without share permission + Given user "user0" exists + And user "user1" exists + And user "user0" created a folder "/parent" + And user "user0" created a folder "/parent/z-child" + And folder "/parent" of user "user0" is shared with user "user1" with permissions 31 + And user "user1" accepts last share + And folder "/parent/z-child" of user "user0" is shared with user "user1" with permissions 1 + And user "user1" accepts last share + When as "user1" creating a share with + | path | parent/z-child | + | shareType | 3 | + | permissions | 1 | + Then the OCS status code should be "100" + And the HTTP status code should be "200" diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index db7a907de6b70..68bfffe94220c 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1029,7 +1029,7 @@ public static function dataGeneralChecks(): array { File::class, [ 'isShareable' => true, - 'getPermissions' => Constants::PERMISSION_READ, + 'getPermissions' => Constants::PERMISSION_READ | Constants::PERMISSION_SHARE, 'getId' => 108, 'getPath' => 'path', 'getName' => 'name', @@ -1045,7 +1045,7 @@ public static function dataGeneralChecks(): array { $limitedPermissions[1]['getMountPoint'] = IMovableMount::class; // increase permissions of a re-share - $data[] = [[null, IShare::TYPE_GROUP, $limitedPermissions, $group0, $user0, $user0, 17, null, null], 'You cannot share "path" with more permission than you have yourself.', true]; + $data[] = [[null, IShare::TYPE_GROUP, $limitedPermissions, $group0, $user0, $user0, 19, null, null], 'You cannot share "path" with more permission than you have yourself.', true]; $data[] = [[null, IShare::TYPE_USER, $limitedPermissions, $user2, $user0, $user0, 3, null, null], 'You cannot share "path" with more permission than you have yourself.', true]; $nonMoveableMountPermissions = [ @@ -1075,9 +1075,9 @@ public static function dataGeneralChecks(): array { 'none', ]; - $data[] = [[null, IShare::TYPE_USER, $rootFolder, $user2, $user0, $user0, 30, null, null], 'You are not allowed to share "".', true]; - $data[] = [[null, IShare::TYPE_GROUP, $rootFolder, $group0, $user0, $user0, 2, null, null], 'You are not allowed to share "".', true]; - $data[] = [[null, IShare::TYPE_LINK, $rootFolder, null, $user0, $user0, 16, null, null], 'You are not allowed to share "".', true]; + $data[] = [[null, IShare::TYPE_USER, $rootFolder, $user2, $user0, $user0, 30, null, null], 'You cannot share your home folder.', true]; + $data[] = [[null, IShare::TYPE_GROUP, $rootFolder, $group0, $user0, $user0, 2, null, null], 'You cannot share your home folder.', true]; + $data[] = [[null, IShare::TYPE_LINK, $rootFolder, null, $user0, $user0, 16, null, null], 'You cannot share your home folder.', true]; $allPermissionsFiles = [ File::class, @@ -1123,7 +1123,7 @@ public static function dataGeneralChecks(): array { Folder::class, [ 'isShareable' => true, - 'getPermissions' => Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE, + 'getPermissions' => Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE | Constants::PERMISSION_SHARE, 'getId' => 108, 'getOwner' => $user0, ],