Merge pull request #59241 from nextcloud/backport/57374/stable33

[stable33] fix: allow renaming files with just update permissions

Author: AndyScherzinger

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

@@ -207,10 +207,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

@@ -23,6 +23,7 @@
 use OCP\Server;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
+use Sabre\DAV\Exception\Forbidden;
 
 abstract class Node implements \Sabre\DAV\INode {
 	/**
@@ -103,6 +104,13 @@ public function getPath() {
 		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
 	 *
@@ -111,10 +119,8 @@ public function getPath() {
 	 * @throws \Sabre\DAV\Exception\Forbidden
 	 */
 	public function setName($name) {
-		// 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 \Sabre\DAV\Exception\Forbidden();
+		if (!$this->canRename()) {
+			throw new Forbidden('');
 		}
 
 		[$parentPath,] = \Sabre\Uri\split($this->path);
@@ -330,11 +336,8 @@ public function getNoteFromShare(?string $user): ?string {
 		return null;
 	}
 
-	/**
-	 * @return string
-	 */
-	public function getDavPermissions() {
-		return DavUtil::getDavPermissions($this->info);
+	public function getDavPermissions(): string {
+		return DavUtil::getDavPermissions($this->info, $this->node->getParent());
 	}
 
 	public function getOwner() {

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 = null): string {
 		$permissions = $info->getPermissions();
 		$p = '';
 		if ($info->isShared()) {
@@ -51,8 +52,17 @@ public static function getDavPermissions(FileInfo $info): string {
 		if ($permissions & Constants::PERMISSION_DELETE) {
 			$p .= 'D';
 		}
-		if ($permissions & Constants::PERMISSION_UPDATE) {
-			$p .= 'NV'; // Renameable, Movable
+		if ($parent) {
+			if (self::canRename($info, $parent)) {
+				$p .= 'N'; // Renamable
+			}
+			if ($permissions & Constants::PERMISSION_UPDATE) {
+				$p .= 'V'; // Movable
+			}
+		} else {
+			if ($permissions & Constants::PERMISSION_UPDATE) {
+				$p .= 'NV'; // Renamable, Movable
+			}
 		}
 
 		// since we always add update permissions for the root of movable mounts
@@ -76,4 +86,35 @@ public static function getDavPermissions(FileInfo $info): string {
 		}
 		return $p;
 	}
+
+	/**
+	 * Check if a node has rename permissions.
+	 *
+	 * We allow renaming the file if either the file has update permissions
+	 * or the file can be deleted and the parent has create permissions
+	 *
+	 * @param FileInfo $info
+	 * @param FileInfo $parent
+	 * @return bool
+	 * @since 33.0.4
+	 */
+	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();
+	}
 }