From 4381cc8f84c03a9571888ec0ef5c4cd5c52a6915 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 25 Sep 2025 10:29:17 +0200 Subject: [PATCH 1/3] feat(files): Add appconfig value to disable fixed userfolder permissions optimization Signed-off-by: Robin Appelman Signed-off-by: Louis Chemineau --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/files/lib/AppInfo/Application.php | 4 ++ apps/files/lib/ConfigLexicon.php | 46 +++++++++++++ lib/private/Files/Node/LazyUserFolder.php | 64 +++++++++++-------- lib/private/Files/Node/Root.php | 36 ++++------- lib/private/Server.php | 1 + tests/lib/Files/Node/FileTest.php | 15 +++-- tests/lib/Files/Node/FolderTest.php | 46 ++++++------- tests/lib/Files/Node/HookConnectorTest.php | 2 + tests/lib/Files/Node/IntegrationTest.php | 2 + tests/lib/Files/Node/NodeTestCase.php | 39 ++++++----- tests/lib/Files/Node/RootTest.php | 36 ++++++----- 13 files changed, 178 insertions(+), 115 deletions(-) create mode 100644 apps/files/lib/ConfigLexicon.php diff --git a/apps/files/composer/composer/autoload_classmap.php b/apps/files/composer/composer/autoload_classmap.php index 25f9c0eaf3f..09cd5bf8c5e 100644 --- a/apps/files/composer/composer/autoload_classmap.php +++ b/apps/files/composer/composer/autoload_classmap.php @@ -50,6 +50,7 @@ return array( 'OCA\\Files\\Command\\ScanAppData' => $baseDir . '/../lib/Command/ScanAppData.php', 'OCA\\Files\\Command\\TransferOwnership' => $baseDir . '/../lib/Command/TransferOwnership.php', 'OCA\\Files\\Command\\WindowsCompatibleFilenames' => $baseDir . '/../lib/Command/WindowsCompatibleFilenames.php', + 'OCA\\Files\\ConfigLexicon' => $baseDir . '/../lib/ConfigLexicon.php', 'OCA\\Files\\Controller\\ApiController' => $baseDir . '/../lib/Controller/ApiController.php', 'OCA\\Files\\Controller\\ConversionApiController' => $baseDir . '/../lib/Controller/ConversionApiController.php', 'OCA\\Files\\Controller\\DirectEditingController' => $baseDir . '/../lib/Controller/DirectEditingController.php', diff --git a/apps/files/composer/composer/autoload_static.php b/apps/files/composer/composer/autoload_static.php index 75c5f40cd81..5234caa6764 100644 --- a/apps/files/composer/composer/autoload_static.php +++ b/apps/files/composer/composer/autoload_static.php @@ -65,6 +65,7 @@ class ComposerStaticInitFiles 'OCA\\Files\\Command\\ScanAppData' => __DIR__ . '/..' . '/../lib/Command/ScanAppData.php', 'OCA\\Files\\Command\\TransferOwnership' => __DIR__ . '/..' . '/../lib/Command/TransferOwnership.php', 'OCA\\Files\\Command\\WindowsCompatibleFilenames' => __DIR__ . '/..' . '/../lib/Command/WindowsCompatibleFilenames.php', + 'OCA\\Files\\ConfigLexicon' => __DIR__ . '/..' . '/../lib/ConfigLexicon.php', 'OCA\\Files\\Controller\\ApiController' => __DIR__ . '/..' . '/../lib/Controller/ApiController.php', 'OCA\\Files\\Controller\\ConversionApiController' => __DIR__ . '/..' . '/../lib/Controller/ConversionApiController.php', 'OCA\\Files\\Controller\\DirectEditingController' => __DIR__ . '/..' . '/../lib/Controller/DirectEditingController.php', diff --git a/apps/files/lib/AppInfo/Application.php b/apps/files/lib/AppInfo/Application.php index 1de8e60ab5a..8ffdd3479c4 100644 --- a/apps/files/lib/AppInfo/Application.php +++ b/apps/files/lib/AppInfo/Application.php @@ -13,6 +13,7 @@ use OCA\Files\AdvancedCapabilities; use OCA\Files\Capabilities; use OCA\Files\Collaboration\Resources\Listener; use OCA\Files\Collaboration\Resources\ResourceProvider; +use OCA\Files\ConfigLexicon; use OCA\Files\Controller\ApiController; use OCA\Files\Dashboard\FavoriteWidget; use OCA\Files\DirectEditingCapabilities; @@ -124,6 +125,9 @@ class Application extends App implements IBootstrap { $context->registerNotifierService(Notifier::class); $context->registerDashboardWidget(FavoriteWidget::class); + + $context->registerConfigLexicon(ConfigLexicon::class); + } public function boot(IBootContext $context): void { diff --git a/apps/files/lib/ConfigLexicon.php b/apps/files/lib/ConfigLexicon.php new file mode 100644 index 00000000000..a2df81cf78e --- /dev/null +++ b/apps/files/lib/ConfigLexicon.php @@ -0,0 +1,46 @@ +user = $user; - $this->mountManager = $mountManager; + public function __construct( + IRootFolder $rootFolder, + private IUser $user, + private IMountManager $mountManager, + bool $useDefaultHomeFoldersPermissions = true, + ) { $this->path = '/' . $user->getUID() . '/files'; - parent::__construct($rootFolder, function () use ($user): Folder { - try { - $node = $this->getRootFolder()->get($this->path); - if ($node instanceof File) { - $e = new \RuntimeException(); - \OCP\Server::get(LoggerInterface::class)->error('User root storage is not a folder: ' . $this->path, [ - 'exception' => $e, - ]); - throw $e; - } - return $node; - } catch (NotFoundException $e) { - if (!$this->getRootFolder()->nodeExists('/' . $user->getUID())) { - $this->getRootFolder()->newFolder('/' . $user->getUID()); - } - return $this->getRootFolder()->newFolder($this->path); - } - }, [ + $data = [ 'path' => $this->path, - // Sharing user root folder is not allowed - 'permissions' => Constants::PERMISSION_ALL ^ Constants::PERMISSION_SHARE, 'type' => FileInfo::TYPE_FOLDER, 'mimetype' => FileInfo::MIMETYPE_FOLDER, - ]); + ]; + + // By default, we assume the permissions for the users' home folders. + // If a mount point is mounted on a user's home folder, the permissions cannot be assumed. + if ($useDefaultHomeFoldersPermissions) { + // Sharing user root folder is not allowed + $data['permissions'] = Constants::PERMISSION_ALL ^ Constants::PERMISSION_SHARE; + } + + parent::__construct( + $rootFolder, + function () use ($user): Folder { + try { + $node = $this->getRootFolder()->get($this->path); + if ($node instanceof File) { + $e = new \RuntimeException(); + \OCP\Server::get(LoggerInterface::class)->error('User root storage is not a folder: ' . $this->path, [ + 'exception' => $e, + ]); + throw $e; + } + return $node; + } catch (NotFoundException $e) { + if (!$this->getRootFolder()->nodeExists('/' . $user->getUID())) { + $this->getRootFolder()->newFolder('/' . $user->getUID()); + } + return $this->getRootFolder()->newFolder($this->path); + } + }, + $data, + ); } public function getMountPoint() { diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index 76afca9dee8..9b0a48fa296 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -14,6 +14,8 @@ use OC\Files\Utils\PathHelper; use OC\Files\View; use OC\Hooks\PublicEmitter; use OC\User\NoUserException; +use OCA\Files\AppInfo\Application; +use OCA\Files\ConfigLexicon; use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\ICacheEntry; @@ -24,6 +26,7 @@ use OCP\Files\Mount\IMountPoint; use OCP\Files\Node as INode; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\IAppConfig; use OCP\ICache; use OCP\ICacheFactory; use OCP\IUser; @@ -51,43 +54,30 @@ use Psr\Log\LoggerInterface; * @package OC\Files\Node */ class Root extends Folder implements IRootFolder { - private Manager $mountManager; private PublicEmitter $emitter; - private ?IUser $user; private CappedMemoryCache $userFolderCache; - private IUserMountCache $userMountCache; - private LoggerInterface $logger; - private IUserManager $userManager; - private IEventDispatcher $eventDispatcher; private ICache $pathByIdCache; + private bool $useDefaultHomeFoldersPermissions = true; - /** - * @param Manager $manager - * @param View $view - * @param IUser|null $user - */ public function __construct( - $manager, - $view, - $user, - IUserMountCache $userMountCache, - LoggerInterface $logger, - IUserManager $userManager, + private Manager $mountManager, + View $view, + private ?IUser $user, + private IUserMountCache $userMountCache, + private LoggerInterface $logger, + private IUserManager $userManager, IEventDispatcher $eventDispatcher, ICacheFactory $cacheFactory, + IAppConfig $appConfig, ) { parent::__construct($this, $view, ''); - $this->mountManager = $manager; - $this->user = $user; $this->emitter = new PublicEmitter(); $this->userFolderCache = new CappedMemoryCache(); - $this->userMountCache = $userMountCache; - $this->logger = $logger; - $this->userManager = $userManager; $eventDispatcher->addListener(FilesystemTornDownEvent::class, function () { $this->userFolderCache = new CappedMemoryCache(); }); $this->pathByIdCache = $cacheFactory->createLocal('path-by-id'); + $this->useDefaultHomeFoldersPermissions = count($appConfig->getValueArray(Application::APP_ID, ConfigLexicon::OVERWRITES_HOME_FOLDERS)) === 0; } /** @@ -367,7 +357,7 @@ class Root extends Folder implements IRootFolder { $folder = $this->newFolder('/' . $userId . '/files'); } } else { - $folder = new LazyUserFolder($this, $userObject, $this->mountManager); + $folder = new LazyUserFolder($this, $userObject, $this->mountManager, $this->useDefaultHomeFoldersPermissions); } $this->userFolderCache->set($userId, $folder); diff --git a/lib/private/Server.php b/lib/private/Server.php index 89c5510029d..c8df46826cd 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -405,6 +405,7 @@ class Server extends ServerContainer implements IServerContainer { $this->get(IUserManager::class), $this->get(IEventDispatcher::class), $this->get(ICacheFactory::class), + $this->get(IAppConfig::class), ); $previewConnector = new \OC\Preview\WatcherConnector( diff --git a/tests/lib/Files/Node/FileTest.php b/tests/lib/Files/Node/FileTest.php index eec34d156ad..0ab8a32b6cb 100644 --- a/tests/lib/Files/Node/FileTest.php +++ b/tests/lib/Files/Node/FileTest.php @@ -44,7 +44,7 @@ class FileTest extends NodeTestCase { public function testGetContent(): void { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $hook = function ($file): void { @@ -74,7 +74,7 @@ class FileTest extends NodeTestCase { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) @@ -93,7 +93,7 @@ class FileTest extends NodeTestCase { public function testPutContent(): void { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) @@ -120,7 +120,7 @@ class FileTest extends NodeTestCase { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $this->view->expects($this->once()) @@ -135,7 +135,7 @@ class FileTest extends NodeTestCase { public function testGetMimeType(): void { /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $this->view->expects($this->once()) @@ -161,6 +161,7 @@ class FileTest extends NodeTestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $hook = function ($file): void { @@ -198,6 +199,7 @@ class FileTest extends NodeTestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $hooksCalled = 0; $hook = function ($file) use (&$hooksCalled): void { @@ -239,6 +241,7 @@ class FileTest extends NodeTestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $hook = function ($file): void { throw new \Exception('Hooks are not supposed to be called'); @@ -266,6 +269,7 @@ class FileTest extends NodeTestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $hook = function (): void { throw new \Exception('Hooks are not supposed to be called'); @@ -293,6 +297,7 @@ class FileTest extends NodeTestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $hook = function (): void { throw new \Exception('Hooks are not supposed to be called'); diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 439535cf2c1..fc8b778cb34 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -76,7 +76,7 @@ class FolderTest extends NodeTestCase { * @var View|\PHPUnit\Framework\MockObject\MockObject $view */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -109,7 +109,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -128,7 +128,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -148,7 +148,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -166,7 +166,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -190,7 +190,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -217,7 +217,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -234,7 +234,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -261,7 +261,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -278,7 +278,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -295,7 +295,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -344,7 +344,7 @@ class FolderTest extends NodeTestCase { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -387,7 +387,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->method('getUser') ->willReturn($this->user); @@ -430,7 +430,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') @@ -506,7 +506,7 @@ class FolderTest extends NodeTestCase { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $storage = $this->createMock(Storage::class); $mount = new MountPoint($storage, '/bar'); @@ -555,7 +555,7 @@ class FolderTest extends NodeTestCase { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $storage = $this->createMock(Storage::class); $mount = new MountPoint($storage, '/bar'); @@ -600,7 +600,7 @@ class FolderTest extends NodeTestCase { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $storage = $this->createMock(Storage::class); $mount = new MountPoint($storage, '/bar'); @@ -644,7 +644,7 @@ class FolderTest extends NodeTestCase { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $storage = $this->createMock(Storage::class); $mount1 = new MountPoint($storage, '/bar'); @@ -703,7 +703,7 @@ class FolderTest extends NodeTestCase { $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $view->expects($this->any()) @@ -728,7 +728,7 @@ class FolderTest extends NodeTestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\Node\Root $root */ $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); /** @var \PHPUnit\Framework\MockObject\MockObject|FileInfo $folderInfo */ $folderInfo = $this->getMockBuilder(FileInfo::class) @@ -797,7 +797,7 @@ class FolderTest extends NodeTestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\Node\Root $root */ $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); /** @var \PHPUnit\Framework\MockObject\MockObject|FileInfo $folderInfo */ $folderInfo = $this->getMockBuilder(FileInfo::class) @@ -864,7 +864,7 @@ class FolderTest extends NodeTestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Files\Node\Root $root */ $root = $this->getMockBuilder(Root::class) ->onlyMethods(['getUser', 'getMountsIn', 'getMount']) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); /** @var \PHPUnit\Framework\MockObject\MockObject|FileInfo $folderInfo */ $folderInfo = $this->getMockBuilder(FileInfo::class) @@ -951,7 +951,7 @@ class FolderTest extends NodeTestCase { $manager = $this->createMock(Manager::class); $view = $this->getRootViewMock(); $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); $root->expects($this->any()) ->method('getUser') diff --git a/tests/lib/Files/Node/HookConnectorTest.php b/tests/lib/Files/Node/HookConnectorTest.php index 3f3957bab1d..f108b279179 100644 --- a/tests/lib/Files/Node/HookConnectorTest.php +++ b/tests/lib/Files/Node/HookConnectorTest.php @@ -32,6 +32,7 @@ use OCP\Files\Events\Node\NodeRenamedEvent; use OCP\Files\Events\Node\NodeTouchedEvent; use OCP\Files\Events\Node\NodeWrittenEvent; use OCP\Files\Node; +use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IUserManager; use OCP\Server; @@ -88,6 +89,7 @@ class HookConnectorTest extends TestCase { $this->createMock(IUserManager::class), $this->createMock(IEventDispatcher::class), $cacheFactory, + $this->createMock(IAppConfig::class), ); $this->eventDispatcher = Server::get(IEventDispatcher::class); $this->logger = Server::get(LoggerInterface::class); diff --git a/tests/lib/Files/Node/IntegrationTest.php b/tests/lib/Files/Node/IntegrationTest.php index f059afa1625..d7a56a0f99d 100644 --- a/tests/lib/Files/Node/IntegrationTest.php +++ b/tests/lib/Files/Node/IntegrationTest.php @@ -16,6 +16,7 @@ use OC\Memcache\ArrayCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; use OCP\Files\Mount\IMountManager; +use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IUserManager; use OCP\Server; @@ -72,6 +73,7 @@ class IntegrationTest extends \Test\TestCase { $this->createMock(IUserManager::class), $this->createMock(IEventDispatcher::class), $cacheFactory, + $this->createMock(IAppConfig::class), ); $storage = new Temporary([]); $subStorage = new Temporary([]); diff --git a/tests/lib/Files/Node/NodeTestCase.php b/tests/lib/Files/Node/NodeTestCase.php index 4aecd0fef11..90f7c2c0f65 100644 --- a/tests/lib/Files/Node/NodeTestCase.php +++ b/tests/lib/Files/Node/NodeTestCase.php @@ -16,7 +16,6 @@ use OC\Files\Node\Root; use OC\Files\Storage\Storage; use OC\Files\View; use OC\Memcache\ArrayCache; -use OC\User\User; use OCP\Constants; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; @@ -27,9 +26,11 @@ use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Files\Storage\IStorage; +use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IUser; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; /** @@ -38,24 +39,16 @@ use Psr\Log\LoggerInterface; * @package Test\Files\Node */ abstract class NodeTestCase extends \Test\TestCase { - /** @var User */ - protected $user; - /** @var \OC\Files\Mount\Manager */ - protected $manager; - /** @var View|\PHPUnit\Framework\MockObject\MockObject */ - protected $view; - /** @var \OC\Files\Node\Root|\PHPUnit\Framework\MockObject\MockObject */ - protected $root; - /** @var IUserMountCache|\PHPUnit\Framework\MockObject\MockObject */ - protected $userMountCache; - /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ - protected $logger; - /** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */ - protected $userManager; - /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ - protected $eventDispatcher; - /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ - protected $cacheFactory; + protected IUser&MockObject $user; + protected Manager&MockObject $manager; + protected View&MockObject $view; + protected Root&MockObject $root; + protected IUserMountCache&MockObject $userMountCache; + protected LoggerInterface&MockObject $logger; + protected IUserManager&MockObject $userManager; + protected IEventDispatcher&MockObject $eventDispatcher; + protected ICacheFactory&MockObject $cacheFactory; + protected IAppConfig&MockObject $appConfig; protected function setUp(): void { parent::setUp(); @@ -81,8 +74,10 @@ abstract class NodeTestCase extends \Test\TestCase { ->willReturnCallback(function () { return new ArrayCache(); }); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); } @@ -194,6 +189,7 @@ abstract class NodeTestCase extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->createMock(IAppConfig::class), ); $root->listen('\OC\Files', 'preDelete', $preListener); @@ -443,6 +439,7 @@ abstract class NodeTestCase extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->createMock(IAppConfig::class), ); $root->listen('\OC\Files', 'preTouch', $preListener); $root->listen('\OC\Files', 'postTouch', $postListener); @@ -619,7 +616,7 @@ abstract class NodeTestCase extends \Test\TestCase { public function testMoveCopyHooks($operationMethod, $viewMethod, $preHookName, $postHookName): void { /** @var IRootFolder|\PHPUnit\Framework\MockObject\MockObject $root */ $root = $this->getMockBuilder(Root::class) - ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory]) + ->setConstructorArgs([$this->manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->onlyMethods(['get']) ->getMock(); diff --git a/tests/lib/Files/Node/RootTest.php b/tests/lib/Files/Node/RootTest.php index d90e6a2cc6e..3821b6943b1 100644 --- a/tests/lib/Files/Node/RootTest.php +++ b/tests/lib/Files/Node/RootTest.php @@ -16,15 +16,16 @@ use OC\Files\Storage\Storage; use OC\Files\View; use OC\Memcache\ArrayCache; use OC\User\NoUserException; -use OC\User\User; use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IUser; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; /** @@ -33,20 +34,14 @@ use Psr\Log\LoggerInterface; * @package Test\Files\Node */ class RootTest extends \Test\TestCase { - /** @var User */ - private $user; - /** @var \OC\Files\Mount\Manager */ - private $manager; - /** @var IUserMountCache|\PHPUnit\Framework\MockObject\MockObject */ - private $userMountCache; - /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ - private $logger; - /** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */ - private $userManager; - /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ - private $eventDispatcher; - /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ - protected $cacheFactory; + private IUser&MockObject $user; + private Manager&MockObject $manager; + private IUserMountCache&MockObject $userMountCache; + private LoggerInterface&MockObject $logger; + private IUserManager&MockObject $userManager; + private IEventDispatcher&MockObject $eventDispatcher; + protected ICacheFactory&MockObject $cacheFactory; + protected IAppConfig&MockObject $appConfig; protected function setUp(): void { parent::setUp(); @@ -66,10 +61,11 @@ class RootTest extends \Test\TestCase { ->willReturnCallback(function () { return new ArrayCache(); }); + $this->appConfig = $this->createMock(IAppConfig::class); } /** - * @return View|\PHPUnit\Framework\MockObject\MockObject $view + * @return View&MockObject $view */ protected function getRootViewMock() { $view = $this->createMock(View::class); @@ -100,6 +96,7 @@ class RootTest extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $view->expects($this->once()) @@ -133,6 +130,7 @@ class RootTest extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $view->expects($this->once()) @@ -158,6 +156,7 @@ class RootTest extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $root->get('/../foo'); @@ -177,6 +176,7 @@ class RootTest extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $root->get('/bar/foo'); @@ -192,6 +192,7 @@ class RootTest extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $user = $this->createMock(IUser::class); $user @@ -203,7 +204,7 @@ class RootTest extends \Test\TestCase { ->method('get') ->with('MyUserId') ->willReturn($user); - /** @var CappedMemoryCache|\PHPUnit\Framework\MockObject\MockObject $cappedMemoryCache */ + /** @var CappedMemoryCache&MockObject $cappedMemoryCache */ $cappedMemoryCache = $this->createMock(CappedMemoryCache::class); $cappedMemoryCache ->expects($this->once()) @@ -234,6 +235,7 @@ class RootTest extends \Test\TestCase { $this->userManager, $this->eventDispatcher, $this->cacheFactory, + $this->appConfig, ); $this->userManager ->expects($this->once()) From 70c6e9cae67552f5af2272a5451b4e06e4798599 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Fri, 26 Sep 2025 17:10:05 +0200 Subject: [PATCH 2/3] feat(files): Mark homefolder as overwritten when an external storage mounted at / exists Signed-off-by: Robin Appelman Signed-off-by: Louis Chemineau --- apps/files_external/appinfo/info.xml | 2 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../Version1025Date20250228162604.php | 35 +++++++++++++++++++ .../lib/Service/DBConfigService.php | 28 +++++++++++---- .../lib/Service/StoragesService.php | 28 +++++++++++++++ .../lib/Service/UserGlobalStoragesService.php | 12 ++----- .../lib/Service/UserStoragesService.php | 10 ++---- .../Service/GlobalStoragesServiceTest.php | 2 +- .../tests/Service/StoragesServiceTestCase.php | 5 ++- .../Service/UserGlobalStoragesServiceTest.php | 1 + .../tests/Service/UserStoragesServiceTest.php | 4 +-- 12 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 apps/files_external/lib/Migration/Version1025Date20250228162604.php diff --git a/apps/files_external/appinfo/info.xml b/apps/files_external/appinfo/info.xml index e746f052619..b652e7533c5 100644 --- a/apps/files_external/appinfo/info.xml +++ b/apps/files_external/appinfo/info.xml @@ -14,7 +14,7 @@ This application enables administrators to configure connections to external sto External storage can be configured using the GUI or at the command line. This second option provides the administration with more flexibility for configuring bulk external storage mounts and setting mount priorities. More information is available in the external storage GUI documentation and the external storage Configuration File documentation. - 1.24.0 + 1.24.1 agpl Robin Appelman Michael Gapczynski diff --git a/apps/files_external/composer/composer/autoload_classmap.php b/apps/files_external/composer/composer/autoload_classmap.php index 8ac551013bd..61165ee67fb 100644 --- a/apps/files_external/composer/composer/autoload_classmap.php +++ b/apps/files_external/composer/composer/autoload_classmap.php @@ -107,6 +107,7 @@ return array( 'OCA\\Files_External\\Migration\\Version1011Date20200630192246' => $baseDir . '/../lib/Migration/Version1011Date20200630192246.php', 'OCA\\Files_External\\Migration\\Version1015Date20211104103506' => $baseDir . '/../lib/Migration/Version1015Date20211104103506.php', 'OCA\\Files_External\\Migration\\Version1016Date20220324154536' => $baseDir . '/../lib/Migration/Version1016Date20220324154536.php', + 'OCA\\Files_External\\Migration\\Version1025Date20250228162604' => $baseDir . '/../lib/Migration/Version1025Date20250228162604.php', 'OCA\\Files_External\\Migration\\Version22000Date20210216084416' => $baseDir . '/../lib/Migration/Version22000Date20210216084416.php', 'OCA\\Files_External\\MountConfig' => $baseDir . '/../lib/MountConfig.php', 'OCA\\Files_External\\NotFoundException' => $baseDir . '/../lib/NotFoundException.php', diff --git a/apps/files_external/composer/composer/autoload_static.php b/apps/files_external/composer/composer/autoload_static.php index 4468ce1b6bb..5b8b6ab0294 100644 --- a/apps/files_external/composer/composer/autoload_static.php +++ b/apps/files_external/composer/composer/autoload_static.php @@ -122,6 +122,7 @@ class ComposerStaticInitFiles_External 'OCA\\Files_External\\Migration\\Version1011Date20200630192246' => __DIR__ . '/..' . '/../lib/Migration/Version1011Date20200630192246.php', 'OCA\\Files_External\\Migration\\Version1015Date20211104103506' => __DIR__ . '/..' . '/../lib/Migration/Version1015Date20211104103506.php', 'OCA\\Files_External\\Migration\\Version1016Date20220324154536' => __DIR__ . '/..' . '/../lib/Migration/Version1016Date20220324154536.php', + 'OCA\\Files_External\\Migration\\Version1025Date20250228162604' => __DIR__ . '/..' . '/../lib/Migration/Version1025Date20250228162604.php', 'OCA\\Files_External\\Migration\\Version22000Date20210216084416' => __DIR__ . '/..' . '/../lib/Migration/Version22000Date20210216084416.php', 'OCA\\Files_External\\MountConfig' => __DIR__ . '/..' . '/../lib/MountConfig.php', 'OCA\\Files_External\\NotFoundException' => __DIR__ . '/..' . '/../lib/NotFoundException.php', diff --git a/apps/files_external/lib/Migration/Version1025Date20250228162604.php b/apps/files_external/lib/Migration/Version1025Date20250228162604.php new file mode 100644 index 00000000000..f3d76ed7baa --- /dev/null +++ b/apps/files_external/lib/Migration/Version1025Date20250228162604.php @@ -0,0 +1,35 @@ +globalStoragesServices->updateOverwriteHomeFolders(); + } +} diff --git a/apps/files_external/lib/Service/DBConfigService.php b/apps/files_external/lib/Service/DBConfigService.php index 41ec4512d70..2338f0bac5d 100644 --- a/apps/files_external/lib/Service/DBConfigService.php +++ b/apps/files_external/lib/Service/DBConfigService.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OCA\Files_External\Service; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; @@ -64,16 +65,16 @@ class DBConfigService { ->where($builder->expr()->orX( $builder->expr()->andX( // global mounts $builder->expr()->eq('a.type', $builder->createNamedParameter(self::APPLICABLE_TYPE_GLOBAL, IQueryBuilder::PARAM_INT)), - $builder->expr()->isNull('a.value') + $builder->expr()->isNull('a.value'), ), $builder->expr()->andX( // mounts for user $builder->expr()->eq('a.type', $builder->createNamedParameter(self::APPLICABLE_TYPE_USER, IQueryBuilder::PARAM_INT)), - $builder->expr()->eq('a.value', $builder->createNamedParameter($userId)) + $builder->expr()->eq('a.value', $builder->createNamedParameter($userId)), ), $builder->expr()->andX( // mounts for group $builder->expr()->eq('a.type', $builder->createNamedParameter(self::APPLICABLE_TYPE_GROUP, IQueryBuilder::PARAM_INT)), - $builder->expr()->in('a.value', $builder->createNamedParameter($groupIds, IQueryBuilder::PARAM_STR_ARRAY)) - ) + $builder->expr()->in('a.value', $builder->createNamedParameter($groupIds, IQueryBuilder::PARAM_STR_ARRAY)), + ), )); return $this->getMountsFromQuery($query); @@ -94,8 +95,8 @@ class DBConfigService { ->leftJoin('a', 'external_applicable', 'b', $builder->expr()->eq('a.mount_id', 'b.mount_id')) ->where($builder->expr()->andX( $builder->expr()->eq('b.type', $builder->createNamedParameter($applicableType, IQueryBuilder::PARAM_INT)), - $builder->expr()->eq('b.value', $builder->createNamedParameter($applicableId)) - ) + $builder->expr()->eq('b.value', $builder->createNamedParameter($applicableId)), + ), ) ->groupBy(['a.mount_id']); $stmt = $query->executeQuery(); @@ -227,7 +228,7 @@ class DBConfigService { 'storage_backend' => $builder->createNamedParameter($storageBackend, IQueryBuilder::PARAM_STR), 'auth_backend' => $builder->createNamedParameter($authBackend, IQueryBuilder::PARAM_STR), 'priority' => $builder->createNamedParameter($priority, IQueryBuilder::PARAM_INT), - 'type' => $builder->createNamedParameter($type, IQueryBuilder::PARAM_INT) + 'type' => $builder->createNamedParameter($type, IQueryBuilder::PARAM_INT), ]); $query->executeStatement(); return $query->getLastInsertId(); @@ -498,4 +499,17 @@ class DBConfigService { return $value; } } + + /** + * Check if any mountpoint is configured that overwrite the home folder + */ + public function hasHomeFolderOverwriteMount(): bool { + $builder = $this->connection->getQueryBuilder(); + $query = $builder->select('mount_id') + ->from('external_mounts') + ->where($builder->expr()->eq('mount_point', $builder->createNamedParameter('/'))) + ->setMaxResults(1); + $result = $query->executeQuery(); + return count($result->fetchAll()) > 0; + } } diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php index a12a8fc245a..7b1b7ba2dc1 100644 --- a/apps/files_external/lib/Service/StoragesService.php +++ b/apps/files_external/lib/Service/StoragesService.php @@ -9,6 +9,9 @@ namespace OCA\Files_External\Service; use OC\Files\Cache\Storage; use OC\Files\Filesystem; +use OCA\Files\AppInfo\Application as FilesApplication; +use OCA\Files\ConfigLexicon; +use OCA\Files_External\AppInfo\Application; use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\Auth\InvalidAuth; use OCA\Files_External\Lib\Backend\Backend; @@ -20,6 +23,7 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; use OCP\Files\Events\InvalidateMountCacheEvent; use OCP\Files\StorageNotAvailableException; +use OCP\IAppConfig; use OCP\Server; use OCP\Util; use Psr\Log\LoggerInterface; @@ -40,6 +44,7 @@ abstract class StoragesService { protected DBConfigService $dbConfig, protected IUserMountCache $userMountCache, protected IEventDispatcher $eventDispatcher, + protected IAppConfig $appConfig, ) { } @@ -242,6 +247,9 @@ abstract class StoragesService { $this->triggerHooks($newStorage, Filesystem::signal_create_mount); $newStorage->setStatus(StorageNotAvailableException::STATUS_SUCCESS); + + $this->updateOverwriteHomeFolders(); + return $newStorage; } @@ -425,6 +433,8 @@ abstract class StoragesService { } } + $this->updateOverwriteHomeFolders(); + return $this->getStorage($id); } @@ -449,6 +459,8 @@ abstract class StoragesService { // delete oc_storages entries and oc_filecache Storage::cleanByMountId($id); + + $this->updateOverwriteHomeFolders(); } /** @@ -473,4 +485,20 @@ abstract class StoragesService { return -1; } } + + public function updateOverwriteHomeFolders(): void { + $appIdsList = $this->appConfig->getValueArray(FilesApplication::APP_ID, ConfigLexicon::OVERWRITES_HOME_FOLDERS); + + if ($this->dbConfig->hasHomeFolderOverwriteMount()) { + if (!in_array(Application::APP_ID, $appIdsList)) { + $appIdsList[] = Application::APP_ID; + $this->appConfig->setValueArray(FilesApplication::APP_ID, ConfigLexicon::OVERWRITES_HOME_FOLDERS, $appIdsList); + } + } else { + if (in_array(Application::APP_ID, $appIdsList)) { + $appIdsList = array_values(array_filter($appIdsList, fn ($v) => $v !== Application::APP_ID)); + $this->appConfig->setValueArray(FilesApplication::APP_ID, ConfigLexicon::OVERWRITES_HOME_FOLDERS, $appIdsList); + } + } + } } diff --git a/apps/files_external/lib/Service/UserGlobalStoragesService.php b/apps/files_external/lib/Service/UserGlobalStoragesService.php index aaa59c85d62..6c943247b20 100644 --- a/apps/files_external/lib/Service/UserGlobalStoragesService.php +++ b/apps/files_external/lib/Service/UserGlobalStoragesService.php @@ -10,6 +10,7 @@ namespace OCA\Files_External\Service; use OCA\Files_External\Lib\StorageConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; +use OCP\IAppConfig; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserSession; @@ -21,14 +22,6 @@ use OCP\IUserSession; class UserGlobalStoragesService extends GlobalStoragesService { use UserTrait; - /** - * @param BackendService $backendService - * @param DBConfigService $dbConfig - * @param IUserSession $userSession - * @param IGroupManager $groupManager - * @param IUserMountCache $userMountCache - * @param IEventDispatcher $eventDispatcher - */ public function __construct( BackendService $backendService, DBConfigService $dbConfig, @@ -36,8 +29,9 @@ class UserGlobalStoragesService extends GlobalStoragesService { protected IGroupManager $groupManager, IUserMountCache $userMountCache, IEventDispatcher $eventDispatcher, + IAppConfig $appConfig, ) { - parent::__construct($backendService, $dbConfig, $userMountCache, $eventDispatcher); + parent::__construct($backendService, $dbConfig, $userMountCache, $eventDispatcher, $appConfig); $this->userSession = $userSession; } diff --git a/apps/files_external/lib/Service/UserStoragesService.php b/apps/files_external/lib/Service/UserStoragesService.php index 9d4192734b6..bd8dd2d348c 100644 --- a/apps/files_external/lib/Service/UserStoragesService.php +++ b/apps/files_external/lib/Service/UserStoragesService.php @@ -13,6 +13,7 @@ use OCA\Files_External\MountConfig; use OCA\Files_External\NotFoundException; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; +use OCP\IAppConfig; use OCP\IUserSession; /** @@ -24,12 +25,6 @@ class UserStoragesService extends StoragesService { /** * Create a user storages service - * - * @param BackendService $backendService - * @param DBConfigService $dbConfig - * @param IUserSession $userSession user session - * @param IUserMountCache $userMountCache - * @param IEventDispatcher $eventDispatcher */ public function __construct( BackendService $backendService, @@ -37,9 +32,10 @@ class UserStoragesService extends StoragesService { IUserSession $userSession, IUserMountCache $userMountCache, IEventDispatcher $eventDispatcher, + IAppConfig $appConfig, ) { $this->userSession = $userSession; - parent::__construct($backendService, $dbConfig, $userMountCache, $eventDispatcher); + parent::__construct($backendService, $dbConfig, $userMountCache, $eventDispatcher, $appConfig); } protected function readDBConfig() { diff --git a/apps/files_external/tests/Service/GlobalStoragesServiceTest.php b/apps/files_external/tests/Service/GlobalStoragesServiceTest.php index 0a3749981c8..b4c8617830b 100644 --- a/apps/files_external/tests/Service/GlobalStoragesServiceTest.php +++ b/apps/files_external/tests/Service/GlobalStoragesServiceTest.php @@ -19,7 +19,7 @@ use OCA\Files_External\Service\GlobalStoragesService; class GlobalStoragesServiceTest extends StoragesServiceTestCase { protected function setUp(): void { parent::setUp(); - $this->service = new GlobalStoragesService($this->backendService, $this->dbConfig, $this->mountCache, $this->eventDispatcher); + $this->service = new GlobalStoragesService($this->backendService, $this->dbConfig, $this->mountCache, $this->eventDispatcher, $this->appConfig); } protected function tearDown(): void { diff --git a/apps/files_external/tests/Service/StoragesServiceTestCase.php b/apps/files_external/tests/Service/StoragesServiceTestCase.php index b41eb409468..991921880e8 100644 --- a/apps/files_external/tests/Service/StoragesServiceTestCase.php +++ b/apps/files_external/tests/Service/StoragesServiceTestCase.php @@ -28,6 +28,7 @@ use OCP\Files\Cache\ICache; use OCP\Files\Config\IUserMountCache; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IDBConnection; use OCP\IUser; @@ -57,12 +58,13 @@ class CleaningDBConfig extends DBConfigService { */ abstract class StoragesServiceTestCase extends \Test\TestCase { protected StoragesService $service; - protected BackendService $backendService; + protected BackendService&MockObject $backendService; protected string $dataDir; protected CleaningDBConfig $dbConfig; protected static array $hookCalls; protected IUserMountCache&MockObject $mountCache; protected IEventDispatcher&MockObject $eventDispatcher; + protected IAppConfig&MockObject $appConfig; protected function setUp(): void { parent::setUp(); @@ -77,6 +79,7 @@ abstract class StoragesServiceTestCase extends \Test\TestCase { $this->mountCache = $this->createMock(IUserMountCache::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->appConfig = $this->createMock(IAppConfig::class); // prepare BackendService mock $this->backendService = $this->createMock(BackendService::class); diff --git a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php index 2a2f4596fda..2119872ea5b 100644 --- a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php +++ b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php @@ -75,6 +75,7 @@ class UserGlobalStoragesServiceTest extends GlobalStoragesServiceTest { $this->groupManager, $this->mountCache, $this->eventDispatcher, + $this->appConfig, ); } diff --git a/apps/files_external/tests/Service/UserStoragesServiceTest.php b/apps/files_external/tests/Service/UserStoragesServiceTest.php index 0a2f291f6e4..8c7c4d1f2db 100644 --- a/apps/files_external/tests/Service/UserStoragesServiceTest.php +++ b/apps/files_external/tests/Service/UserStoragesServiceTest.php @@ -36,7 +36,7 @@ class UserStoragesServiceTest extends StoragesServiceTestCase { protected function setUp(): void { parent::setUp(); - $this->globalStoragesService = new GlobalStoragesService($this->backendService, $this->dbConfig, $this->mountCache, $this->eventDispatcher); + $this->globalStoragesService = new GlobalStoragesService($this->backendService, $this->dbConfig, $this->mountCache, $this->eventDispatcher, $this->appConfig); $this->userId = $this->getUniqueID('user_'); $this->createUser($this->userId, $this->userId); @@ -49,7 +49,7 @@ class UserStoragesServiceTest extends StoragesServiceTestCase { ->method('getUser') ->willReturn($this->user); - $this->service = new UserStoragesService($this->backendService, $this->dbConfig, $userSession, $this->mountCache, $this->eventDispatcher); + $this->service = new UserStoragesService($this->backendService, $this->dbConfig, $userSession, $this->mountCache, $this->eventDispatcher, $this->appConfig); } private function makeTestStorageData() { From 28301211fa48e032124e05eb11304f7703fc6ad8 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Fri, 26 Sep 2025 17:40:31 +0200 Subject: [PATCH 3/3] test(files_external): Ensure Home folder permissions are correct Signed-off-by: Louis Chemineau --- cypress/e2e/files_external/StorageUtils.ts | 22 +++++++++ .../home-folder-root-mount-permissions.cy.ts | 46 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 cypress/e2e/files_external/home-folder-root-mount-permissions.cy.ts diff --git a/cypress/e2e/files_external/StorageUtils.ts b/cypress/e2e/files_external/StorageUtils.ts index 0f7fec65edf..33402feac1f 100644 --- a/cypress/e2e/files_external/StorageUtils.ts +++ b/cypress/e2e/files_external/StorageUtils.ts @@ -9,10 +9,15 @@ export type StorageConfig = { [key: string]: string } +export type StorageMountOption = { + readonly: boolean +} + export enum StorageBackend { DAV = 'dav', SMB = 'smb', SFTP = 'sftp', + LOCAL = 'local', } export enum AuthBackend { @@ -22,6 +27,7 @@ export enum AuthBackend { SessionCredentials = 'password::sessioncredentials', UserGlobalAuth = 'password::global::user', UserProvided = 'password::userprovided', + Null = 'null::null', } /** @@ -35,4 +41,20 @@ export function createStorageWithConfig(mountPoint: string, storageBackend: Stor cy.log(`Creating storage with command: ${command}`) return cy.runOccCommand(command) + .then(({ stdout }) => { + return stdout.replace('Storage created with id ', '') + }) +} + +export function setStorageMountOptions(mountId: string, options: StorageMountOption) { + for (const [key, value] of Object.entries(options)) { + cy.runOccCommand(`files_external:option ${mountId} ${key} ${value}`) + } +} + +export function deleteAllExternalStorages() { + cy.runOccCommand('files_external:list --all --output=json').then(({ stdout }) => { + const list = JSON.parse(stdout) + list.forEach((storage) => cy.runOccCommand(`files_external:delete --yes ${storage.mount_id}`), { failOnNonZeroExit: false }) + }) } diff --git a/cypress/e2e/files_external/home-folder-root-mount-permissions.cy.ts b/cypress/e2e/files_external/home-folder-root-mount-permissions.cy.ts new file mode 100644 index 00000000000..b2938e3106b --- /dev/null +++ b/cypress/e2e/files_external/home-folder-root-mount-permissions.cy.ts @@ -0,0 +1,46 @@ +/** + * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { User } from '@nextcloud/cypress' +import { AuthBackend, createStorageWithConfig, deleteAllExternalStorages, setStorageMountOptions, StorageBackend } from './StorageUtils' + +describe('Home folder root mount permissions', { testIsolation: true }, () => { + let user1: User + + before(() => { + cy.runOccCommand('app:enable files_external') + cy.createRandomUser().then((user) => { user1 = user }) + }) + + after(() => { + deleteAllExternalStorages() + cy.runOccCommand('app:disable files_external') + }) + + it('Does not show write actions on read-only storage mounted at the root of the user\'s home folder', () => { + cy.login(user1) + cy.visit('/apps/files/') + cy.runOccCommand('config:app:get files overwrites_home_folders --default-value=[]') + .then(({ stdout }) => assert.equal(stdout.trim(), '[]')) + + cy.get('[data-cy-upload-picker=""]').should('exist') + + createStorageWithConfig('/', StorageBackend.LOCAL, AuthBackend.Null, { datadir: '/tmp' }) + .then((id) => setStorageMountOptions(id, { readonly: true })) + // HACK: somehow, we need to create an external folder targeting a subpath for the previous one to show. + createStorageWithConfig('/a', StorageBackend.LOCAL, AuthBackend.Null, { datadir: '/tmp' }) + cy.visit('/apps/files/') + cy.visit('/apps/files/') + cy.runOccCommand('config:app:get files overwrites_home_folders') + .then(({ stdout }) => assert.equal(stdout.trim(), '["files_external"]')) + cy.get('[data-cy-upload-picker=""]').should('not.exist') + + deleteAllExternalStorages() + cy.visit('/apps/files/') + cy.runOccCommand('config:app:get files overwrites_home_folders') + .then(({ stdout }) => assert.equal(stdout.trim(), '[]')) + cy.get('[data-cy-upload-picker=""]').should('exist') + }) +})