Skip to content

Commit a7a64de

Browse files
sorbaughsorbaugh
authored
Merge pull request #54441 from nextcloud/fix/directoryAsINodeByPath
Add INodeByPath to Directory
2 parents d15feb4 + 871262d commit a7a64de

File tree

3 files changed

+227
-3
lines changed

3 files changed

+227
-3
lines changed

‎3rdparty‎

‎apps/dav/lib/Connector/Sabre/Directory.php‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OCA\DAV\Connector\Sabre;
99

1010
use OC\Files\Mount\MoveableMount;
11+
use OC\Files\Utils\PathHelper;
1112
use OC\Files\View;
1213
use OCA\DAV\AppInfo\Application;
1314
use OCA\DAV\Connector\Sabre\Exception\FileLocked;
@@ -38,8 +39,14 @@
3839
use Sabre\DAV\Exception\ServiceUnavailable;
3940
use Sabre\DAV\IFile;
4041
use Sabre\DAV\INode;
41-
42-
class Directory extends Node implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuota, \Sabre\DAV\IMoveTarget, \Sabre\DAV\ICopyTarget {
42+
use Sabre\DAV\INodeByPath;
43+
44+
class Directory extends Node implements
45+
\Sabre\DAV\ICollection,
46+
\Sabre\DAV\IQuota,
47+
\Sabre\DAV\IMoveTarget,
48+
\Sabre\DAV\ICopyTarget,
49+
INodeByPath {
4350
/**
4451
* Cached directory content
4552
* @var FileInfo[]
@@ -490,4 +497,79 @@ public function copyInto($targetName, $sourcePath, INode $sourceNode) {
490497
public function getNode(): Folder {
491498
return $this->node;
492499
}
500+
501+
public function getNodeForPath($path): INode {
502+
$storage = $this->info->getStorage();
503+
$allowDirectory = false;
504+
505+
// Checking if we're in a file drop
506+
// If we are, then only PUT and MKCOL are allowed (see plugin)
507+
// so we are safe to return the directory without a risk of
508+
// leaking files and folders structure.
509+
if ($storage->instanceOfStorage(PublicShareWrapper::class)) {
510+
$share = $storage->getShare();
511+
$allowDirectory = ($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ;
512+
}
513+
514+
// For file drop we need to be allowed to read the directory with the nickname
515+
if (!$allowDirectory && !$this->info->isReadable()) {
516+
// avoid detecting files through this way
517+
throw new NotFound();
518+
}
519+
520+
$destinationPath = PathHelper::normalizePath($this->getPath() . '/' . $path);
521+
$destinationDir = dirname($destinationPath);
522+
523+
try {
524+
$info = $this->getNode()->get($path);
525+
} catch (NotFoundException $e) {
526+
throw new \Sabre\DAV\Exception\NotFound('File with name ' . $destinationPath
527+
. ' could not be located');
528+
} catch (StorageNotAvailableException $e) {
529+
throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage(), 0, $e);
530+
} catch (NotPermittedException $ex) {
531+
throw new InvalidPath($ex->getMessage(), false, $ex);
532+
}
533+
534+
// if not in a public share with no read permissions, throw Forbidden
535+
if (!$allowDirectory && !$info->isReadable()) {
536+
if (Server::get(IAppManager::class)->isEnabledForAnyone('files_accesscontrol')) {
537+
throw new Forbidden('No read permissions. This might be caused by files_accesscontrol, check your configured rules');
538+
}
539+
540+
throw new Forbidden('No read permissions');
541+
}
542+
543+
if ($info->getMimeType() === FileInfo::MIMETYPE_FOLDER) {
544+
$node = new \OCA\DAV\Connector\Sabre\Directory($this->fileView, $info, $this->tree, $this->shareManager);
545+
} else {
546+
// In case reading a directory was allowed but it turns out the node was a not a directory, reject it now.
547+
if (!$this->info->isReadable()) {
548+
throw new NotFound();
549+
}
550+
551+
$node = new File($this->fileView, $info, $this->shareManager);
552+
}
553+
$this->tree?->cacheNode($node);
554+
555+
// recurse upwards until the root and check for read permissions to keep
556+
// ACL checks working in files_accesscontrol
557+
if (!$allowDirectory && $destinationDir !== '') {
558+
$scanPath = $destinationPath;
559+
while (($scanPath = dirname($scanPath)) !== '/') {
560+
// fileView can get the parent info in a cheaper way compared
561+
// to the node API
562+
/** @psalm-suppress InternalMethod */
563+
$info = $this->fileView->getFileInfo($scanPath, false);
564+
$directory = new Directory($this->fileView, $info, $this->tree, $this->shareManager);
565+
$readable = $directory->getNode()->isReadable();
566+
if (!$readable) {
567+
throw new \Sabre\DAV\Exception\NotFound('File with name ' . $destinationPath
568+
. ' could not be located');
569+
}
570+
}
571+
}
572+
573+
return $node;
574+
}
493575
}

‎apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use OC\Files\FileInfo;
1212
use OC\Files\Filesystem;
13+
use OC\Files\Node\Folder;
1314
use OC\Files\Node\Node;
1415
use OC\Files\Storage\Wrapper\Quota;
1516
use OC\Files\View;
@@ -24,6 +25,7 @@
2425
use OCP\Files\Storage\IStorage;
2526
use OCP\Files\StorageNotAvailableException;
2627
use PHPUnit\Framework\MockObject\MockObject;
28+
use Sabre\DAV\Exception\NotFound;
2729
use Test\Traits\UserTrait;
2830

2931
class TestViewDirectory extends View {
@@ -271,6 +273,146 @@ public function testGetChildThrowInvalidPath(): void {
271273
$dir->getChild('.');
272274
}
273275

276+
public function testGetNodeForPath(): void {
277+
$directoryNode = $this->createMock(Folder::class);
278+
$pathNode = $this->createMock(Folder::class);
279+
$pathParentNode = $this->createMock(Folder::class);
280+
$storage = $this->createMock(IStorage::class);
281+
282+
$directoryNode->expects($this->once())
283+
->method('getStorage')
284+
->willReturn($storage);
285+
$storage->expects($this->once())
286+
->method('instanceOfStorage')
287+
->willReturn(false);
288+
289+
$directoryNode->expects($this->once())
290+
->method('isReadable')
291+
->willReturn(true);
292+
$directoryNode->expects($this->once())
293+
->method('getPath')
294+
->willReturn('/admin/files/');
295+
$directoryNode->expects($this->once())
296+
->method('get')
297+
->willReturn($pathNode);
298+
299+
$pathNode->expects($this->once())
300+
->method('getPath')
301+
->willReturn('/admin/files/my/deep/folder/');
302+
$pathNode->expects($this->once())
303+
->method('isReadable')
304+
->willReturn(true);
305+
$pathNode->expects($this->once())
306+
->method('getMimetype')
307+
->willReturn(FileInfo::MIMETYPE_FOLDER);
308+
309+
$this->view->method('getRelativePath')
310+
->willReturnCallback(function ($path) {
311+
return str_replace('/admin/files/', '', $path);
312+
});
313+
314+
$this->view->expects($this->exactly(2))
315+
->method('getFileInfo')
316+
->willReturn($pathParentNode);
317+
318+
$pathParentNode->expects($this->exactly(2))
319+
->method('getPath')
320+
->willReturnOnConsecutiveCalls('/my/deep', '/my');
321+
$pathParentNode->expects($this->exactly(2))
322+
->method('isReadable')
323+
->willReturn(true);
324+
325+
$dir = new Directory($this->view, $directoryNode);
326+
$dir->getNodeForPath('/my/deep/folder/');
327+
}
328+
329+
public function testGetNodeForPathFailsWithNoReadPermissionsForParent(): void {
330+
$directoryNode = $this->createMock(Folder::class);
331+
$pathNode = $this->createMock(Folder::class);
332+
$pathParentNode = $this->createMock(Folder::class);
333+
$storage = $this->createMock(IStorage::class);
334+
335+
$directoryNode->expects($this->once())
336+
->method('getStorage')
337+
->willReturn($storage);
338+
$storage->expects($this->once())
339+
->method('instanceOfStorage')
340+
->willReturn(false);
341+
342+
$directoryNode->expects($this->once())
343+
->method('isReadable')
344+
->willReturn(true);
345+
$directoryNode->expects($this->once())
346+
->method('getPath')
347+
->willReturn('/admin/files/');
348+
$directoryNode->expects($this->once())
349+
->method('get')
350+
->willReturn($pathNode);
351+
352+
$pathNode->expects($this->once())
353+
->method('getPath')
354+
->willReturn('/admin/files/my/deep/folder/');
355+
$pathNode->expects($this->once())
356+
->method('isReadable')
357+
->willReturn(true);
358+
$pathNode->expects($this->once())
359+
->method('getMimetype')
360+
->willReturn(FileInfo::MIMETYPE_FOLDER);
361+
362+
$this->view->method('getRelativePath')
363+
->willReturnCallback(function ($path) {
364+
return str_replace('/admin/files/', '', $path);
365+
});
366+
367+
$this->view->expects($this->exactly(2))
368+
->method('getFileInfo')
369+
->willReturn($pathParentNode);
370+
371+
$pathParentNode->expects($this->exactly(2))
372+
->method('getPath')
373+
->willReturnOnConsecutiveCalls('/my/deep', '/my');
374+
$pathParentNode->expects($this->exactly(2))
375+
->method('isReadable')
376+
->willReturnOnConsecutiveCalls(true, false);
377+
378+
$this->expectException(NotFound::class);
379+
380+
$dir = new Directory($this->view, $directoryNode);
381+
$dir->getNodeForPath('/my/deep/folder/');
382+
}
383+
384+
public function testGetNodeForPathFailsWithNoReadPermissionsForPath(): void {
385+
$directoryNode = $this->createMock(Folder::class);
386+
$pathNode = $this->createMock(Folder::class);
387+
$storage = $this->createMock(IStorage::class);
388+
389+
$directoryNode->expects($this->once())
390+
->method('getStorage')
391+
->willReturn($storage);
392+
$storage->expects($this->once())
393+
->method('instanceOfStorage')
394+
->willReturn(false);
395+
396+
$directoryNode->expects($this->once())
397+
->method('isReadable')
398+
->willReturn(true);
399+
$directoryNode->expects($this->once())
400+
->method('getPath')
401+
->willReturn('/admin/files/');
402+
$directoryNode->expects($this->once())
403+
->method('get')
404+
->willReturn($pathNode);
405+
406+
$pathNode->expects($this->once())
407+
->method('isReadable')
408+
->willReturn(false);
409+
410+
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
411+
412+
$dir = new Directory($this->view, $directoryNode);
413+
$dir->getNodeForPath('/my/deep/folder/');
414+
}
415+
274416
public function testGetQuotaInfoUnlimited(): void {
275417
$this->createUser('user', 'password');
276418
self::loginAsUser('user');

0 commit comments

Comments
 (0)