From 3571207bd956de5dc8aece2ba879f31f3696fef6 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 30 Jun 2016 11:09:20 +0200 Subject: add some additonal permission checks to the webdav backend --- apps/dav/lib/Connector/Sabre/ObjectTree.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'apps') diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php index 9e7d876187d..07052e30301 100644 --- a/apps/dav/lib/Connector/Sabre/ObjectTree.php +++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php @@ -71,7 +71,7 @@ class ObjectTree extends \Sabre\DAV\Tree { * is present. * * @param string $path chunk file path to convert - * + * * @return string path to real file */ private function resolveChunkFile($path) { @@ -196,6 +196,15 @@ class ObjectTree extends \Sabre\DAV\Tree { throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup'); } + $infoDestination = $this->fileView->getFileInfo(dirname($destinationPath)); + $infoSource = $this->fileView->getFileInfo($sourcePath); + $destinationPermission = $infoDestination && $infoDestination->isUpdateable(); + $sourcePermission = $infoSource && $infoSource->isDeletable(); + + if (!$destinationPermission || !$sourcePermission) { + throw new Forbidden(); + } + $targetNodeExists = $this->nodeExists($destinationPath); $sourceNode = $this->getNodeForPath($sourcePath); if ($sourceNode instanceof \Sabre\DAV\ICollection && $targetNodeExists) { @@ -273,6 +282,12 @@ class ObjectTree extends \Sabre\DAV\Tree { throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup'); } + + $info = $this->fileView->getFileInfo(dirname($destination)); + if ($info && !$info->isUpdateable()) { + throw new Forbidden(); + } + // this will trigger existence check $this->getNodeForPath($source); -- cgit v1.2.3 From 1e7f0f73410cf3b8a7fd52cade27de92bc5994c3 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 30 Jun 2016 13:17:53 +0200 Subject: Add required $message parameter --- apps/dav/lib/Connector/Sabre/ObjectTree.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'apps') diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php index 07052e30301..100e5ec56a2 100644 --- a/apps/dav/lib/Connector/Sabre/ObjectTree.php +++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php @@ -202,7 +202,7 @@ class ObjectTree extends \Sabre\DAV\Tree { $sourcePermission = $infoSource && $infoSource->isDeletable(); if (!$destinationPermission || !$sourcePermission) { - throw new Forbidden(); + throw new Forbidden('No permissions to move object.'); } $targetNodeExists = $this->nodeExists($destinationPath); @@ -285,7 +285,7 @@ class ObjectTree extends \Sabre\DAV\Tree { $info = $this->fileView->getFileInfo(dirname($destination)); if ($info && !$info->isUpdateable()) { - throw new Forbidden(); + throw new Forbidden('No permissions to move object.'); } // this will trigger existence check -- cgit v1.2.3 From c771368c4e3e8fdac4ef95758f308f85d5a29edf Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 30 Jun 2016 13:19:50 +0200 Subject: Add proper throws PHP docs --- apps/dav/lib/Connector/Sabre/ObjectTree.php | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'apps') diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php index 100e5ec56a2..8f5f7be2505 100644 --- a/apps/dav/lib/Connector/Sabre/ObjectTree.php +++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php @@ -186,9 +186,13 @@ class ObjectTree extends \Sabre\DAV\Tree { * * @param string $sourcePath The path to the file which should be moved * @param string $destinationPath The full destination path, so not just the destination parent node - * @throws \Sabre\DAV\Exception\BadRequest - * @throws \Sabre\DAV\Exception\ServiceUnavailable + * @throws FileLocked + * @throws Forbidden + * @throws InvalidPath * @throws \Sabre\DAV\Exception\Forbidden + * @throws \Sabre\DAV\Exception\Locked + * @throws \Sabre\DAV\Exception\NotFound + * @throws \Sabre\DAV\Exception\ServiceUnavailable * @return int */ public function move($sourcePath, $destinationPath) { @@ -274,6 +278,13 @@ class ObjectTree extends \Sabre\DAV\Tree { * * @param string $source * @param string $destination + * @throws FileLocked + * @throws Forbidden + * @throws InvalidPath + * @throws \Exception + * @throws \Sabre\DAV\Exception\Forbidden + * @throws \Sabre\DAV\Exception\Locked + * @throws \Sabre\DAV\Exception\NotFound * @throws \Sabre\DAV\Exception\ServiceUnavailable * @return void */ -- cgit v1.2.3 From 149218ead9c43d79e45ba19ea59cd4966a1e046b Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 30 Jun 2016 13:46:08 +0200 Subject: Fix tests --- .../tests/unit/Connector/Sabre/ObjectTreeTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'apps') diff --git a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php index 4a5e43376c0..96d4357660e 100644 --- a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php @@ -35,6 +35,7 @@ class TestDoubleFileView extends \OC\Files\View { $this->updatables = $updatables; $this->deletables = $deletables; $this->canRename = $canRename; + $this->lockingProvider = \OC::$server->getLockingProvider(); } public function isUpdatable($path) { @@ -56,6 +57,11 @@ class TestDoubleFileView extends \OC\Files\View { public function getRelativePath($path) { return $path; } + + public function getFileInfo($path, $includeMountPoints = true) { + $objectTreeTest = new ObjectTreeTest(); + return $objectTreeTest->getFileInfoMock(); + } } /** @@ -67,6 +73,20 @@ class TestDoubleFileView extends \OC\Files\View { */ class ObjectTreeTest extends \Test\TestCase { + public function getFileInfoMock() { + $mock = $this->getMock('\OCP\Files\FileInfo'); + $mock + ->expects($this->any()) + ->method('isDeletable') + ->willReturn(true); + $mock + ->expects($this->any()) + ->method('isUpdateable') + ->willReturn(true); + + return $mock; + } + /** * @dataProvider moveFailedProvider * @expectedException \Sabre\DAV\Exception\Forbidden -- cgit v1.2.3 From 26e14529be942e3cc3c2bb2b388b155073daecb1 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 30 Jun 2016 13:50:31 +0200 Subject: fix error message --- apps/dav/lib/Connector/Sabre/ObjectTree.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apps') diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php index 8f5f7be2505..d8c1d71e7f1 100644 --- a/apps/dav/lib/Connector/Sabre/ObjectTree.php +++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php @@ -296,7 +296,7 @@ class ObjectTree extends \Sabre\DAV\Tree { $info = $this->fileView->getFileInfo(dirname($destination)); if ($info && !$info->isUpdateable()) { - throw new Forbidden('No permissions to move object.'); + throw new Forbidden('No permissions to copy object.'); } // this will trigger existence check -- cgit v1.2.3