fix(dav): ensure moving or copying a file is possible

Signed-off-by: Ferdinand Thiessen <[email protected]>

apps/dav/lib/Connector/Sabre/SharesPlugin.php

@@ -29,13 +29,16 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use OC\Share20\Exception\BackendError;
+use OCA\DAV\Connector\Sabre\Exception\Forbidden;
 use OCA\DAV\Connector\Sabre\Node as DavNode;
 use OCP\Files\Folder;
 use OCP\Files\Node;
 use OCP\Files\NotFoundException;
+use OCP\Files\Storage\ISharedStorage;
 use OCP\IUserSession;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
+use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\Server;
 use Sabre\DAV\Tree;
@@ -88,7 +91,9 @@
 		$server->protectedProperties[] = self::SHAREES_PROPERTYNAME;
 
 		$this->server = $server;
-		$this->server->on('propFind', [$this, 'handleGetProperties']);
+		$this->server->on('propFind', $this->handleGetProperties(...));
+		$this->server->on('beforeCopy', $this->validateMoveOrCopy(...));
+		$this->server->on('beforeMove', $this->validateMoveOrCopy(...));
 	}
 
 	/**
@@ -225,4 +230,49 @@
 			return new ShareeList($shares);
 		});
 	}
+
+	/**
+	 * Ensure that when copying or moving a node it is not transferred from one share to another,
+	 * if the user is neither the owner nor has re-share permissions.
+	 * For share creation we already ensure this in the share manager.
+	 */
+	public function validateMoveOrCopy(string $source, string $target): bool {
+		try {
+			$targetNode = $this->tree->getNodeForPath($target);
+		} catch (NotFound) {
+			[$targetPath,] = \Sabre\Uri\split($target);
+			$targetNode = $this->tree->getNodeForPath($targetPath);
+		}
+
+		$sourceNode = $this->tree->getNodeForPath($source);
+		if ((!$sourceNode instanceof DavNode) || (!$targetNode instanceof DavNode)) {
+			return true;
+		}
+
+		$sourceNode = $sourceNode->getNode();
+		if ($sourceNode->isShareable()) {
+			return true;
+		}
+
+		$targetShares = $this->getShare($targetNode->getNode());
+		if (empty($targetShares)) {
+			// Target is not a share so no re-sharing inprogress
+			return true;
+		}
+
+		$sourceStorage = $sourceNode->getStorage();
+		if ($sourceStorage->instanceOfStorage(ISharedStorage::class)) {
+			// source is also a share - check if it is the same share
+
+			/** @var ISharedStorage $sourceStorage */
+			$sourceShare = $sourceStorage->getShare();
+			foreach ($targetShares as $targetShare) {
+				if ($targetShare->getId() === $sourceShare->getId()) {
+					return true;
+				}
+			}
+		}
+
+		throw new Forbidden('You cannot move a non-shareable node into a share');
+	}
 }

build/integration/sharing_features/sharing-v1-part4.feature

@@ -180,3 +180,110 @@ Scenario: publicUpload overrides permissions
       | uid_file_owner | user0 |
       | share_type     |     3 |
       | permissions    |     1 |
+
+Scenario: Cannot copy files from share without share permission into other share
+    Given user "user0" exists
+    Given user "user1" exists
+    Given user "user2" exists
+    And As an "user0"
+    And user "user0" created a folder "/share"
+    When creating a share with
+      | path        | share |
+      | shareType   |     0 |
+      | shareWith   | user1 |
+      | permissions |    15 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    And User "user0" uploads file with content "test" to "/share/test.txt"
+    And As an "user1"
+    And user "user1" created a folder "/re-share"
+    When creating a share with
+      | path        | re-share |
+      | shareType   |        0 |
+      | shareWith   |    user2 |
+      | permissions |       31 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    When User "user1" copies file "/share/test.txt" to "/re-share/copytest.txt"
+    Then the HTTP status code should be "403"
+
+Scenario: Cannot move files from share without share permission into other share
+    Given user "user0" exists
+    Given user "user1" exists
+    Given user "user2" exists
+    And As an "user0"
+    And user "user0" created a folder "/share"
+    When creating a share with
+      | path        | share |
+      | shareType   |     0 |
+      | shareWith   | user1 |
+      | permissions |    15 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    And User "user0" uploads file with content "test" to "/share/test.txt"
+    And As an "user1"
+    And user "user1" created a folder "/re-share"
+    When creating a share with
+      | path        | re-share |
+      | shareType   |        0 |
+      | shareWith   |    user2 |
+      | permissions |       31 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    When User "user1" moves file "/share/test.txt" to "/re-share/movetest.txt"
+    Then the HTTP status code should be "403"
+
+Scenario: Cannot move folder containing share without share permission into other share
+    Given user "user0" exists
+    Given user "user1" exists
+    Given user "user2" exists
+    And As an "user0"
+    And user "user0" created a folder "/share"
+    When creating a share with
+      | path        | share |
+      | shareType   |     0 |
+      | shareWith   | user1 |
+      | permissions |    15 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    And User "user0" uploads file with content "test" to "/share/test.txt"
+    And As an "user1"
+    And user "user1" created a folder "/contains-share"
+    When User "user1" moves file "/share" to "/contains-share/share"
+    Then the HTTP status code should be "201"
+    And user "user1" created a folder "/re-share"
+    When creating a share with
+      | path        | re-share |
+      | shareType   |        0 |
+      | shareWith   |    user2 |
+      | permissions |       31 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    When User "user1" moves file "/contains-share" to "/re-share/movetest"
+    Then the HTTP status code should be "403"
+
+Scenario: Can copy file between shares if share permissions
+    Given user "user0" exists
+    Given user "user1" exists
+    Given user "user2" exists
+    And As an "user0"
+    And user "user0" created a folder "/share"
+    When creating a share with
+      | path        | share |
+      | shareType   |     0 |
+      | shareWith   | user1 |
+      | permissions |    31 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    And User "user0" uploads file with content "test" to "/share/test.txt"
+    And As an "user1"
+    And user "user1" created a folder "/re-share"
+    When creating a share with
+      | path        | re-share |
+      | shareType   |        0 |
+      | shareWith   |    user2 |
+      | permissions |       31 |
+    Then the HTTP status code should be "200"
+    And the OCS status code should be "100"
+    When User "user1" copies file "/share/test.txt" to "/re-share/movetest.txt"
+    Then the HTTP status code should be "201"