From cb39c55073ebf7bc64e4bf0b7a29e2061f6cbbca Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 22 Sep 2014 12:19:34 +0200 Subject: [PATCH] WebDAV now throws 403 when deletion did not work Assume a permission issue whenever a file could not be deleted. This is because some storages are not able to return permissions, so a permission denied situation can only be triggered during direct deletion. --- lib/private/connector/sabre/directory.php | 5 +- lib/private/connector/sabre/file.php | 6 ++- tests/lib/connector/sabre/directory.php | 64 +++++++++++++++++++++-- tests/lib/connector/sabre/file.php | 63 ++++++++++++++++++++++ 4 files changed, 131 insertions(+), 7 deletions(-) diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index 9904c3525c4..8815a261db8 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -192,7 +192,10 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node throw new \Sabre\DAV\Exception\Forbidden(); } - $this->fileView->rmdir($this->path); + if (!$this->fileView->rmdir($this->path)) { + // assume it wasn't possible to remove due to permission issue + throw new \Sabre\DAV\Exception\Forbidden(); + } } diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 9b9a4596542..13b33382165 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -166,7 +166,11 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements \Sabre\ if (!$this->info->isDeletable()) { throw new \Sabre\DAV\Exception\Forbidden(); } - $this->fileView->unlink($this->path); + + if (!$this->fileView->unlink($this->path)) { + // assume it wasn't possible to delete due to permissions + throw new \Sabre\DAV\Exception\Forbidden(); + } // remove properties $this->removeProperties(); diff --git a/tests/lib/connector/sabre/directory.php b/tests/lib/connector/sabre/directory.php index 8a1550ffa95..453d8e8d42a 100644 --- a/tests/lib/connector/sabre/directory.php +++ b/tests/lib/connector/sabre/directory.php @@ -8,18 +8,24 @@ */ class Test_OC_Connector_Sabre_Directory extends PHPUnit_Framework_TestCase { + private $view; + private $info; + + public function setUp() { + $this->view = $this->getMock('OC\Files\View', array(), array(), '', false); + $this->info = $this->getMock('OC\Files\FileInfo', array(), array(), '', false); + } + private function getRootDir() { - $view = $this->getMock('OC\Files\View', array(), array(), '', false); - $view->expects($this->once()) + $this->view->expects($this->once()) ->method('getRelativePath') ->will($this->returnValue('')); - $info = $this->getMock('OC\Files\FileInfo', array(), array(), '', false); - $info->expects($this->once()) + $this->info->expects($this->once()) ->method('getPath') ->will($this->returnValue('')); - return new OC_Connector_Sabre_Directory($view, $info); + return new OC_Connector_Sabre_Directory($this->view, $this->info); } /** @@ -45,4 +51,52 @@ class Test_OC_Connector_Sabre_Directory extends PHPUnit_Framework_TestCase { $dir = $this->getRootDir(); $dir->delete(); } + + /** + * + */ + public function testDeleteFolderWhenAllowed() { + // deletion allowed + $this->info->expects($this->once()) + ->method('isDeletable') + ->will($this->returnValue(true)); + + // but fails + $this->view->expects($this->once()) + ->method('rmdir') + ->will($this->returnValue(true)); + + $dir = $this->getRootDir(); + $dir->delete(); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testDeleteFolderFailsWhenNotAllowed() { + $this->info->expects($this->once()) + ->method('isDeletable') + ->will($this->returnValue(false)); + + $dir = $this->getRootDir(); + $dir->delete(); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testDeleteFolderThrowsWhenDeletionFailed() { + // deletion allowed + $this->info->expects($this->once()) + ->method('isDeletable') + ->will($this->returnValue(true)); + + // but fails + $this->view->expects($this->once()) + ->method('rmdir') + ->will($this->returnValue(false)); + + $dir = $this->getRootDir(); + $dir->delete(); + } } diff --git a/tests/lib/connector/sabre/file.php b/tests/lib/connector/sabre/file.php index acc2634fcaf..2ce69d7499c 100644 --- a/tests/lib/connector/sabre/file.php +++ b/tests/lib/connector/sabre/file.php @@ -145,4 +145,67 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase { // action $file->put('test data'); } + + /** + * + */ + public function testDeleteWhenAllowed() { + // setup + $view = $this->getMock('\OC\Files\View', + array()); + + $view->expects($this->once()) + ->method('unlink') + ->will($this->returnValue(true)); + + $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + 'permissions' => \OCP\PERMISSION_ALL + )); + + $file = new OC_Connector_Sabre_File($view, $info); + + // action + $file->delete(); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testDeleteThrowsWhenDeletionNotAllowed() { + // setup + $view = $this->getMock('\OC\Files\View', + array()); + + $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + 'permissions' => 0 + )); + + $file = new OC_Connector_Sabre_File($view, $info); + + // action + $file->delete(); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testDeleteThrowsWhenDeletionFailed() { + // setup + $view = $this->getMock('\OC\Files\View', + array()); + + // but fails + $view->expects($this->once()) + ->method('unlink') + ->will($this->returnValue(false)); + + $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + 'permissions' => \OCP\PERMISSION_ALL + )); + + $file = new OC_Connector_Sabre_File($view, $info); + + // action + $file->delete(); + } } -- 2.39.5