From e2cfcd992cf6f4bb5f1cdb9070d3d0ea2a1504e2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 13 Nov 2015 14:13:16 +0100 Subject: [PATCH] Allow storage wrappers to through a forbidden exception with retry information --- apps/dav/lib/connector/sabre/directory.php | 8 +++ .../connector/sabre/exception/forbidden.php | 64 +++++++++++++++++++ apps/dav/lib/connector/sabre/file.php | 12 ++++ apps/dav/lib/connector/sabre/objecttree.php | 6 ++ .../tests/unit/connector/sabre/directory.php | 21 ++++++ .../sabre/exception/forbiddentest.php | 44 +++++++++++++ .../sabre/exception/invalidpathtest.php | 10 +-- apps/dav/tests/unit/connector/sabre/file.php | 46 +++++++++++++ lib/private/cache/file.php | 2 + lib/private/files.php | 5 ++ lib/private/files/cache/scanner.php | 2 + lib/public/files/forbiddenexception.php | 55 ++++++++++++++++ 12 files changed, 270 insertions(+), 5 deletions(-) create mode 100644 apps/dav/lib/connector/sabre/exception/forbidden.php create mode 100644 apps/dav/tests/unit/connector/sabre/exception/forbiddentest.php create mode 100644 lib/public/files/forbiddenexception.php diff --git a/apps/dav/lib/connector/sabre/directory.php b/apps/dav/lib/connector/sabre/directory.php index 8c736ea010..b602dd2f7b 100644 --- a/apps/dav/lib/connector/sabre/directory.php +++ b/apps/dav/lib/connector/sabre/directory.php @@ -28,8 +28,10 @@ */ namespace OCA\DAV\Connector\Sabre; +use OCA\DAV\Connector\Sabre\Exception\Forbidden; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; use OCA\DAV\Connector\Sabre\Exception\FileLocked; +use OCP\Files\ForbiddenException; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; use Sabre\DAV\Exception\Locked; @@ -117,6 +119,8 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); } catch (\OCP\Files\InvalidPathException $ex) { throw new InvalidPath($ex->getMessage()); + } catch (ForbiddenException $ex) { + throw new Forbidden($ex->getMessage(), $ex->getRetry()); } catch (LockedException $e) { throw new FileLocked($e->getMessage(), $e->getCode(), $e); } @@ -146,6 +150,8 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); } catch (\OCP\Files\InvalidPathException $ex) { throw new InvalidPath($ex->getMessage()); + } catch (ForbiddenException $ex) { + throw new Forbidden($ex->getMessage(), $ex->getRetry()); } catch (LockedException $e) { throw new FileLocked($e->getMessage(), $e->getCode(), $e); } @@ -247,6 +253,8 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node // assume it wasn't possible to remove due to permission issue throw new \Sabre\DAV\Exception\Forbidden(); } + } catch (ForbiddenException $ex) { + throw new Forbidden($ex->getMessage(), $ex->getRetry()); } catch (LockedException $e) { throw new FileLocked($e->getMessage(), $e->getCode(), $e); } diff --git a/apps/dav/lib/connector/sabre/exception/forbidden.php b/apps/dav/lib/connector/sabre/exception/forbidden.php new file mode 100644 index 0000000000..673958349f --- /dev/null +++ b/apps/dav/lib/connector/sabre/exception/forbidden.php @@ -0,0 +1,64 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\DAV\Connector\Sabre\Exception; + +class Forbidden extends \Sabre\DAV\Exception\Forbidden { + + const NS_OWNCLOUD = 'http://owncloud.org/ns'; + + /** + * @var bool + */ + private $retry; + + /** + * @param string $message + * @param bool $retry + * @param \Exception $previous + */ + public function __construct($message, $retry = false, \Exception $previous = null) { + parent::__construct($message, 0, $previous); + $this->retry = $retry; + } + + /** + * This method allows the exception to include additional information + * into the WebDAV error response + * + * @param \Sabre\DAV\Server $server + * @param \DOMElement $errorNode + * @return void + */ + public function serialize(\Sabre\DAV\Server $server,\DOMElement $errorNode) { + + // set ownCloud namespace + $errorNode->setAttribute('xmlns:o', self::NS_OWNCLOUD); + + // adding the retry node + $error = $errorNode->ownerDocument->createElementNS('o:','o:retry', var_export($this->retry, true)); + $errorNode->appendChild($error); + + // adding the message node + $error = $errorNode->ownerDocument->createElementNS('o:','o:reason', $this->getMessage()); + $errorNode->appendChild($error); + } +} diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index 961532daf5..a0c35fb2ba 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -35,9 +35,11 @@ namespace OCA\DAV\Connector\Sabre; use OC\Files\Filesystem; use OCA\DAV\Connector\Sabre\Exception\EntityTooLarge; use OCA\DAV\Connector\Sabre\Exception\FileLocked; +use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException; use OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType; use OCP\Encryption\Exceptions\GenericEncryptionException; use OCP\Files\EntityTooLargeException; +use OCP\Files\ForbiddenException; use OCP\Files\InvalidContentException; use OCP\Files\InvalidPathException; use OCP\Files\LockNotAcquiredException; @@ -175,6 +177,8 @@ class File extends Node implements IFile { \OCP\Util::writeLog('webdav', 'renaming part file to final file failed', \OCP\Util::ERROR); throw new Exception('Could not rename part file to final file'); } + } catch (ForbiddenException $ex) { + throw new DAVForbiddenException($ex->getMessage(), $ex->getRetry()); } catch (\Exception $e) { $partStorage->unlink($internalPartPath); $this->convertToSabreException($e); @@ -273,6 +277,8 @@ class File extends Node implements IFile { throw new ServiceUnavailable("Encryption not ready: " . $e->getMessage()); } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to open file: " . $e->getMessage()); + } catch (ForbiddenException $ex) { + throw new DAVForbiddenException($ex->getMessage(), $ex->getRetry()); } catch (LockedException $e) { throw new FileLocked($e->getMessage(), $e->getCode(), $e); } @@ -296,6 +302,8 @@ class File extends Node implements IFile { } } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to unlink: " . $e->getMessage()); + } catch (ForbiddenException $ex) { + throw new DAVForbiddenException($ex->getMessage(), $ex->getRetry()); } catch (LockedException $e) { throw new FileLocked($e->getMessage(), $e->getCode(), $e); } @@ -474,6 +482,10 @@ class File extends Node implements IFile { // a more general case - due to whatever reason the content could not be written throw new Forbidden($e->getMessage(), 0, $e); } + if ($e instanceof ForbiddenException) { + // the path for the file was forbidden + throw new DAVForbiddenException($e->getMessage(), $e->getRetry(), $e); + } if ($e instanceof EntityTooLargeException) { // the file is too big to be stored throw new EntityTooLarge($e->getMessage(), 0, $e); diff --git a/apps/dav/lib/connector/sabre/objecttree.php b/apps/dav/lib/connector/sabre/objecttree.php index 80c0ef7461..2e9c1b9916 100644 --- a/apps/dav/lib/connector/sabre/objecttree.php +++ b/apps/dav/lib/connector/sabre/objecttree.php @@ -25,10 +25,12 @@ namespace OCA\DAV\Connector\Sabre; +use OCA\DAV\Connector\Sabre\Exception\Forbidden; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; use OCA\DAV\Connector\Sabre\Exception\FileLocked; use OC\Files\FileInfo; use OC\Files\Mount\MoveableMount; +use OCP\Files\ForbiddenException; use OCP\Files\StorageInvalidException; use OCP\Files\StorageNotAvailableException; use OCP\Lock\LockedException; @@ -235,6 +237,8 @@ class ObjectTree extends \Sabre\DAV\Tree { } } catch (StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); + } catch (ForbiddenException $ex) { + throw new Forbidden($ex->getMessage(), $ex->getRetry()); } catch (LockedException $e) { throw new FileLocked($e->getMessage(), $e->getCode(), $e); } @@ -274,6 +278,8 @@ class ObjectTree extends \Sabre\DAV\Tree { $this->fileView->copy($source, $destination); } catch (StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); + } catch (ForbiddenException $ex) { + throw new Forbidden($ex->getMessage(), $ex->getRetry()); } catch (LockedException $e) { throw new FileLocked($e->getMessage(), $e->getCode(), $e); } diff --git a/apps/dav/tests/unit/connector/sabre/directory.php b/apps/dav/tests/unit/connector/sabre/directory.php index 148a91d26d..75c4828641 100644 --- a/apps/dav/tests/unit/connector/sabre/directory.php +++ b/apps/dav/tests/unit/connector/sabre/directory.php @@ -9,6 +9,8 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre; +use OCP\Files\ForbiddenException; + class Directory extends \Test\TestCase { /** @var \OC\Files\View | \PHPUnit_Framework_MockObject_MockObject */ @@ -48,6 +50,25 @@ class Directory extends \Test\TestCase { $dir->delete(); } + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testDeleteForbidden() { + // deletion allowed + $this->info->expects($this->once()) + ->method('isDeletable') + ->will($this->returnValue(true)); + + // but fails + $this->view->expects($this->once()) + ->method('rmdir') + ->with('sub') + ->willThrowException(new ForbiddenException('', true)); + + $dir = $this->getDir('sub'); + $dir->delete(); + } + /** * */ diff --git a/apps/dav/tests/unit/connector/sabre/exception/forbiddentest.php b/apps/dav/tests/unit/connector/sabre/exception/forbiddentest.php new file mode 100644 index 0000000000..19799c71b9 --- /dev/null +++ b/apps/dav/tests/unit/connector/sabre/exception/forbiddentest.php @@ -0,0 +1,44 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\DAV\Tests\Unit\Connector\Sabre\Exception; + +use OCA\DAV\Connector\Sabre\Exception\Forbidden; + +class ForbiddenTest extends \Test\TestCase { + + public function testSerialization() { + + // create xml doc + $DOM = new \DOMDocument('1.0','utf-8'); + $DOM->formatOutput = true; + $error = $DOM->createElementNS('DAV:','d:error'); + $error->setAttribute('xmlns:s', \Sabre\DAV\Server::NS_SABREDAV); + $DOM->appendChild($error); + + // serialize the exception + $message = "1234567890"; + $retry = false; + $expectedXml = << + + false + 1234567890 + + +EOD; + + $ex = new Forbidden($message, $retry); + $server = $this->getMock('Sabre\DAV\Server'); + $ex->serialize($server, $error); + + // assert + $xml = $DOM->saveXML(); + $this->assertEquals($expectedXml, $xml); + } +} diff --git a/apps/dav/tests/unit/connector/sabre/exception/invalidpathtest.php b/apps/dav/tests/unit/connector/sabre/exception/invalidpathtest.php index 19e82320d5..4296a4d561 100644 --- a/apps/dav/tests/unit/connector/sabre/exception/invalidpathtest.php +++ b/apps/dav/tests/unit/connector/sabre/exception/invalidpathtest.php @@ -1,15 +1,15 @@ * This file is licensed under the Affero General Public License version 3 or * later. * See the COPYING-README file. */ + +namespace OCA\DAV\Tests\Unit\Connector\Sabre\Exception; + +use OCA\DAV\Connector\Sabre\Exception\InvalidPath; + class InvalidPathTest extends \Test\TestCase { public function testSerialization() { diff --git a/apps/dav/tests/unit/connector/sabre/file.php b/apps/dav/tests/unit/connector/sabre/file.php index 94dadf88fe..399634f8be 100644 --- a/apps/dav/tests/unit/connector/sabre/file.php +++ b/apps/dav/tests/unit/connector/sabre/file.php @@ -9,6 +9,7 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre; use OC\Files\Storage\Local; +use OCP\Files\ForbiddenException; use Test\HookHelper; use OC\Files\Filesystem; use OCP\Lock\ILockingProvider; @@ -72,6 +73,10 @@ class File extends \Test\TestCase { new \OCP\Files\InvalidPathException(), 'Sabre\DAV\Exception\Forbidden' ], + [ + new \OCP\Files\ForbiddenException('', true), + 'OCA\DAV\Connector\Sabre\Exception\Forbidden' + ], [ new \OCP\Files\LockNotAcquiredException('/test.txt', 1), 'OCA\DAV\Connector\Sabre\Exception\FileLocked' @@ -690,6 +695,29 @@ class File extends \Test\TestCase { $file->delete(); } + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testDeleteThrowsWhenDeletionThrows() { + // setup + $view = $this->getMock('\OC\Files\View', + array()); + + // but fails + $view->expects($this->once()) + ->method('unlink') + ->willThrowException(new ForbiddenException('', true)); + + $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + 'permissions' => \OCP\Constants::PERMISSION_ALL + ), null); + + $file = new \OCA\DAV\Connector\Sabre\File($view, $info); + + // action + $file->delete(); + } + /** * Asserts hook call * @@ -835,4 +863,22 @@ class File extends \Test\TestCase { $file->get(); } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testGetFopenThrows() { + $view = $this->getMock('\OC\Files\View', ['fopen'], array()); + $view->expects($this->atLeastOnce()) + ->method('fopen') + ->willThrowException(new ForbiddenException('', true)); + + $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + 'permissions' => \OCP\Constants::PERMISSION_ALL + ), null); + + $file = new \OCA\DAV\Connector\Sabre\File($view, $info); + + $file->get(); + } } diff --git a/lib/private/cache/file.php b/lib/private/cache/file.php index 1cda05f28e..31d4718d18 100644 --- a/lib/private/cache/file.php +++ b/lib/private/cache/file.php @@ -185,6 +185,8 @@ class File implements ICache { } catch (\OCP\Lock\LockedException $e) { // ignore locked chunks \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core')); + } catch (\OCP\Files\ForbiddenException $e) { + \OC::$server->getLogger()->debug('Could not cleanup forbidden chunk "' . $file . '"', array('app' => 'core')); } catch (\OCP\Files\LockNotAcquiredException $e) { \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core')); } diff --git a/lib/private/files.php b/lib/private/files.php index 9be5fc9a12..af10f3e1e3 100644 --- a/lib/private/files.php +++ b/lib/private/files.php @@ -142,6 +142,11 @@ class OC_Files { $l = \OC::$server->getL10N('core'); $hint = method_exists($ex, 'getHint') ? $ex->getHint() : ''; \OC_Template::printErrorPage($l->t('File is currently busy, please try again later'), $hint); + } catch (\OCP\Files\ForbiddenException $ex) { + self::unlockAllTheFiles($dir, $files, $getType, $view, $filename); + OC::$server->getLogger()->logException($ex); + $l = \OC::$server->getL10N('core'); + \OC_Template::printErrorPage($l->t('Can\'t read file'), $ex->getMessage()); } catch (\Exception $ex) { self::unlockAllTheFiles($dir, $files, $getType, $view, $filename); OC::$server->getLogger()->logException($ex); diff --git a/lib/private/files/cache/scanner.php b/lib/private/files/cache/scanner.php index 35043029dc..983e12d763 100644 --- a/lib/private/files/cache/scanner.php +++ b/lib/private/files/cache/scanner.php @@ -432,6 +432,8 @@ class Scanner extends BasicEmitter { // skip unavailable storages } catch (\OCP\Files\StorageNotAvailableException $e) { // skip unavailable storages + } catch (\OCP\Files\ForbiddenException $e) { + // skip forbidden storages } catch (\OCP\Lock\LockedException $e) { // skip unavailable storages } diff --git a/lib/public/files/forbiddenexception.php b/lib/public/files/forbiddenexception.php new file mode 100644 index 0000000000..13490c6eae --- /dev/null +++ b/lib/public/files/forbiddenexception.php @@ -0,0 +1,55 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +// use OCP namespace for all classes that are considered public. +// This means that they should be used by apps instead of the internal ownCloud classes +namespace OCP\Files; + +/** + * Class ForbiddenException + * + * @package OCP\Files + * @since 9.0.0 + */ +class ForbiddenException extends \Exception { + + /** @var bool */ + private $retry; + + /** + * @param string $message + * @param bool $retry + * @param \Exception $previous previous exception for cascading + * @since 9.0.0 + */ + public function __construct($message, $retry, \Exception $previous = null) { + parent::__construct($message, 0, $previous); + $this->retry = $retry; + } + + /** + * @return bool + * @since 9.0.0 + */ + public function getRetry() { + return (bool) $this->retry; + } +}