Merge pull request #1321 from nextcloud/backport-1301-fix-required-permissions-for-webdav-move-and-copy-9

[stable9] Fix required permissions for webdav move and copy

2 files changed, 49 insertions, 21 deletions

diff --git a/apps/dav/lib/connector/sabre/objecttree.php b/apps/dav/lib/connector/sabre/objecttree.php
index 24ce4acdf22..da17f1a268d 100644
--- a/apps/dav/lib/connector/sabre/objecttree.php
+++ b/apps/dav/lib/connector/sabre/objecttree.php
@@ -201,9 +201,18 @@ class ObjectTree extends \Sabre\DAV\Tree {
 		}
 
 		$infoDestination = $this->fileView->getFileInfo(dirname($destinationPath));
-		$infoSource = $this->fileView->getFileInfo($sourcePath);
-		$destinationPermission = $infoDestination && $infoDestination->isUpdateable();
-		$sourcePermission =  $infoSource && $infoSource->isDeletable();
+		if (dirname($destinationPath) === dirname($sourcePath)) {
+			$sourcePermission = $infoDestination && $infoDestination->isUpdateable();
+			$destinationPermission = $sourcePermission;
+		} else {
+			$infoSource = $this->fileView->getFileInfo($sourcePath);
+			if ($this->fileView->file_exists($destinationPath)) {
+				$destinationPermission = $infoDestination && $infoDestination->isUpdateable();
+			} else {
+				$destinationPermission = $infoDestination && $infoDestination->isCreatable();
+			}
+			$sourcePermission =  $infoSource && $infoSource->isDeletable();
+		}
 
 		if (!$destinationPermission || !$sourcePermission) {
 			throw new Forbidden('No permissions to move object.');
@@ -292,7 +301,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.');
 		}
 		
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);