From 0c647d36f0026fda4863f119d47e27a617ccfbe6 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 20 Aug 2024 11:31:21 +0200 Subject: [PATCH 1/2] fix: Display 'Leave share' instead of 'Delete' Signed-off-by: Marcel Klehr --- apps/files_sharing/lib/Controller/ShareAPIController.php | 4 ++++ apps/files_sharing/lib/ResponseDefinitions.php | 2 ++ apps/files_sharing/openapi.json | 8 ++++++++ 3 files changed, 14 insertions(+) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index c7b9235bb2711..5b32de90787b9 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -191,6 +191,10 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array $result['item_permissions'] = $node->getPermissions(); } + // See MOUNT_ROOT_PROPERTYNAME dav property + $result['is-mount-root'] = $node->getInternalPath() === ''; + $result['mount-type'] = $node->getMountPoint()->getMountType(); + $result['mimetype'] = $node->getMimetype(); $result['has_preview'] = $this->previewManager->isAvailable($node); $result['storage_id'] = $node->getStorage()->getId(); diff --git a/apps/files_sharing/lib/ResponseDefinitions.php b/apps/files_sharing/lib/ResponseDefinitions.php index aa1ee004971a5..2d2e30eb790ff 100644 --- a/apps/files_sharing/lib/ResponseDefinitions.php +++ b/apps/files_sharing/lib/ResponseDefinitions.php @@ -39,6 +39,7 @@ * file_target: string, * has_preview: bool, * hide_download: 0|1, + * is-mount-root: bool, * id: string, * item_mtime: int, * item_permissions?: int, @@ -48,6 +49,7 @@ * label: string, * mail_send: 0|1, * mimetype: string, + * mount-type: string, * note: string, * parent: null, * password?: string, diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json index 661134126d3b8..772b6643a3c37 100644 --- a/apps/files_sharing/openapi.json +++ b/apps/files_sharing/openapi.json @@ -480,6 +480,7 @@ "file_target", "has_preview", "hide_download", + "is-mount-root", "id", "item_mtime", "item_size", @@ -488,6 +489,7 @@ "label", "mail_send", "mimetype", + "mount-type", "note", "parent", "path", @@ -543,6 +545,9 @@ 1 ] }, + "is-mount-root": { + "type": "boolean" + }, "id": { "type": "string" }, @@ -591,6 +596,9 @@ "mimetype": { "type": "string" }, + "mount-type": { + "type": "string" + }, "note": { "type": "string" }, From 903f717f7d22c73dc50c0470cbd826f74014d3c5 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Thu, 22 Aug 2024 10:02:37 +0200 Subject: [PATCH 2/2] fix(files_sharing): adjust permissions from custom edit and delete check methods Signed-off-by: skjnldsv --- .../lib/Controller/ShareAPIController.php | 17 ++++ .../Controller/ShareAPIControllerTest.php | 42 ++++++++++ .../sharing_features/sharing-v1-part4.feature | 83 +++++++++++++++++++ 3 files changed, 142 insertions(+) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 5b32de90787b9..266c1f9474ce0 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -190,6 +190,23 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array if ($isOwnShare) { $result['item_permissions'] = $node->getPermissions(); } + + // If we're on the recipient side, the node permissions + // are bound to the share permissions. So we need to + // adjust the permissions to the share permissions if necessary. + if (!$isOwnShare) { + $result['item_permissions'] = $share->getPermissions(); + + // For some reason, single files share are forbidden to have the delete permission + // since we have custom methods to check those, let's adjust straight away. + // DAV permissions does not have that issue though. + if ($this->canDeleteShare($share) || $this->canDeleteShareFromSelf($share)) { + $result['item_permissions'] |= Constants::PERMISSION_DELETE; + } + if ($this->canEditShare($share)) { + $result['item_permissions'] |= Constants::PERMISSION_UPDATE; + } + } // See MOUNT_ROOT_PROPERTYNAME dav property $result['is-mount-root'] = $node->getInternalPath() === ''; diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 0a0e267857313..139c8996ae011 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -3757,6 +3757,12 @@ public function dataFormatShare() { $folder->method('getStorage')->willReturn($storage); $fileWithPreview->method('getStorage')->willReturn($storage); + + $mountPoint = $this->getMockBuilder(IMountPoint::class)->getMock(); + $mountPoint->method('getMountType')->willReturn(''); + $file->method('getMountPoint')->willReturn($mountPoint); + $folder->method('getMountPoint')->willReturn($mountPoint); + $owner = $this->getMockBuilder(IUser::class)->getMock(); $owner->method('getDisplayName')->willReturn('ownerDN'); $initiator = $this->getMockBuilder(IUser::class)->getMock(); @@ -3911,6 +3917,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -3962,6 +3970,8 @@ public function dataFormatShare() { 'can_delete' => true, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4014,6 +4024,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4063,6 +4075,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4119,6 +4133,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4175,6 +4191,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4225,6 +4243,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4275,6 +4295,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4328,6 +4350,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4378,6 +4402,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4428,6 +4454,8 @@ public function dataFormatShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4495,6 +4523,8 @@ public function dataFormatShare() { 'password_expiration_time' => null, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4548,6 +4578,8 @@ public function dataFormatShare() { 'password_expiration_time' => null, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4599,6 +4631,8 @@ public function dataFormatShare() { 'can_delete' => true, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, [], false ]; @@ -4702,6 +4736,10 @@ public function dataFormatRoomShare() { $file->method('getSize')->willReturn(123456); $file->method('getMTime')->willReturn(1234567890); + $mountPoint = $this->getMockBuilder(IMountPoint::class)->getMock(); + $mountPoint->method('getMountType')->willReturn(''); + $file->method('getMountPoint')->willReturn($mountPoint); + $cache = $this->getMockBuilder('OCP\Files\Cache\ICache')->getMock(); $cache->method('getNumericStorageId')->willReturn(100); $storage = $this->createMock(Storage::class); @@ -4757,6 +4795,8 @@ public function dataFormatRoomShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, false, [] ]; @@ -4806,6 +4846,8 @@ public function dataFormatRoomShare() { 'can_delete' => false, 'item_size' => 123456, 'item_mtime' => 1234567890, + 'is-mount-root' => false, + 'mount-type' => '', 'attributes' => null, ], $share, true, [ 'share_with_displayname' => 'recipientRoomName' diff --git a/build/integration/sharing_features/sharing-v1-part4.feature b/build/integration/sharing_features/sharing-v1-part4.feature index ae4e69f262279..b180ad0807271 100644 --- a/build/integration/sharing_features/sharing-v1-part4.feature +++ b/build/integration/sharing_features/sharing-v1-part4.feature @@ -39,3 +39,86 @@ Scenario: Creating a new share of a file you own shows the file permissions And the HTTP status code should be "200" And Share fields of last share match with | item_permissions | 27 | + +Scenario: Receiving a share of a file gives no create permission + Given user "user0" exists + And user "user1" exists + And As an "user0" + And parameter "shareapi_default_permissions" of app "core" is set to "31" + And file "welcome.txt" of user "user0" is shared with user "user1" + And sending "GET" to "/apps/files_sharing/api/v1/shares" + And share 0 is returned with + | path | /welcome.txt | + | permissions | 19 | + | item_permissions | 27 | + When As an "user1" + And user "user1" accepts last share + And sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true" + Then the list of returned shares has 1 shares + And share 0 is returned with + | path | /welcome (2).txt | + | permissions | 19 | + | item_permissions | 27 | + +Scenario: Receiving a share of a folder gives create permission + Given user "user0" exists + And user "user1" exists + And As an "user0" + And parameter "shareapi_default_permissions" of app "core" is set to "31" + And file "PARENT/CHILD" of user "user0" is shared with user "user1" + And sending "GET" to "/apps/files_sharing/api/v1/shares" + And share 0 is returned with + | path | /PARENT/CHILD | + | permissions | 31 | + | item_permissions | 31 | + When As an "user1" + And user "user1" accepts last share + And sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true" + Then the list of returned shares has 1 shares + And share 0 is returned with + | path | /CHILD | + | permissions | 31 | + | item_permissions | 31 | + +# User can remove itself from a share +Scenario: Receiving a share of a file without delete permission gives delete permission anyway + Given user "user0" exists + And user "user1" exists + And As an "user0" + And parameter "shareapi_default_permissions" of app "core" is set to "23" + And file "welcome.txt" of user "user0" is shared with user "user1" + And sending "GET" to "/apps/files_sharing/api/v1/shares" + And share 0 is returned with + | path | /welcome.txt | + | permissions | 19 | + | item_permissions | 27 | + When As an "user1" + And user "user1" accepts last share + And sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true" + Then the list of returned shares has 1 shares + And share 0 is returned with + | path | /welcome (2).txt | + | permissions | 19 | + | item_permissions | 27 | + +Scenario: Receiving a share of a file without delete permission gives delete permission anyway + Given user "user0" exists + And user "user1" exists + And As an "user0" + And group "group1" exists + And user "user1" belongs to group "group1" + And parameter "shareapi_default_permissions" of app "core" is set to "23" + And file "welcome.txt" of user "user0" is shared with group "group1" + And sending "GET" to "/apps/files_sharing/api/v1/shares" + And share 0 is returned with + | path | /welcome.txt | + | permissions | 19 | + | item_permissions | 27 | + When As an "user1" + And user "user1" accepts last share + And sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true" + Then the list of returned shares has 1 shares + And share 0 is returned with + | path | /welcome (2).txt | + | permissions | 19 | + | item_permissions | 27 |