]> source.dussan.org Git - nextcloud-server.git/commitdiff
fix(files_sharing): adjust permissions from custom edit and delete check methods backport/47339/stable30 47894/head
authorskjnldsv <skjnldsv@protonmail.com>
Thu, 22 Aug 2024 08:02:37 +0000 (10:02 +0200)
committerbackportbot[bot] <backportbot[bot]@users.noreply.github.com>
Wed, 11 Sep 2024 11:48:25 +0000 (11:48 +0000)
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
apps/files_sharing/lib/Controller/ShareAPIController.php
apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
build/integration/sharing_features/sharing-v1-part4.feature

index 8b7a711c0ee0766773e4c5de756dabd54335b578..1be1fdbbde90ec59e11f67360b79f9b0c2d94d0c 100644 (file)
@@ -158,6 +158,23 @@ class ShareAPIController extends OCSController {
                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() === '';
index c47bb6d4395dffe9f3c981804d05225dfb61f9ad..85dfb9145cf08e6b273758d288a2222d6a80242e 100644 (file)
@@ -3770,6 +3770,12 @@ class ShareAPIControllerTest extends TestCase {
                $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();
@@ -3830,6 +3836,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => '[{"scope":"permissions","key":"download","value":true}]',
                        ], $share, [], false
                ];
@@ -3869,6 +3877,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => '[{"scope":"permissions","key":"download","value":true}]',
                        ], $share, [
                                ['owner', $owner],
@@ -3924,6 +3934,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -3975,6 +3987,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => true,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4027,6 +4041,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4076,6 +4092,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4132,6 +4150,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4188,6 +4208,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4238,6 +4260,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4288,6 +4312,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4341,6 +4367,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4391,6 +4419,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4441,6 +4471,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4508,6 +4540,8 @@ class ShareAPIControllerTest extends TestCase {
                                'password_expiration_time' => null,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4561,6 +4595,8 @@ class ShareAPIControllerTest extends TestCase {
                                'password_expiration_time' => null,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4612,6 +4648,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => true,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, [], false
                ];
@@ -4715,6 +4753,10 @@ class ShareAPIControllerTest extends TestCase {
                $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);
@@ -4770,6 +4812,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, false, []
                ];
@@ -4819,6 +4863,8 @@ class ShareAPIControllerTest extends TestCase {
                                'can_delete' => false,
                                'item_size' => 123456,
                                'item_mtime' => 1234567890,
+                               'is-mount-root' => false,
+                               'mount-type' => '',
                                'attributes' => null,
                        ], $share, true, [
                                'share_with_displayname' => 'recipientRoomName'
index a51561513967e4a84e2f0f7118234828081649c3..1b32f99a0a283f8df0fbc97196c3559007d47f71 100644 (file)
@@ -41,3 +41,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 |