From 2a4ee2df9f4b9a0374603ea689a608d7c380d342 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Fri, 15 Aug 2025 16:14:19 +0200 Subject: [PATCH 1/2] fix: add INodeByPath to Directory Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- apps/dav/lib/Connector/Sabre/Directory.php | 86 ++++++++++- .../unit/Connector/Sabre/DirectoryTest.php | 142 ++++++++++++++++++ 2 files changed, 226 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index 87685c5bb2aeb..9baea2d027c5e 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -8,6 +8,7 @@ namespace OCA\DAV\Connector\Sabre; use OC\Files\Mount\MoveableMount; +use OC\Files\Utils\PathHelper; use OC\Files\View; use OCA\DAV\AppInfo\Application; use OCA\DAV\Connector\Sabre\Exception\FileLocked; @@ -38,8 +39,14 @@ use Sabre\DAV\Exception\ServiceUnavailable; use Sabre\DAV\IFile; use Sabre\DAV\INode; - -class Directory extends Node implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuota, \Sabre\DAV\IMoveTarget, \Sabre\DAV\ICopyTarget { +use Sabre\DAV\INodeByPath; + +class Directory extends Node implements + \Sabre\DAV\ICollection, + \Sabre\DAV\IQuota, + \Sabre\DAV\IMoveTarget, + \Sabre\DAV\ICopyTarget, + INodeByPath { /** * Cached directory content * @var FileInfo[] @@ -490,4 +497,79 @@ public function copyInto($targetName, $sourcePath, INode $sourceNode) { public function getNode(): Folder { return $this->node; } + + public function getNodeForPath($path): INode { + $storage = $this->info->getStorage(); + $allowDirectory = false; + + // Checking if we're in a file drop + // If we are, then only PUT and MKCOL are allowed (see plugin) + // so we are safe to return the directory without a risk of + // leaking files and folders structure. + if ($storage->instanceOfStorage(PublicShareWrapper::class)) { + $share = $storage->getShare(); + $allowDirectory = ($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ; + } + + // For file drop we need to be allowed to read the directory with the nickname + if (!$allowDirectory && !$this->info->isReadable()) { + // avoid detecting files through this way + throw new NotFound(); + } + + $destinationPath = PathHelper::normalizePath($this->getPath() . '/' . $path); + $destinationDir = dirname($destinationPath); + + try { + $info = $this->getNode()->get($path); + } catch (NotFoundException $e) { + throw new \Sabre\DAV\Exception\NotFound('File with name ' . $destinationPath + . ' could not be located'); + } catch (StorageNotAvailableException $e) { + throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage(), 0, $e); + } catch (NotPermittedException $ex) { + throw new InvalidPath($ex->getMessage(), false, $ex); + } + + // if not in a public share with no read permissions, throw Forbidden + if (!$allowDirectory && !$info->isReadable()) { + if (Server::get(IAppManager::class)->isEnabledForAnyone('files_accesscontrol')) { + throw new Forbidden('No read permissions. This might be caused by files_accesscontrol, check your configured rules'); + } + + throw new Forbidden('No read permissions'); + } + + if ($info->getMimeType() === FileInfo::MIMETYPE_FOLDER) { + $node = new \OCA\DAV\Connector\Sabre\Directory($this->fileView, $info, $this->tree, $this->shareManager); + } else { + // In case reading a directory was allowed but it turns out the node was a not a directory, reject it now. + if (!$this->info->isReadable()) { + throw new NotFound(); + } + + $node = new File($this->fileView, $info, $this->shareManager); + } + $this->tree?->cacheNode($node); + + // recurse upwards until the root and check for read permissions to keep + // ACL checks working in files_accesscontrol + if (!$allowDirectory && $destinationDir !== '') { + $scanPath = $destinationPath; + while (($scanPath = dirname($scanPath)) !== '/') { + // fileView can get the parent info in a cheaper way compared + // to the node API + /** @psalm-suppress InternalMethod */ + $info = $this->fileView->getFileInfo($scanPath, false); + $directory = new Directory($this->fileView, $info, $this->tree, $this->shareManager); + $readable = $directory->getNode()->isReadable(); + if (!$readable) { + throw new \Sabre\DAV\Exception\NotFound('File with name ' . $destinationPath + . ' could not be located'); + } + } + } + + return $node; + } } diff --git a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php index 1da7126987b94..845d5fc5f02dd 100644 --- a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php @@ -10,6 +10,7 @@ use OC\Files\FileInfo; use OC\Files\Filesystem; +use OC\Files\Node\Folder; use OC\Files\Node\Node; use OC\Files\Storage\Wrapper\Quota; use OC\Files\View; @@ -24,6 +25,7 @@ use OCP\Files\Storage\IStorage; use OCP\Files\StorageNotAvailableException; use PHPUnit\Framework\MockObject\MockObject; +use Sabre\DAV\Exception\NotFound; use Test\Traits\UserTrait; class TestViewDirectory extends View { @@ -271,6 +273,146 @@ public function testGetChildThrowInvalidPath(): void { $dir->getChild('.'); } + public function testGetNodeForPath(): void { + $directoryNode = $this->createMock(Folder::class); + $pathNode = $this->createMock(Folder::class); + $pathParentNode = $this->createMock(Folder::class); + $storage = $this->createMock(IStorage::class); + + $directoryNode->expects($this->once()) + ->method('getStorage') + ->willReturn($storage); + $storage->expects($this->once()) + ->method('instanceOfStorage') + ->willReturn(false); + + $directoryNode->expects($this->once()) + ->method('isReadable') + ->willReturn(true); + $directoryNode->expects($this->once()) + ->method('getPath') + ->willReturn('/admin/files/'); + $directoryNode->expects($this->once()) + ->method('get') + ->willReturn($pathNode); + + $pathNode->expects($this->once()) + ->method('getPath') + ->willReturn('/admin/files/my/deep/folder/'); + $pathNode->expects($this->once()) + ->method('isReadable') + ->willReturn(true); + $pathNode->expects($this->once()) + ->method('getMimetype') + ->willReturn(FileInfo::MIMETYPE_FOLDER); + + $this->view->method('getRelativePath') + ->willReturnCallback(function ($path) { + return str_replace('/admin/files/', '', $path); + }); + + $this->view->expects($this->exactly(2)) + ->method('getFileInfo') + ->willReturn($pathParentNode); + + $pathParentNode->expects($this->exactly(2)) + ->method('getPath') + ->willReturnOnConsecutiveCalls('/my/deep', '/my'); + $pathParentNode->expects($this->exactly(2)) + ->method('isReadable') + ->willReturn(true); + + $dir = new Directory($this->view, $directoryNode); + $dir->getNodeForPath('/my/deep/folder/'); + } + + public function testGetNodeForPathFailsWithNoReadPermissionsForParent(): void { + $directoryNode = $this->createMock(Folder::class); + $pathNode = $this->createMock(Folder::class); + $pathParentNode = $this->createMock(Folder::class); + $storage = $this->createMock(IStorage::class); + + $directoryNode->expects($this->once()) + ->method('getStorage') + ->willReturn($storage); + $storage->expects($this->once()) + ->method('instanceOfStorage') + ->willReturn(false); + + $directoryNode->expects($this->once()) + ->method('isReadable') + ->willReturn(true); + $directoryNode->expects($this->once()) + ->method('getPath') + ->willReturn('/admin/files/'); + $directoryNode->expects($this->once()) + ->method('get') + ->willReturn($pathNode); + + $pathNode->expects($this->once()) + ->method('getPath') + ->willReturn('/admin/files/my/deep/folder/'); + $pathNode->expects($this->once()) + ->method('isReadable') + ->willReturn(true); + $pathNode->expects($this->once()) + ->method('getMimetype') + ->willReturn(FileInfo::MIMETYPE_FOLDER); + + $this->view->method('getRelativePath') + ->willReturnCallback(function ($path) { + return str_replace('/admin/files/', '', $path); + }); + + $this->view->expects($this->exactly(2)) + ->method('getFileInfo') + ->willReturn($pathParentNode); + + $pathParentNode->expects($this->exactly(2)) + ->method('getPath') + ->willReturnOnConsecutiveCalls('/my/deep', '/my'); + $pathParentNode->expects($this->exactly(2)) + ->method('isReadable') + ->willReturnOnConsecutiveCalls(true, false); + + $this->expectException(NotFound::class); + + $dir = new Directory($this->view, $directoryNode); + $dir->getNodeForPath('/my/deep/folder/'); + } + + public function testGetNodeForPathFailsWithNoReadPermissionsForPath(): void { + $directoryNode = $this->createMock(Folder::class); + $pathNode = $this->createMock(Folder::class); + $storage = $this->createMock(IStorage::class); + + $directoryNode->expects($this->once()) + ->method('getStorage') + ->willReturn($storage); + $storage->expects($this->once()) + ->method('instanceOfStorage') + ->willReturn(false); + + $directoryNode->expects($this->once()) + ->method('isReadable') + ->willReturn(true); + $directoryNode->expects($this->once()) + ->method('getPath') + ->willReturn('/admin/files/'); + $directoryNode->expects($this->once()) + ->method('get') + ->willReturn($pathNode); + + $pathNode->expects($this->once()) + ->method('isReadable') + ->willReturn(false); + + $this->expectException(\Sabre\DAV\Exception\Forbidden::class); + + $dir = new Directory($this->view, $directoryNode); + $dir->getNodeForPath('/my/deep/folder/'); + } + public function testGetQuotaInfoUnlimited(): void { $this->createUser('user', 'password'); self::loginAsUser('user'); From 871262d6f6049b5e4663d41805870f5c76c22eec Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Mon, 1 Dec 2025 16:53:28 +0100 Subject: [PATCH 2/2] chore: update 3rdparty Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index 3e8f824169a81..a030dd69eaca2 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit 3e8f824169a8144420a3071ffabb116799e18016 +Subproject commit a030dd69eaca2934d3898aaea2bbd3baaa38d3e4