Merge pull request #53280 from nextcloud/fix/dav-nickname-master

Author: skjnldsv

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

@@ -8,6 +8,7 @@
 use OCP\Files\Folder;
 use OCP\Files\NotFoundException;
 use OCP\Share\IShare;
+use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\MethodNotAllowed;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
@@ -71,13 +72,12 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
 			? 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 MethodNotAllowed('A nickname header is required when uploading subfolders');
+					throw new BadRequest('A nickname header is required when uploading subfolders');
 				}
 			} else {
 				throw new MethodNotAllowed('Only PUT is allowed on files drop');
@@ -113,20 +113,32 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
 
 		// We need a valid nickname for file requests
 		if ($isFileRequest && !$nickname) {
-			throw new MethodNotAllowed('A nickname header is required for file requests');
+			throw new BadRequest('A nickname header is required for file requests');
 		}
 
 		// 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');
+			throw new BadRequest('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
+			try {
+				$node->verifyPath($nickname);
+			} catch (\Exception $e) {
+				// If the path is not valid, we throw an exception
+				throw new BadRequest('Invalid nickname: ' . $nickname);
+			}
+
+			// Forbid nicknames starting with a dot
+			if (str_starts_with($nickname, '.')) {
+				throw new BadRequest('Invalid nickname: ' . $nickname);
+			}
+
+			// If we have a nickname, let's put
+			// all files in the subfolder
 			$relativePath = '/' . $nickname . '/' . $relativePath;
 			$relativePath = str_replace('//', '/', $relativePath);
 		}

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

@@ -13,7 +13,7 @@
 use OCP\Share\IAttributes;
 use OCP\Share\IShare;
 use PHPUnit\Framework\MockObject\MockObject;
-use Sabre\DAV\Exception\MethodNotAllowed;
+use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Server;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
@@ -119,7 +119,7 @@ public function testNoMKCOLWithoutNickname(): void {
 		$this->request->method('getMethod')
 			->willReturn('MKCOL');
 
-		$this->expectException(MethodNotAllowed::class);
+		$this->expectException(BadRequest::class);
 
 		$this->plugin->beforeMethod($this->request, $this->response);
 	}

apps/files_sharing/src/services/GuestNameValidity.ts

@@ -0,0 +1,45 @@
+/*!
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+import { InvalidFilenameError, InvalidFilenameErrorReason, validateFilename } from '@nextcloud/files'
+import { t } from '@nextcloud/l10n'
+
+/**
+ * Get the validity of a filename (empty if valid).
+ * This can be used for `setCustomValidity` on input elements
+ * @param name The filename
+ * @param escape Escape the matched string in the error (only set when used in HTML)
+ */
+export function getGuestNameValidity(name: string, escape = false): string {
+	if (name.trim() === '') {
+		return t('files', 'Filename must not be empty.')
+	}
+
+	if (name.startsWith('.')) {
+		return t('files', 'Names must not start with a dot.')
+	}
+
+	try {
+		validateFilename(name)
+		return ''
+	} catch (error) {
+		if (!(error instanceof InvalidFilenameError)) {
+			throw error
+		}
+
+		switch (error.reason) {
+		case InvalidFilenameErrorReason.Character:
+			return t('files', '"{char}" is not allowed inside a name.', { char: error.segment }, undefined, { escape })
+		case InvalidFilenameErrorReason.ReservedName:
+			return t('files', '"{segment}" is a reserved name and not allowed.', { segment: error.segment }, undefined, { escape: false })
+		case InvalidFilenameErrorReason.Extension:
+			if (error.segment.match(/\.[a-z]/i)) {
+				return t('files', '"{extension}" is not an allowed name.', { extension: error.segment }, undefined, { escape: false })
+			}
+			return t('files', 'Names must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
+		default:
+			return t('files', 'Invalid name.')
+		}
+	}
+}

apps/files_sharing/src/views/PublicAuthPrompt.vue

@@ -35,12 +35,14 @@
 
 <script lang="ts">
 import { defineComponent } from 'vue'
+import { loadState } from '@nextcloud/initial-state'
 import { t } from '@nextcloud/l10n'
 
 import NcDialog from '@nextcloud/vue/components/NcDialog'
 import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'
 import NcTextField from '@nextcloud/vue/components/NcTextField'
-import { loadState } from '@nextcloud/initial-state'
+
+import { getGuestNameValidity } from '../services/GuestNameValidity'
 
 export default defineComponent({
 	name: 'PublicAuthPrompt',
@@ -101,6 +103,19 @@ export default defineComponent({
 			},
 			immediate: true,
 		},
+
+		name() {
+			// Check validity of the new name
+			const newName = this.name.trim?.() || ''
+			const input = (this.$refs.input as Vue|undefined)?.$el.querySelector('input')
+			if (!input) {
+				return
+			}
+
+			const validity = getGuestNameValidity(newName)
+			input.setCustomValidity(validity)
+			input.reportValidity()
+		},
 	},
 })
 </script>

build/integration/filesdrop_features/filesdrop.feature

@@ -44,7 +44,7 @@ Feature: FilesDrop
     And Updating last share with
       | permissions | 4 |
     When Dropping file "/folder/a.txt" with "abc"
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
 
   Scenario: Files drop forbid MKCOL without a nickname
     Given user "user0" exists
@@ -57,7 +57,7 @@ Feature: FilesDrop
     And Updating last share with
       | permissions | 4 |
     When Creating folder "folder" in drop
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
 
   Scenario: Files drop allows MKCOL with a nickname
     Given user "user0" exists
@@ -83,7 +83,7 @@ Feature: FilesDrop
     And Updating last share with
       | permissions | 4 |
     When dropping file "/folder/a.txt" with "abc"
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
 
   Scenario: Files request drop
     Given user "user0" exists
@@ -195,4 +195,43 @@ Feature: FilesDrop
       | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
       | shareWith |  |
     When Dropping file "/folder/a.txt" with "abc"
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
+
+  Scenario: Files request drop with invalid nickname with slashes
+    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" as "Alice/Bob/Mallory"
+    Then the HTTP status code should be "400"
+
+  Scenario: Files request drop with invalid nickname with forbidden characters
+    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" as ".htaccess"
+    Then the HTTP status code should be "400"
+
+  Scenario: Files request drop with invalid nickname with forbidden characters
+    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" as ".Mallory"
+    Then the HTTP status code should be "400"

dist/6127-6127.js

@@ -1,29 +1,65 @@
 SPDX-License-Identifier: MIT
+SPDX-License-Identifier: ISC
 SPDX-License-Identifier: GPL-3.0-or-later
 SPDX-License-Identifier: AGPL-3.0-or-later
 SPDX-License-Identifier: (MPL-2.0 OR Apache-2.0)
+SPDX-FileCopyrightText: inherits developers
 SPDX-FileCopyrightText: escape-html developers
 SPDX-FileCopyrightText: Tobias Koppers @sokra
+SPDX-FileCopyrightText: Roman Shtylman <shtylman@gmail.com>
+SPDX-FileCopyrightText: Roeland Jago Douma
 SPDX-FileCopyrightText: Nextcloud GmbH and Nextcloud contributors
+SPDX-FileCopyrightText: Joyent
+SPDX-FileCopyrightText: Jonas Schade <derzade@gmail.com>
 SPDX-FileCopyrightText: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
 SPDX-FileCopyrightText: Guillaume Chau <guillaume.b.chau@gmail.com>
+SPDX-FileCopyrightText: GitHub Inc.
 SPDX-FileCopyrightText: Evan You
 SPDX-FileCopyrightText: Dr.-Ing. Mario Heiderich, Cure53 <mario@cure53.de> (https://cure53.de/)
 SPDX-FileCopyrightText: David Clark
+SPDX-FileCopyrightText: Christoph Wurst <christoph@winzerhof-wurst.at>
 SPDX-FileCopyrightText: Christoph Wurst
 SPDX-FileCopyrightText: Anthony Fu <https://github.com/antfu>
+SPDX-FileCopyrightText: Alkemics
 
 
 This file is generated from multiple sources. Included packages:
+- @nextcloud/auth
+	- version: 2.5.1
+	- license: GPL-3.0-or-later
+- @nextcloud/browser-storage
+	- version: 0.4.0
+	- license: GPL-3.0-or-later
+- @nextcloud/capabilities
+	- version: 1.2.0
+	- license: GPL-3.0-or-later
+- semver
+	- version: 7.6.3
+	- license: ISC
+- @nextcloud/event-bus
+	- version: 3.3.2
+	- license: GPL-3.0-or-later
+- @nextcloud/files
+	- version: 3.10.2
+	- license: AGPL-3.0-or-later
 - @nextcloud/initial-state
 	- version: 2.2.0
 	- license: GPL-3.0-or-later
 - @nextcloud/l10n
 	- version: 3.2.0
 	- license: GPL-3.0-or-later
+- @nextcloud/logger
+	- version: 3.0.2
+	- license: GPL-3.0-or-later
+- @nextcloud/paths
+	- version: 2.2.1
+	- license: GPL-3.0-or-later
 - @nextcloud/router
 	- version: 3.0.1
 	- license: GPL-3.0-or-later
+- @nextcloud/sharing
+	- version: 0.2.4
+	- license: GPL-3.0-or-later
 - @nextcloud/vue
 	- version: 8.27.0
 	- license: AGPL-3.0-or-later
@@ -33,6 +69,9 @@ This file is generated from multiple sources. Included packages:
 - @vueuse/shared
 	- version: 11.3.0
 	- license: MIT
+- cancelable-promise
+	- version: 4.3.1
+	- license: MIT
 - css-loader
 	- version: 7.1.2
 	- license: MIT
@@ -48,12 +87,27 @@ This file is generated from multiple sources. Included packages:
 - focus-trap
 	- version: 7.6.5
 	- license: MIT
+- inherits
+	- version: 2.0.3
+	- license: ISC
+- util
+	- version: 0.10.4
+	- license: MIT
+- path
+	- version: 0.12.7
+	- license: MIT
+- process
+	- version: 0.11.10
+	- license: MIT
 - style-loader
 	- version: 4.0.0
 	- license: MIT
 - tabbable
 	- version: 6.2.0
 	- license: MIT
+- typescript-event-target
+	- version: 1.1.1
+	- license: MIT
 - vue-loader
 	- version: 15.11.1
 	- license: MIT