From 1cd31259f2fedf84989ef7cec76d704faad7a980 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 11 Dec 2019 08:25:12 +0100 Subject: [PATCH 1/2] On the public preview route the share password needs to be verified again to not grant unauthorized access --- apps/files_sharing/ajax/publicpreview.php | 8 ++++++++ changelog/unreleased/36571 | 5 +++++ 2 files changed, 13 insertions(+) create mode 100644 changelog/unreleased/36571 diff --git a/apps/files_sharing/ajax/publicpreview.php b/apps/files_sharing/ajax/publicpreview.php index 90153fb574a3..dcc6004d64e3 100644 --- a/apps/files_sharing/ajax/publicpreview.php +++ b/apps/files_sharing/ajax/publicpreview.php @@ -48,6 +48,14 @@ $shareManager = \OC::$server->getShareManager(); try { $linkedItem = $shareManager->getShareByToken($token); + if ($linkedItem->getPassword() !== null) { + $session = \OC::$server->getSession(); + if (! $session->exists('public_link_authenticated') + || $session->get('public_link_authenticated') !== (string)$linkedItem->getId()) { + // sending back 404 in case access is not allowed - not 401 because this way we would expose existence of a share + throw new ShareNotFound(); + } + } } catch (ShareNotFound $e) { \OC_Response::setStatus(\OC_Response::STATUS_NOT_FOUND); \OCP\Util::writeLog('core-preview', 'Passed token parameter is not valid', \OCP\Util::DEBUG); diff --git a/changelog/unreleased/36571 b/changelog/unreleased/36571 new file mode 100644 index 000000000000..638453d5285f --- /dev/null +++ b/changelog/unreleased/36571 @@ -0,0 +1,5 @@ +Change: Protect public preview with password + +The preview route for password protected shares was accessible without the password. + +https://github.com/owncloud/core/pull/36571 From d47fa251d8fbda4bde81fb60a3488772e5a0678b Mon Sep 17 00:00:00 2001 From: Dipak Acharya Date: Wed, 11 Dec 2019 16:52:01 +0545 Subject: [PATCH 2/2] Add api acceptance tests for checking the preview of public link shared file --- .../apiShareOperations/accessToShare.feature | 48 +++++++++++++++++++ .../acceptance/features/bootstrap/Sharing.php | 29 +++++++++++ 2 files changed, 77 insertions(+) diff --git a/tests/acceptance/features/apiShareOperations/accessToShare.feature b/tests/acceptance/features/apiShareOperations/accessToShare.feature index c0ede03ffde8..991f557c8498 100644 --- a/tests/acceptance/features/apiShareOperations/accessToShare.feature +++ b/tests/acceptance/features/apiShareOperations/accessToShare.feature @@ -63,3 +63,51 @@ Feature: sharing | ocs_api_version | ocs_status_code | | 1 | 100 | | 2 | 200 | + + @public_link_share-feature-required + Scenario: Access to the preview of password protected public link + Given the administrator has enabled DAV tech_preview + And user "user0" has uploaded file "filesForUpload/testavatar.jpg" to "testavatar.jpg" + And user "user0" has created a public link share with settings + | path | /testavatar.jpg | + | permissions | change | + | password | testpass1 | + When the public accesses the preview of file "testavatar.jpg" from the last shared public link using the sharing API + Then the HTTP status code should be "404" + + @public_link_share-feature-required + Scenario: Access to the preview of public shared file without password + Given the administrator has enabled DAV tech_preview + And user "user0" has uploaded file "filesForUpload/testavatar.jpg" to "testavatar.jpg" + And user "user0" has created a public link share with settings + | path | /testavatar.jpg | + | permissions | change | + When the public accesses the preview of file "testavatar.jpg" from the last shared public link using the sharing API + Then the HTTP status code should be "200" + + @public_link_share-feature-required + Scenario: Access to the preview of password protected public shared file inside a folder + Given the administrator has enabled DAV tech_preview + And user "user0" has uploaded file "filesForUpload/testavatar.jpg" to "FOLDER/testavatar.jpg" + And user "user0" has moved file "textfile0.txt" to "FOLDER/textfile0.txt" + And user "user0" has created a public link share with settings + | path | /FOLDER | + | permissions | change | + | password | testpass1 | + When the public accesses the preview of file "testavatar.jpg" from the last shared public link using the sharing API + Then the HTTP status code should be "404" + When the public accesses the preview of file "textfile0.txt" from the last shared public link using the sharing API + Then the HTTP status code should be "404" + + @public_link_share-feature-required + Scenario: Access to the preview of public shared file inside a folder without password + Given the administrator has enabled DAV tech_preview + And user "user0" has uploaded file "filesForUpload/testavatar.jpg" to "FOLDER/testavatar.jpg" + And user "user0" has moved file "textfile0.txt" to "FOLDER/textfile0.txt" + And user "user0" has created a public link share with settings + | path | /FOLDER | + | permissions | change | + When the public accesses the preview of file "testavatar.jpg" from the last shared public link using the sharing API + Then the HTTP status code should be "200" + When the public accesses the preview of file "textfile0.txt" from the last shared public link using the sharing API + Then the HTTP status code should be "200" diff --git a/tests/acceptance/features/bootstrap/Sharing.php b/tests/acceptance/features/bootstrap/Sharing.php index d8570968690a..70173cf48395 100644 --- a/tests/acceptance/features/bootstrap/Sharing.php +++ b/tests/acceptance/features/bootstrap/Sharing.php @@ -1958,6 +1958,35 @@ public function getLastShareToken() { return $this->lastShareData->data->token; } + /** + * Send request for preview of a file in a public link + * + * @param string $fileName + * @param string $token + * + * @return void + */ + public function getPublicPreviewOfFile($fileName, $token) { + $url = $this->getBaseUrl() . + "/index.php/apps/files_sharing/ajax/publicpreview.php" . + "?file=$fileName&t=$token"; + $resp = HttpRequestHelper::get($url); + $this->setResponse($resp); + } + + /** + * @When the public accesses the preview of file :path from the last shared public link using the sharing API + * + * @param string $path + * + * @return void + */ + public function thePublicAccessesThePreviewOfTheSharedFileUsingTheSharingApi($path) { + $shareData = $this->getLastShareData(); + $token = (string)$shareData->data->token; + $this->getPublicPreviewOfFile($path, $token); + } + /** * replace values from table *