Better check reshare permissions when creating a share

Signed-off-by: Joas Schilling <coding@schilljs.com>

diff --git a/build/integration/features/sharing-v1-part2.feature b/build/integration/features/sharing-v1-part2.feature
index f6532ea564d8ab500a8357f26d367dd45bb9eae2..9fbb4cda9479a312445d339dcf76dce2a82cb40c 100644 (file)
--- a/build/integration/features/sharing-v1-part2.feature
+++ b/build/integration/features/sharing-v1-part2.feature
@@ -251,6 +251,66 @@ Feature: sharing
     Then the OCS status code should be "404"
     And the HTTP status code should be "200"
 
+  Scenario: User is not allowed to reshare file with additional delete permissions
+  As an "admin"
+    Given user "user0" exists
+    And user "user1" exists
+    And user "user2" exists
+    And As an "user0"
+    And creating a share with
+      | path | /PARENT |
+      | shareType | 0 |
+      | shareWith | user1 |
+      | permissions | 16 |
+    And As an "user1"
+    When creating a share with
+      | path | /PARENT (2) |
+      | shareType | 0 |
+      | shareWith | user2 |
+      | permissions | 25 |
+    Then the OCS status code should be "404"
+    And the HTTP status code should be "200"
+
+  Scenario: User is not allowed to reshare file with additional delete permissions for files
+  As an "admin"
+    Given user "user0" exists
+    And user "user1" exists
+    And user "user2" exists
+    And As an "user0"
+    And creating a share with
+      | path | /textfile0.txt |
+      | shareType | 0 |
+      | shareWith | user1 |
+      | permissions | 16 |
+    And As an "user1"
+    When creating a share with
+      | path | /textfile0 (2).txt |
+      | shareType | 0 |
+      | shareWith | user2 |
+      | permissions | 25 |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    When Getting info of last share
+    Then Share fields of last share match with
+      | id | A_NUMBER |
+      | item_type | file |
+      | item_source | A_NUMBER |
+      | share_type | 0 |
+      | share_with | user2 |
+      | file_source | A_NUMBER |
+      | file_target | /textfile0 (2).txt |
+      | path | /textfile0 (2).txt |
+      | permissions | 17 |
+      | stime | A_NUMBER |
+      | storage | A_NUMBER |
+      | mail_send | 0 |
+      | uid_owner | user1 |
+      | storage_id | shared::/textfile0 (2).txt |
+      | file_parent | A_NUMBER |
+      | share_with_displayname | user2 |
+      | displayname_owner | user1 |
+      | mimetype          | text/plain |
+
   Scenario: Get a share with a user which didn't received the share
     Given user "user0" exists
     And user "user1" exists

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 18fefc021fa8da85eff92faeed611fb90add2477..4e80e1d0668b4693d782fc5ed54a969424aae45c 100644 (file)
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -269,11 +269,13 @@ class Manager implements IManager {
 
                // And you can't share your rootfolder
                if ($this->userManager->userExists($share->getSharedBy())) {
-                       $sharedPath = $this->rootFolder->getUserFolder($share->getSharedBy())->getPath();
+                       $userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
+                       $userFolderPath = $userFolder->getPath();
                } else {
-                       $sharedPath = $this->rootFolder->getUserFolder($share->getShareOwner())->getPath();
+                       $userFolder = $this->rootFolder->getUserFolder($share->getShareOwner());
+                       $userFolderPath = $userFolder->getPath();
                }
-               if ($sharedPath === $share->getNode()->getPath()) {
+               if ($userFolderPath === $share->getNode()->getPath()) {
                        throw new \InvalidArgumentException('You can’t share your root folder');
                }
 
@@ -297,6 +299,23 @@ class Manager implements IManager {
                $mount = $share->getNode()->getMountPoint();
                if (!($mount instanceof MoveableMount)) {
                        $permissions |= \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_UPDATE;
+               } else if ($share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
+                       $userMountPointId = $mount->getStorageRootId();
+                       $userMountPoints = $userFolder->getById($userMountPointId);
+                       $userMountPoint = array_shift($userMountPoints);
+
+                       /* Check if this is an incoming share */
+                       $incomingShares = $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_USER, $userMountPoint, -1, 0);
+                       $incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_GROUP, $userMountPoint, -1, 0));
+                       $incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_ROOM, $userMountPoint, -1, 0));
+
+                       /** @var \OCP\Share\IShare[] $incomingShares */
+                       if (!empty($incomingShares)) {
+                               $permissions = 0;
+                               foreach ($incomingShares as $incomingShare) {
+                                       $permissions |= $incomingShare->getPermissions();
+                               }
+                       }
                }
 
                // Check that we do not share with more permissions than we have

diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index ddbfe857222d386821f9e368992ebcebee31e0dd..747ad556c05462cf30471f875be819604e85f6bf 100644 (file)
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -590,6 +590,12 @@ class ManagerTest extends \Test\TestCase {
                $limitedPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ);
                $limitedPermssions->method('getPath')->willReturn('path');
 
+               $owner = $this->createMock(IUser::class);
+               $owner->method('getUID')
+                       ->willReturn($user0);
+               $limitedPermssions->method('getOwner')
+                       ->willReturn($owner);
+
                $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER,  $limitedPermssions, $user2, $user0, $user0, null, null, null), 'A share requires permissions', true];
                $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, null, null, null), 'A share requires permissions', true];
                $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK,  $limitedPermssions, null, $user0, $user0, null, null, null), 'A share requires permissions', true];