From 3a242166f793d39eff938bb2322c28217c961972 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Wed, 15 Oct 2025 23:24:07 +0200 Subject: [PATCH] fix(FilesDropPlugin): Fix request method and nickname header checks Signed-off-by: provokateurin --- .../dav/lib/Files/Sharing/FilesDropPlugin.php | 22 ++++++------------- .../Files/Sharing/FilesDropPluginTest.php | 22 +------------------ .../features/bootstrap/FilesDropContext.php | 14 +----------- .../filesdrop_features/filesdrop.feature | 17 ++------------ 4 files changed, 11 insertions(+), 64 deletions(-) diff --git a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php index d74449b7d1e..930f06cb232 100644 --- a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php +++ b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php @@ -83,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 @@ -135,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'); diff --git a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php index 2b9864be8d9..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; @@ -105,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"