Merge pull request #1276 from nextcloud/fix/1272/do_not_return_trashed_shares

Filter returned shares on accessibility

3 files changed, 307 insertions, 8 deletions

diff --git a/lib/Share/RoomShareProvider.php b/lib/Share/RoomShareProvider.php
index 7b5f7678c..89e48d60b 100644
--- a/lib/Share/RoomShareProvider.php
+++ b/lib/Share/RoomShareProvider.php
@@ -611,11 +611,17 @@ class RoomShareProvider implements IShareProvider {
 	 */
 	public function getShareById($id, $recipientId = null) {
 		$qb = $this->dbConnection->getQueryBuilder();
-
-		$qb->select('*')
-			->from('share')
-			->where($qb->expr()->eq('id', $qb->createNamedParameter($id)))
-			->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_ROOM)));
+		$qb->select('s.*',
+			'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
+			'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
+			'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
+		)
+			->selectAlias('st.id', 'storage_string_id')
+			->from('share', 's')
+			->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
+			->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'))
+			->where($qb->expr()->eq('s.id', $qb->createNamedParameter($id)))
+			->andWhere($qb->expr()->eq('s.share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_ROOM)));
 
 		$cursor = $qb->execute();
 		$data = $cursor->fetch();
@@ -625,6 +631,10 @@ class RoomShareProvider implements IShareProvider {
 			throw new ShareNotFound();
 		}
 
+		if (!$this->isAccessibleResult($data)) {
+			throw new ShareNotFound();
+		}
+
 		$share = $this->createShareObject($data);
 
 		if ($recipientId !== null) {
@@ -745,10 +755,16 @@ class RoomShareProvider implements IShareProvider {
 			}
 
 			$qb = $this->dbConnection->getQueryBuilder();
-			$qb->select('*')
-				->from('share')
+			$qb->select('s.*',
+				'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
+				'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
+				'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
+			)
+				->selectAlias('st.id', 'storage_string_id')
+				->from('share', 's')
 				->orderBy('id')
-				->setFirstResult(0);
+				->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
+				->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'));
 
 			if ($limit !== -1) {
 				$qb->setMaxResults($limit);
@@ -773,6 +789,10 @@ class RoomShareProvider implements IShareProvider {
 
 			$cursor = $qb->execute();
 			while ($data = $cursor->fetch()) {
+				if (!$this->isAccessibleResult($data)) {
+					continue;
+				}
+
 				if ($offset > 0) {
 					$offset--;
 					continue;
@@ -788,6 +808,22 @@ class RoomShareProvider implements IShareProvider {
 		return $shares;
 	}
 
+	private function isAccessibleResult($data) {
+		// exclude shares leading to deleted file entries
+		if ($data['fileid'] === null) {
+			return false;
+		}
+
+		// exclude shares leading to trashbin on home storages
+		$pathSections = explode('/', $data['path'], 2);
+		// FIXME: would not detect rare md5'd home storage case properly
+		if ($pathSections[0] !== 'files'
+			&& in_array(explode(':', $data['storage_string_id'], 2)[0], array('home', 'object'))) {
+			return false;
+		}
+		return true;
+	}
+
 	/**
 	 * Get a share by token
 	 *
diff --git a/tests/integration/features/bootstrap/SharingContext.php b/tests/integration/features/bootstrap/SharingContext.php
index f29aaa23a..9c2e1d1ec 100644
--- a/tests/integration/features/bootstrap/SharingContext.php
+++ b/tests/integration/features/bootstrap/SharingContext.php
@@ -104,6 +104,22 @@ class SharingContext implements Context {
 	}
 
 	/**
+	 * @Given user :user deletes file :file
+	 *
+	 * @param string $user
+	 * @param string $file
+	 */
+	public function userDeletesFile($user, $file) {
+		$this->currentUser = $user;
+	
+		$url = "/$user/$file";
+
+		$this->sendingToDav('DELETE', $url);
+
+		$this->theHTTPStatusCodeShouldBe(204);
+	}
+
+	/**
 	 * @When user :user shares :path with user :sharee
 	 *
 	 * @param string $user
@@ -349,6 +365,22 @@ class SharingContext implements Context {
 	}
 
 	/**
+	 * @When user :user gets the DAV properties for :path
+	 *
+	 * @param string $user
+	 * @param string $path
+	 */
+	public function userGetsTheDavPropertiesFor(string $user, string $path) {
+		$this->currentUser = $user;
+
+		$url = "/$user/$path";
+
+		$this->sendingToDav('PROPFIND', $url);
+
+		$this->theHTTPStatusCodeShouldBe(207);
+	}
+
+	/**
 	 * @When user :user gets the share-type DAV property for :path
 	 *
 	 * @param string $user
@@ -546,6 +578,31 @@ class SharingContext implements Context {
 	}
 
 	/**
+	 * @Then the list of returned files for :user is
+	 *
+	 * @param string $user
+	 * @param TableNode|null $table
+	 */
+	public function theListOfReturnedFilesForIs(string $user, TableNode $table = null) {
+		$xmlResponse = $this->getXmlResponse();
+		$xmlResponse->registerXPathNamespace('d', 'DAV:');
+
+		$hrefs = [];
+		foreach ($xmlResponse->xpath('//d:response/d:href') as $href) {
+			$hrefs[] = (string)$href;
+		}
+
+		$expectedHrefs = [];
+		if ($table !== null) {
+			foreach ($table->getRows() as $row) {
+				$expectedHrefs[] = '/remote.php/dav/files/' . $user . (string)$row[0];
+			}
+		}
+
+		PHPUnit_Framework_Assert::assertEquals($expectedHrefs, $hrefs);
+	}
+
+	/**
 	 * @Then the response contains a share-types DAV property with
 	 *
 	 * @param TableNode|null $table
diff --git a/tests/integration/features/sharing/get.feature b/tests/integration/features/sharing/get.feature
index 4e043af6e..1410ed980 100644
--- a/tests/integration/features/sharing/get.feature
+++ b/tests/integration/features/sharing/get.feature
@@ -142,6 +142,22 @@ Feature: get
 
 
 
+  Scenario: get a share after deleting its file
+    Given user "participant1" creates room "group room"
+      | roomType | 2 |
+    And user "participant1" renames room "group room" to "Group room" with 200
+    And user "participant1" adds "participant2" to room "group room" with 200
+    And user "participant1" shares "welcome.txt" with room "group room" with OCS 100
+    And user "participant1" deletes file "welcome.txt"
+    When user "participant1" gets last share
+    Then the OCS status code should be "404"
+    And the HTTP status code should be "200"
+    And user "participant2" gets last share
+    And the OCS status code should be "404"
+    And the HTTP status code should be "200"
+
+
+
   Scenario: get all shares of a user
     Given user "participant1" creates room "own group room"
       | roomType | 2 |
@@ -163,6 +179,9 @@ Feature: get
     And user "participant1" shares "test" with room "own one-to-one room" with OCS 100
     And user "participant2" shares "welcome (2).txt" with user "participant3" with OCS 100
     And user "participant3" shares "welcome (2).txt" with room "one-to-one room not invited to" with OCS 100
+    And user "participant1" creates folder "/deleted"
+    And user "participant1" shares "deleted" with room "group room invited to" with OCS 100
+    And user "participant1" deletes file "deleted"
     When user "participant1" gets all shares
     Then the list of returned shares has 4 shares
     And share 0 is returned with
@@ -229,6 +248,10 @@ Feature: get
     And user "participant1" shares "test" with room "own one-to-one room" with OCS 100
     And user "participant2" shares "welcome (2).txt" with user "participant3" with OCS 100
     And user "participant3" shares "welcome (2).txt" with room "one-to-one room not invited to" with OCS 100
+    And user "participant1" creates folder "/deleted"
+    And user "participant1" shares "deleted" with room "group room invited to" with OCS 100
+    And user "participant2" shares "deleted" with user "participant3" with OCS 100
+    And user "participant1" deletes file "deleted"
     When user "participant1" gets all shares and reshares
     Then the list of returned shares has 6 shares
     And share 0 is returned with
@@ -343,6 +366,32 @@ Feature: get
       | share_with             | group room invited to |
       | share_with_displayname | Group room invited to |
 
+  Scenario: get all shares of a deleted file
+    Given user "participant1" creates room "own group room"
+      | roomType | 2 |
+    And user "participant1" renames room "own group room" to "Own group room" with 200
+    And user "participant2" creates room "group room invited to"
+      | roomType | 2 |
+    And user "participant2" renames room "group room invited to" to "Group room invited to" with 200
+    And user "participant2" adds "participant1" to room "group room invited to" with 200
+    And user "participant1" creates room "own one-to-one room"
+      | roomType | 1 |
+      | invite   | participant3 |
+    And user "participant3" creates room "one-to-one room not invited to"
+      | roomType | 1 |
+      | invite   | participant4 |
+    And user "participant1" creates folder "/test"
+    And user "participant1" shares "welcome.txt" with room "own group room" with OCS 100
+    And user "participant1" shares "test" with room "group room invited to" with OCS 100
+    And user "participant1" shares "welcome.txt" with room "group room invited to" with OCS 100
+    And user "participant1" shares "test" with room "own one-to-one room" with OCS 100
+    And user "participant2" shares "welcome (2).txt" with user "participant3" with OCS 100
+    And user "participant3" shares "welcome (2).txt" with room "one-to-one room not invited to" with OCS 100
+    And user "participant1" deletes file "welcome.txt"
+    When user "participant1" gets all shares for "/welcome.txt"
+    Then the OCS status code should be "404"
+    And the HTTP status code should be "200"
+
   Scenario: get all shares and reshares of a file
     Given user "participant1" creates room "own group room"
       | roomType | 2 |
@@ -412,6 +461,32 @@ Feature: get
       | share_with             | one-to-one room not invited to |
       | share_with_displayname | participant3-displayname |
 
+  Scenario: get all shares and reshares of a deleted file
+    Given user "participant1" creates room "own group room"
+      | roomType | 2 |
+    And user "participant1" renames room "own group room" to "Own group room" with 200
+    And user "participant2" creates room "group room invited to"
+      | roomType | 2 |
+    And user "participant2" renames room "group room invited to" to "Group room invited to" with 200
+    And user "participant2" adds "participant1" to room "group room invited to" with 200
+    And user "participant1" creates room "own one-to-one room"
+      | roomType | 1 |
+      | invite   | participant3 |
+    And user "participant3" creates room "one-to-one room not invited to"
+      | roomType | 1 |
+      | invite   | participant4 |
+    And user "participant1" creates folder "/test"
+    And user "participant1" shares "welcome.txt" with room "own group room" with OCS 100
+    And user "participant1" shares "test" with room "group room invited to" with OCS 100
+    And user "participant1" shares "welcome.txt" with room "group room invited to" with OCS 100
+    And user "participant1" shares "test" with room "own one-to-one room" with OCS 100
+    And user "participant2" shares "welcome (2).txt" with user "participant3" with OCS 100
+    And user "participant3" shares "welcome (2).txt" with room "one-to-one room not invited to" with OCS 100
+    And user "participant1" deletes file "welcome.txt"
+    When user "participant1" gets all shares and reshares for "/welcome.txt"
+    Then the OCS status code should be "404"
+    And the HTTP status code should be "200"
+
   Scenario: get all shares of a folder
     Given user "participant1" creates room "own group room"
       | roomType | 2 |
@@ -473,6 +548,37 @@ Feature: get
       | share_with_displayname | Group room invited to |
       | permissions            | 31 |
 
+  Scenario: get all shares of a deleted folder
+    Given user "participant1" creates room "own group room"
+      | roomType | 2 |
+    And user "participant1" renames room "own group room" to "Own group room" with 200
+    And user "participant2" creates room "group room invited to"
+      | roomType | 2 |
+    And user "participant2" renames room "group room invited to" to "Group room invited to" with 200
+    And user "participant2" adds "participant1" to room "group room invited to" with 200
+    And user "participant1" creates room "own one-to-one room"
+      | roomType | 1 |
+      | invite   | participant3 |
+    And user "participant3" creates room "one-to-one room not invited to"
+      | roomType | 1 |
+      | invite   | participant4 |
+    And user "participant1" creates folder "/test"
+    And user "participant1" creates folder "/test/subfolder"
+    And user "participant1" creates folder "/test/subfolder/subsubfolder"
+    And user "participant1" creates folder "/test2"
+    And user "participant1" shares "welcome.txt" with room "own group room" with OCS 100
+    And user "participant1" shares "test/subfolder" with room "group room invited to" with OCS 100
+    And user "participant1" shares "test/subfolder/subsubfolder" with room "group room invited to" with OCS 100
+    And user "participant1" shares "welcome.txt" with room "group room invited to" with OCS 100
+    And user "participant1" shares "test2" with room "own one-to-one room" with OCS 100
+    And user "participant1" moves file "/welcome.txt" to "/test/renamed.txt" with 201
+    And user "participant2" shares "subfolder" with user "participant3" with OCS 100
+    And user "participant3" shares "subfolder" with room "one-to-one room not invited to" with OCS 100
+    And user "participant1" deletes file "test"
+    When user "participant1" gets all shares for "/test" and its subfiles
+    Then the OCS status code should be "404"
+    And the HTTP status code should be "200"
+
 
 
   Scenario: get all received shares of a user
@@ -493,6 +599,9 @@ Feature: get
     And user "participant3" shares "test" with room "group room invited to" with OCS 100
     And user "participant2" shares "welcome.txt" with room "group room invited to" with OCS 100
     And user "participant3" shares "test" with room "own one-to-one room" with OCS 100
+    And user "participant2" creates folder "/deleted"
+    And user "participant2" shares "deleted" with room "group room invited to" with OCS 100
+    And user "participant2" deletes file "deleted"
     When user "participant1" gets all received shares
     Then the list of returned shares has 4 shares
     And share 0 is returned with
@@ -579,6 +688,29 @@ Feature: get
       | share_with             | group room invited to |
       | share_with_displayname | Group room invited to |
 
+  Scenario: get all received shares of a deleted file
+    Given user "participant1" creates room "own group room"
+      | roomType | 2 |
+    And user "participant1" renames room "own group room" to "Own group room" with 200
+    And user "participant1" adds "participant2" to room "own group room" with 200
+    And user "participant2" creates room "group room invited to"
+      | roomType | 2 |
+    And user "participant2" renames room "group room invited to" to "Group room invited to" with 200
+    And user "participant2" adds "participant1" to room "group room invited to" with 200
+    And user "participant2" adds "participant3" to room "group room invited to" with 200
+    And user "participant1" creates room "own one-to-one room"
+      | roomType | 1 |
+      | invite   | participant3 |
+    And user "participant3" creates folder "/test"
+    And user "participant2" shares "welcome.txt" with room "own group room" with OCS 100
+    And user "participant3" shares "test" with room "group room invited to" with OCS 100
+    And user "participant2" shares "welcome.txt" with room "group room invited to" with OCS 100
+    And user "participant3" shares "test" with room "own one-to-one room" with OCS 100
+    And user "participant2" deletes file "welcome.txt"
+    When user "participant1" gets all received shares for "/welcome (2).txt"
+    Then the OCS status code should be "404"
+    And the HTTP status code should be "200"
+
 
 
   Scenario: get deleted shares when deleting an own share
@@ -620,6 +752,17 @@ Feature: get
     And user "participant3" gets deleted shares
     And the list of returned shares has 0 shares
 
+  Scenario: get deleted shares when deleting the file of an own share
+    Given user "participant1" creates room "group room"
+      | roomType | 2 |
+    And user "participant1" adds "participant2" to room "group room" with 200
+    And user "participant1" shares "welcome.txt" with room "group room" with OCS 100
+    And user "participant1" deletes file "welcome.txt"
+    When user "participant1" gets deleted shares
+    Then the list of returned shares has 0 shares
+    And user "participant2" gets deleted shares
+    And the list of returned shares has 0 shares
+
 
 
   Scenario: get DAV properties for a share
@@ -734,6 +877,69 @@ Feature: get
 
 
 
+  Scenario: get files after sharing a file
+    Given user "participant1" creates room "group room"
+      | roomType | 2 |
+    And user "participant1" adds "participant2" to room "group room" with 200
+    And user "participant1" shares "welcome.txt" with room "group room" with OCS 100
+    When user "participant1" gets the DAV properties for "/"
+    Then the list of returned files for "participant1" is
+      | / |
+      | /welcome.txt |
+    And user "participant2" gets the DAV properties for "/"
+    And the list of returned files for "participant2" is
+      | / |
+      | /welcome.txt |
+      | /welcome%20(2).txt |
+
+
+
+  Scenario: get files after deleting a share
+    Given user "participant1" creates room "group room"
+      | roomType | 2 |
+    And user "participant1" adds "participant2" to room "group room" with 200
+    And user "participant1" shares "welcome.txt" with room "group room" with OCS 100
+    And user "participant1" deletes last share
+    When user "participant1" gets the DAV properties for "/"
+    Then the list of returned files for "participant1" is
+      | / |
+      | /welcome.txt |
+    And user "participant2" gets the DAV properties for "/"
+    And the list of returned files for "participant2" is
+      | / |
+      | /welcome.txt |
+
+  Scenario: get files after deleting a received share
+    Given user "participant1" creates room "group room"
+      | roomType | 2 |
+    And user "participant1" adds "participant2" to room "group room" with 200
+    And user "participant1" shares "welcome.txt" with room "group room" with OCS 100
+    And user "participant2" deletes last share
+    When user "participant2" gets the DAV properties for "/"
+    Then the list of returned files for "participant2" is
+      | / |
+      | /welcome.txt |
+    And user "participant1" gets the DAV properties for "/"
+    And the list of returned files for "participant1" is
+      | / |
+      | /welcome.txt |
+
+  Scenario: get files after deleting the file of a share
+    Given user "participant1" creates room "group room"
+      | roomType | 2 |
+    And user "participant1" adds "participant2" to room "group room" with 200
+    And user "participant1" shares "welcome.txt" with room "group room" with OCS 100
+    And user "participant1" deletes file "welcome.txt"
+    When user "participant1" gets the DAV properties for "/"
+    Then the list of returned files for "participant1" is
+      | / |
+    And user "participant2" gets the DAV properties for "/"
+    And the list of returned files for "participant2" is
+      | / |
+      | /welcome.txt |
+
+
+
   Scenario: get recent files including a share
     Given user "participant1" creates room "group room"
       | roomType | 2 |