Merge pull request #52785 from nextcloud/feat/file-drop-recursive

7 files changed, 305 insertions, 66 deletions

diff --git a/apps/dav/lib/Connector/Sabre/ChecksumUpdatePlugin.php b/apps/dav/lib/Connector/Sabre/ChecksumUpdatePlugin.php
index 64a61a43a9b..18009080585 100644
--- a/apps/dav/lib/Connector/Sabre/ChecksumUpdatePlugin.php
+++ b/apps/dav/lib/Connector/Sabre/ChecksumUpdatePlugin.php
@@ -27,20 +27,6 @@ class ChecksumUpdatePlugin extends ServerPlugin {
 	}
 
 	/** @return string[] */
-	public function getHTTPMethods($path): array {
-		$tree = $this->server->tree;
-
-		if ($tree->nodeExists($path)) {
-			$node = $tree->getNodeForPath($path);
-			if ($node instanceof File) {
-				return ['PATCH'];
-			}
-		}
-
-		return [];
-	}
-
-	/** @return string[] */
 	public function getFeatures(): array {
 		return ['nextcloud-checksum-update'];
 	}
diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php
index 3ebaa55786c..fe09c3f423f 100644
--- a/apps/dav/lib/Connector/Sabre/Directory.php
+++ b/apps/dav/lib/Connector/Sabre/Directory.php
@@ -176,13 +176,14 @@ class Directory extends Node implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuot
 	public function getChild($name, $info = null, ?IRequest $request = null, ?IL10N $l10n = null) {
 		$storage = $this->info->getStorage();
 		$allowDirectory = false;
+
+		// Checking if we're in a file drop
+		// If we are, then only PUT and MKCOL are allowed (see plugin)
+		// so we are safe to return the directory without a risk of
+		// leaking files and folders structure.
 		if ($storage instanceof PublicShareWrapper) {
 			$share = $storage->getShare();
-			$allowDirectory =
-				// Only allow directories for file drops
-				($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ &&
-				// And only allow it for directories which are a direct child of the share root
-				$this->info->getId() === $share->getNodeId();
+			$allowDirectory = ($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ;
 		}
 
 		// For file drop we need to be allowed to read the directory with the nickname
diff --git a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php
index ccf7cd28f4a..ad7648795da 100644
--- a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php
+++ b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php
@@ -36,57 +36,136 @@ class FilesDropPlugin extends ServerPlugin {
 
 	/**
 	 * This initializes the plugin.
-	 *
-	 * @param \Sabre\DAV\Server $server Sabre server
-	 *
-	 * @return void
-	 * @throws MethodNotAllowed
+	 * It is ONLY initialized by the server on a file drop request.
 	 */
 	public function initialize(\Sabre\DAV\Server $server): void {
 		$server->on('beforeMethod:*', [$this, 'beforeMethod'], 999);
+		$server->on('method:MKCOL', [$this, 'onMkcol']);
 		$this->enabled = false;
 	}
 
-	public function beforeMethod(RequestInterface $request, ResponseInterface $response): void {
+	public function onMkcol(RequestInterface $request, ResponseInterface $response) {
 		if (!$this->enabled || $this->share === null || $this->view === null) {
 			return;
 		}
 
-		// Only allow file drop
+		// If this is a folder creation request we need
+		// to fake a success so we can pretend every
+		// folder now exists.
+		$response->setStatus(201);
+		return false;
+	}
+
+	public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
+		if (!$this->enabled || $this->share === null || $this->view === null) {
+			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') {
-			throw new MethodNotAllowed('Only PUT is allowed on files drop');
+			// If uploading subfolders we need to ensure they get created
+			// within the nickname folder
+			if ($request->getMethod() === 'MKCOL') {
+				if (!$nickname) {
+					throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
+				}
+			} else {
+				throw new MethodNotAllowed('Only PUT is allowed on files drop');
+			}
+		}
+
+		// If this is a folder creation request
+		// let's stop there and let the onMkcol handle it
+		if ($request->getMethod() === 'MKCOL') {
+			return;
 		}
 
-		// Always upload at the root level
-		$path = explode('/', $request->getPath());
-		$path = array_pop($path);
+		// Now if we create a file, we need to create the
+		// 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();
+		$token = $this->share->getToken();
+
+		// e.g files/dCP8yn3N86EK9sL
+		$rootPath = substr($path, 0, strpos($path, $token) + strlen($token));
+		// e.g /Folder/image.jpg
+		$relativePath = substr($path, strlen($rootPath));
+		$isRootUpload = substr_count($relativePath, '/') === 1;
 
 		// Extract the attributes for the file request
 		$isFileRequest = false;
 		$attributes = $this->share->getAttributes();
-		$nickName = $request->hasHeader('X-NC-Nickname') ? urldecode($request->getHeader('X-NC-Nickname')) : null;
 		if ($attributes !== null) {
 			$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
 		}
 
 		// We need a valid nickname for file requests
-		if ($isFileRequest && ($nickName == null || trim($nickName) === '')) {
-			throw new MethodNotAllowed('Nickname is required for file requests');
+		if ($isFileRequest && !$nickname) {
+			throw new MethodNotAllowed('A nickname header is required for file requests');
 		}
 
-		// If this is a file request we need to create a folder for the user
-		if ($isFileRequest) {
-			// Check if the folder already exists
-			if (!($this->view->file_exists($nickName) === true)) {
-				$this->view->mkdir($nickName);
-			}
+		// We're only allowing the upload of
+		// long path with subfolders if a nickname is set.
+		// This prevents confusion when uploading files and help
+		// classify them by uploaders.
+		if (!$nickname && !$isRootUpload) {
+			throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
+		}
+
+		// If we have a nickname, let's put everything inside
+		if ($nickname) {
 			// Put all files in the subfolder
-			$path = $nickName . '/' . $path;
+			$relativePath = '/' . $nickname . '/' . $relativePath;
+			$relativePath = str_replace('//', '/', $relativePath);
 		}
 
-		$newName = \OC_Helper::buildNotExistingFileNameForView('/', $path, $this->view);
-		$url = $request->getBaseUrl() . '/files/' . $this->share->getToken() . $newName;
+		// Create the folders along the way
+		$folders = $this->getPathSegments(dirname($relativePath));
+		foreach ($folders as $folder) {
+			if ($folder === '') {
+				continue;
+			} // skip empty parts
+			if (!$this->view->file_exists($folder)) {
+				$this->view->mkdir($folder);
+			}
+		}
+
+		// Finally handle conflicts on the end files
+		$noConflictPath = \OC_Helper::buildNotExistingFileNameForView(dirname($relativePath), basename($relativePath), $this->view);
+		$path = '/files/' . $token . '/' . $noConflictPath;
+		$url = $request->getBaseUrl() . str_replace('//', '/', $path);
 		$request->setUrl($url);
 	}
 
+	private function getPathSegments(string $path): array {
+		// Normalize slashes and remove trailing slash
+		$path = rtrim(str_replace('\\', '/', $path), '/');
+
+		// Handle absolute paths starting with /
+		$isAbsolute = str_starts_with($path, '/');
+
+		$segments = explode('/', $path);
+
+		// Add back the leading slash for the first segment if needed
+		$result = [];
+		$current = $isAbsolute ? '/' : '';
+
+		foreach ($segments as $segment) {
+			if ($segment === '') {
+				// skip empty parts
+				continue;
+			}
+			$current = rtrim($current, '/') . '/' . $segment;
+			$result[] = $current;
+		}
+
+		return $result;
+	}
 }
diff --git a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php
index ce469a1c749..dc1e067dafd 100644
--- a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php
+++ b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php
@@ -46,9 +46,6 @@ class FilesDropPluginTest extends TestCase {
 		$this->request = $this->createMock(RequestInterface::class);
 		$this->response = $this->createMock(ResponseInterface::class);
 
-		$this->response->expects($this->never())
-			->method($this->anything());
-
 		$attributes = $this->createMock(IAttributes::class);
 		$this->share->expects($this->any())
 			->method('getAttributes')
@@ -60,13 +57,19 @@ class FilesDropPluginTest extends TestCase {
 	}
 
 	public function testInitialize(): void {
-		$this->server->expects($this->once())
+		$this->server->expects($this->at(0))
 			->method('on')
 			->with(
 				$this->equalTo('beforeMethod:*'),
 				$this->equalTo([$this->plugin, 'beforeMethod']),
 				$this->equalTo(999)
 			);
+		$this->server->expects($this->at(1))
+			->method('on')
+			->with(
+				$this->equalTo('method:MKCOL'),
+				$this->equalTo([$this->plugin, 'onMkcol']),
+			);
 
 		$this->plugin->initialize($this->server);
 	}
@@ -136,7 +139,7 @@ class FilesDropPluginTest extends TestCase {
 		$this->plugin->beforeMethod($this->request, $this->response);
 	}
 
-	public function testNoMKCOL(): void {
+	public function testNoMKCOLWithoutNickname(): void {
 		$this->plugin->enable();
 		$this->plugin->setView($this->view);
 		$this->plugin->setShare($this->share);
@@ -149,7 +152,27 @@ class FilesDropPluginTest extends TestCase {
 		$this->plugin->beforeMethod($this->request, $this->response);
 	}
 
-	public function testNoSubdirPut(): void {
+	public function testMKCOLWithNickname(): void {
+		$this->plugin->enable();
+		$this->plugin->setView($this->view);
+		$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);
+	}
+
+	public function testSubdirPut(): void {
 		$this->plugin->enable();
 		$this->plugin->setView($this->view);
 		$this->plugin->setShare($this->share);
@@ -157,6 +180,13 @@ class FilesDropPluginTest extends TestCase {
 		$this->request->method('getMethod')
 			->willReturn('PUT');
 
+		$this->request->method('hasHeader')
+			->with('X-NC-Nickname')
+			->willReturn(true);
+		$this->request->method('getHeader')
+			->with('X-NC-Nickname')
+			->willReturn('nickname');
+
 		$this->request->method('getPath')
 			->willReturn('/files/token/folder/file.txt');
 
@@ -165,7 +195,7 @@ class FilesDropPluginTest extends TestCase {
 
 		$this->view->method('file_exists')
 			->willReturnCallback(function ($path) {
-				if ($path === 'file.txt' || $path === '/file.txt') {
+				if ($path === 'file.txt' || $path === '/folder/file.txt') {
 					return true;
 				} else {
 					return false;
@@ -174,8 +204,70 @@ class FilesDropPluginTest extends TestCase {
 
 		$this->request->expects($this->once())
 			->method('setUrl')
-			->with($this->equalTo('https://example.com/files/token/file (2).txt'));
+			->with($this->equalTo('https://example.com/files/token/nickname/folder/file.txt'));
 
 		$this->plugin->beforeMethod($this->request, $this->response);
 	}
+
+	public function testRecursiveFolderCreation(): void {
+		$this->plugin->enable();
+		$this->plugin->setView($this->view);
+		$this->plugin->setShare($this->share);
+
+		$this->request->method('getMethod')
+			->willReturn('PUT');
+		$this->request->method('hasHeader')
+			->with('X-NC-Nickname')
+			->willReturn(true);
+		$this->request->method('getHeader')
+			->with('X-NC-Nickname')
+			->willReturn('nickname');
+		
+		$this->request->method('getPath')
+			->willReturn('/files/token/folder/subfolder/file.txt');
+		$this->request->method('getBaseUrl')
+			->willReturn('https://example.com');
+		$this->view->method('file_exists')
+			->willReturn(false);
+
+		$this->view->expects($this->exactly(4))
+			->method('file_exists')
+			->withConsecutive(
+				['/nickname'],
+				['/nickname/folder'],
+				['/nickname/folder/subfolder'],
+				['/nickname/folder/subfolder/file.txt']
+			)
+			->willReturnOnConsecutiveCalls(
+				false,
+				false,
+				false,
+				false,
+			);
+		$this->view->expects($this->exactly(3))
+			->method('mkdir')
+			->withConsecutive(
+				['/nickname'],
+				['/nickname/folder'],
+				['/nickname/folder/subfolder'],
+			);
+
+		$this->request->expects($this->once())
+			->method('setUrl')
+			->with($this->equalTo('https://example.com/files/token/nickname/folder/subfolder/file.txt'));
+		$this->plugin->beforeMethod($this->request, $this->response);
+	}
+
+	public function testOnMkcol(): void {
+		$this->plugin->enable();
+		$this->plugin->setView($this->view);
+		$this->plugin->setShare($this->share);
+
+		$this->response->expects($this->once())
+			->method('setStatus')
+			->with(201);
+
+		$response = $this->plugin->onMkcol($this->request, $this->response);
+		$this->assertFalse($response);
+	}
 }
diff --git a/build/integration/features/bootstrap/FilesDropContext.php b/build/integration/features/bootstrap/FilesDropContext.php
index 1b9d598645f..00c2cd346df 100644
--- a/build/integration/features/bootstrap/FilesDropContext.php
+++ b/build/integration/features/bootstrap/FilesDropContext.php
@@ -15,7 +15,7 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @When Dropping file :path with :content
 	 */
-	public function droppingFileWith($path, $content, $nickName = null) {
+	public function droppingFileWith($path, $content, $nickname = null) {
 		$client = new Client();
 		$options = [];
 		if (count($this->lastShareData->data->element) > 0) {
@@ -28,11 +28,11 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
 		$fullUrl = str_replace('//', '/', $base . "/public.php/dav/files/$token/$path");
 
 		$options['headers'] = [
-			'X-REQUESTED-WITH' => 'XMLHttpRequest'
+			'X-REQUESTED-WITH' => 'XMLHttpRequest',
 		];
 
-		if ($nickName) {
-			$options['headers']['X-NC-NICKNAME'] = $nickName;
+		if ($nickname) {
+			$options['headers']['X-NC-NICKNAME'] = $nickname;
 		}
 
 		$options['body'] = \GuzzleHttp\Psr7\Utils::streamFor($content);
@@ -48,15 +48,15 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @When Dropping file :path with :content as :nickName
 	 */
-	public function droppingFileWithAs($path, $content, $nickName) {
-		$this->droppingFileWith($path, $content, $nickName);
+	public function droppingFileWithAs($path, $content, $nickname) {
+		$this->droppingFileWith($path, $content, $nickname);
 	}
 
 
 	/**
 	 * @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) {
@@ -69,13 +69,25 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
 		$fullUrl = str_replace('//', '/', $base . "/public.php/dav/files/$token/$folder");
 
 		$options['headers'] = [
-			'X-REQUESTED-WITH' => 'XMLHttpRequest'
+			'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/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php
index e71502b6b0c..e9e08844767 100644
--- a/build/integration/features/bootstrap/WebDav.php
+++ b/build/integration/features/bootstrap/WebDav.php
@@ -262,7 +262,11 @@ trait WebDav {
 			'Accept' => 'application/zip'
 		];
 
-		$this->response = $client->request('GET', $fullUrl, $options);
+		try {
+			$this->response = $client->request('GET', $fullUrl, $options);
+		} catch (\GuzzleHttp\Exception\ClientException $e) {
+			$this->response = $e->getResponse();
+		}
 	}
 
 	/**
diff --git a/build/integration/filesdrop_features/filesdrop.feature b/build/integration/filesdrop_features/filesdrop.feature
index 2c9156dea02..eacd19a4b13 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 ignores directory
+  Scenario: Files drop forbid directory without a nickname
     Given user "user0" exists
     And As an "user0"
     And user "user0" created a folder "/drop"
@@ -44,10 +44,9 @@ Feature: FilesDrop
     And Updating last share with
       | permissions | 4 |
     When Dropping file "/folder/a.txt" with "abc"
-    And Downloading file "/drop/a.txt"
-    Then Downloaded content should be "abc"
+    Then the HTTP status code should be "405"
 
-  Scenario: Files drop forbis MKCOL
+  Scenario: Files drop forbid MKCOL without a nickname
     Given user "user0" exists
     And As an "user0"
     And user "user0" created a folder "/drop"
@@ -60,6 +59,32 @@ Feature: FilesDrop
     When Creating folder "folder" in drop
     Then the HTTP status code should be "405"
 
+  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
+    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 dropping file "/folder/a.txt" with "abc"
+    Then the HTTP status code should be "405"
+
   Scenario: Files request drop
     Given user "user0" exists
     And As an "user0"
@@ -71,7 +96,7 @@ Feature: FilesDrop
       | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
       | shareWith |  |
     When Dropping file "/folder/a.txt" with "abc" as "Alice"
-    And Downloading file "/drop/Alice/a.txt"
+    And Downloading file "/drop/Alice/folder/a.txt"
     Then Downloaded content should be "abc"
 
   Scenario: Put file same file multiple times via files drop
@@ -86,7 +111,47 @@ Feature: FilesDrop
       | shareWith |  |
     When Dropping file "/folder/a.txt" with "abc" as "Mallory"
     And Dropping file "/folder/a.txt" with "def" as "Mallory"
-    And Downloading file "/drop/Mallory/a.txt"
+    # Ensure folder structure and that we only checked
+    # for files duplicates, but merged the existing folders
+    Then as "user0" the folder "/drop/Mallory" exists
+    Then as "user0" the folder "/drop/Mallory/folder" exists
+    Then as "user0" the folder "/drop/Mallory (2)" does not exist
+    Then as "user0" the folder "/drop/Mallory/folder (2)" does not exist
+    Then as "user0" the file "/drop/Mallory/folder/a.txt" exists
+    Then as "user0" the file "/drop/Mallory/folder/a (2).txt" exists
+    And Downloading file "/drop/Mallory/folder/a.txt"
     Then Downloaded content should be "abc"
-    And Downloading file "/drop/Mallory/a (2).txt"
+    And Downloading file "/drop/Mallory/folder/a (2).txt"
     Then Downloaded content should be "def"
+
+  Scenario: Files drop prevents GET
+    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 | 4 |
+      | permissions | 4 |
+      | shareWith |  |
+      | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
+    When Dropping file "/folder/a.txt" with "abc" as "Mallory"
+    When as "user0" the file "/drop/Mallory/folder/a.txt" exists
+    And Downloading public folder "Mallory"
+    Then the HTTP status code should be "405"
+    And Downloading public folder "Mallory/folder"
+    Then the HTTP status code should be "405"
+    And Downloading public file "Mallory/folder/a.txt"
+    Then the HTTP status code should be "405"
+
+  Scenario: Files drop requires nickname if file request is enabled
+    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 | 4 |
+      | permissions | 4 |
+      | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
+      | shareWith |  |
+    When Dropping file "/folder/a.txt" with "abc"
+    Then the HTTP status code should be "405"