From 7cfa95e48b227ac6db8d9243d748b2f8a3a9fc8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 May 2026 10:54:27 +0200 Subject: [PATCH] fix(files_external): Move MountConfig to non-static services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/files_external/lib/MountConfig.php | 136 ++---------------- .../lib/Service/BackendService.php | 77 +++++++++- .../lib/Service/EncryptionService.php | 85 +++++++++++ .../Controller/StoragesControllerTestCase.php | 3 - .../tests/Service/StoragesServiceTestCase.php | 3 - 7 files changed, 171 insertions(+), 135 deletions(-) create mode 100644 apps/files_external/lib/Service/EncryptionService.php diff --git a/apps/files_external/composer/composer/autoload_classmap.php b/apps/files_external/composer/composer/autoload_classmap.php index ea66a333a15..e8bf64ac168 100644 --- a/apps/files_external/composer/composer/autoload_classmap.php +++ b/apps/files_external/composer/composer/autoload_classmap.php @@ -117,6 +117,7 @@ return array( 'OCA\\Files_External\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php', 'OCA\\Files_External\\Service\\BackendService' => $baseDir . '/../lib/Service/BackendService.php', 'OCA\\Files_External\\Service\\DBConfigService' => $baseDir . '/../lib/Service/DBConfigService.php', + 'OCA\\Files_External\\Service\\EncryptionService' => $baseDir . '/../lib/Service/EncryptionService.php', 'OCA\\Files_External\\Service\\GlobalStoragesService' => $baseDir . '/../lib/Service/GlobalStoragesService.php', 'OCA\\Files_External\\Service\\ImportLegacyStoragesService' => $baseDir . '/../lib/Service/ImportLegacyStoragesService.php', 'OCA\\Files_External\\Service\\LegacyStoragesService' => $baseDir . '/../lib/Service/LegacyStoragesService.php', diff --git a/apps/files_external/composer/composer/autoload_static.php b/apps/files_external/composer/composer/autoload_static.php index 37b3bb40524..9b0a1df75b0 100644 --- a/apps/files_external/composer/composer/autoload_static.php +++ b/apps/files_external/composer/composer/autoload_static.php @@ -132,6 +132,7 @@ class ComposerStaticInitFiles_External 'OCA\\Files_External\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php', 'OCA\\Files_External\\Service\\BackendService' => __DIR__ . '/..' . '/../lib/Service/BackendService.php', 'OCA\\Files_External\\Service\\DBConfigService' => __DIR__ . '/..' . '/../lib/Service/DBConfigService.php', + 'OCA\\Files_External\\Service\\EncryptionService' => __DIR__ . '/..' . '/../lib/Service/EncryptionService.php', 'OCA\\Files_External\\Service\\GlobalStoragesService' => __DIR__ . '/..' . '/../lib/Service/GlobalStoragesService.php', 'OCA\\Files_External\\Service\\ImportLegacyStoragesService' => __DIR__ . '/..' . '/../lib/Service/ImportLegacyStoragesService.php', 'OCA\\Files_External\\Service\\LegacyStoragesService' => __DIR__ . '/..' . '/../lib/Service/LegacyStoragesService.php', diff --git a/apps/files_external/lib/MountConfig.php b/apps/files_external/lib/MountConfig.php index 3b7d26925e0..d1b3d9eafaa 100644 --- a/apps/files_external/lib/MountConfig.php +++ b/apps/files_external/lib/MountConfig.php @@ -5,64 +5,34 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OCA\Files_External; -use OC\Files\Storage\Common; -use OCA\Files_External\Config\IConfigHandler; -use OCA\Files_External\Config\UserContext; use OCA\Files_External\Lib\Backend\Backend; use OCA\Files_External\Service\BackendService; -use OCA\Files_External\Service\GlobalStoragesService; -use OCA\Files_External\Service\UserGlobalStoragesService; -use OCA\Files_External\Service\UserStoragesService; -use OCP\Files\StorageNotAvailableException; -use OCP\IConfig; -use OCP\Security\ISecureRandom; +use OCA\Files_External\Service\EncryptionService; use OCP\Server; -use phpseclib\Crypt\AES; use Psr\Container\ContainerExceptionInterface; -use Psr\Log\LoggerInterface; /** * Class to configure mount.json globally and for users */ class MountConfig { - // TODO: make this class non-static and give it a proper namespace - public const MOUNT_TYPE_GLOBAL = 'global'; public const MOUNT_TYPE_GROUP = 'group'; public const MOUNT_TYPE_USER = 'user'; public const MOUNT_TYPE_PERSONAL = 'personal'; - // whether to skip backend test (for unit tests, as this static class is not mockable) - public static $skipTest = false; - - public function __construct( - private UserGlobalStoragesService $userGlobalStorageService, - private UserStoragesService $userStorageService, - private GlobalStoragesService $globalStorageService, - ) { - } - /** * @param mixed $input * @param string|null $userId * @return mixed * @throws ContainerExceptionInterface * @since 16.0.0 + * @deprecated 34.0.0 use BackendService instead */ public static function substitutePlaceholdersInConfig($input, ?string $userId = null) { - /** @var BackendService $backendService */ - $backendService = Server::get(BackendService::class); - /** @var IConfigHandler[] $handlers */ - $handlers = $backendService->getConfigHandlers(); - foreach ($handlers as $handler) { - if ($handler instanceof UserContext && $userId !== null) { - $handler->setUserId($userId); - } - $input = $handler->handle($input); - } - return $input; + return Server::get(BackendService::class)->applyConfigHandlers($input, $userId); } /** @@ -73,39 +43,10 @@ class MountConfig { * @param boolean $isPersonal * @return int see self::STATUS_* * @throws \Exception + * @deprecated 34.0.0 use BackendService instead */ public static function getBackendStatus($class, $options) { - if (self::$skipTest) { - return StorageNotAvailableException::STATUS_SUCCESS; - } - foreach ($options as $key => &$option) { - if ($key === 'password') { - // no replacements in passwords - continue; - } - $option = self::substitutePlaceholdersInConfig($option); - } - if (class_exists($class)) { - try { - /** @var Common $storage */ - $storage = new $class($options); - - try { - $result = $storage->test(); - $storage->setAvailability($result); - if ($result) { - return StorageNotAvailableException::STATUS_SUCCESS; - } - } catch (\Exception $e) { - $storage->setAvailability(false); - throw $e; - } - } catch (\Exception $exception) { - Server::get(LoggerInterface::class)->error($exception->getMessage(), ['exception' => $exception, 'app' => 'files_external']); - throw $exception; - } - } - return StorageNotAvailableException::STATUS_ERROR; + return Server::get(BackendService::class)->getBackendStatus($class, $options); } /** @@ -113,15 +54,10 @@ class MountConfig { * * @param array $options mount options * @return array updated options + * @deprecated 34.0.0 use EncryptionService instead */ public static function encryptPasswords($options) { - if (isset($options['password'])) { - $options['password_encrypted'] = self::encryptPassword($options['password']); - // do not unset the password, we want to keep the keys order - // on load... because that's how the UI currently works - $options['password'] = ''; - } - return $options; + return Server::get(EncryptionService::class)->encryptPasswords($options); } /** @@ -129,64 +65,19 @@ class MountConfig { * * @param array $options mount options * @return array updated options + * @deprecated 34.0.0 use EncryptionService instead */ public static function decryptPasswords($options) { - // note: legacy options might still have the unencrypted password in the "password" field - if (isset($options['password_encrypted'])) { - $options['password'] = self::decryptPassword($options['password_encrypted']); - unset($options['password_encrypted']); - } - return $options; - } - - /** - * Encrypt a single password - * - * @param string $password plain text password - * @return string encrypted password - */ - private static function encryptPassword($password) { - $cipher = self::getCipher(); - $iv = Server::get(ISecureRandom::class)->generate(16); - $cipher->setIV($iv); - return base64_encode($iv . $cipher->encrypt($password)); - } - - /** - * Decrypts a single password - * - * @param string $encryptedPassword encrypted password - * @return string plain text password - */ - private static function decryptPassword($encryptedPassword) { - $cipher = self::getCipher(); - $binaryPassword = base64_decode($encryptedPassword); - $iv = substr($binaryPassword, 0, 16); - $cipher->setIV($iv); - $binaryPassword = substr($binaryPassword, 16); - return $cipher->decrypt($binaryPassword); - } - - /** - * Returns the encryption cipher - * - * @return AES - */ - private static function getCipher() { - $cipher = new AES(AES::MODE_CBC); - $cipher->setKey(Server::get(IConfig::class)->getSystemValue('passwordsalt', null)); - return $cipher; + return Server::get(EncryptionService::class)->decryptPasswords($options); } /** * Computes a hash based on the given configuration. * This is mostly used to find out whether configurations * are the same. - * - * @param array $config - * @return string + * @throws \JsonException */ - public static function makeConfigHash($config) { + public static function makeConfigHash(array $config): string { $data = json_encode( [ 'c' => $config['backend'], @@ -195,7 +86,8 @@ class MountConfig { 'o' => $config['options'], 'p' => $config['priority'] ?? -1, 'mo' => $config['mountOptions'] ?? [], - ] + ], + JSON_THROW_ON_ERROR ); return hash('md5', $data); } diff --git a/apps/files_external/lib/Service/BackendService.php b/apps/files_external/lib/Service/BackendService.php index 1da8c618cca..7b12f41153a 100644 --- a/apps/files_external/lib/Service/BackendService.php +++ b/apps/files_external/lib/Service/BackendService.php @@ -7,8 +7,10 @@ */ namespace OCA\Files_External\Service; +use OC\Files\Storage\Common; use OCA\Files_External\AppInfo\Application; use OCA\Files_External\Config\IConfigHandler; +use OCA\Files_External\Config\UserContext; use OCA\Files_External\ConfigLexicon; use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\Backend\Backend; @@ -18,6 +20,8 @@ use OCP\EventDispatcher\GenericEvent; use OCP\EventDispatcher\IEventDispatcher; use OCP\IAppConfig; use OCP\Server; +use Psr\Container\ContainerExceptionInterface; +use Psr\Log\LoggerInterface; /** * Service class to manage backend definitions @@ -40,24 +44,26 @@ class BackendService { private array $userMountingBackends = []; /** @var Backend[] */ - private $backends = []; + private array $backends = []; /** @var IBackendProvider[] */ - private $backendProviders = []; + private array $backendProviders = []; /** @var AuthMechanism[] */ - private $authMechanisms = []; + private array $authMechanisms = []; /** @var IAuthMechanismProvider[] */ - private $authMechanismProviders = []; + private array $authMechanismProviders = []; /** @var callable[] */ - private $configHandlerLoaders = []; + private array $configHandlerLoaders = []; - private $configHandlers = []; + /** @var IConfigHandler[] */ + private array $configHandlers = []; public function __construct( protected readonly IAppConfig $appConfig, + private readonly LoggerInterface $logger, ) { } @@ -332,9 +338,66 @@ class BackendService { /** * @since 16.0.0 + * @return IConfigHandler[] */ - public function getConfigHandlers() { + public function getConfigHandlers(): array { $this->loadConfigHandlers(); return $this->configHandlers; } + + /** + * @param mixed $input + * @return mixed + * @throws ContainerExceptionInterface + */ + public function applyConfigHandlers($input, ?string $userId = null) { + /** @var IConfigHandler[] $handlers */ + $handlers = $this->getConfigHandlers(); + foreach ($handlers as $handler) { + if ($handler instanceof UserContext && $userId !== null) { + $handler->setUserId($userId); + } + $input = $handler->handle($input); + } + return $input; + } + + /** + * Test connecting using the given backend configuration + * + * @param string $class backend class name + * @param array $options backend configuration options + * @return StorageNotAvailableException::STATUS_* + * @throws \Exception + */ + public function getBackendStatus(string $class, array $options): int { + foreach ($options as $key => &$option) { + if ($key === 'password') { + // no replacements in passwords + continue; + } + $option = $this->applyConfigHandlers($option); + } + if (class_exists($class)) { + try { + /** @var Common $storage */ + $storage = new $class($options); + + try { + $result = $storage->test(); + $storage->setAvailability($result); + if ($result) { + return StorageNotAvailableException::STATUS_SUCCESS; + } + } catch (\Exception $e) { + $storage->setAvailability(false); + throw $e; + } + } catch (\Exception $exception) { + $this->logger->error($exception->getMessage(), ['exception' => $exception]); + throw $exception; + } + } + return StorageNotAvailableException::STATUS_ERROR; + } } diff --git a/apps/files_external/lib/Service/EncryptionService.php b/apps/files_external/lib/Service/EncryptionService.php new file mode 100644 index 00000000000..c36f6d06132 --- /dev/null +++ b/apps/files_external/lib/Service/EncryptionService.php @@ -0,0 +1,85 @@ +encryptPassword($options['password']); + // do not unset the password, we want to keep the keys order + // on load... because that's how the UI currently works + $options['password'] = ''; + } + return $options; + } + + /** + * Decrypt passwords in the given config options + * + * @param array $options mount options + * @return array updated options + */ + public function decryptPasswords(array $options): array { + // note: legacy options might still have the unencrypted password in the "password" field + if (isset($options['password_encrypted'])) { + $options['password'] = $this->decryptPassword($options['password_encrypted']); + unset($options['password_encrypted']); + } + return $options; + } + + /** + * Encrypt a single password + */ + private function encryptPassword(string $password): string { + $cipher = $this->getCipher(); + $iv = $this->secureRandom->generate(16); + $cipher->setIV($iv); + return base64_encode($iv . $cipher->encrypt($password)); + } + + /** + * Decrypts a single password + */ + private function decryptPassword(string $encryptedPassword): string { + $cipher = $this->getCipher(); + $binaryPassword = base64_decode($encryptedPassword); + $iv = substr($binaryPassword, 0, 16); + $cipher->setIV($iv); + $binaryPassword = substr($binaryPassword, 16); + return $cipher->decrypt($binaryPassword); + } + + /** + * Returns the encryption cipher + */ + private function getCipher(): AES { + $cipher = new AES(AES::MODE_CBC); + $cipher->setKey($this->config->getSystemValue('passwordsalt', null)); + return $cipher; + } +} diff --git a/apps/files_external/tests/Controller/StoragesControllerTestCase.php b/apps/files_external/tests/Controller/StoragesControllerTestCase.php index 12e3e13f943..33c8eb26f5f 100644 --- a/apps/files_external/tests/Controller/StoragesControllerTestCase.php +++ b/apps/files_external/tests/Controller/StoragesControllerTestCase.php @@ -15,7 +15,6 @@ use OCA\Files_External\Lib\Auth\NullMechanism; use OCA\Files_External\Lib\Backend\Backend; use OCA\Files_External\Lib\Backend\SMB; use OCA\Files_External\Lib\StorageConfig; -use OCA\Files_External\MountConfig; use OCA\Files_External\NotFoundException; use OCA\Files_External\Service\GlobalStoragesService; use OCA\Files_External\Service\UserStoragesService; @@ -28,11 +27,9 @@ abstract class StoragesControllerTestCase extends \Test\TestCase { protected function setUp(): void { parent::setUp(); - MountConfig::$skipTest = true; } protected function tearDown(): void { - MountConfig::$skipTest = false; parent::tearDown(); } diff --git a/apps/files_external/tests/Service/StoragesServiceTestCase.php b/apps/files_external/tests/Service/StoragesServiceTestCase.php index 1ad78a34a60..5eed8294b8f 100644 --- a/apps/files_external/tests/Service/StoragesServiceTestCase.php +++ b/apps/files_external/tests/Service/StoragesServiceTestCase.php @@ -17,7 +17,6 @@ use OCA\Files_External\Lib\Backend\Backend; use OCA\Files_External\Lib\Backend\InvalidBackend; use OCA\Files_External\Lib\Backend\SMB; use OCA\Files_External\Lib\StorageConfig; -use OCA\Files_External\MountConfig; use OCA\Files_External\NotFoundException; use OCA\Files_External\Service\BackendService; use OCA\Files_External\Service\DBConfigService; @@ -72,7 +71,6 @@ abstract class StoragesServiceTestCase extends \Test\TestCase { 'datadirectory', \OC::$SERVERROOT . '/data/' ); - MountConfig::$skipTest = true; $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->appConfig = $this->createMock(IAppConfig::class); @@ -140,7 +138,6 @@ abstract class StoragesServiceTestCase extends \Test\TestCase { } protected function tearDown(): void { - MountConfig::$skipTest = false; self::$hookCalls = []; if ($this->dbConfig) { $this->dbConfig->clean();