From 20772c28f56d4ea6ebd39f511fcaf71b24bbb821 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 24 Nov 2021 16:54:03 +0100 Subject: [PATCH 1/7] Add hidden user folder this can be used by apps to have apps that are within the users home folder but are hidden from the normal ui bits Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/Directory.php | 8 ++++++-- lib/private/Files/Cache/QuerySearchHelper.php | 3 +++ lib/private/Files/Node/Folder.php | 6 ++++++ lib/private/Files/Node/LazyRoot.php | 4 ++++ lib/private/Files/Node/Root.php | 14 ++++++++++++++ lib/private/Files/View.php | 7 +++++++ lib/public/Files/IRootFolder.php | 18 ++++++++++++++++++ 7 files changed, 58 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index f4b1ee62190f3..9bb80be7074cb 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -266,10 +266,14 @@ public function getChildren() { throw new Locked(); } + $hiddenName = 'hidden_' . \OC_Util::getInstanceId(); + $nodes = []; foreach ($folderContent as $info) { - $node = $this->getChild($info->getName(), $info); - $nodes[] = $node; + if ($info->getName() !== $hiddenName) { + $node = $this->getChild($info->getName(), $info); + $nodes[] = $node; + } } $this->dirContent = $nodes; return $this->dirContent; diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index eba2aac927bc4..e474c5bcf797b 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -139,6 +139,9 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array $query->andWhere($searchExpr); } + // exclude entries from the hidden folder in the user storage + $query->andWhere($query->expr()->notLike('path', $query->createNamedParameter('files/hidden_' . \OC_Util::getInstanceId() . '%'))); + $this->searchBuilder->addSearchOrdersToQuery($query, $searchQuery->getOrder()); if ($searchQuery->getLimit()) { diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index bf9ae3c148d5c..8881b4bb6e5ba 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -250,8 +250,14 @@ public function search($query) { $mountByMountPoint = ['' => $mount]; if (!$limitToHome) { + $hiddenFolder = 'hidden_' . \OC_Util::getInstanceId(); $mounts = $this->root->getMountsIn($this->path); foreach ($mounts as $mount) { + // don't search in mounts inside the hidden folder + if (strpos($mount->getMountPoint(), '/' . $hiddenFolder .'/') !== false) { + continue; + } + $storage = $mount->getStorage(); if ($storage) { $relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/'); diff --git a/lib/private/Files/Node/LazyRoot.php b/lib/private/Files/Node/LazyRoot.php index c01b9fdbb839c..e1d7b37d96acf 100644 --- a/lib/private/Files/Node/LazyRoot.php +++ b/lib/private/Files/Node/LazyRoot.php @@ -43,4 +43,8 @@ public function getUserFolder($userId) { public function getByIdInPath(int $id, string $path) { return $this->__call(__FUNCTION__, func_get_args()); } + + public function getHiddenUserFolder($userId) { + return $this->__call(__FUNCTION__, func_get_args()); + } } diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index ca930c1002c0f..a054b82a6eb50 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -404,6 +404,20 @@ public function getUserFolder($userId) { return $this->userFolderCache->get($userId); } + public function getHiddenUserFolder($userId) { + $userFolder = $this->getUserFolder($userId); + $hiddenName = 'hidden_' . \OC_Util::getInstanceId(); + try { + return $userFolder->get($hiddenName); + } catch (NotFoundException $e) { + return $userFolder->newFolder($hiddenName); + } + } + + public function clearCache() { + $this->userFolderCache = new CappedMemoryCache(); + } + public function getUserMountCache() { return $this->userMountCache; } diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 8f073da9164c8..f65f3471179a6 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1665,9 +1665,16 @@ private function searchCommon($method, $args) { } } + $hiddenFolder = 'hidden_' . \OC_Util::getInstanceId(); $mounts = Filesystem::getMountManager()->findIn($this->fakeRoot); foreach ($mounts as $mount) { $mountPoint = $mount->getMountPoint(); + + // don't search in mounts inside the hidden folder + if (strpos($mountPoint, '/' . $hiddenFolder .'/') !== false) { + continue; + } + $storage = $mount->getStorage(); if ($storage) { $cache = $storage->getCache(''); diff --git a/lib/public/Files/IRootFolder.php b/lib/public/Files/IRootFolder.php index 452c0fd31573b..5a00bac5083e7 100644 --- a/lib/public/Files/IRootFolder.php +++ b/lib/public/Files/IRootFolder.php @@ -55,4 +55,22 @@ public function getUserFolder($userId); * @since 24.0.0 */ public function getByIdInPath(int $id, string $path); + + /** + * Returns a hidden files directory + * + * This directory can be used to place files hidden for user, + * but still usable through most normal api as it is still inside the user folder + * + * Note that an experienced user can still browser this folder if they manually navigate into it. + * Do not rely on it being hidden for security purposes + * + * @param string $userId user ID + * @return \OCP\Files\Folder + * @throws NoUserException + * @throws NotPermittedException + * + * @since 24.0.0 + */ + public function getHiddenUserFolder($userId); } From 6aba15f7202e83ec199d31414de75e6e96a9f255 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 25 Nov 2021 14:23:54 +0100 Subject: [PATCH 2/7] factor out hidden folder name into a seperate function Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/Directory.php | 3 ++- lib/private/Files/Cache/QuerySearchHelper.php | 3 ++- lib/private/Files/Filesystem.php | 4 ++++ lib/private/Files/Node/Folder.php | 3 ++- lib/private/Files/Node/Root.php | 5 +++-- lib/private/Files/View.php | 2 +- 6 files changed, 14 insertions(+), 6 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index 9bb80be7074cb..224a695f811c6 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -32,6 +32,7 @@ */ namespace OCA\DAV\Connector\Sabre; +use OC\Files\Filesystem; use OC\Files\Mount\MoveableMount; use OC\Files\View; use OC\Metadata\FileMetadata; @@ -266,7 +267,7 @@ public function getChildren() { throw new Locked(); } - $hiddenName = 'hidden_' . \OC_Util::getInstanceId(); + $hiddenName = Filesystem::getHiddenFolderName(); $nodes = []; foreach ($folderContent as $info) { diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index e474c5bcf797b..67435241bcc18 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -25,6 +25,7 @@ */ namespace OC\Files\Cache; +use OC\Files\Filesystem; use OC\Files\Search\QueryOptimizer\QueryOptimizer; use OC\Files\Search\SearchBinaryOperator; use OC\SystemConfig; @@ -140,7 +141,7 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array } // exclude entries from the hidden folder in the user storage - $query->andWhere($query->expr()->notLike('path', $query->createNamedParameter('files/hidden_' . \OC_Util::getInstanceId() . '%'))); + $query->andWhere($query->expr()->notLike('path', $query->createNamedParameter('files/' . Filesystem::getHiddenFolderName() . '%'))); $this->searchBuilder->addSearchOrdersToQuery($query, $searchQuery->getOrder()); diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index 64a6fc57b27f9..f209a76604c3a 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -794,4 +794,8 @@ public static function getOwner($path) { public static function getETag($path) { return self::$defaultInstance->getETag($path); } + + public static function getHiddenFolderName(): string { + return '.hidden_' . \OC_Util::getInstanceId(); + } } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 8881b4bb6e5ba..909bb8653104b 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -31,6 +31,7 @@ namespace OC\Files\Node; use OC\Files\Cache\QuerySearchHelper; +use OC\Files\Filesystem; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Cache\Wrapper\CacheJail; use OC\Files\Search\SearchComparison; @@ -250,7 +251,7 @@ public function search($query) { $mountByMountPoint = ['' => $mount]; if (!$limitToHome) { - $hiddenFolder = 'hidden_' . \OC_Util::getInstanceId(); + $hiddenFolder = Filesystem::getHiddenFolderName(); $mounts = $this->root->getMountsIn($this->path); foreach ($mounts as $mount) { // don't search in mounts inside the hidden folder diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index a054b82a6eb50..fe9bef0024b93 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -32,8 +32,9 @@ namespace OC\Files\Node; -use OCP\Cache\CappedMemoryCache; +use OC\Cache\CappedMemoryCache; use OC\Files\FileInfo; +use OC\Files\Filesystem; use OC\Files\Mount\Manager; use OC\Files\Mount\MountPoint; use OC\Files\Utils\PathHelper; @@ -406,7 +407,7 @@ public function getUserFolder($userId) { public function getHiddenUserFolder($userId) { $userFolder = $this->getUserFolder($userId); - $hiddenName = 'hidden_' . \OC_Util::getInstanceId(); + $hiddenName = Filesystem::getHiddenFolderName(); try { return $userFolder->get($hiddenName); } catch (NotFoundException $e) { diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index f65f3471179a6..75f4e0c243569 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1665,7 +1665,7 @@ private function searchCommon($method, $args) { } } - $hiddenFolder = 'hidden_' . \OC_Util::getInstanceId(); + $hiddenFolder = Filesystem::getHiddenFolderName(); $mounts = Filesystem::getMountManager()->findIn($this->fakeRoot); foreach ($mounts as $mount) { $mountPoint = $mount->getMountPoint(); From bb6d6ee2145eb03b0ddbdf35dce52e8dea322fc9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 7 Jan 2022 14:14:49 +0100 Subject: [PATCH 3/7] Add sabre plugin to block dav bind/unbind on the hidden folder Signed-off-by: Robin Appelman --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + .../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 ++++++++++ 8 files changed, 141 insertions(+), 14 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/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index ce98cece3a192..7f68b8d958822 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -233,6 +233,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 a5a7d34d128d4..572bbdf4f9de8 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -248,6 +248,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/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 4c57f3412e3f9..4eca61729148a 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -107,6 +107,7 @@ public function createServer(string $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()); $server->addPlugin(new RequestIdHeaderPlugin(\OC::$server->get(IRequest::class))); 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 a5833e5175f48..88d1d64fa6992 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -155,6 +155,7 @@ public function __construct(IRequest $request, string $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 afe17cd75a406..1b246716d3f86 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::assertStringContainsString($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 680db01a260c0..e16de4a719af8 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; @@ -273,11 +275,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]; } @@ -290,7 +292,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); @@ -574,12 +576,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) { @@ -591,6 +600,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 @@ -622,7 +659,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); } @@ -861,7 +898,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; @@ -871,7 +908,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]); } @@ -880,7 +917,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]); } @@ -913,14 +950,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) { @@ -957,7 +994,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" From e158974c23c8069c96320b1b678c50daebe5c3aa Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 11 Jan 2022 16:10:21 +0100 Subject: [PATCH 4/7] Block the hidden folder from being shared Signed-off-by: Robin Appelman --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/files/lib/AppInfo/Application.php | 8 ++++ .../lib/Listener/HiddenFolderListener.php | 44 +++++++++++++++++++ .../features/bootstrap/CommandLine.php | 9 ++++ .../features/bootstrap/CommandLineContext.php | 9 ---- .../sharing_features/hidden-folder.feature | 18 ++++++++ 7 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 apps/files/lib/Listener/HiddenFolderListener.php create mode 100644 build/integration/sharing_features/hidden-folder.feature diff --git a/apps/files/composer/composer/autoload_classmap.php b/apps/files/composer/composer/autoload_classmap.php index ef3480081e0c4..b3f32fe77ec59 100644 --- a/apps/files/composer/composer/autoload_classmap.php +++ b/apps/files/composer/composer/autoload_classmap.php @@ -48,6 +48,7 @@ 'OCA\\Files\\Event\\LoadSidebar' => $baseDir . '/../lib/Event/LoadSidebar.php', 'OCA\\Files\\Exception\\TransferOwnershipException' => $baseDir . '/../lib/Exception/TransferOwnershipException.php', 'OCA\\Files\\Helper' => $baseDir . '/../lib/Helper.php', + 'OCA\\Files\\Listener\\HiddenFolderListener' => $baseDir . '/../lib/Listener/HiddenFolderListener.php', 'OCA\\Files\\Listener\\LegacyLoadAdditionalScriptsAdapter' => $baseDir . '/../lib/Listener/LegacyLoadAdditionalScriptsAdapter.php', 'OCA\\Files\\Listener\\LoadSidebarListener' => $baseDir . '/../lib/Listener/LoadSidebarListener.php', 'OCA\\Files\\Migration\\Version11301Date20191205150729' => $baseDir . '/../lib/Migration/Version11301Date20191205150729.php', diff --git a/apps/files/composer/composer/autoload_static.php b/apps/files/composer/composer/autoload_static.php index 4f7872e39dff1..3c565dbf72ddc 100644 --- a/apps/files/composer/composer/autoload_static.php +++ b/apps/files/composer/composer/autoload_static.php @@ -63,6 +63,7 @@ class ComposerStaticInitFiles 'OCA\\Files\\Event\\LoadSidebar' => __DIR__ . '/..' . '/../lib/Event/LoadSidebar.php', 'OCA\\Files\\Exception\\TransferOwnershipException' => __DIR__ . '/..' . '/../lib/Exception/TransferOwnershipException.php', 'OCA\\Files\\Helper' => __DIR__ . '/..' . '/../lib/Helper.php', + 'OCA\\Files\\Listener\\HiddenFolderListener' => __DIR__ . '/..' . '/../lib/Listener/HiddenFolderListener.php', 'OCA\\Files\\Listener\\LegacyLoadAdditionalScriptsAdapter' => __DIR__ . '/..' . '/../lib/Listener/LegacyLoadAdditionalScriptsAdapter.php', 'OCA\\Files\\Listener\\LoadSidebarListener' => __DIR__ . '/..' . '/../lib/Listener/LoadSidebarListener.php', 'OCA\\Files\\Migration\\Version11301Date20191205150729' => __DIR__ . '/..' . '/../lib/Migration/Version11301Date20191205150729.php', diff --git a/apps/files/lib/AppInfo/Application.php b/apps/files/lib/AppInfo/Application.php index 01fe46bb877fb..235c32ec527e9 100644 --- a/apps/files/lib/AppInfo/Application.php +++ b/apps/files/lib/AppInfo/Application.php @@ -42,6 +42,7 @@ use OCA\Files\DirectEditingCapabilities; use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCA\Files\Event\LoadSidebar; +use OCA\Files\Listener\HiddenFolderListener; use OCA\Files\Listener\LegacyLoadAdditionalScriptsAdapter; use OCA\Files\Listener\LoadSidebarListener; use OCA\Files\Notification\Notifier; @@ -65,6 +66,7 @@ use OCP\Share\IManager as IShareManager; use OCP\Util; use Psr\Container\ContainerInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; class Application extends App implements IBootstrap { public const APP_ID = 'files'; @@ -131,6 +133,7 @@ public function boot(IBootContext $context): void { $this->registerTemplates(); $context->injectFn(Closure::fromCallable([$this, 'registerNavigation'])); $this->registerHooks(); + $context->injectFn(Closure::fromCallable([$this, 'registerLegacyEvents'])); } private function registerCollaboration(IProviderManager $providerManager): void { @@ -181,4 +184,9 @@ private function registerNavigation(IL10N $l10n): void { private function registerHooks(): void { Util::connectHook('\OCP\Config', 'js', '\OCA\Files\App', 'extendJsConfig'); } + + private function registerLegacyEvents(EventDispatcherInterface $dispatcher): void { + $listener = new HiddenFolderListener(); + $dispatcher->addListener('OCP\Share::preShare', [$listener, 'handlePreShare']); + } } diff --git a/apps/files/lib/Listener/HiddenFolderListener.php b/apps/files/lib/Listener/HiddenFolderListener.php new file mode 100644 index 0000000000000..ad9090a2da137 --- /dev/null +++ b/apps/files/lib/Listener/HiddenFolderListener.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\Files\Listener; + +use OC\Files\Filesystem; +use OCP\Share\IShare; +use Symfony\Component\EventDispatcher\GenericEvent; + +class HiddenFolderListener { + public function handlePreShare(GenericEvent $event): void { + + $share = $event->getSubject(); + if (!$share instanceof IShare) { + return; + } + + $hiddenName = Filesystem::getHiddenFolderName(); + if ($share->getNode()->getName() === $hiddenName) { + $event->stopPropagation(); + $event->setArgument("error", "hidden root folder can't be shared"); + } + } +} diff --git a/build/integration/features/bootstrap/CommandLine.php b/build/integration/features/bootstrap/CommandLine.php index cba254551e0c4..15adedef8da8e 100644 --- a/build/integration/features/bootstrap/CommandLine.php +++ b/build/integration/features/bootstrap/CommandLine.php @@ -150,4 +150,13 @@ public function theCommandOutputContainsTheText($text) { public function theCommandErrorOutputContainsTheText($text) { Assert::assertStringContainsString($text, $this->lastStdErr, 'The command did not output the expected text on stderr'); } + + /** + * @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/CommandLineContext.php b/build/integration/features/bootstrap/CommandLineContext.php index 1b246716d3f86..afe17cd75a406 100644 --- a/build/integration/features/bootstrap/CommandLineContext.php +++ b/build/integration/features/bootstrap/CommandLineContext.php @@ -149,13 +149,4 @@ public function usingTransferFolderAsDavPath($user) { public function transferFolderNameContains($text) { Assert::assertStringContainsString($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/sharing_features/hidden-folder.feature b/build/integration/sharing_features/hidden-folder.feature new file mode 100644 index 0000000000000..069a3b3448a0f --- /dev/null +++ b/build/integration/sharing_features/hidden-folder.feature @@ -0,0 +1,18 @@ +Feature: Hidden folder sharing + 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: Share the hidden folder fails + Given user "user0" exists + And user "user1" exists + And As an "user0" + When creating a share with + | path | .hidden_instance | + | shareWith | user1 | + | shareType | 0 | + Then the OCS status code should be "403" + And the HTTP status code should be "200" From 14faaee6bbd8cdb437c2eecffa4327a2a7f760c4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 11 Jan 2022 16:32:05 +0100 Subject: [PATCH 5/7] run hidden folder integration tests in drone Signed-off-by: Robin Appelman --- .drone.yml | 17 +++++++++++++++++ .../sharing_features/hidden-folder.feature | 18 ------------------ .../sharing_features/sharing-v1-part3.feature | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 18 deletions(-) delete mode 100644 build/integration/sharing_features/hidden-folder.feature diff --git a/.drone.yml b/.drone.yml index 0b51b4ac47fc5..7196c8a49ee8d 100644 --- a/.drone.yml +++ b/.drone.yml @@ -1093,6 +1093,23 @@ steps: - cd build/integration - ./run.sh features/webdav-related.feature +--- +kind: pipeline +name: integration-webdav-hidden-folder + +steps: + - name: submodules + image: ghcr.io/nextcloud/continuous-integration-alpine-git:latest + commands: + - git submodule update --init + - name: integration-webdav-related + image: ghcr.io/nextcloud/continuous-integration-integration-php7.3:latest + commands: + - bash tests/drone-run-integration-tests.sh || exit 0 + - ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int + - cd build/integration + - ./run.sh features/hidden-folder.feature + trigger: branch: - master diff --git a/build/integration/sharing_features/hidden-folder.feature b/build/integration/sharing_features/hidden-folder.feature deleted file mode 100644 index 069a3b3448a0f..0000000000000 --- a/build/integration/sharing_features/hidden-folder.feature +++ /dev/null @@ -1,18 +0,0 @@ -Feature: Hidden folder sharing - 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: Share the hidden folder fails - Given user "user0" exists - And user "user1" exists - And As an "user0" - When creating a share with - | path | .hidden_instance | - | shareWith | user1 | - | shareType | 0 | - Then the OCS status code should be "403" - And the HTTP status code should be "200" diff --git a/build/integration/sharing_features/sharing-v1-part3.feature b/build/integration/sharing_features/sharing-v1-part3.feature index 1331d5b2ba6d1..837a46db63b97 100644 --- a/build/integration/sharing_features/sharing-v1-part3.feature +++ b/build/integration/sharing_features/sharing-v1-part3.feature @@ -608,3 +608,17 @@ Feature: sharing | permissions | 23 | | file_target | /subfolder | | expireDate | | + + Scenario: Share the hidden folder fails + Given 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" + And user "user1" exists + And As an "user0" + When creating a share with + | path | .hidden_instance | + | shareWith | user1 | + | shareType | 0 | + Then the OCS status code should be "403" + And the HTTP status code should be "200" From 69f39b2c3384fce23bf5bc5d84b7bf748d2a53eb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 13 Jan 2022 17:31:34 +0100 Subject: [PATCH 6/7] add convenience method to check if a file is in the hidden folder Signed-off-by: Robin Appelman --- lib/private/Files/Filesystem.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index f209a76604c3a..6b0e7c561c533 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -798,4 +798,8 @@ public static function getETag($path) { public static function getHiddenFolderName(): string { return '.hidden_' . \OC_Util::getInstanceId(); } + + public static function isPathHidden($path): string { + return strpos($path, self::getHiddenFolderName()) !== false; + } } From 77bbbf74ea26c48f23f20ded6b9740769d266980 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Nov 2022 15:42:27 +0100 Subject: [PATCH 7/7] minor fixes Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/Directory.php | 4 ++-- apps/dav/lib/Files/HiddenFolderPlugin.php | 4 ++-- lib/private/Files/Filesystem.php | 2 +- lib/private/Files/Node/Folder.php | 3 +-- lib/private/Files/Node/Root.php | 6 +++--- lib/private/Files/View.php | 3 +-- 6 files changed, 10 insertions(+), 12 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index 224a695f811c6..8989766bb64b3 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -267,11 +267,11 @@ public function getChildren() { throw new Locked(); } - $hiddenName = Filesystem::getHiddenFolderName(); + $hiddenFolderName = Filesystem::getHiddenFolderName(); $nodes = []; foreach ($folderContent as $info) { - if ($info->getName() !== $hiddenName) { + if ($info->getName() !== $hiddenFolderName) { $node = $this->getChild($info->getName(), $info); $nodes[] = $node; } diff --git a/apps/dav/lib/Files/HiddenFolderPlugin.php b/apps/dav/lib/Files/HiddenFolderPlugin.php index 0b719cd802b3b..939d6247d1ed1 100644 --- a/apps/dav/lib/Files/HiddenFolderPlugin.php +++ b/apps/dav/lib/Files/HiddenFolderPlugin.php @@ -29,12 +29,12 @@ use Sabre\DAV\ServerPlugin; class HiddenFolderPlugin extends ServerPlugin { - public function initialize(Server $server) { + public function initialize(Server $server): void { $server->on('beforeBind', [$this, 'onBind'], 1000); $server->on('beforeUnbind', [$this, 'onBind'], 1000); } - public function onBind($path) { + public function onBind($path): bool { $hiddenName = Filesystem::getHiddenFolderName(); if (basename($path) === $hiddenName) { throw new Forbidden("Can't modify hidden base folder"); diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index 6b0e7c561c533..2dd870141e8af 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -799,7 +799,7 @@ public static function getHiddenFolderName(): string { return '.hidden_' . \OC_Util::getInstanceId(); } - public static function isPathHidden($path): string { + public static function isPathHidden($path): bool { return strpos($path, self::getHiddenFolderName()) !== false; } } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 909bb8653104b..7c7ecbdbd307b 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -251,11 +251,10 @@ public function search($query) { $mountByMountPoint = ['' => $mount]; if (!$limitToHome) { - $hiddenFolder = Filesystem::getHiddenFolderName(); $mounts = $this->root->getMountsIn($this->path); foreach ($mounts as $mount) { // don't search in mounts inside the hidden folder - if (strpos($mount->getMountPoint(), '/' . $hiddenFolder .'/') !== false) { + if (Filesystem::isPathHidden($mount->getMountPoint())) { continue; } diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index fe9bef0024b93..102f42cd1e416 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -407,11 +407,11 @@ public function getUserFolder($userId) { public function getHiddenUserFolder($userId) { $userFolder = $this->getUserFolder($userId); - $hiddenName = Filesystem::getHiddenFolderName(); + $hiddenFolderName = Filesystem::getHiddenFolderName(); try { - return $userFolder->get($hiddenName); + return $userFolder->get($hiddenFolderName); } catch (NotFoundException $e) { - return $userFolder->newFolder($hiddenName); + return $userFolder->newFolder($hiddenFolderName); } } diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 75f4e0c243569..d741dddc373ad 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1665,13 +1665,12 @@ private function searchCommon($method, $args) { } } - $hiddenFolder = Filesystem::getHiddenFolderName(); $mounts = Filesystem::getMountManager()->findIn($this->fakeRoot); foreach ($mounts as $mount) { $mountPoint = $mount->getMountPoint(); // don't search in mounts inside the hidden folder - if (strpos($mountPoint, '/' . $hiddenFolder .'/') !== false) { + if (Filesystem::isPathHidden($mountPoint)) { continue; }