[WEBDAV] check if delete of source is allowed on move
Fixes #5251 If we perform a move we need to make sure first that the source can be deleted. Else the dest might be cleared but the move will fail later. * Added unit tests Eventually we need more and better checking here.
This commit is contained in:
parent
cd818e7419
commit
1ee56c702d
3 changed files with 91 additions and 6 deletions
|
@ -64,10 +64,20 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin {
|
|||
private $isPublic;
|
||||
|
||||
/**
|
||||
* @param \Sabre\DAV\Tree $tree
|
||||
* @var \OC\Files\View
|
||||
*/
|
||||
public function __construct(\Sabre\DAV\Tree $tree, $isPublic = false) {
|
||||
private $fileView;
|
||||
|
||||
/**
|
||||
* @param \Sabre\DAV\Tree $tree
|
||||
* @param \OC\Files\View $view
|
||||
* @param bool $isPublic
|
||||
*/
|
||||
public function __construct(\Sabre\DAV\Tree $tree,
|
||||
\OC\Files\View $view,
|
||||
$isPublic = false) {
|
||||
$this->tree = $tree;
|
||||
$this->fileView = $view;
|
||||
$this->isPublic = $isPublic;
|
||||
}
|
||||
|
||||
|
@ -106,6 +116,26 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin {
|
|||
fclose($body);
|
||||
}
|
||||
});
|
||||
$this->server->on('beforeMove', [$this, 'checkMove']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Plugin that checks if a move can actually be performed.
|
||||
* @param string $source source path
|
||||
* @param string $destination destination path
|
||||
* @throws \Sabre\DAV\Exception\Forbidden
|
||||
*/
|
||||
function checkMove($source, $destination) {
|
||||
list($sourceDir,) = \Sabre\HTTP\URLUtil::splitPath($source);
|
||||
list($destinationDir,) = \Sabre\HTTP\URLUtil::splitPath($destination);
|
||||
|
||||
if ($sourceDir !== $destinationDir) {
|
||||
$sourceFileInfo = $this->fileView->getFileInfo($source);
|
||||
|
||||
if (!$sourceFileInfo->isDeletable()) {
|
||||
throw new \Sabre\DAV\Exception\Forbidden($source . " cannot be deleted");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -72,7 +72,6 @@ class ServerFactory {
|
|||
$server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend, $defaults->getName()));
|
||||
// FIXME: The following line is a workaround for legacy components relying on being able to send a GET to /
|
||||
$server->addPlugin(new \OC\Connector\Sabre\DummyGetResponsePlugin());
|
||||
$server->addPlugin(new \OC\Connector\Sabre\FilesPlugin($objectTree));
|
||||
$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav', $this->logger));
|
||||
$server->addPlugin(new \OC\Connector\Sabre\LockPlugin($objectTree));
|
||||
$server->addPlugin(new \OC\Connector\Sabre\ListenerPlugin($this->dispatcher));
|
||||
|
@ -91,6 +90,7 @@ class ServerFactory {
|
|||
}
|
||||
$objectTree->init($root, $view, $this->mountManager);
|
||||
|
||||
$server->addPlugin(new \OC\Connector\Sabre\FilesPlugin($objectTree, $view));
|
||||
$server->addPlugin(new \OC\Connector\Sabre\QuotaPlugin($view));
|
||||
|
||||
if($this->userSession->isLoggedIn()) {
|
||||
|
|
|
@ -31,13 +31,24 @@ class FilesPlugin extends \Test\TestCase {
|
|||
*/
|
||||
private $plugin;
|
||||
|
||||
/**
|
||||
* @var \OC\Files\View
|
||||
*/
|
||||
private $view;
|
||||
|
||||
public function setUp() {
|
||||
parent::setUp();
|
||||
$this->server = new \Sabre\DAV\Server();
|
||||
$this->server = $this->getMockBuilder('\Sabre\DAV\Server')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->tree = $this->getMockBuilder('\Sabre\DAV\Tree')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->plugin = new \OC\Connector\Sabre\FilesPlugin($this->tree);
|
||||
$this->view = $this->getMockBuilder('\OC\Files\View')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
|
||||
$this->plugin = new \OC\Connector\Sabre\FilesPlugin($this->tree, $this->view);
|
||||
$this->plugin->initialize($this->server);
|
||||
}
|
||||
|
||||
|
@ -104,7 +115,7 @@ class FilesPlugin extends \Test\TestCase {
|
|||
}
|
||||
|
||||
public function testGetPublicPermissions() {
|
||||
$this->plugin = new \OC\Connector\Sabre\FilesPlugin($this->tree, true);
|
||||
$this->plugin = new \OC\Connector\Sabre\FilesPlugin($this->tree, $this->view, true);
|
||||
$this->plugin->initialize($this->server);
|
||||
|
||||
$propFind = new \Sabre\DAV\PropFind(
|
||||
|
@ -196,4 +207,48 @@ class FilesPlugin extends \Test\TestCase {
|
|||
$this->assertEquals(200, $result[self::GETETAG_PROPERTYNAME]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Testcase from https://github.com/owncloud/core/issues/5251
|
||||
*
|
||||
* |-FolderA
|
||||
* |-text.txt
|
||||
* |-test.txt
|
||||
*
|
||||
* FolderA is an incomming shared folder and there are no delete permissions.
|
||||
* Thus moving /FolderA/test.txt to /test.txt should fail already on that check
|
||||
*
|
||||
* @expectedException \Sabre\DAV\Exception\Forbidden
|
||||
* @expectedExceptionMessage FolderA/test.txt cannot be deleted
|
||||
*/
|
||||
public function testMoveSrcNotDeletable() {
|
||||
$fileInfoFolderATestTXT = $this->getMockBuilder('\OCP\Files\FileInfo')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$fileInfoFolderATestTXT->expects($this->once())
|
||||
->method('isDeletable')
|
||||
->willReturn(false);
|
||||
|
||||
$this->view->expects($this->once())
|
||||
->method('getFileInfo')
|
||||
->with('FolderA/test.txt')
|
||||
->willReturn($fileInfoFolderATestTXT);
|
||||
|
||||
$this->plugin->checkMove('FolderA/test.txt', 'test.txt');
|
||||
}
|
||||
|
||||
public function testMoveSrcDeletable() {
|
||||
$fileInfoFolderATestTXT = $this->getMockBuilder('\OCP\Files\FileInfo')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$fileInfoFolderATestTXT->expects($this->once())
|
||||
->method('isDeletable')
|
||||
->willReturn(true);
|
||||
|
||||
$this->view->expects($this->once())
|
||||
->method('getFileInfo')
|
||||
->with('FolderA/test.txt')
|
||||
->willReturn($fileInfoFolderATestTXT);
|
||||
|
||||
$this->plugin->checkMove('FolderA/test.txt', 'test.txt');
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue