From 425231dc116629dc789365945008d1821c758a80 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Tue, 16 Sep 2025 16:58:09 +0200 Subject: [PATCH 1/4] refactor: extract tree initialization logic Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- .../dav/lib/Connector/Sabre/ServerFactory.php | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 8279cd7c3f381..093b7f85929e3 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -27,6 +27,7 @@ use OCP\IRequest; use OCP\ITagManager; use OCP\IUserSession; +use OCP\L10N\IFactory; use OCP\SabrePluginEvent; use Psr\Log\LoggerInterface; use Sabre\DAV\Auth\Plugin; @@ -187,4 +188,61 @@ public function createServer(string $baseUri, }, 30); // priority 30: after auth (10) and acl(20), before lock(50) and handling the request return $server; } + + /** + * Returns a Tree object and, if $useCollection is true, the collection used + * as root. + * + * @param bool $useCollection Whether to use a collection or the legacy + * ObjectTree, which doesn't use collections. + * @return array{0: CachingTree|ObjectTree, 1: SimpleCollection|null} + */ + public function getTree(bool $useCollection): array { + if ($useCollection) { + $rootCollection = new SimpleCollection('root'); + $tree = new CachingTree($rootCollection); + return [$tree, $rootCollection]; + } + + return [new ObjectTree(), null]; + } + + /** + * Adds the user's principal backend to $rootCollection. + */ + private function initRootCollection(SimpleCollection $rootCollection, Directory|File $root): void { + $userPrincipalBackend = new Principal( + \OCP\Server::get(IUserManager::class), + \OCP\Server::get(IGroupManager::class), + \OCP\Server::get(IAccountManager::class), + \OCP\Server::get(\OCP\Share\IManager::class), + \OCP\Server::get(IUserSession::class), + \OCP\Server::get(IAppManager::class), + \OCP\Server::get(ProxyMapper::class), + \OCP\Server::get(KnownUserService::class), + \OCP\Server::get(IConfig::class), + \OCP\Server::get(IFactory::class), + ); + + // Mount the share collection at /public.php/dav/files/ + $rootCollection->addChild( + new RootCollection( + $root, + $userPrincipalBackend, + 'principals/shares', + ) + ); + + // Mount the upload collection at /public.php/dav/uploads/ + $rootCollection->addChild( + new \OCA\DAV\Upload\RootCollection( + $userPrincipalBackend, + 'principals/shares', + \OCP\Server::get(CleanupService::class), + \OCP\Server::get(IRootFolder::class), + \OCP\Server::get(IUserSession::class), + \OCP\Server::get(\OCP\Share\IManager::class), + ) + ); + } } From 04c0709980bcd7163455737283b8db21c8e2b27a Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Tue, 16 Sep 2025 17:08:23 +0200 Subject: [PATCH 2/4] fix: isPublicShare =true when share is public The isPublicShare was set to false in one instance where it should have been true. Flipping the value to true, would break the functionality for PROPFIND /public.php/webdav/ which returns properties of files in a share identified by the username being the share token. Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> From 4aa038030ed4167f68dac6f3c9eacb98dbea2933 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Tue, 16 Sep 2025 17:10:54 +0200 Subject: [PATCH 3/4] fix: check instance of storage using helper function instanceof cannot be used to check the instance of a storage, doing so breaks the check in certain cases. In this case, enabling the `files_accesscontrol` app breaks the check. Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php index f6c19787e94cc..2c6d587a7df21 100644 --- a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php @@ -19,6 +19,7 @@ use OCP\Files\ForbiddenException; use OCP\Files\InvalidPathException; use OCP\Files\Mount\IMountPoint; +use OCP\Files\Storage\IStorage; use OCP\Files\StorageNotAvailableException; use Test\Traits\UserTrait; From 14ebd254d22552f6b0e23be7b746e90cf22cd15d Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Wed, 15 Oct 2025 14:58:28 +0200 Subject: [PATCH 4/4] style: apply cs-fixer to publicwebdav.php Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> [skip ci] --- apps/dav/appinfo/v1/publicwebdav.php | 39 ++++++++++++++----- .../dav/lib/Connector/Sabre/ServerFactory.php | 2 +- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index dc74fe214afbb..62db0dcda265c 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -68,15 +68,36 @@ throw new \Sabre\DAV\Exception\NotAuthenticated(); } - $share = $authBackend->getShare(); - $owner = $share->getShareOwner(); - $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; - $fileId = $share->getNodeId(); - - // FIXME: should not add storage wrappers outside of preSetup, need to find a better way - $previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false); - Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($share) { - return new PermissionsMask(['storage' => $storage, 'mask' => $share->getPermissions() | Constants::PERMISSION_SHARE]); + $share = $authBackend->getShare(); + $owner = $share->getShareOwner(); + $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; + $fileId = $share->getNodeId(); + + // FIXME: should not add storage wrappers outside of preSetup, need to find a better way + $previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false); + Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($share) { + return new PermissionsMask(['storage' => $storage, 'mask' => $share->getPermissions() | Constants::PERMISSION_SHARE]); + }); + Filesystem::addStorageWrapper('shareOwner', function ($mountPoint, $storage) use ($share) { + return new PublicOwnerWrapper(['storage' => $storage, 'owner' => $share->getShareOwner()]); + }); + Filesystem::logWarningWhenAddingStorageWrapper($previousLog); + + $rootFolder = Server::get(IRootFolder::class); + $userFolder = $rootFolder->getUserFolder($owner); + $node = $userFolder->getFirstNodeById($fileId); + if (!$node) { + throw new \Sabre\DAV\Exception\NotFound(); + } + $linkCheckPlugin->setFileInfo($node); + + // If not readable (files_drop) enable the filesdrop plugin + if (!$isReadable) { + $filesDropPlugin->enable(); + } + $filesDropPlugin->setShare($share); + + return new View($node->getPath()); }); Filesystem::addStorageWrapper('shareOwner', function ($mountPoint, $storage) use ($share) { return new PublicOwnerWrapper(['storage' => $storage, 'owner' => $share->getShareOwner()]); diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 093b7f85929e3..1a4dc30728c02 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -194,7 +194,7 @@ public function createServer(string $baseUri, * as root. * * @param bool $useCollection Whether to use a collection or the legacy - * ObjectTree, which doesn't use collections. + * ObjectTree, which doesn't use collections. * @return array{0: CachingTree|ObjectTree, 1: SimpleCollection|null} */ public function getTree(bool $useCollection): array {