From d0641c959f8997fc02fff6df2ff00dfd48210681 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 26 Mar 2026 17:20:45 +0100 Subject: [PATCH] fix: properly expose rename permissions over dav Signed-off-by: Robin Appelman --- apps/dav/lib/BulkUpload/BulkUploadPlugin.php | 2 +- apps/dav/lib/Connector/Sabre/Node.php | 27 ++------------ .../tests/unit/Connector/Sabre/NodeTest.php | 2 +- lib/public/Files/DavUtil.php | 36 +++++++++++++++++-- 4 files changed, 38 insertions(+), 29 deletions(-) diff --git a/apps/dav/lib/BulkUpload/BulkUploadPlugin.php b/apps/dav/lib/BulkUpload/BulkUploadPlugin.php index d4faf3764e1..6a99dc82ec7 100644 --- a/apps/dav/lib/BulkUpload/BulkUploadPlugin.php +++ b/apps/dav/lib/BulkUpload/BulkUploadPlugin.php @@ -76,7 +76,7 @@ class BulkUploadPlugin extends ServerPlugin { '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']]); diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 46589476e98..38450be4630 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -108,25 +108,7 @@ abstract class Node implements \Sabre\DAV\INode { * Check if this node can be renamed */ public function canRename(): bool { - // the root of a movable mountpoint can be renamed regardless of the file permissions - if ($this->info->getMountPoint() instanceof MoveableMount && $this->info->getInternalPath() === '') { - return true; - } - - // we allow renaming the file if either the file has update permissions - if ($this->info->isUpdateable()) { - return true; - } - - // or the file can be deleted and the parent has create permissions - [$parentPath,] = \Sabre\Uri\split($this->path); - if ($parentPath === null) { - // can't rename the users home - return false; - } - - $parent = $this->node->getParent(); - return $this->info->isDeletable() && $parent->isCreatable(); + return DavUtil::canRename($this->node, $this->node->getParent()); } /** @@ -354,11 +336,8 @@ abstract class Node implements \Sabre\DAV\INode { 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() { diff --git a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php index 896f36f1253..9450f8f6ff2 100644 --- a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php @@ -42,7 +42,7 @@ class NodeTest extends \Test\TestCase { [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'], diff --git a/lib/public/Files/DavUtil.php b/lib/public/Files/DavUtil.php index 6dde3179bb8..1d067209606 100644 --- a/lib/public/Files/DavUtil.php +++ b/lib/public/Files/DavUtil.php @@ -7,6 +7,7 @@ namespace OCP\Files; +use OC\Files\Mount\MoveableMount; use OCP\Constants; use OCP\Files\Mount\IMovableMount; @@ -33,7 +34,7 @@ class DavUtil { * * @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 @@ class DavUtil { 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,24 @@ class DavUtil { } 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(); + } }