From cd4ce921fc43c251c9dbdcfe53c943877856aa10 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 27 Jul 2026 09:39:21 +0200 Subject: [PATCH 1/2] perf: Ensure encryption wrapper is only setup when needed The app needs to be enabled and the settings need to be on. Signed-off-by: Carl Schwan --- lib/private/Encryption/Manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Encryption/Manager.php b/lib/private/Encryption/Manager.php index 84540684df831..4f4ca5a2ecd58 100644 --- a/lib/private/Encryption/Manager.php +++ b/lib/private/Encryption/Manager.php @@ -206,7 +206,7 @@ public function getDefaultEncryptionModuleId() { */ public function setupStorage(): void { // If encryption is disabled and there are no loaded modules it makes no sense to load the wrapper - if (!empty($this->encryptionModules) || $this->isEnabled()) { + if ($this->encryptionModules !== [] && $this->isEnabled()) { $encryptionWrapper = new EncryptionWrapper($this->arrayCache, $this, $this->logger); Filesystem::addStorageWrapper('oc_encryption', $encryptionWrapper->wrapStorage(...), 2); } From 755d6ce3e49a810ae2b2769a52abcc3535a2b806 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 27 Jul 2026 09:54:56 +0200 Subject: [PATCH 2/2] refactor: Cleanup encryption manager - Port to IAppConfig - Port to IRootFolder - Add typing - Add Consumable attribute to OCP interface Signed-off-by: Carl Schwan --- build/psalm-baseline.xml | 5 - lib/private/Encryption/EncryptionWrapper.php | 6 +- lib/private/Encryption/Manager.php | 112 ++++++------------- lib/private/Server.php | 3 +- lib/public/Encryption/IManager.php | 37 +++--- tests/lib/Encryption/ManagerTest.php | 72 +++++------- tests/lib/Traits/EncryptionTrait.php | 17 +-- 7 files changed, 89 insertions(+), 163 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 7e59e0d212c8e..e2d6a6a5a4866 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3661,11 +3661,6 @@ - - - - - diff --git a/lib/private/Encryption/EncryptionWrapper.php b/lib/private/Encryption/EncryptionWrapper.php index 23a7af4486319..a6bf2aefa401f 100644 --- a/lib/private/Encryption/EncryptionWrapper.php +++ b/lib/private/Encryption/EncryptionWrapper.php @@ -45,13 +45,9 @@ public function __construct( /** * Wraps the given storage when it is not a shared storage * - * @param string $mountPoint - * @param IStorage $storage - * @param IMountPoint $mount * @param bool $force apply the wrapper even if the storage normally has encryption disabled, helpful for repair steps - * @return Encryption|IStorage */ - public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $mount, bool $force = false) { + public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $mount, bool $force = false): Encryption|IStorage { $parameters = [ 'storage' => $storage, 'mountPoint' => $mountPoint, diff --git a/lib/private/Encryption/Manager.php b/lib/private/Encryption/Manager.php index 4f4ca5a2ecd58..924fa978e9405 100644 --- a/lib/private/Encryption/Manager.php +++ b/lib/private/Encryption/Manager.php @@ -1,5 +1,7 @@ + */ protected array $encryptionModules; public function __construct( - protected IConfig $config, - protected LoggerInterface $logger, - protected IL10N $l, - protected View $rootView, - protected Util $util, - protected ArrayCache $arrayCache, + protected readonly IConfig $config, + protected readonly IAppConfig $appConfig, + protected readonly LoggerInterface $logger, + protected readonly IL10N $l, + protected readonly IRootFolder $rootFolder, + protected readonly Util $util, + protected readonly ArrayCache $arrayCache, ) { $this->encryptionModules = []; } - /** - * Check if encryption is enabled - * - * @return bool true if enabled, false if not - */ #[\Override] - public function isEnabled() { + public function isEnabled(): bool { $installed = $this->config->getSystemValueBool('installed', false); if (!$installed) { return false; } - return Server::get(IAppConfig::class)->getValueBool('core', 'encryption_enabled'); + return $this->appConfig->getValueBool('core', 'encryption_enabled'); } /** - * check if new encryption is ready + * Check if new encryption is ready * - * @return bool * @throws ServiceUnavailableException */ - public function isReady() { + public function isReady(): bool { if ($this->isKeyStorageReady() === false) { throw new ServiceUnavailableException('Key Storage is not ready'); } @@ -68,10 +68,7 @@ public function isReady() { return true; } - /** - * @param string $user - */ - public function isReadyForUser($user) { + public function isReadyForUser(string $user): bool { if (!$this->isReady()) { return false; } @@ -87,16 +84,8 @@ public function isReadyForUser($user) { return true; } - /** - * Registers an callback function which must return an encryption module instance - * - * @param string $id - * @param string $displayName - * @param callable $callback - * @throws Exceptions\ModuleAlreadyExistsException - */ #[\Override] - public function registerEncryptionModule($id, $displayName, callable $callback) { + public function registerEncryptionModule(string $id, string $displayName, callable $callback): void { if (isset($this->encryptionModules[$id])) { throw new ModuleAlreadyExistsException($id, $displayName); } @@ -114,35 +103,18 @@ public function registerEncryptionModule($id, $displayName, callable $callback) } } - /** - * Unregisters an encryption module - * - * @param string $moduleId - */ #[\Override] - public function unregisterEncryptionModule($moduleId) { + public function unregisterEncryptionModule(string $moduleId): void { unset($this->encryptionModules[$moduleId]); } - /** - * get a list of all encryption modules - * - * @return array [id => ['id' => $id, 'displayName' => $displayName, 'callback' => callback]] - */ #[\Override] - public function getEncryptionModules() { + public function getEncryptionModules(): array { return $this->encryptionModules; } - /** - * get a specific encryption module - * - * @param string $moduleId - * @return IEncryptionModule - * @throws Exceptions\ModuleDoesNotExistsException - */ #[\Override] - public function getEncryptionModule($moduleId = '') { + public function getEncryptionModule(string $moduleId = ''): IEncryptionModule { if (empty($moduleId)) { return $this->getDefaultEncryptionModule(); } @@ -155,12 +127,11 @@ public function getEncryptionModule($moduleId = '') { } /** - * get default encryption module + * Get default encryption module * - * @return IEncryptionModule * @throws Exceptions\ModuleDoesNotExistsException */ - protected function getDefaultEncryptionModule() { + protected function getDefaultEncryptionModule(): IEncryptionModule { $defaultModuleId = $this->getDefaultEncryptionModuleId(); if (empty($defaultModuleId)) { $message = 'No default encryption module defined'; @@ -173,32 +144,21 @@ protected function getDefaultEncryptionModule() { throw new ModuleDoesNotExistsException($message); } - /** - * set default encryption module Id - * - * @param string $moduleId - * @return bool - */ #[\Override] - public function setDefaultEncryptionModule($moduleId) { + public function setDefaultEncryptionModule(string $moduleId): bool { try { $this->getEncryptionModule($moduleId); } catch (\Exception $e) { return false; } - $this->config->setAppValue('core', 'default_encryption_module', $moduleId); + $this->appConfig->setValueString('core', 'default_encryption_module', $moduleId); return true; } - /** - * get default encryption module Id - * - * @return string - */ #[\Override] - public function getDefaultEncryptionModuleId() { - return $this->config->getAppValue('core', 'default_encryption_module'); + public function getDefaultEncryptionModuleId(): string { + return $this->appConfig->getValueString('core', 'default_encryption_module'); } /** @@ -212,17 +172,15 @@ public function setupStorage(): void { } } - public function forceWrapStorage(IMountPoint $mountPoint, IStorage $storage) { + public function forceWrapStorage(IMountPoint $mountPoint, IStorage $storage): Encryption|IStorage { $encryptionWrapper = new EncryptionWrapper($this->arrayCache, $this, $this->logger); return $encryptionWrapper->wrapStorage($mountPoint->getMountPoint(), $storage, $mountPoint, true); } /** - * check if key storage is ready - * - * @return bool + * Check if key storage is ready */ - protected function isKeyStorageReady() { + protected function isKeyStorageReady(): bool { $rootDir = $this->util->getKeyStorageRoot(); // the default root is always valid @@ -231,10 +189,6 @@ protected function isKeyStorageReady() { } // check if key storage is mounted correctly - if ($this->rootView->file_exists($rootDir . '/' . Storage::KEY_STORAGE_MARKER)) { - return true; - } - - return false; + return $this->rootFolder->nodeExists($rootDir . '/' . Storage::KEY_STORAGE_MARKER); } } diff --git a/lib/private/Server.php b/lib/private/Server.php index a1caf17da3984..5d0bb96d5e7a7 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -347,9 +347,10 @@ public function __construct( $this->registerService(Encryption\Manager::class, function (Server $c): Encryption\Manager { return new Encryption\Manager( $c->get(IConfig::class), + $c->get(IAppConfig::class), $c->get(LoggerInterface::class), $c->get(IFactory::class)->get('core'), - $c->get(View::class), + $c->get(IRootFolder::class), $c->get(Encryption\Util::class), new ArrayCache() ); diff --git a/lib/public/Encryption/IManager.php b/lib/public/Encryption/IManager.php index d33276ee089f6..afed3c8bcbf74 100644 --- a/lib/public/Encryption/IManager.php +++ b/lib/public/Encryption/IManager.php @@ -1,5 +1,7 @@ ['id' => $id, 'displayName' => $displayName, 'callback' => callback]] + * @return array * @since 8.1.0 */ - public function getEncryptionModules(); + public function getEncryptionModules(): array; /** - * get a specific encryption module + * Get a specific encryption module. * * @param string $moduleId Empty to get the default module - * @return IEncryptionModule * @throws ModuleDoesNotExistsException * @since 8.1.0 */ - public function getEncryptionModule($moduleId = ''); + public function getEncryptionModule(string $moduleId = ''): IEncryptionModule; /** - * get default encryption module Id + * Get default encryption module Id * - * @return string * @since 8.1.0 */ - public function getDefaultEncryptionModuleId(); + public function getDefaultEncryptionModuleId(): string; /** - * set default encryption module Id + * Set default encryption module Id. * - * @param string $moduleId - * @return string * @since 8.1.0 */ - public function setDefaultEncryptionModule($moduleId); + public function setDefaultEncryptionModule(string $moduleId): bool; } diff --git a/tests/lib/Encryption/ManagerTest.php b/tests/lib/Encryption/ManagerTest.php index fb7aec3a1c833..f02b637663402 100644 --- a/tests/lib/Encryption/ManagerTest.php +++ b/tests/lib/Encryption/ManagerTest.php @@ -1,5 +1,7 @@ config = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->l10n = $this->createMock(IL10N::class); - $this->view = $this->createMock(View::class); + $this->rootFolder = $this->createMock(IRootFolder::class); $this->util = $this->createMock(Util::class); $this->arrayCache = $this->createMock(ArrayCache::class); - $this->manager = new Manager($this->config, $this->logger, $this->l10n, $this->view, $this->util, $this->arrayCache); + $this->manager = new Manager($this->config, $this->appConfig, $this->logger, $this->l10n, $this->rootFolder, $this->util, $this->arrayCache); } public function testManagerIsDisabled(): void { @@ -76,20 +67,17 @@ public function testManagerIsDisabledIfDisabledButModules(): void { $this->assertFalse($this->manager->isEnabled()); } - #[Group(name: 'DB')] public function testManagerIsEnabled(): void { - $appConfig = Server::get(IAppConfig::class); - $appConfig->setValueBool('core', 'encryption_enabled', true); + $this->appConfig->expects($this->any())->method('getValueBool')->with('core', 'encryption_enabled')->willReturn(true); $this->config->expects($this->any())->method('getSystemValueBool')->willReturn(true); $result = $this->manager->isEnabled(); - $appConfig->deleteKey('core', 'encryption_enabled'); $this->assertTrue($result); } public function testModuleRegistration() { - $this->config->expects($this->any())->method('getAppValue')->willReturn('yes'); + $this->appConfig->expects($this->any())->method('getValueBool')->willReturn(true); $this->addNewEncryptionModule($this->manager, 0); $this->assertCount(1, $this->manager->getEncryptionModules()); @@ -106,7 +94,7 @@ public function testModuleReRegistration($manager): void { } public function testModuleUnRegistration(): void { - $this->config->expects($this->any())->method('getAppValue')->willReturn(true); + $this->appConfig->expects($this->any())->method('getValueBool')->willReturn(true); $this->addNewEncryptionModule($this->manager, 0); $this->assertCount(1, $this->manager->getEncryptionModules()); @@ -118,7 +106,7 @@ public function testGetEncryptionModuleUnknown(): void { $this->expectException(ModuleDoesNotExistsException::class); $this->expectExceptionMessage('Module with ID: unknown does not exist.'); - $this->config->expects($this->any())->method('getAppValue')->willReturn(true); + $this->appConfig->expects($this->any())->method('getValueBool')->willReturn(true); $this->addNewEncryptionModule($this->manager, 0); $this->assertCount(1, $this->manager->getEncryptionModules()); $this->manager->getEncryptionModule('unknown'); @@ -126,10 +114,10 @@ public function testGetEncryptionModuleUnknown(): void { public function testGetEncryptionModuleEmpty(): void { global $defaultId; - $defaultId = null; + $defaultId = ''; - $this->config->expects($this->any()) - ->method('getAppValue') + $this->appConfig->expects($this->any()) + ->method('getValueString') ->with('core', 'default_encryption_module') ->willReturnCallback(function () { global $defaultId; @@ -150,10 +138,10 @@ public function testGetEncryptionModuleEmpty(): void { public function testGetEncryptionModule(): void { global $defaultId; - $defaultId = null; + $defaultId = ''; - $this->config->expects($this->any()) - ->method('getAppValue') + $this->appConfig->expects($this->any()) + ->method('getValueString') ->with('core', 'default_encryption_module') ->willReturnCallback(function () { global $defaultId; @@ -175,10 +163,10 @@ public function testGetEncryptionModule(): void { public function testSetDefaultEncryptionModule(): void { global $defaultId; - $defaultId = null; + $defaultId = ''; - $this->config->expects($this->any()) - ->method('getAppValue') + $this->appConfig->expects($this->any()) + ->method('getValueString') ->with('core', 'default_encryption_module') ->willReturnCallback(function () { global $defaultId; @@ -195,8 +183,8 @@ public function testSetDefaultEncryptionModule(): void { $this->assertEquals('ID0', $this->manager->getDefaultEncryptionModuleId()); // Set to an existing module - $this->config->expects($this->once()) - ->method('setAppValue') + $this->appConfig->expects($this->once()) + ->method('setValueString') ->with('core', 'default_encryption_module', 'ID1'); $this->assertTrue($this->manager->setDefaultEncryptionModule('ID1')); $defaultId = 'ID1'; diff --git a/tests/lib/Traits/EncryptionTrait.php b/tests/lib/Traits/EncryptionTrait.php index 7b3c016d52276..187221c7c8765 100644 --- a/tests/lib/Traits/EncryptionTrait.php +++ b/tests/lib/Traits/EncryptionTrait.php @@ -19,7 +19,6 @@ use OCP\Encryption\IManager; use OCP\Files\ISetupManager; use OCP\IAppConfig; -use OCP\IConfig; use OCP\IUserManager; use OCP\IUserSession; use OCP\Server; @@ -43,11 +42,6 @@ abstract protected static function assertTrue($condition, string $message = ''): private IUserManager $userManagerEncTrait; private ISetupManager $setupManagerEncTrait; - /** - * @var IConfig - */ - private $config; - private IAppConfig $appConfig; /** @@ -110,20 +104,19 @@ protected function setUpEncryptionTrait() { $this->encryptionApp = new Application([], $isReady); - $this->config = Server::get(IConfig::class); $this->appConfig = Server::get(IAppConfig::class); $this->encryptionWasEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled'); - $this->originalEncryptionModule = $this->config->getAppValue('core', 'default_encryption_module'); - $this->config->setAppValue('core', 'default_encryption_module', Encryption::ID); + $this->originalEncryptionModule = $this->appConfig->getValueString('core', 'default_encryption_module'); + $this->appConfig->setValueString('core', 'default_encryption_module', Encryption::ID); $this->appConfig->setValueBool('core', 'encryption_enabled', true); $this->assertTrue(Server::get(\OCP\Encryption\IManager::class)->isEnabled()); } protected function tearDownEncryptionTrait() { - if ($this->config) { + if ($this->appConfig) { $this->appConfig->setValueBool('core', 'encryption_enabled', $this->encryptionWasEnabled); - $this->config->setAppValue('core', 'default_encryption_module', $this->originalEncryptionModule); - $this->config->deleteAppValue('encryption', 'useMasterKey'); + $this->appConfig->setValueBool('core', 'default_encryption_module', $this->originalEncryptionModule); + $this->appConfig->deleteKey('encryption', 'useMasterKey'); } } }