diff --git a/apps/dav/appinfo/v2/publicremote.php b/apps/dav/appinfo/v2/publicremote.php index e089fa7bb62..8d509e636a8 100644 --- a/apps/dav/appinfo/v2/publicremote.php +++ b/apps/dav/appinfo/v2/publicremote.php @@ -81,7 +81,7 @@ $linkCheckPlugin = new PublicLinkCheckPlugin(); $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 @@ -103,8 +103,16 @@ $server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, $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 */ diff --git a/apps/dav/lib/Capabilities.php b/apps/dav/lib/Capabilities.php index eb777769b61..f9bad25bf31 100644 --- a/apps/dav/lib/Capabilities.php +++ b/apps/dav/lib/Capabilities.php @@ -24,7 +24,7 @@ class Capabilities implements ICapability { $capabilities = [ 'dav' => [ 'chunking' => '1.0', - 'public_shares_chunking' => false, + 'public_shares_chunking' => true, ] ]; if ($this->config->getSystemValueBool('bulkupload.enabled', true)) { diff --git a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php index a3dbd32ce6b..606f526b18b 100644 --- a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php +++ b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php @@ -42,6 +42,10 @@ class FilesDropPlugin extends ServerPlugin { } public function onMkcol(RequestInterface $request, ResponseInterface $response) { + if ($this->isChunkedUpload($request)) { + return; + } + if (!$this->enabled || $this->share === null) { return; } @@ -58,7 +62,18 @@ class FilesDropPlugin extends ServerPlugin { 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; } @@ -68,21 +83,25 @@ class FilesDropPlugin extends ServerPlugin { 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 @@ -95,35 +114,22 @@ class FilesDropPlugin extends ServerPlugin { // 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 { @@ -187,7 +193,11 @@ class FilesDropPlugin extends ServerPlugin { $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 { diff --git a/apps/dav/tests/unit/CapabilitiesTest.php b/apps/dav/tests/unit/CapabilitiesTest.php index 873500646c2..ad70d576d48 100644 --- a/apps/dav/tests/unit/CapabilitiesTest.php +++ b/apps/dav/tests/unit/CapabilitiesTest.php @@ -30,7 +30,7 @@ class CapabilitiesTest extends TestCase { $expected = [ 'dav' => [ 'chunking' => '1.0', - 'public_shares_chunking' => false, + 'public_shares_chunking' => true, ], ]; $this->assertSame($expected, $capabilities->getCapabilities()); @@ -50,7 +50,7 @@ class CapabilitiesTest extends TestCase { $expected = [ 'dav' => [ 'chunking' => '1.0', - 'public_shares_chunking' => false, + 'public_shares_chunking' => true, 'bulkupload' => '1.0', ], ]; @@ -71,7 +71,7 @@ class CapabilitiesTest extends TestCase { $expected = [ 'dav' => [ 'chunking' => '1.0', - 'public_shares_chunking' => false, + 'public_shares_chunking' => true, 'absence-supported' => true, 'absence-replacement' => true, ], diff --git a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php index 1a7ab7179e1..a9c03e67a9c 100644 --- a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php +++ b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php @@ -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; @@ -46,23 +47,16 @@ class FilesDropPluginTest extends TestCase { $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); @@ -112,25 +106,47 @@ class FilesDropPluginTest extends TestCase { $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); @@ -138,8 +154,6 @@ class FilesDropPluginTest extends TestCase { ->with('X-NC-Nickname') ->willReturn('nickname'); - $this->expectNotToPerformAssertions(); - $this->plugin->beforeMethod($this->request, $this->response); } diff --git a/build/integration/filesdrop_features/filesdrop.feature b/build/integration/filesdrop_features/filesdrop.feature index 7618a31a1d0..52a0399f4b4 100644 --- a/build/integration/filesdrop_features/filesdrop.feature +++ b/build/integration/filesdrop_features/filesdrop.feature @@ -33,7 +33,7 @@ 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" @@ -41,12 +41,26 @@ Feature: FilesDrop | 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" @@ -54,12 +68,13 @@ Feature: FilesDrop | 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" @@ -67,12 +82,13 @@ Feature: FilesDrop | 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" @@ -80,6 +96,7 @@ Feature: FilesDrop | 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" @@ -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"