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/Files/Sharing/FilesDropPlugin.php b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php index a3dbd32ce6b..930f06cb232 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 final MOVE request of a chunked upload it is necessary to modify the Destination header. + if ($isChunkedUpload && $request->getMethod() !== 'MOVE') { + return; + } + if (!$this->enabled || $this->share === null) { return; } @@ -68,21 +83,8 @@ class FilesDropPlugin extends ServerPlugin { return; } - // 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'); - } + if ($request->getMethod() !== 'PUT' && $request->getMethod() !== 'MKCOL' && (!$isChunkedUpload || $request->getMethod() !== 'MOVE')) { + throw new MethodNotAllowed('Only PUT, MKCOL and MOVE are allowed on files drop'); } // If this is a folder creation request @@ -95,8 +97,16 @@ 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 @@ -112,6 +122,11 @@ class FilesDropPlugin extends ServerPlugin { $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; + // We need a valid nickname for file requests if ($isFileRequest && !$nickname) { throw new BadRequest('A nickname header is required for file requests'); @@ -187,7 +202,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/Files/Sharing/FilesDropPluginTest.php b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php index 1a7ab7179e1..57e4b874fd6 100644 --- a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php +++ b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php @@ -13,7 +13,6 @@ use OCP\Files\NotFoundException; use OCP\Share\IAttributes; use OCP\Share\IShare; use PHPUnit\Framework\MockObject\MockObject; -use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Server; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; @@ -56,13 +55,6 @@ class FilesDropPluginTest extends TestCase { ->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,32 +104,13 @@ class FilesDropPluginTest extends TestCase { $this->plugin->beforeMethod($this->request, $this->response); } - public function testNoMKCOLWithoutNickname(): void { + public function testMKCOL(): void { $this->plugin->enable(); $this->plugin->setShare($this->share); $this->request->method('getMethod') ->willReturn('MKCOL'); - $this->expectException(BadRequest::class); - - $this->plugin->beforeMethod($this->request, $this->response); - } - - public function testMKCOLWithNickname(): void { - $this->plugin->enable(); - $this->plugin->setShare($this->share); - - $this->request->method('getMethod') - ->willReturn('MKCOL'); - - $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); diff --git a/build/integration/features/bootstrap/FilesDropContext.php b/build/integration/features/bootstrap/FilesDropContext.php index 0c437f28a72..af04503d7a1 100644 --- a/build/integration/features/bootstrap/FilesDropContext.php +++ b/build/integration/features/bootstrap/FilesDropContext.php @@ -57,7 +57,7 @@ class FilesDropContext implements Context, SnippetAcceptingContext { /** * @When Creating folder :folder in drop */ - public function creatingFolderInDrop($folder, $nickname = null) { + public function creatingFolderInDrop($folder) { $client = new Client(); $options = []; if (count($this->lastShareData->data->element) > 0) { @@ -73,22 +73,10 @@ class FilesDropContext implements Context, SnippetAcceptingContext { 'X-REQUESTED-WITH' => 'XMLHttpRequest', ]; - if ($nickname) { - $options['headers']['X-NC-NICKNAME'] = $nickname; - } - try { $this->response = $client->request('MKCOL', $fullUrl, $options); } catch (\GuzzleHttp\Exception\ClientException $e) { $this->response = $e->getResponse(); } } - - - /** - * @When Creating folder :folder in drop as :nickName - */ - public function creatingFolderInDropWithNickname($folder, $nickname) { - return $this->creatingFolderInDrop($folder, $nickname); - } } diff --git a/build/integration/filesdrop_features/filesdrop.feature b/build/integration/filesdrop_features/filesdrop.feature index 7618a31a1d0..d8a1464fa7d 100644 --- a/build/integration/filesdrop_features/filesdrop.feature +++ b/build/integration/filesdrop_features/filesdrop.feature @@ -46,7 +46,7 @@ Feature: FilesDrop 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 allows MKCOL Given user "user0" exists And As an "user0" And user "user0" created a folder "/drop" @@ -57,19 +57,6 @@ Feature: FilesDrop 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 - 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 as "nickname" Then the HTTP status code should be "201" Scenario: Files drop forbid subfolder creation without a nickname @@ -139,7 +126,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"