Merge pull request #57374 from nextcloud/rename-dav-permissions

fix: allow renaming files with just update permissions

Author: icewind1991

apps/dav/lib/BulkUpload/BulkUploadPlugin.php

@@ -76,7 +76,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
 					'error' => false,
 					'etag' => $node->getETag(),
 					'fileid' => DavUtil::getDavFileId($node->getId()),
-					'permissions' => DavUtil::getDavPermissions($node),
+					'permissions' => DavUtil::getDavPermissions($node, $node->getParent()),
 				];
 			} catch (\Exception $e) {
 				$this->logger->error($e->getMessage(), ['path' => $headers['x-file-path']]);

apps/dav/lib/Connector/Sabre/FilesPlugin.php

@@ -203,10 +203,19 @@ public function checkMove(string $source, string $target): void {
 		// First check copyable (move only needs additional delete permission)
 		$this->checkCopy($source, $target);
 
-		// The source needs to be deletable for moving
-		$sourceNodeFileInfo = $sourceNode->getFileInfo();
-		if (!$sourceNodeFileInfo->isDeletable()) {
-			throw new Forbidden($source . ' cannot be deleted');
+		[$sourceDir] = \Sabre\Uri\split($source);
+		[$destinationDir, ] = \Sabre\Uri\split($target);
+
+		if ($sourceDir === $destinationDir) {
+			if (!$sourceNode->canRename()) {
+				throw new Forbidden($source . ' cannot be renamed');
+			}
+		} else {
+			// The source needs to be deletable for moving
+			$sourceNodeFileInfo = $sourceNode->getFileInfo();
+			if (!$sourceNodeFileInfo->isDeletable()) {
+				throw new Forbidden($source . ' cannot be deleted');
+			}
 		}
 
 		// The source is not allowed to be the parent of the target

apps/dav/lib/Connector/Sabre/Node.php

@@ -112,6 +112,13 @@ public function getPath(): string {
 		return $this->path;
 	}
 
+	/**
+	 * Check if this node can be renamed
+	 */
+	public function canRename(): bool {
+		return DavUtil::canRename($this->node, $this->node->getParent());
+	}
+
 	/**
 	 * Renames the node
 	 *
@@ -123,10 +130,8 @@ public function getPath(): string {
 	 * @throws LockedException
 	 */
 	public function setName($name): void {
-		// rename is only allowed if the delete privilege is granted
-		// (basically rename is a copy with delete of the original node)
-		if (!$this->info->isDeletable() && !($this->info->getMountPoint() instanceof MoveableMount && $this->info->getInternalPath() === '')) {
-			throw new Forbidden();
+		if (!$this->canRename()) {
+			throw new Forbidden('');
 		}
 
 		/** @var string $parentPath */
@@ -320,7 +325,7 @@ public function getNoteFromShare(?string $user): ?string {
 	}
 
 	public function getDavPermissions(): string {
-		return DavUtil::getDavPermissions($this->info);
+		return DavUtil::getDavPermissions($this->info, $this->node->getParent());
 	}
 
 	/**

apps/dav/tests/unit/Connector/Sabre/NodeTest.php

@@ -42,7 +42,7 @@ public static function davPermissionsProvider(): array {
 			[Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL, true, '' , 'SRMGDNVW'],
 			[Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE, true, '' , 'SRMGDNV'],
 			[Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, 'file', true, Constants::PERMISSION_ALL, false, 'test', 'SGDNVW'],
-			[Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGD'],
+			[Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGDN'],
 			[Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGNVW'],
 			[Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNVW'],
 			[Constants::PERMISSION_ALL - Constants::PERMISSION_READ, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RDNVW'],

apps/files/src/actions/renameAction.ts

@@ -37,10 +37,15 @@ export const action: IFileAction = {
 			: filesStore.getNode(dirname(node.source))
 		const parentPermissions = parentNode?.permissions || Permission.NONE
 
-		// Only enable if the node have the delete permission
-		// and if the parent folder allows creating files
-		return Boolean(node.permissions & Permission.DELETE)
-			&& Boolean(parentPermissions & Permission.CREATE)
+		// Enable if the node has update permissions or the node
+		// has delete permission and the parent folder allows creating files
+		return (
+			(
+				Boolean(node.permissions & Permission.DELETE)
+				&& Boolean(parentPermissions & Permission.CREATE)
+			)
+			|| Boolean(node.permissions & Permission.UPDATE)
+		)
 	},
 
 	async exec({ nodes }) {

dist/files-init.js

@@ -7,6 +7,7 @@
 
 namespace OCP\Files;
 
+use OC\Files\Mount\MoveableMount;
 use OCP\Constants;
 use OCP\Files\Mount\IMovableMount;
 
@@ -33,7 +34,7 @@ public static function getDavFileId(int $id): string {
 	 *
 	 * @since 25.0.0
 	 */
-	public static function getDavPermissions(FileInfo $info): string {
+	public static function getDavPermissions(FileInfo $info, FileInfo $parent): string {
 		$permissions = $info->getPermissions();
 		$p = '';
 		if ($info->isShared()) {
@@ -51,8 +52,11 @@ public static function getDavPermissions(FileInfo $info): string {
 		if ($permissions & Constants::PERMISSION_DELETE) {
 			$p .= 'D';
 		}
+		if (self::canRename($info, $parent)) {
+			$p .= 'N'; // Renamable
+		}
 		if ($permissions & Constants::PERMISSION_UPDATE) {
-			$p .= 'NV'; // Renameable, Movable
+			$p .= 'V'; // Movable
 		}
 
 		// since we always add update permissions for the root of movable mounts
@@ -76,4 +80,24 @@ public static function getDavPermissions(FileInfo $info): string {
 		}
 		return $p;
 	}
+
+	public static function canRename(FileInfo $info, FileInfo $parent): bool {
+		// the root of a movable mountpoint can be renamed regardless of the file permissions
+		if ($info->getMountPoint() instanceof MoveableMount && $info->getInternalPath() === '') {
+			return true;
+		}
+
+		// we allow renaming the file if either the file has update permissions
+		if ($info->isUpdateable()) {
+			return true;
+		}
+
+		// or the file can be deleted and the parent has create permissions
+		if ($info->getStorage() instanceof IHomeStorage && $info->getInternalPath() === 'files') {
+			// can't rename the users home
+			return false;
+		}
+
+		return $info->isDeletable() && $parent->isCreatable();
+	}
 }