Merge pull request #47339 from nextcloud/fix/leave-share-instead-of-delete

fix: Display 'Leave share' instead of 'Delete'

5 files changed, 160 insertions, 0 deletions

diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index 3f8062ac2c9..9ca9774013c 100644
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -158,6 +158,27 @@ 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() === '';
+		$result['mount-type'] = $node->getMountPoint()->getMountType();
 
 		$result['mimetype'] = $node->getMimetype();
 		$result['has_preview'] = $this->previewManager->isAvailable($node);
diff --git a/apps/files_sharing/lib/ResponseDefinitions.php b/apps/files_sharing/lib/ResponseDefinitions.php
index 9a6ef199169..774e4c17e00 100644
--- a/apps/files_sharing/lib/ResponseDefinitions.php
+++ b/apps/files_sharing/lib/ResponseDefinitions.php
@@ -22,6 +22,7 @@ namespace OCA\Files_Sharing;
  *     file_target: string,
  *     has_preview: bool,
  *     hide_download: 0|1,
+ *     is-mount-root: bool,
  *     id: string,
  *     item_mtime: int,
  *     item_permissions?: int,
@@ -31,6 +32,7 @@ namespace OCA\Files_Sharing;
  *     label: ?string,
  *     mail_send: 0|1,
  *     mimetype: string,
+ *     mount-type: string,
  *     note: string,
  *     parent: null,
  *     password?: null|string,
diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json
index d6df6e40382..362e9aefc32 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"
                     },
@@ -592,6 +597,9 @@
                     "mimetype": {
                         "type": "string"
                     },
+                    "mount-type": {
+                        "type": "string"
+                    },
                     "note": {
                         "type": "string"
                     },
diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
index e9e35799d84..fd1c60ff842 100644
--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@@ -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'
diff --git a/build/integration/sharing_features/sharing-v1-part4.feature b/build/integration/sharing_features/sharing-v1-part4.feature
index a5156151396..1b32f99a0a2 100644
--- a/build/integration/sharing_features/sharing-v1-part4.feature
+++ b/build/integration/sharing_features/sharing-v1-part4.feature
@@ -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 |