From cc132a80306a5114bd65789fd03883c008ef7c77 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 7 Jan 2022 14:14:49 +0100 Subject: [PATCH] Add sabre plugin to block dav bind/unbind on the hidden folder Signed-off-by: Robin Appelman --- apps/dav/composer/composer/ClassLoader.php | 2 +- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/composer/composer/installed.php | 4 +- .../dav/lib/Connector/Sabre/ServerFactory.php | 1 + apps/dav/lib/Files/HiddenFolderPlugin.php | 44 +++++++++++++ apps/dav/lib/Server.php | 1 + .../features/bootstrap/CommandLineContext.php | 9 +++ .../integration/features/bootstrap/WebDav.php | 65 +++++++++++++++---- .../features/hidden-folder.feature | 33 ++++++++++ 10 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 apps/dav/lib/Files/HiddenFolderPlugin.php create mode 100644 build/integration/features/hidden-folder.feature diff --git a/apps/dav/composer/composer/ClassLoader.php b/apps/dav/composer/composer/ClassLoader.php index 0cd6055d1b794..afef3fa2ad83f 100644 --- a/apps/dav/composer/composer/ClassLoader.php +++ b/apps/dav/composer/composer/ClassLoader.php @@ -149,7 +149,7 @@ public function getFallbackDirsPsr4() /** * @return string[] Array of classname => path - * @psalm-var array + * @psalm-return array */ public function getClassMap() { diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index fa83c05578d6f..5399e7d5569bb 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -225,6 +225,7 @@ 'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => $baseDir . '/../lib/Files/BrowserErrorPagePlugin.php', 'OCA\\DAV\\Files\\FileSearchBackend' => $baseDir . '/../lib/Files/FileSearchBackend.php', 'OCA\\DAV\\Files\\FilesHome' => $baseDir . '/../lib/Files/FilesHome.php', + 'OCA\\DAV\\Files\\HiddenFolderPlugin' => $baseDir . '/../lib/Files/HiddenFolderPlugin.php', 'OCA\\DAV\\Files\\LazySearchBackend' => $baseDir . '/../lib/Files/LazySearchBackend.php', 'OCA\\DAV\\Files\\RootCollection' => $baseDir . '/../lib/Files/RootCollection.php', 'OCA\\DAV\\Files\\Sharing\\FilesDropPlugin' => $baseDir . '/../lib/Files/Sharing/FilesDropPlugin.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 8f8ca0da10b2e..6e17756bfdfcf 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -240,6 +240,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/BrowserErrorPagePlugin.php', 'OCA\\DAV\\Files\\FileSearchBackend' => __DIR__ . '/..' . '/../lib/Files/FileSearchBackend.php', 'OCA\\DAV\\Files\\FilesHome' => __DIR__ . '/..' . '/../lib/Files/FilesHome.php', + 'OCA\\DAV\\Files\\HiddenFolderPlugin' => __DIR__ . '/..' . '/../lib/Files/HiddenFolderPlugin.php', 'OCA\\DAV\\Files\\LazySearchBackend' => __DIR__ . '/..' . '/../lib/Files/LazySearchBackend.php', 'OCA\\DAV\\Files\\RootCollection' => __DIR__ . '/..' . '/../lib/Files/RootCollection.php', 'OCA\\DAV\\Files\\Sharing\\FilesDropPlugin' => __DIR__ . '/..' . '/../lib/Files/Sharing/FilesDropPlugin.php', diff --git a/apps/dav/composer/composer/installed.php b/apps/dav/composer/composer/installed.php index 5440719fa40ef..9eef9501ab92a 100644 --- a/apps/dav/composer/composer/installed.php +++ b/apps/dav/composer/composer/installed.php @@ -5,7 +5,7 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), - 'reference' => 'c6429e6cd19c57582364338362e543580821cf99', + 'reference' => '50f7b50a261ac99413cfd0f02582b51e895be12b', 'name' => '__root__', 'dev' => false, ), @@ -16,7 +16,7 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), - 'reference' => 'c6429e6cd19c57582364338362e543580821cf99', + 'reference' => '50f7b50a261ac99413cfd0f02582b51e895be12b', 'dev_requirement' => false, ), ), diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 7be24014881c9..7bbe3e3f37e83 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -130,6 +130,7 @@ public function createServer($baseUri, $server->addPlugin(new \OCA\DAV\Connector\Sabre\DummyGetResponsePlugin()); $server->addPlugin(new \OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin('webdav', $this->logger)); $server->addPlugin(new \OCA\DAV\Connector\Sabre\LockPlugin()); + $server->addPlugin(new \OCA\DAV\Files\HiddenFolderPlugin()); // Some WebDAV clients do require Class 2 WebDAV support (locking), since // we do not provide locking we emulate it using a fake locking plugin. if ($this->request->isUserAgent([ diff --git a/apps/dav/lib/Files/HiddenFolderPlugin.php b/apps/dav/lib/Files/HiddenFolderPlugin.php new file mode 100644 index 0000000000000..0b719cd802b3b --- /dev/null +++ b/apps/dav/lib/Files/HiddenFolderPlugin.php @@ -0,0 +1,44 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OCA\DAV\Files; + +use OC\Files\Filesystem; +use Sabre\DAV\Exception\Forbidden; +use Sabre\DAV\Server; +use Sabre\DAV\ServerPlugin; + +class HiddenFolderPlugin extends ServerPlugin { + public function initialize(Server $server) { + $server->on('beforeBind', [$this, 'onBind'], 1000); + $server->on('beforeUnbind', [$this, 'onBind'], 1000); + } + + public function onBind($path) { + $hiddenName = Filesystem::getHiddenFolderName(); + if (basename($path) === $hiddenName) { + throw new Forbidden("Can't modify hidden base folder"); + } + return true; + } +} diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 055c37f8472f5..9413713514b6b 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -144,6 +144,7 @@ public function __construct(IRequest $request, $baseUri) { $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin('webdav', $logger)); $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\LockPlugin()); + $this->server->addPlugin(new \OCA\DAV\Files\HiddenFolderPlugin()); $this->server->addPlugin(new \Sabre\DAV\Sync\Plugin()); // acl diff --git a/build/integration/features/bootstrap/CommandLineContext.php b/build/integration/features/bootstrap/CommandLineContext.php index 41756e448bfb9..8e44031840984 100644 --- a/build/integration/features/bootstrap/CommandLineContext.php +++ b/build/integration/features/bootstrap/CommandLineContext.php @@ -149,4 +149,13 @@ public function usingTransferFolderAsDavPath($user) { public function transferFolderNameContains($text) { Assert::assertContains($text, $this->lastTransferPath); } + + /** + * @Given /^system parameter "([^"]*)" is set to "([^"]*)"$/ + * @param string $parameter + * @param string $value + */ + public function setSystemConfig(string $parameter, string $value) { + $this->runOcc(['config:system:set', $parameter, '--value', $value]); + } } diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 2db51d8b22f7a..382cee7b589d2 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -31,6 +31,8 @@ * along with this program. If not, see . * */ + +use Behat\Gherkin\Node\TableNode; use GuzzleHttp\Client as GClient; use GuzzleHttp\Message\ResponseInterface; use PHPUnit\Framework\Assert; @@ -252,11 +254,11 @@ public function downloadedContentShouldStartWith($start) { * @param string $user * @param string $elementType * @param string $path - * @param \Behat\Gherkin\Node\TableNode|null $propertiesTable + * @param TableNode|null $propertiesTable */ public function asGetsPropertiesOfFolderWith($user, $elementType, $path, $propertiesTable) { $properties = null; - if ($propertiesTable instanceof \Behat\Gherkin\Node\TableNode) { + if ($propertiesTable instanceof TableNode) { foreach ($propertiesTable->getRows() as $row) { $properties[] = $row[0]; } @@ -269,7 +271,7 @@ public function asGetsPropertiesOfFolderWith($user, $elementType, $path, $proper * @param string $user * @param string $entry * @param string $path - * @param \Behat\Gherkin\Node\TableNode|null $propertiesTable + * @param TableNode|null $propertiesTable */ public function asTheFileOrFolderDoesNotExist($user, $entry, $path) { $client = $this->getSabreClient($user); @@ -432,12 +434,19 @@ public function getSabreClient($user) { /** * @Then /^user "([^"]*)" should see following elements$/ + * @Then /^user "([^"]*)" should see following elements in folder "([^"]*)"$/ * @param string $user - * @param \Behat\Gherkin\Node\TableNode|null $expectedElements + * @param string|TableNode|null $folder + * @param TableNode|null $expectedElements */ - public function checkElementList($user, $expectedElements) { - $elementList = $this->listFolder($user, '/', 3); - if ($expectedElements instanceof \Behat\Gherkin\Node\TableNode) { + public function checkElementList($user, $folder, $expectedElements = null) { + if ($folder instanceof TableNode and $expectedElements === null) { + $expectedElements = $folder; + $folder = null; + } + $path = $folder ?? '/'; + $elementList = $this->listFolder($user, $path, 3); + if ($expectedElements instanceof TableNode) { $elementRows = $expectedElements->getRows(); $elementsSimplified = $this->simplifyArray($elementRows); foreach ($elementsSimplified as $expectedElement) { @@ -449,6 +458,34 @@ public function checkElementList($user, $expectedElements) { } } + /** + * @Then /^user "([^"]*)" should not see following elements$/ + * @param string $user + * @param TableNode|null $expectedElements + */ + public function checkElementNotList($user, $expectedElements) { + try { + $elementList = $this->listFolder($user, '/', 3); + } catch (\Sabre\HTTP\ClientHttpException $e) { + $status = $e->getResponse()->getStatus(); + if ($status === 403 || $status === 404) { + // if listing fails the elements are also not listed, so this is fine + return; + } + } + + if ($expectedElements instanceof TableNode) { + $elementRows = $expectedElements->getRows(); + $elementsSimplified = $this->simplifyArray($elementRows); + foreach ($elementsSimplified as $expectedElement) { + $webdavPath = "/" . $this->getDavFilesPath($user) . $expectedElement; + if (array_key_exists($webdavPath, $elementList)) { + Assert::fail("$webdavPath" . " is in propfind answer"); + } + } + } + } + /** * @When User :user uploads file :source to :destination * @param string $user @@ -480,7 +517,7 @@ public function userAddsAFileTo($user, $bytes, $destination) { Assert::assertEquals(1, file_exists("work/$filename")); $this->userUploadsAFileTo($user, "work/$filename", $destination); $this->removeFile("work/", $filename); - $expectedElements = new \Behat\Gherkin\Node\TableNode([["$destination"]]); + $expectedElements = new TableNode([["$destination"]]); $this->checkElementList($user, $expectedElements); } @@ -716,7 +753,7 @@ public function changeFavStateOfAnElement($user, $path, $favOrUnfav, $folderDept * @Given user :user stores etag of element :path */ public function userStoresEtagOfElement($user, $path) { - $propertiesTable = new \Behat\Gherkin\Node\TableNode([['{DAV:}getetag']]); + $propertiesTable = new TableNode([['{DAV:}getetag']]); $this->asGetsPropertiesOfFolderWith($user, 'entry', $path, $propertiesTable); $pathETAG[$path] = $this->response['{DAV:}getetag']; $this->storedETAG[$user] = $pathETAG; @@ -726,7 +763,7 @@ public function userStoresEtagOfElement($user, $path) { * @Then etag of element :path of user :user has not changed */ public function checkIfETAGHasNotChanged($path, $user) { - $propertiesTable = new \Behat\Gherkin\Node\TableNode([['{DAV:}getetag']]); + $propertiesTable = new TableNode([['{DAV:}getetag']]); $this->asGetsPropertiesOfFolderWith($user, 'entry', $path, $propertiesTable); Assert::assertEquals($this->response['{DAV:}getetag'], $this->storedETAG[$user][$path]); } @@ -735,7 +772,7 @@ public function checkIfETAGHasNotChanged($path, $user) { * @Then etag of element :path of user :user has changed */ public function checkIfETAGHasChanged($path, $user) { - $propertiesTable = new \Behat\Gherkin\Node\TableNode([['{DAV:}getetag']]); + $propertiesTable = new TableNode([['{DAV:}getetag']]); $this->asGetsPropertiesOfFolderWith($user, 'entry', $path, $propertiesTable); Assert::assertNotEquals($this->response['{DAV:}getetag'], $this->storedETAG[$user][$path]); } @@ -768,14 +805,14 @@ public function thereAreNoDuplicateHeaders() { * @Then /^user "([^"]*)" in folder "([^"]*)" should have favorited the following elements$/ * @param string $user * @param string $folder - * @param \Behat\Gherkin\Node\TableNode|null $expectedElements + * @param TableNode|null $expectedElements */ public function checkFavoritedElements($user, $folder, $expectedElements) { $elementList = $this->reportFolder($user, $folder, '', '1'); - if ($expectedElements instanceof \Behat\Gherkin\Node\TableNode) { + if ($expectedElements instanceof TableNode) { $elementRows = $expectedElements->getRows(); $elementsSimplified = $this->simplifyArray($elementRows); foreach ($elementsSimplified as $expectedElement) { @@ -812,7 +849,7 @@ public function userDeletesEverythingInFolder($user, $folder) { * @return int */ private function getFileIdForPath($user, $path) { - $propertiesTable = new \Behat\Gherkin\Node\TableNode([["{http://owncloud.org/ns}fileid"]]); + $propertiesTable = new TableNode([["{http://owncloud.org/ns}fileid"]]); $this->asGetsPropertiesOfFolderWith($user, 'file', $path, $propertiesTable); return (int)$this->response['{http://owncloud.org/ns}fileid']; } diff --git a/build/integration/features/hidden-folder.feature b/build/integration/features/hidden-folder.feature new file mode 100644 index 0000000000000..fc354f690b262 --- /dev/null +++ b/build/integration/features/hidden-folder.feature @@ -0,0 +1,33 @@ +Feature: Hidden folder + Background: + Given using api version "1" + And user "user0" exists + And system parameter "instanceid" is set to "dummy" + And User "user0" created a folder "/.hidden_instance" + And system parameter "instanceid" is set to "instance" + + Scenario: The hidden folder should not be listed in the root + When User "user0" created a folder "/folder" + Then user "user0" should see following elements + | /folder/ | + And user "user0" should not see following elements + | /.hidden_instance/ | + + Scenario: Folders can be created inside the hidden folder + When User "user0" created a folder "/.hidden_instance/sub" + Then the HTTP status code should be "201" + And user "user0" should see following elements in folder "/.hidden_instance/" + | /.hidden_instance/sub/ | + + Scenario: Trying to delete the hidden folder should fail + Given User "user0" deletes folder "/.hidden_instance" + Then the HTTP status code should be "403" + + Scenario: Trying to rename the hidden folder should fail + Given User "user0" moves folder "/.hidden_instance" to "/foo" + Then the HTTP status code should be "403" + + Scenario: Trying to overwrite the hidden folder with a rename should fail + When User "user0" created a folder "/folder" + And User "user0" moves folder "/folder" to "/.hidden_instance" + Then the HTTP status code should be "403"