]> source.dussan.org Git - nextcloud-server.git/commitdiff
fix(dav): Public WebDAV endpoint should allow `GET` requests backport/dav-get 48631/head
authorFerdinand Thiessen <opensource@fthiessen.de>
Tue, 8 Oct 2024 21:51:38 +0000 (23:51 +0200)
committerFerdinand Thiessen <opensource@fthiessen.de>
Wed, 9 Oct 2024 16:17:19 +0000 (18:17 +0200)
`GET` should be allowed even without Ajax header to allow downloading files,
or show files in the viewer. All other requests could be guarded, but this should not.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
apps/dav/appinfo/v2/publicremote.php
build/integration/config/behat.yml
build/integration/dav_features/dav-v2-public.feature [new file with mode: 0644]
build/integration/features/bootstrap/CommandLineContext.php
build/integration/features/bootstrap/CommentsContext.php
build/integration/features/bootstrap/DavFeatureContext.php [new file with mode: 0644]
build/integration/features/bootstrap/Download.php
build/integration/features/bootstrap/FeatureContext.php
build/integration/features/bootstrap/WebDav.php

index 0381614a328890ffc5788bb1e04618ff82803a18..c48f582ab4a379231c4b7285d2625e1990046226 100644 (file)
@@ -96,11 +96,15 @@ preg_match('/(^files\/\w+)/i', substr($requestUri, strlen($baseuri)), $match);
 $baseuri = $baseuri . $match[0];
 
 $server = $serverFactory->createServer($baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($authBackend, $linkCheckPlugin, $filesDropPlugin) {
-       $isAjax = in_array('XMLHttpRequest', explode(',', $_SERVER['HTTP_X_REQUESTED_WITH'] ?? ''));
-       $federatedShareProvider = \OCP\Server::get(FederatedShareProvider::class);
-       if ($federatedShareProvider->isOutgoingServer2serverShareEnabled() === false && !$isAjax) {
-               // this is what is thrown when trying to access a non-existing share
-               throw new NotAuthenticated();
+       // GET must be allowed for e.g. showing images and allowing Zip downloads
+       if ($server->httpRequest->getMethod() !== 'GET') {
+               // If this is *not* a GET request we only allow access to public DAV from AJAX or when Server2Server is allowed
+               $isAjax = in_array('XMLHttpRequest', explode(',', $_SERVER['HTTP_X_REQUESTED_WITH'] ?? ''));
+               $federatedShareProvider = \OCP\Server::get(FederatedShareProvider::class);
+               if ($federatedShareProvider->isOutgoingServer2serverShareEnabled() === false && $isAjax === false) {
+                       // this is what is thrown when trying to access a non-existing share
+                       throw new NotAuthenticated();
+               }
        }
 
        $share = $authBackend->getShare();
@@ -152,4 +156,4 @@ $server->addPlugin($linkCheckPlugin);
 $server->addPlugin($filesDropPlugin);
 
 // And off we go!
-$server->exec();
+$server->start();
index 8401b841cdde10c38bfa6084266da374df1ade67..896b5cd2ec1713f035f27bda1d1b93bdaf88449f 100644 (file)
@@ -58,7 +58,7 @@ default:
       paths:
         - "%paths.base%/../dav_features"
       contexts:
-        - FeatureContext:
+        - DavFeatureContext:
             baseUrl:  http://localhost:8080/ocs/
             admin:
               - admin
diff --git a/build/integration/dav_features/dav-v2-public.feature b/build/integration/dav_features/dav-v2-public.feature
new file mode 100644 (file)
index 0000000..66102f8
--- /dev/null
@@ -0,0 +1,41 @@
+# SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
+# SPDX-License-Identifier: AGPL-3.0-or-later
+Feature: dav-v2-public
+       Background:
+               Given using api version "1"
+
+       Scenario: Downloading a file from public share with Ajax header
+               Given using new dav path
+               And As an "admin"
+               And user "user0" exists
+               And user "user1" exists
+               And As an "user1"
+               And user "user1" created a folder "/testshare"
+               When User "user1" uploads file "data/green-square-256.png" to "/testshare/image.png"
+               And as "user1" creating a share with
+                 | path | testshare |
+                 | shareType | 3 |
+                 | permissions | 1 |
+               And As an "user0"
+               Given using new public dav path
+               When Downloading public file "/image.png"
+               Then the downloaded file has the content of "/testshare/image.png" from "user1" data
+
+       # Test that downloading files work to ensure e.g. the viewer works or files can be downloaded
+       Scenario: Downloading a file from public share without Ajax header and disabled s2s share
+               Given using new dav path
+               And As an "admin"
+               And user "user0" exists
+               And user "user1" exists
+               And As an "user1"
+               And user "user1" created a folder "/testshare"
+               When User "user1" uploads file "data/green-square-256.png" to "/testshare/image.png"
+               And as "user1" creating a share with
+                 | path | testshare |
+                 | shareType | 3 |
+                 | permissions | 1 |
+               And As an "user0"
+               Given parameter "outgoing_server2server_share_enabled" of app "files_sharing" is set to "no"
+               Given using new public dav path
+               When Downloading public file "/image.png" without ajax header
+               Then the downloaded file has the content of "/testshare/image.png" from "user1" data
index 8f9cfc7b45ae8373cd2ba75937227481e5ec6b5d..7be6863deaee4ccd400ff39e3766ba6fe8a6521f 100644 (file)
@@ -26,6 +26,7 @@
  */
 require __DIR__ . '/../../vendor/autoload.php';
 
+use Behat\Behat\Context\Exception\ContextNotFoundException;
 use Behat\Behat\Hook\Scope\BeforeScenarioScope;
 use PHPUnit\Framework\Assert;
 
@@ -61,8 +62,12 @@ class CommandLineContext implements \Behat\Behat\Context\Context {
        /** @BeforeScenario */
        public function gatherContexts(BeforeScenarioScope $scope) {
                $environment = $scope->getEnvironment();
-               // this should really be "WebDavContext" ...
-               $this->featureContext = $environment->getContext('FeatureContext');
+               // this should really be "WebDavContext"
+               try {
+                       $this->featureContext = $environment->getContext('FeatureContext');
+               } catch (ContextNotFoundException) {
+                       $this->featureContext = $environment->getContext('DavFeatureContext');
+               }
        }
 
        private function findLastTransferFolderForUser($sourceUser, $targetUser) {
index 0eddf972bc1b067cd83ed571514304f1f9285cc7..70fb23a2b2def6b6f5df6e92afcffbe0cd982844 100644 (file)
@@ -49,8 +49,6 @@ class CommentsContext implements \Behat\Behat\Context\Context {
                }
        }
 
-
-
        /**
         * get a named entry from response instead of picking a random entry from values
         *
diff --git a/build/integration/features/bootstrap/DavFeatureContext.php b/build/integration/features/bootstrap/DavFeatureContext.php
new file mode 100644 (file)
index 0000000..acca52c
--- /dev/null
@@ -0,0 +1,23 @@
+<?php
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+use Behat\Behat\Context\Context;
+use Behat\Behat\Context\SnippetAcceptingContext;
+
+require __DIR__ . '/../../vendor/autoload.php';
+
+class DavFeatureContext implements Context, SnippetAcceptingContext {
+       use AppConfiguration;
+       use ContactsMenu;
+       use ExternalStorage;
+       use Search;
+       use WebDav;
+       use Trashbin;
+
+       protected function resetAppConfigs() {
+               $this->deleteServerConfig('files_sharing', 'outgoing_server2server_share_enabled');
+       }
+}
index e5e6dc64853ef0a36415025adfb3645231a6a461..8b871b1cd787eb602f3d937530616ccbc58a6a72 100644 (file)
@@ -138,4 +138,18 @@ trait Download {
                        "Local header for folder did not appear once in zip file"
                );
        }
+
+       /**
+        * @Then the downloaded file has the content of :sourceFilename from :user data
+        */
+       public function theDownloadedFileHasContentOfUserFile($sourceFilename, $user) {
+               $this->getDownloadedFile();
+               $expectedFileContents = file_get_contents($this->getDataDirectory() . "/$user/files" . $sourceFilename);
+
+               // prevent the whole file from being printed in case of error.
+               Assert::assertEquals(
+                       0, strcmp($expectedFileContents, $this->downloadedFile),
+                       'Downloaded file content does not match local file content'
+               );
+       }
 }
index 27ad69857e26764755935e709c2852748073ee0b..8f49a1f14912514cb635b13cbaec3c2d4b9e45fa 100644 (file)
@@ -28,7 +28,6 @@ use Behat\Behat\Context\SnippetAcceptingContext;
 
 require __DIR__ . '/../../vendor/autoload.php';
 
-
 /**
  * Features context.
  */
index a832e26ec18a64f1621272abd3128ae3af04c130..3e63a974fba5edd60cb7ee5aa92973261bfc3af6 100644 (file)
@@ -80,6 +80,14 @@ trait WebDav {
                $this->usingOldDavPath = false;
        }
 
+       /**
+        * @Given /^using new public dav path$/
+        */
+       public function usingNewPublicDavPath() {
+               $this->davPath = 'public.php/dav';
+               $this->usingOldDavPath = false;
+       }
+
        public function getDavFilesPath($user) {
                if ($this->usingOldDavPath === true) {
                        return $this->davPath;
@@ -270,6 +278,42 @@ trait WebDav {
                }
        }
 
+       /**
+        * @When Downloading public file :filename
+        */
+       public function downloadingPublicFile(string $filename) {
+               $token = $this->lastShareData->data->token;
+               $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/dav/files/$token/$filename";
+
+               $client = new GClient();
+               $options = [
+                       'headers' => [
+                               'X-Requested-With' => 'XMLHttpRequest',
+                       ]
+               ];
+
+               try {
+                       $this->response = $client->request('GET', $fullUrl, $options);
+               } catch (\GuzzleHttp\Exception\ClientException $e) {
+                       $this->response = $e->getResponse();
+               }
+       }
+
+       /**
+        * @When Downloading public file :filename without ajax header
+        */
+       public function downloadingPublicFileWithoutHeader(string $filename) {
+               $token = $this->lastShareData->data->token;
+               $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/dav/files/$token/$filename";
+
+               $client = new GClient();
+               try {
+                       $this->response = $client->request('GET', $fullUrl);
+               } catch (\GuzzleHttp\Exception\ClientException $e) {
+                       $this->response = $e->getResponse();
+               }
+       }
+
        /**
         * @Then Downloaded content should start with :start
         * @param int $start