Return "password fields" only if public-link password is set. #35541

The iOS client uses the presence of these fields to determine if a password is required. Despite their names they are historically used to store pw-hashes for public-links.
owncloud · Jun 17, 2019 · d78e246 · d78e246
1 parent 597b6e8
commit d78e246
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 2 deletions.
diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php
@@ -201,8 +201,12 @@ protected function formatShare(IShare $share, $received = false) {
 			$result['share_with'] = $share->getSharedWith();
 			$result['share_with_displayname'] = $group !== null ? $group->getDisplayName() : $share->getSharedWith();
 		} elseif ($share->getShareType() === Share::SHARE_TYPE_LINK) {
-			$result['share_with'] = '***redacted***';
-			$result['share_with_displayname'] = '***redacted***';
+			if ($share->getPassword() !== null) {
+				// Misleading names ahead!: This fields are miss-used to
+				// read/write public link password-hashes
+				$result['share_with'] = '***redacted***';
+				$result['share_with_displayname'] = '***redacted***';
+			}
 			$result['name'] = $share->getName();
 
 			$result['token'] = $share->getToken();

diff --git a/tests/acceptance/features/apiShareManagement/createShare.feature b/tests/acceptance/features/apiShareManagement/createShare.feature
@@ -123,6 +123,51 @@ Feature: sharing
       | 1               | 100             |
       | 2               | 200             |
 
+  @public_link_share-feature-required
+  Scenario Outline: Getting the share information of public link share form OCS API does not expose sensitive information
+    Given using OCS API version "<ocs_api_version>"
+    When user "user0" creates a public link share using the sharing API with settings
+      | path     | welcome.txt   |
+      | password | %public%      |
+    Then the OCS status code should be "<ocs_status_code>"
+    And the HTTP status code should be "200"
+    And the fields of the last response should include
+      | file_target            | /welcome.txt   |
+      | path                   | /welcome.txt   |
+      | item_type              | file           |
+      | share_type             | 3              |
+      | permissions            | 1              |
+      | uid_owner              | user0          |
+      | share_with             | ***redacted*** |
+      | share_with_displayname | ***redacted*** |
+    Examples:
+      | ocs_api_version | ocs_status_code |
+      | 1               | 100             |
+      | 2               | 200             |
+
+  @public_link_share-feature-required
+  Scenario Outline: Getting the share information of passwordless public-links hides credential placeholders
+    Given using OCS API version "<ocs_api_version>"
+    When user "user0" creates a public link share using the sharing API with settings
+      | path     | welcome.txt   |
+    Then the OCS status code should be "<ocs_status_code>"
+    And the HTTP status code should be "200"
+    And the fields of the last response should include
+      | file_target            | /welcome.txt   |
+      | path                   | /welcome.txt   |
+      | item_type              | file           |
+      | share_type             | 3              |
+      | permissions            | 1              |
+      | uid_owner              | user0          |
+    And the fields of the last response should not include
+      | share_with             | ***redacted*** |
+      | share_with_displayname | ***redacted*** |
+
+    Examples:
+      | ocs_api_version | ocs_status_code |
+      | 1               | 100             |
+      | 2               | 200             |
+
   Scenario Outline: Creating a new share with a disabled user
     Given using OCS API version "<ocs_api_version>"
     And user "user1" has been created with default attributes

diff --git a/tests/acceptance/features/bootstrap/Sharing.php b/tests/acceptance/features/bootstrap/Sharing.php
@@ -1353,6 +1353,28 @@ public function checkFields($body) {
 		}
 	}
 
+	/**
+	 * @Then the fields of the last response should not include
+	 *
+	 * @param TableNode|null $body
+	 *
+	 * @return void
+	 */
+	public function checkFieldsNotInResponse($body) {
+		if ($body instanceof TableNode) {
+			$fd = $body->getRowsHash();
+
+			foreach ($fd as $field => $value) {
+				$value = $this->replaceValuesFromTable($field, $value);
+				if ($this->isFieldInResponse($field, $value)) {
+					PHPUnit\Framework\Assert::fail(
+						"$field has value $value"
+					);
+				}
+			}
+		}
+	}
+
 	/**
 	 * @When user :user removes all shares from the file named :fileName using the sharing API
 	 * @Given user :user has removed all shares from the file named :fileName