From b737336e4d26889d64b478d4a11651e2f2f900c1 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Thu, 16 Oct 2025 12:27:08 +0200 Subject: [PATCH] fix(FilesDropPlugin): Ensure all request for file request have a nickname Signed-off-by: provokateurin --- .../dav/lib/Files/Sharing/FilesDropPlugin.php | 34 +++++++------- .../Files/Sharing/FilesDropPluginTest.php | 47 +++++++++++++++++-- .../features/bootstrap/FilesDropContext.php | 14 +++++- .../filesdrop_features/filesdrop.feature | 36 ++++++++++++-- 4 files changed, 107 insertions(+), 24 deletions(-) diff --git a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php index d414a337820..606f526b18b 100644 --- a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php +++ b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php @@ -87,6 +87,23 @@ class FilesDropPlugin extends ServerPlugin { 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; + + // 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 // let's stop there and let the onMkcol handle it if ($request->getMethod() === 'MKCOL') { @@ -114,23 +131,6 @@ class FilesDropPlugin extends ServerPlugin { // e.g /Folder/image.jpg $relativePath = substr($path, strlen($rootPath)); - // 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; - - // We need a valid nickname for file requests - if ($isFileRequest && !$nickname) { - throw new BadRequest('A nickname header is required for file requests'); - } - if ($nickname) { try { $node->verifyPath($nickname); diff --git a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php index 57e4b874fd6..a9c03e67a9c 100644 --- a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php +++ b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php @@ -13,6 +13,7 @@ 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; @@ -23,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; @@ -45,10 +47,10 @@ 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') @@ -104,7 +106,7 @@ class FilesDropPluginTest extends TestCase { $this->plugin->beforeMethod($this->request, $this->response); } - public function testMKCOL(): void { + public function testFileDropMKCOLWithoutNickname(): void { $this->plugin->enable(); $this->plugin->setShare($this->share); @@ -116,6 +118,45 @@ class FilesDropPluginTest extends TestCase { $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 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->plugin->beforeMethod($this->request, $this->response); + } + public function testSubdirPut(): void { $this->plugin->enable(); $this->plugin->setShare($this->share); diff --git a/build/integration/features/bootstrap/FilesDropContext.php b/build/integration/features/bootstrap/FilesDropContext.php index af04503d7a1..0c437f28a72 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) { + public function creatingFolderInDrop($folder, $nickname = null) { $client = new Client(); $options = []; if (count($this->lastShareData->data->element) > 0) { @@ -73,10 +73,22 @@ 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 d8a1464fa7d..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 allows MKCOL +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,27 @@ 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 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" @@ -67,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"