fix(FilesDropPlugin): Ensure all request for file request have a nickname

Signed-off-by: provokateurin <kate@provokateurin.de>

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);

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);

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);
+	}
 }

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,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 "201"
+    Then the HTTP status code should be "400"
 
-  Scenario: Files drop forbid subfolder creation without 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,6 +82,21 @@ 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 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"