Merge pull request #11215 from owncloud/dav-throwwhendeletefailed

WebDAV now throws 403 when deletion did not work

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();
+		}
 
 	}
 

lib/private/connector/sabre/file.php

@@ -167,7 +167,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();

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();
+	}
 }

tests/lib/connector/sabre/file.php

@@ -143,4 +143,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();
+	}
 }