fix(FilesDropPlugin): Fix request method and nickname header checks

Signed-off-by: provokateurin <[email protected]>

Author: provokateurin

apps/dav/lib/Files/Sharing/FilesDropPlugin.php

@@ -83,21 +83,8 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
 			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 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
 			$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');

apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php

@@ -13,7 +13,6 @@
 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 @@ public function testFileAlreadyExistsValid(): void {
 		$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);

build/integration/features/bootstrap/FilesDropContext.php

@@ -57,7 +57,7 @@ public function droppingFileWithAs($path, $content, $nickname) {
 	/**
 	 * @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 @@ public function creatingFolderInDrop($folder, $nickname = null) {
 			'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

@@ -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"