Skip to content

[stable34] fix: Creating shares with multiple mount permissions #63372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
backportbot wants to merge 1 commit into
base: stable34
Choose a base branch
from
+49 −7
Open

[stable34] fix: Creating shares with multiple mount permissions #63372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion apps/files_sharing/lib/Listener/RestrictInteractionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,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()]));
}

Expand Down
22 changes: 22 additions & 0 deletions apps/files_sharing/tests/Listener/RestrictInteractionListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,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());

Expand Down
18 changes: 18 additions & 0 deletions build/integration/sharing_features/sharing-v1-part4.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
12 changes: 6 additions & 6 deletions tests/lib/Share20/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 = [
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
],
Expand Down
Loading