Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions apps/dav/appinfo/v2/publicremote.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,14 @@
$filesDropPlugin = new FilesDropPlugin();

/** @var string $baseuri defined in public.php */
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($baseuri, $requestUri, $authBackend, $bearerAuthBackend, $linkCheckPlugin, $filesDropPlugin) {
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($baseuri, $requestUri, $request, $authBackend, $bearerAuthBackend, $linkCheckPlugin, $filesDropPlugin) {
// 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
// If this is *not* a GET request we only allow access to public DAV from AJAX,
// from a form with a valid request token (e.g. Zip downloads via POST) or when Server2Server is allowed
$isAjax = in_array('XMLHttpRequest', explode(',', $_SERVER['HTTP_X_REQUESTED_WITH'] ?? ''), true);
$federatedShareProvider = Server::get(FederatedShareProvider::class);
if ($federatedShareProvider->isOutgoingServer2serverShareEnabled() === false && $isAjax === false) {
if ($federatedShareProvider->isOutgoingServer2serverShareEnabled() === false && $isAjax === false && !$request->passesCSRFCheck()) {
// this is what is thrown when trying to access a non-existing share
throw new NotAuthenticated();
}
Expand Down
26 changes: 20 additions & 6 deletions apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,10 @@ public function __construct(
public function initialize(Server $server): void {
$this->server = $server;
$this->server->on('method:GET', $this->handleDownload(...), 100);
$this->server->on('method:POST', $this->handleDownload(...), 100);
Comment thread
CarlSchwan marked this conversation as resolved.
// low priority to give any other afterMethod:* a chance to fire before we cancel everything
$this->server->on('afterMethod:GET', $this->afterDownload(...), 999);
$this->server->on('afterMethod:POST', $this->afterDownload(...), 999);
}

/**
Expand Down Expand Up @@ -97,8 +99,9 @@ protected function streamNode(Streamer $streamer, NcNode $node, string $rootPath
/**
* Download a folder as an archive.
* It is possible to filter / limit the files that should be downloaded,
* either by passing (multiple) `X-NC-Files: the-file` headers
* or by setting a `files=JSON_ARRAY_OF_FILES` URL query.
* either by passing (multiple) `X-NC-Files: the-file` headers,
* by setting a `files=JSON_ARRAY_OF_FILES` URL query,
* or by sending a POST request with `files` in the request body.
*/
public function handleDownload(Request $request, Response $response): ?false {
if ($request->getHeader('X-Sabre-Original-Method') === 'HEAD') {
Expand All @@ -111,10 +114,16 @@ public function handleDownload(Request $request, Response $response): ?false {
}

$query = $request->getQueryParameters();
// POST requests pass the parameters in the request body,
// as other plugins do not support query parameters on POST requests
$params = $request->getMethod() === 'POST' ? $request->getPostData() : $query;

// Get accept header - or if set overwrite with accept GET-param
// Get accept header - or if set overwrite with accept parameter
$accept = $request->getHeaderAsArray('Accept');
$acceptParam = $query['accept'] ?? '';
$acceptParam = $params['accept'] ?? '';
if (!is_string($acceptParam)) {
return null;
}
if ($acceptParam !== '') {
$accept = array_map(fn (string $name) => strtolower(trim($name)), explode(',', $acceptParam));
}
Expand All @@ -126,8 +135,13 @@ public function handleDownload(Request $request, Response $response): ?false {
}

$files = $request->getHeaderAsArray('X-NC-Files');
$filesParam = $query['files'] ?? '';
// The preferred way would be headers, but this is not possible for simple browser requests ("links")
$filesParam = $params['files'] ?? '';
if (!is_string($filesParam)) {
$this->logger->notice('Invalid files filter parameter for ZipFolderPlugin', ['filter' => $filesParam]);
return null;
}

// The preferred way would be POST or headers, but this is not possible for simple browser requests ("links")
// so we also need to support GET parameters
if ($filesParam !== '') {
$files = json_decode($filesParam);
Expand Down
28 changes: 25 additions & 3 deletions apps/files/src/actions/downloadAction.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,23 @@ describe('Download action execute tests', () => {
root: '/files/admin',
})

const form = {
style: {},
appendChild: vi.fn(),
submit: vi.fn(),
remove: vi.fn(),
} as unknown as HTMLFormElement
const inputs: HTMLInputElement[] = []
vi.spyOn(document, 'createElement').mockImplementation((tagName: string) => {
if (tagName === 'form') {
return form
}
const input = {} as HTMLInputElement
inputs.push(input)
return input
})
vi.spyOn(document.body, 'appendChild').mockImplementation((node) => node)

const exec = await action.execBatch!({
nodes: [file1, file2],
view,
Expand All @@ -266,9 +283,14 @@ describe('Download action execute tests', () => {

// Silent action
expect(exec).toStrictEqual([null, null])
expect(link.download).toEqual('')
expect(link.href).toMatch('https://cloud.domain.com/remote.php/dav/files/admin/Dir/?accept=zip&files=%5B%22foo.txt%22%2C%22bar.txt%22%5D')
expect(link.click).toHaveBeenCalledTimes(1)
// The file list is sent in the request body to avoid too long URLs
expect(form.method).toBe('POST')
expect(form.action).toBe('https://cloud.domain.com/remote.php/dav/files/admin/Dir/')
expect(inputs).toContainEqual(expect.objectContaining({ type: 'hidden', name: 'accept', value: 'zip' }))
expect(inputs).toContainEqual(expect.objectContaining({ type: 'hidden', name: 'files', value: '["foo.txt","bar.txt"]' }))
expect(inputs).toContainEqual(expect.objectContaining({ type: 'hidden', name: 'requesttoken' }))
expect(form.submit).toHaveBeenCalledTimes(1)
expect(form.remove).toHaveBeenCalledTimes(1)
})

test('Download fails with error', async () => {
Expand Down
37 changes: 36 additions & 1 deletion apps/files/src/actions/downloadAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import type { IFileAction, INode, IView } from '@nextcloud/files'

import ArrowDownSvg from '@mdi/svg/svg/arrow-down.svg?raw'
import { getRequestToken } from '@nextcloud/auth'
import axios from '@nextcloud/axios'
import { showError } from '@nextcloud/dialogs'
import { emit } from '@nextcloud/event-bus'
Expand Down Expand Up @@ -142,7 +143,41 @@ async function downloadNodes(nodes: INode[]) {
// The URL contains the path encoded so we need to decode as the query.append will re-encode it
const filenames = nodes.map((node) => decodeURIComponent(node.encodedSource.slice(url.href.length + 1)))
url.searchParams.append('accept', 'zip')
url.searchParams.append('files', JSON.stringify(filenames))

if (url.pathname.at(-1) !== '/') {
url.pathname = `${url.pathname}/`
}

// try to see if the resource is still available
await axios.head(url.href)

// Send the file list in the request body to avoid too long URLs.
// A native form submission lets the browser stream the archive to disk.
const form = document.createElement('form')
form.method = 'POST'

@pringelmann pringelmann Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally: zipping a folder that contains a view-only share returns a 403 here. With a form submit, wouldn't the browser end up on that error page instead of staying in Files?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit tricky, I will try to figure out a solution

// Pass the parameters in the body as POST requests with query parameters are not supported by all DAV plugins
url.search = ''
form.action = url.href
form.style.display = 'none'
const fields = {
accept: 'zip',
files: JSON.stringify(filenames),
requesttoken: getRequestToken() ?? '',
}
for (const [name, value] of Object.entries(fields)) {
const input = document.createElement('input')
input.type = 'hidden'
input.name = name
input.value = value
form.appendChild(input)
}
document.body.appendChild(form)
try {
form.submit()
Comment thread
CarlSchwan marked this conversation as resolved.
} finally {
form.remove()
}
return
}

if (url.pathname.at(-1) !== '/') {
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 @@ -57,6 +57,42 @@ Feature: dav-v2-public
When Downloading public file "/image.png" without ajax header
Then the downloaded file has the content of "/testshare/image.png" from "user1" data

Scenario: Downloading a zip file from public share using POST with requesttoken and disabled s2s share
Given using new dav path
And As an "admin"
And user "user1" exists
And As an "user1"
And user "user1" created a folder "/testshare"
And User "user1" uploads file "data/green-square-256.png" to "/testshare/image.png"
And User "user1" uploads file with content "text" to "/testshare/text.txt"
And as "user1" creating a share with
| path | testshare |
| shareType | 3 |
| permissions | 1 |
And parameter "outgoing_server2server_share_enabled" of app "files_sharing" is set to "no"
Given using new public dav path
When Downloading public zip file for entries '"image.png","text.txt"' using POST with requesttoken
Then the HTTP status code should be "200"
And the downloaded file is a zip file
And the downloaded zip file contains a file named "image.png" with the contents of "/testshare/image.png" from "user1" data
And the downloaded zip file contains a file named "text.txt" with the contents of "/testshare/text.txt" from "user1" data

Scenario: Downloading a zip file from public share using POST without requesttoken and disabled s2s share
Given using new dav path
And As an "admin"
And user "user1" exists
And As an "user1"
And user "user1" created a folder "/testshare"
And User "user1" uploads file with content "text" to "/testshare/text.txt"
And as "user1" creating a share with
| path | testshare |
| shareType | 3 |
| permissions | 1 |
And parameter "outgoing_server2server_share_enabled" of app "files_sharing" is set to "no"
Given using new public dav path
When Downloading public zip file for entries '"text.txt"' using POST
Then the HTTP status code should be "401"

Scenario: Finalizing a public chunked upload with COPY is not allowed
Given using new dav path
And user "user0" exists
Expand Down
14 changes: 14 additions & 0 deletions build/integration/features/bootstrap/Download.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
use Behat\Gherkin\Node\TableNode;
use PHPUnit\Framework\Assert;
use Psr\Http\Message\StreamInterface;

Expand All @@ -28,6 +29,19 @@ public function userDownloadsZipFileForEntriesInFolder($user, $entries, $folder)
$this->theHTTPStatusCodeShouldBe('200');
}

/**
* @When user :user downloads zip file for entries :entries in folder :folder using POST
*/
public function userDownloadsZipFileForEntriesInFolderUsingPost(string $user, string $entries, string $folder): void {
$folder = trim($folder, '/');
$this->asAn($user);
$this->sendingToWithDirectUrl('POST', "/remote.php/dav/files/$user/$folder", new TableNode([
['accept', 'zip'],
['files', '[' . $entries . ']'],
]));
$this->theHTTPStatusCodeShouldBe('200');
}

private function getDownloadedFile() {
$this->downloadedFile = '';

Expand Down
42 changes: 42 additions & 0 deletions build/integration/features/bootstrap/WebDav.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,48 @@ public function downloadingPublicFileWithoutHeader(string $filename) {
}
}

/**
* @When Downloading public zip file for entries :entries using POST
*/
public function downloadingPublicZipFileUsingPost(string $entries): void {
$this->sendPublicZipDownloadPost($entries, []);
}

/**
* @When Downloading public zip file for entries :entries using POST with requesttoken
*/
public function downloadingPublicZipFileUsingPostWithRequestToken(string $entries): void {
$token = $this->lastShareData->data->token;
$client = new GClient();
$response = $client->get(substr($this->baseUrl, 0, -4) . "index.php/s/$token", [
'cookies' => $this->cookieJar,
]);
$this->extracRequestTokenFromResponse($response);

$this->sendPublicZipDownloadPost($entries, [
'requesttoken' => $this->requestToken,
]);
}

private function sendPublicZipDownloadPost(string $entries, array $formParams): void {
$token = $this->lastShareData->data->token;
$fullUrl = substr($this->baseUrl, 0, -4) . "public.php/dav/files/$token/";

$client = new GClient();
try {
$this->response = $client->request('POST', $fullUrl, [
'cookies' => $this->cookieJar,
'form_params' => [
'accept' => 'zip',
'files' => '[' . $entries . ']',
...$formParams,
],
]);
} catch (\GuzzleHttp\Exception\ClientException $e) {
$this->response = $e->getResponse();
}
}

/**
* @Then Downloaded content should start with :start
* @param int $start
Expand Down
22 changes: 22 additions & 0 deletions build/integration/files_features/download.feature
Original file line number Diff line number Diff line change
Expand Up @@ -294,3 +294,25 @@ Feature: download
And the downloaded zip file contains a file named "crowdedFolder/subFolder7/test.txt-5524" with the contents of "/crowdedFolder/subFolder7/test.txt-5524" from "user0" data
And the downloaded zip file contains a folder named "crowdedFolder/subFolder7/subSubFolder/emptySubSubSubFolder/"
And the downloaded zip file contains a folder named "crowdedFolder/subFolder7/emptySubSubFolder/"

Scenario: downloading 2 small files using POST
Given using new dav path
And user "user0" exists
And User "user0" copies file "/welcome.txt" to "/welcome2.txt"
When user "user0" downloads zip file for entries '"welcome.txt","welcome2.txt"' in folder "/" using POST
Then the downloaded file is a zip file
And the downloaded zip file contains a file named "welcome.txt" with the contents of "/welcome.txt" from "user0" data
And the downloaded zip file contains a file named "welcome2.txt" with the contents of "/welcome2.txt" from "user0" data

Scenario: downloading (from folder) a small file and 2 nested directories using POST
Given using new dav path
And user "user0" exists
And user "user0" created a folder "/baseFolder"
And User "user0" copies file "/welcome.txt" to "/baseFolder/welcome.txt"
And user "user0" created a folder "/baseFolder/subFolder"
And user "user0" created a folder "/baseFolder/subFolder/emptySubSubFolder"
When user "user0" downloads zip file for entries '"welcome.txt","subFolder"' in folder "/baseFolder/" using POST
Then the downloaded file is a zip file
And the downloaded zip file contains a file named "welcome.txt" with the contents of "/baseFolder/welcome.txt" from "user0" data
And the downloaded zip file contains a folder named "subFolder/"
And the downloaded zip file contains a folder named "subFolder/emptySubSubFolder/"
Loading