From 338ce725c70e17aa4d9fccce5a9e926098fa101c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 7 Sep 2016 11:10:48 +0200 Subject: Only require CREATE permissions when the file does not exist yet --- apps/dav/lib/connector/sabre/objecttree.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/connector/sabre/objecttree.php b/apps/dav/lib/connector/sabre/objecttree.php index 24ce4acdf22..adf55f81bb0 100644 --- a/apps/dav/lib/connector/sabre/objecttree.php +++ b/apps/dav/lib/connector/sabre/objecttree.php @@ -202,7 +202,11 @@ class ObjectTree extends \Sabre\DAV\Tree { $infoDestination = $this->fileView->getFileInfo(dirname($destinationPath)); $infoSource = $this->fileView->getFileInfo($sourcePath); - $destinationPermission = $infoDestination && $infoDestination->isUpdateable(); + if ($this->fileView->file_exists($destinationPath)) { + $destinationPermission = $infoDestination && $infoDestination->isUpdateable(); + } else { + $destinationPermission = $infoDestination && $infoDestination->isCreatable(); + } $sourcePermission = $infoSource && $infoSource->isDeletable(); if (!$destinationPermission || !$sourcePermission) { @@ -292,7 +296,12 @@ class ObjectTree extends \Sabre\DAV\Tree { } $info = $this->fileView->getFileInfo(dirname($destination)); - if ($info && !$info->isUpdateable()) { + if ($this->fileView->file_exists($destination)) { + $destinationPermission = $info && $info->isUpdateable(); + } else { + $destinationPermission = $info && $info->isCreatable(); + } + if (!$destinationPermission) { throw new Forbidden('No permissions to copy object.'); } -- cgit v1.2.3 From a69fe583f847e7e8d8ab1fd5cb07d528627df343 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 7 Sep 2016 11:22:57 +0200 Subject: UPDATE permissions qualify for renaming a node --- apps/dav/lib/connector/sabre/objecttree.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/dav/lib/connector/sabre/objecttree.php b/apps/dav/lib/connector/sabre/objecttree.php index adf55f81bb0..da17f1a268d 100644 --- a/apps/dav/lib/connector/sabre/objecttree.php +++ b/apps/dav/lib/connector/sabre/objecttree.php @@ -201,13 +201,18 @@ class ObjectTree extends \Sabre\DAV\Tree { } $infoDestination = $this->fileView->getFileInfo(dirname($destinationPath)); - $infoSource = $this->fileView->getFileInfo($sourcePath); - if ($this->fileView->file_exists($destinationPath)) { - $destinationPermission = $infoDestination && $infoDestination->isUpdateable(); + if (dirname($destinationPath) === dirname($sourcePath)) { + $sourcePermission = $infoDestination && $infoDestination->isUpdateable(); + $destinationPermission = $sourcePermission; } else { - $destinationPermission = $infoDestination && $infoDestination->isCreatable(); + $infoSource = $this->fileView->getFileInfo($sourcePath); + if ($this->fileView->file_exists($destinationPath)) { + $destinationPermission = $infoDestination && $infoDestination->isUpdateable(); + } else { + $destinationPermission = $infoDestination && $infoDestination->isCreatable(); + } + $sourcePermission = $infoSource && $infoSource->isDeletable(); } - $sourcePermission = $infoSource && $infoSource->isDeletable(); if (!$destinationPermission || !$sourcePermission) { throw new Forbidden('No permissions to move object.'); -- cgit v1.2.3 From 86a673a05a30750cef8d250a6f558e201051153e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 7 Sep 2016 18:26:44 +0200 Subject: Fix tests --- apps/dav/tests/unit/connector/sabre/objecttree.php | 48 ++++++++++++++-------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/apps/dav/tests/unit/connector/sabre/objecttree.php b/apps/dav/tests/unit/connector/sabre/objecttree.php index 0d3bdbd9c24..7eb24ab9159 100644 --- a/apps/dav/tests/unit/connector/sabre/objecttree.php +++ b/apps/dav/tests/unit/connector/sabre/objecttree.php @@ -34,22 +34,23 @@ use OC\Files\Storage\Temporary; class TestDoubleFileView extends \OC\Files\View { - public function __construct($updatables, $deletables, $canRename = true) { + public function __construct($creatables, $updatables, $deletables, $canRename = true) { + $this->creatables = $creatables; $this->updatables = $updatables; $this->deletables = $deletables; $this->canRename = $canRename; } public function isUpdatable($path) { - return $this->updatables[$path]; + return !empty($this->updatables[$path]); } public function isCreatable($path) { - return $this->updatables[$path]; + return !empty($this->creatables[$path]); } public function isDeletable($path) { - return $this->deletables[$path]; + return !empty($this->deletables[$path]); } public function rename($path1, $path2) { @@ -62,7 +63,11 @@ class TestDoubleFileView extends \OC\Files\View { public function getFileInfo($path, $includeMountPoints = true) { $objectTreeTest = new ObjectTree(); - return $objectTreeTest->getFileInfoMock(); + return $objectTreeTest->getFileInfoMock( + $this->isCreatable($path), + $this->isUpdatable($path), + $this->isDeletable($path) + ); } } @@ -75,16 +80,22 @@ class TestDoubleFileView extends \OC\Files\View { */ class ObjectTree extends \Test\TestCase { - public function getFileInfoMock() { - $mock = $this->getMock('\OCP\Files\FileInfo'); + public function getFileInfoMock($create = true, $update = true, $delete = true) { + $mock = $this->getMockBuilder('\OCP\Files\FileInfo') + ->disableOriginalConstructor() + ->getMock(); $mock ->expects($this->any()) - ->method('isDeletable') - ->willReturn(true); + ->method('isCreatable') + ->willReturn($create); $mock ->expects($this->any()) ->method('isUpdateable') - ->willReturn(true); + ->willReturn($update); + $mock + ->expects($this->any()) + ->method('isDeletable') + ->willReturn($delete); return $mock; } @@ -95,14 +106,14 @@ class ObjectTree extends \Test\TestCase { * @expectedException \Sabre\DAV\Exception\Forbidden */ public function testMoveFailed($source, $destination, $updatables, $deletables) { - $this->moveTest($source, $destination, $updatables, $deletables); + $this->moveTest($source, $destination, $updatables, $updatables, $deletables, true); } /** * @dataProvider moveSuccessProvider */ public function testMoveSuccess($source, $destination, $updatables, $deletables) { - $this->moveTest($source, $destination, $updatables, $deletables); + $this->moveTest($source, $destination, $updatables, $updatables, $deletables); $this->assertTrue(true); } @@ -111,7 +122,7 @@ class ObjectTree extends \Test\TestCase { * @expectedException \OCA\DAV\Connector\Sabre\Exception\InvalidPath */ public function testMoveFailedInvalidChars($source, $destination, $updatables, $deletables) { - $this->moveTest($source, $destination, $updatables, $deletables); + $this->moveTest($source, $destination, $updatables, $updatables, $deletables); } function moveFailedInvalidCharsProvider() { @@ -142,10 +153,13 @@ class ObjectTree extends \Test\TestCase { /** * @param $source * @param $destination + * @param $creatables * @param $updatables + * @param $deletables + * @param $throwsBeforeGetNode */ - private function moveTest($source, $destination, $updatables, $deletables) { - $view = new TestDoubleFileView($updatables, $deletables); + private function moveTest($source, $destination, $creatables, $updatables, $deletables, $throwsBeforeGetNode = false) { + $view = new TestDoubleFileView($creatables, $updatables, $deletables); $info = new FileInfo('', null, null, array(), null); @@ -154,7 +168,7 @@ class ObjectTree extends \Test\TestCase { array('nodeExists', 'getNodeForPath'), array($rootDir, $view)); - $objectTree->expects($this->once()) + $objectTree->expects($throwsBeforeGetNode ? $this->never() : $this->once()) ->method('getNodeForPath') ->with($this->identicalTo($source)) ->will($this->returnValue(false)); @@ -345,7 +359,7 @@ class ObjectTree extends \Test\TestCase { $updatables = array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false); $deletables = array('a/b' => true); - $view = new TestDoubleFileView($updatables, $deletables); + $view = new TestDoubleFileView($updatables, $updatables, $deletables); $info = new FileInfo('', null, null, array(), null); -- cgit v1.2.3