diff --git a/lib/private/connector/sabre/filesplugin.php b/lib/private/connector/sabre/filesplugin.php index 84620f454aa7605172a12e566a134dadc5d2fe8a..ab7f6884a5e2900dc7dbe1364b0289ce6b9886da 100644 --- a/lib/private/connector/sabre/filesplugin.php +++ b/lib/private/connector/sabre/filesplugin.php @@ -63,11 +63,21 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin { */ private $isPublic; + /** + * @var \OC\Files\View + */ + private $fileView; + /** * @param \Sabre\DAV\Tree $tree + * @param \OC\Files\View $view + * @param bool $isPublic */ - public function __construct(\Sabre\DAV\Tree $tree, $isPublic = false) { + 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"); + } + } } /** diff --git a/lib/private/connector/sabre/serverfactory.php b/lib/private/connector/sabre/serverfactory.php index a0c32c1da5386aad220c57c77fc1d57bb157f888..893e29fd41cece9c52a1dd02517657c544a537ca 100644 --- a/lib/private/connector/sabre/serverfactory.php +++ b/lib/private/connector/sabre/serverfactory.php @@ -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()) { diff --git a/tests/lib/connector/sabre/filesplugin.php b/tests/lib/connector/sabre/filesplugin.php index a4cf9f7bfb98d38f56909375c4493feaab37f398..0aa62cab176498fe7ab741f7134d2b9b59b05da5 100644 --- a/tests/lib/connector/sabre/filesplugin.php +++ b/tests/lib/connector/sabre/filesplugin.php @@ -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'); + } }