From 3491400261c1454a9a30d3ec96969573330120cc Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 30 Jun 2016 15:05:13 +0200 Subject: [PATCH] add some additonal permission checks to the webdav backend --- apps/dav/lib/connector/sabre/objecttree.php | 33 ++++++++++++++++--- .../tests/unit/connector/sabre/objecttree.php | 20 +++++++++++ .../integration/features/bootstrap/WebDav.php | 18 +++++++++- .../features/webdav-related.feature | 32 ++++++++++++++++++ 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/apps/dav/lib/connector/sabre/objecttree.php b/apps/dav/lib/connector/sabre/objecttree.php index f38dfe679c..c952a68e9a 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) { @@ -184,16 +184,29 @@ 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 \Sabre\DAV\Exception\Forbidden * @return int + * @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 */ public function move($sourcePath, $destinationPath) { if (!$this->fileView) { 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('No permissions to move object.'); + } + $targetNodeExists = $this->nodeExists($destinationPath); $sourceNode = $this->getNodeForPath($sourcePath); if ($sourceNode instanceof \Sabre\DAV\ICollection && $targetNodeExists) { @@ -263,14 +276,24 @@ class ObjectTree extends \Sabre\DAV\Tree { * * @param string $source * @param string $destination + * @throws FileLocked + * @throws Forbidden + * @throws InvalidPath + * @throws \Exception + * @throws \Sabre\DAV\Exception\Locked + * @throws \Sabre\DAV\Exception\NotFound * @throws \Sabre\DAV\Exception\ServiceUnavailable - * @return void */ public function copy($source, $destination) { if (!$this->fileView) { throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup'); } + $info = $this->fileView->getFileInfo(dirname($destination)); + if ($info && !$info->isUpdateable()) { + throw new Forbidden('No permissions to copy object.'); + } + // this will trigger existence check $this->getNodeForPath($source); diff --git a/apps/dav/tests/unit/connector/sabre/objecttree.php b/apps/dav/tests/unit/connector/sabre/objecttree.php index e5e858ef17..5b0dcae21b 100644 --- a/apps/dav/tests/unit/connector/sabre/objecttree.php +++ b/apps/dav/tests/unit/connector/sabre/objecttree.php @@ -56,6 +56,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 +72,21 @@ class TestDoubleFileView extends \OC\Files\View { */ class ObjectTree 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 diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 0a4624ccc2..785c20325b 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -64,9 +64,25 @@ trait WebDav { $this->response = $this->makeDavRequest($user, "MOVE", $fileSource, $headers); } + + /** - * @When /^Downloading file "([^"]*)" with range "([^"]*)"$/ + * @When /^User "([^"]*)" copies file "([^"]*)" to "([^"]*)"$/ + * @param string $user + * @param string $fileSource + * @param string $fileDestination */ + public function userCopiesFileTo($user, $fileSource, $fileDestination) { + $fullUrl = substr($this->baseUrl, 0, -4) . $this->davPath; + $headers['Destination'] = $fullUrl . $fileDestination; + try { + $this->response = $this->makeDavRequest($user, 'COPY', $fileSource, $headers); + } catch (\GuzzleHttp\Exception\ClientException $e) { + // 4xx and 5xx responses cause an exception + $this->response = $e->getResponse(); + } + } + public function downloadFileWithRange($fileSource, $range){ $fullUrl = substr($this->baseUrl, 0, -4) . $this->davPath; $headers['Range'] = $range; diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index c6d2919db8..577c85564b 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -241,3 +241,35 @@ Feature: webdav-related | 0 | | 1 | | 3 | + + + Scenario: Copying files into a folder with edit permissions + Given using dav path "remote.php/webdav" + And user "user0" exists + And user "user1" exists + And As an "user1" + And user "user1" created a folder "/testcopypermissionsAllowed" + And as "user1" creating a share with + | path | testcopypermissionsAllowed | + | shareType | 0 | + | permissions | 31 | + | shareWith | user0 | + And User "user0" uploads file with content "copytest" to "/copytest.txt" + When User "user0" copies file "/copytest.txt" to "/testcopypermissionsAllowed/copytest.txt" + Then the HTTP status code should be "201" + + + Scenario: Copying files into a folder without edit permissions + Given using dav path "remote.php/webdav" + And user "user0" exists + And user "user1" exists + And As an "user1" + And user "user1" created a folder "/testcopypermissionsNotAllowed" + And as "user1" creating a share with + | path | testcopypermissionsNotAllowed | + | shareType | 0 | + | permissions | 1 | + | shareWith | user0 | + And User "user0" uploads file with content "copytest" to "/copytest.txt" + When User "user0" copies file "/copytest.txt" to "/testcopypermissionsNotAllowed/copytest.txt" + Then the HTTP status code should be "403"