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
This commit is contained in:
commit
0ffb5e075b
|
@ -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.');
|
||||
}
|
||||
|
||||
|
|
|
@ -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);
|
||||
|
||||
|
|
Loading…
Reference in New Issue