Skip to content
Open
14 changes: 11 additions & 3 deletions apps/dav/appinfo/v2/publicremote.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
$filesDropPlugin = new FilesDropPlugin();

/** @var string $baseuri defined in public.php */
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($authBackend, $linkCheckPlugin, $filesDropPlugin) {
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($baseuri, $requestUri, $authBackend, $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
Expand All @@ -103,8 +103,16 @@
$previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false);

/** @psalm-suppress MissingClosureParamType */
Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($share) {
return new PermissionsMask(['storage' => $storage, 'mask' => $share->getPermissions() | Constants::PERMISSION_SHARE]);
Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($requestUri, $baseuri, $share) {
$mask = $share->getPermissions() | Constants::PERMISSION_SHARE;

// For chunked uploads it is necessary to have read and delete permission,
// so the temporary directory, chunks and destination file can be read and delete after the assembly.
if (str_starts_with(substr($requestUri, strlen($baseuri) - 1), '/uploads/')) {
$mask |= Constants::PERMISSION_READ | Constants::PERMISSION_DELETE;
}

return new PermissionsMask(['storage' => $storage, 'mask' => $mask]);
});

/** @psalm-suppress MissingClosureParamType */
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function getCapabilities() {
$capabilities = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
]
];
if ($this->config->getSystemValueBool('bulkupload.enabled', true)) {
Expand Down
78 changes: 44 additions & 34 deletions apps/dav/lib/Files/Sharing/FilesDropPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ public function initialize(\Sabre\DAV\Server $server): void {
}

public function onMkcol(RequestInterface $request, ResponseInterface $response) {
if ($this->isChunkedUpload($request)) {
return;
}

if (!$this->enabled || $this->share === null) {
return;
}
Expand All @@ -58,7 +62,18 @@ public function onMkcol(RequestInterface $request, ResponseInterface $response)
return false;
}

private function isChunkedUpload(RequestInterface $request): bool {
return str_starts_with(substr($request->getUrl(), strlen($request->getBaseUrl()) - 1), '/uploads/');
}

public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
$isChunkedUpload = $this->isChunkedUpload($request);

// For the PUT and MOVE requests of a chunked upload it is necessary to modify the Destination header.
if ($isChunkedUpload && $request->getMethod() !== 'MOVE' && $request->getMethod() !== 'PUT') {
return;
}

if (!$this->enabled || $this->share === null) {
return;
}
Expand All @@ -68,21 +83,25 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
return;
}

if ($request->getMethod() !== 'PUT' && $request->getMethod() !== 'MKCOL' && (!$isChunkedUpload || $request->getMethod() !== 'MOVE')) {
throw new MethodNotAllowed('Only PUT, MKCOL and MOVE are allowed on files drop');
}

// Extract the attributes for the file request
$isFileRequest = false;
$attributes = $this->share->getAttributes();
if ($attributes !== null) {
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
}

// Retrieve the nickname from the request
$nickname = $request->hasHeader('X-NC-Nickname')
? trim(urldecode($request->getHeader('X-NC-Nickname')))
: null;

if ($request->getMethod() !== 'PUT') {
// If uploading subfolders we need to ensure they get created
// within the nickname folder
if ($request->getMethod() === 'MKCOL') {
if (!$nickname) {
throw new BadRequest('A nickname header is required when uploading subfolders');
}
} else {
throw new MethodNotAllowed('Only PUT is allowed on files drop');
}
// We need a valid nickname for file requests
if ($isFileRequest && !$nickname) {
throw new BadRequest('A nickname header is required for file requests');
}

// If this is a folder creation request
Expand All @@ -95,35 +114,22 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
// full path along the way. We'll only handle conflict
// resolution on file conflicts, but not on folders.

// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
$path = $request->getPath();
if ($isChunkedUpload) {
$destination = $request->getHeader('destination');
$baseUrl = $request->getBaseUrl();
// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
$path = substr($destination, strpos($destination, $baseUrl) + strlen($baseUrl));
} else {
// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
$path = $request->getPath();
}

$token = $this->share->getToken();

// e.g files/dCP8yn3N86EK9sL
$rootPath = substr($path, 0, strpos($path, $token) + strlen($token));
// e.g /Folder/image.jpg
$relativePath = substr($path, strlen($rootPath));
$isRootUpload = substr_count($relativePath, '/') === 1;

// Extract the attributes for the file request
$isFileRequest = false;
$attributes = $this->share->getAttributes();
if ($attributes !== null) {
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
}

// We need a valid nickname for file requests
if ($isFileRequest && !$nickname) {
throw new BadRequest('A nickname header is required for file requests');
}

// We're only allowing the upload of
// long path with subfolders if a nickname is set.
// This prevents confusion when uploading files and help
// classify them by uploaders.
if (!$nickname && !$isRootUpload) {
throw new BadRequest('A nickname header is required when uploading subfolders');
}

if ($nickname) {
try {
Expand Down Expand Up @@ -187,7 +193,11 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
$relativePath = substr($folder->getPath(), strlen($node->getPath()));
$path = '/files/' . $token . '/' . $relativePath . '/' . $uniqueName;
$url = rtrim($request->getBaseUrl(), '/') . str_replace('//', '/', $path);
$request->setUrl($url);
if ($isChunkedUpload) {
$request->setHeader('destination', $url);
} else {
$request->setUrl($url);
}
}

private function getPathSegments(string $path): array {
Expand Down
6 changes: 3 additions & 3 deletions apps/dav/tests/unit/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function testGetCapabilities(): void {
$expected = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
],
];
$this->assertSame($expected, $capabilities->getCapabilities());
Expand All @@ -50,7 +50,7 @@ public function testGetCapabilitiesWithBulkUpload(): void {
$expected = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
'bulkupload' => '1.0',
],
];
Expand All @@ -71,7 +71,7 @@ public function testGetCapabilitiesWithAbsence(): void {
$expected = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
'absence-supported' => true,
'absence-replacement' => true,
],
Expand Down
40 changes: 27 additions & 13 deletions apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class FilesDropPluginTest extends TestCase {
private FilesDropPlugin $plugin;

private Folder&MockObject $node;
private IAttributes&MockObject $attributes;
private IShare&MockObject $share;
private Server&MockObject $server;
private RequestInterface&MockObject $request;
Expand All @@ -46,23 +47,16 @@ protected function setUp(): void {
$this->request = $this->createMock(RequestInterface::class);
$this->response = $this->createMock(ResponseInterface::class);

$attributes = $this->createMock(IAttributes::class);
$this->attributes = $this->createMock(IAttributes::class);
$this->share->expects($this->any())
->method('getAttributes')
->willReturn($attributes);
->willReturn($this->attributes);

$this->share
->method('getToken')
->willReturn('token');
}

public function testNotEnabled(): void {
$this->request->expects($this->never())
->method($this->anything());

$this->plugin->beforeMethod($this->request, $this->response);
}

public function testValid(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);
Expand Down Expand Up @@ -112,34 +106,54 @@ public function testFileAlreadyExistsValid(): void {
$this->plugin->beforeMethod($this->request, $this->response);
}

public function testNoMKCOLWithoutNickname(): void {
public function testFileDropMKCOLWithoutNickname(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);

$this->request->method('getMethod')
->willReturn('MKCOL');

$this->expectNotToPerformAssertions();

$this->plugin->beforeMethod($this->request, $this->response);
}

public function testFileRequestNoMKCOLWithoutNickname(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);

$this->request->method('getMethod')
->willReturn('MKCOL');

$this->attributes
->method('getAttribute')
->with('fileRequest', 'enabled')
->willReturn(true);

$this->expectException(BadRequest::class);

$this->plugin->beforeMethod($this->request, $this->response);
}

public function testMKCOLWithNickname(): void {
public function testFileRequestMKCOLWithNickname(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);

$this->request->method('getMethod')
->willReturn('MKCOL');

$this->attributes
->method('getAttribute')
->with('fileRequest', 'enabled')
->willReturn(true);

$this->request->method('hasHeader')
->with('X-NC-Nickname')
->willReturn(true);
$this->request->method('getHeader')
->with('X-NC-Nickname')
->willReturn('nickname');

$this->expectNotToPerformAssertions();

$this->plugin->beforeMethod($this->request, $this->response);
}

Expand Down
27 changes: 22 additions & 5 deletions build/integration/filesdrop_features/filesdrop.feature
Original file line number Diff line number Diff line change
Expand Up @@ -33,53 +33,70 @@ Feature: FilesDrop
And Downloading file "/drop/a (2).txt"
Then Downloaded content should be "def"

Scenario: Files drop forbid directory without a nickname
Scenario: Files request forbid directory without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When Dropping file "/folder/a.txt" with "abc"
Then the HTTP status code should be "400"

Scenario: Files drop forbid MKCOL without a nickname
Scenario: Files drop allow MKCOL without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop
Then the HTTP status code should be "201"

Scenario: Files request forbid MKCOL without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop
Then the HTTP status code should be "400"

Scenario: Files drop allows MKCOL with a nickname
Scenario: Files request allows MKCOL with a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop as "nickname"
Then the HTTP status code should be "201"

Scenario: Files drop forbid subfolder creation without a nickname
Scenario: Files request forbid subfolder creation without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When dropping file "/folder/a.txt" with "abc"
Expand Down Expand Up @@ -139,7 +156,7 @@ Feature: FilesDrop
When Downloading file "/drop/Alice/folder (2)"
Then the HTTP status code should be "200"
And Downloaded content should be "its a file"

Scenario: Put file same file multiple times via files drop
Given user "user0" exists
And As an "user0"
Expand Down
Loading