Skip to content

Commit

Permalink
fix(dav): Public WebDAV endpoint should allow GET requests
Browse files Browse the repository at this point in the history
`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>
  • Loading branch information
susnux committed Oct 9, 2024
1 parent 8bf6a60 commit b79d2b7
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 12 deletions.
16 changes: 10 additions & 6 deletions apps/dav/appinfo/v2/publicremote.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -132,4 +136,4 @@
$server->addPlugin($filesDropPlugin);

// And off we go!
$server->exec();
$server->start();
2 changes: 1 addition & 1 deletion build/integration/config/behat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ default:
paths:
- "%paths.base%/../dav_features"
contexts:
- FeatureContext:
- DavFeatureContext:
baseUrl: http://localhost:8080/ocs/
admin:
- admin
Expand Down
36 changes: 36 additions & 0 deletions build/integration/dav_features/dav-v2-public.feature
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,39 @@ Feature: dav-v2-public
Given using new public dav path
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
9 changes: 7 additions & 2 deletions build/integration/features/bootstrap/CommandLineContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
require __DIR__ . '/../../vendor/autoload.php';

use Behat\Behat\Context\Exception\ContextNotFoundException;
use Behat\Behat\Hook\Scope\BeforeScenarioScope;
use PHPUnit\Framework\Assert;

Expand Down Expand Up @@ -41,8 +42,12 @@ public function maintenanceModeIsDisabled() {
/** @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) {
Expand Down
2 changes: 0 additions & 2 deletions build/integration/features/bootstrap/CommentsContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ public function __construct($baseUrl) {
}
}



/**
* get a named entry from response instead of picking a random entry from values
*
Expand Down
23 changes: 23 additions & 0 deletions build/integration/features/bootstrap/DavFeatureContext.php
Original file line number Diff line number Diff line change
@@ -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');
}
}
14 changes: 14 additions & 0 deletions build/integration/features/bootstrap/Download.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,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'
);
}
}
1 change: 0 additions & 1 deletion build/integration/features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

require __DIR__ . '/../../vendor/autoload.php';


/**
* Features context.
*/
Expand Down
36 changes: 36 additions & 0 deletions build/integration/features/bootstrap/WebDav.php
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,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
Expand Down

0 comments on commit b79d2b7

Please sign in to comment.