fix(dav): Public WebDAV endpoint should allow GET requests

`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 <[email protected]>

apps/dav/appinfo/v2/publicremote.php

@@ -73,11 +73,15 @@
 $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();
@@ -132,4 +136,4 @@
 $server->addPlugin($filesDropPlugin);
 
 // And off we go!
-$server->exec();
+$server->start();

build/integration/dav_features/dav-v2-public.feature

@@ -21,6 +21,42 @@ Feature: dav-v2-public
 		When Requesting share note on dav endpoint
 		Then the single response should contain a property "{http://nextcloud.org/ns}note" with value "Hello"
 
+	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
+
 	Scenario: Download a folder
 		Given using new dav path
 		And As an "admin"

build/integration/features/bootstrap/CommentsContext.php

@@ -29,8 +29,6 @@ public function __construct($baseUrl) {
 		}
 	}
 
-
-
 	/**
 	 * get a named entry from response instead of picking a random entry from values
 	 *

build/integration/features/bootstrap/Download.php

@@ -137,4 +137,18 @@ public function theDownloadedZipFileContainsAFolderNamed($folderName) {
 			'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'
+		);
+	}
 }

build/integration/features/bootstrap/FeatureContext.php

@@ -14,9 +14,14 @@
  * Features context.
  */
 class FeatureContext 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');
+	}
 }

build/integration/features/bootstrap/WebDav.php

@@ -277,6 +277,42 @@ public function downloadingFile($fileName) {
 		}
 	}
 
+	/**
+	 * @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