fix(files_external): Move MountConfig to non-static services

.github/workflows/files-external-sftp.yml

@@ -98,7 +98,6 @@ jobs:
       - name: PHPUnit
         run: composer run test:files_external -- \
           apps/files_external/tests/Storage/SftpTest.php \
-          apps/files_external/tests/Storage/SFTP_KeyTest.php \
           --log-junit junit.xml \
           ${{ matrix.coverage && '--coverage-clover ./clover.xml' || '' }}
 

apps/files_external/composer/composer/autoload_classmap.php

@@ -117,6 +117,7 @@
     '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',

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',

apps/files_external/lib/Command/Verify.php

@@ -10,8 +10,8 @@
 use OC\Core\Command\Base;
 use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException;
 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\GlobalStoragesService;
 use OCP\AppFramework\Http;
 use OCP\Files\StorageNotAvailableException;
@@ -23,6 +23,7 @@
 class Verify extends Base {
 	public function __construct(
 		protected GlobalStoragesService $globalService,
+		protected BackendService $backendService,
 	) {
 		parent::__construct();
 	}
@@ -96,7 +97,7 @@ private function updateStorageStatus(StorageConfig &$storage, $configInput, Outp
 			$backend = $storage->getBackend();
 			// update status (can be time-consuming)
 			$storage->setStatus(
-				MountConfig::getBackendStatus(
+				$this->backendService->getBackendStatus(
 					$backend->getStorageClass(),
 					$storage->getBackendOptions(),
 				)

apps/files_external/lib/Config/ConfigAdapter.php

@@ -13,7 +13,7 @@
 use OC\Files\Storage\Wrapper\KnownMtime;
 use OCA\Files_External\Lib\PersonalMount;
 use OCA\Files_External\Lib\StorageConfig;
-use OCA\Files_External\MountConfig;
+use OCA\Files_External\Service\BackendService;
 use OCA\Files_External\Service\UserGlobalStoragesService;
 use OCA\Files_External\Service\UserStoragesService;
 use OCP\Files\Config\IAuthoritativeMountProvider;
@@ -39,6 +39,7 @@ class ConfigAdapter implements IMountProvider, IAuthoritativeMountProvider, IPar
 	public function __construct(
 		private UserStoragesService $userStoragesService,
 		private UserGlobalStoragesService $userGlobalStoragesService,
+		private BackendService $backendService,
 		private ClockInterface $clock,
 	) {
 	}
@@ -63,7 +64,7 @@ private function validateObjectStoreClassString(string $class): string {
 	 */
 	private function prepareStorageConfig(StorageConfig &$storage, IUser $user): void {
 		foreach ($storage->getBackendOptions() as $option => $value) {
-			$storage->setBackendOption($option, MountConfig::substitutePlaceholdersInConfig($value, $user->getUID()));
+			$storage->setBackendOption($option, $this->backendService->applyConfigHandlers($value, $user->getUID()));
 		}
 
 		$objectStore = $storage->getBackendOption('objectstore');

apps/files_external/lib/Controller/GlobalStoragesController.php

@@ -8,6 +8,7 @@
 namespace OCA\Files_External\Controller;
 
 use OCA\Files_External\NotFoundException;
+use OCA\Files_External\Service\BackendService;
 use OCA\Files_External\Service\GlobalStoragesService;
 use OCA\Files_External\Settings\Admin;
 use OCP\AppFramework\Http;
@@ -37,6 +38,7 @@ public function __construct(
 		IUserSession $userSession,
 		IGroupManager $groupManager,
 		IConfig $config,
+		BackendService $backendService,
 	) {
 		parent::__construct(
 			$appName,
@@ -46,7 +48,8 @@ public function __construct(
 			$logger,
 			$userSession,
 			$groupManager,
-			$config
+			$config,
+			$backendService,
 		);
 	}
 

apps/files_external/lib/Controller/StoragesController.php

@@ -11,8 +11,8 @@
 use OCA\Files_External\Lib\Backend\Backend;
 use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException;
 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\StoragesService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
@@ -48,6 +48,7 @@ public function __construct(
 		protected IUserSession $userSession,
 		protected IGroupManager $groupManager,
 		protected IConfig $config,
+		protected BackendService $backendService,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -222,7 +223,7 @@ protected function updateStorageStatus(StorageConfig &$storage) {
 			$backend = $storage->getBackend();
 			// update status (can be time-consuming)
 			$storage->setStatus(
-				MountConfig::getBackendStatus(
+				$this->backendService->getBackendStatus(
 					$backend->getStorageClass(),
 					$storage->getBackendOptions(),
 				)

apps/files_external/lib/Controller/UserGlobalStoragesController.php

@@ -14,6 +14,7 @@
 use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException;
 use OCA\Files_External\Lib\StorageConfig;
 use OCA\Files_External\NotFoundException;
+use OCA\Files_External\Service\BackendService;
 use OCA\Files_External\Service\UserGlobalStoragesService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -50,6 +51,7 @@ public function __construct(
 		IUserSession $userSession,
 		IGroupManager $groupManager,
 		IConfig $config,
+		BackendService $backendService,
 	) {
 		parent::__construct(
 			$appName,
@@ -59,7 +61,8 @@ public function __construct(
 			$logger,
 			$userSession,
 			$groupManager,
-			$config
+			$config,
+			$backendService,
 		);
 	}
 

apps/files_external/lib/Controller/UserStoragesController.php

@@ -11,6 +11,7 @@
 use OCA\Files_External\Lib\Backend\Backend;
 use OCA\Files_External\Lib\StorageConfig;
 use OCA\Files_External\NotFoundException;
+use OCA\Files_External\Service\BackendService;
 use OCA\Files_External\Service\UserStoragesService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -40,6 +41,7 @@ public function __construct(
 		IUserSession $userSession,
 		IGroupManager $groupManager,
 		IConfig $config,
+		BackendService $backendService,
 	) {
 		parent::__construct(
 			$appName,
@@ -49,7 +51,8 @@ public function __construct(
 			$logger,
 			$userSession,
 			$groupManager,
-			$config
+			$config,
+			$backendService,
 		);
 	}
 

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,120 +43,41 @@ public static function substitutePlaceholdersInConfig($input, ?string $userId =
 	 * @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);
 	}
 
 	/**
 	 * Encrypt passwords in the given config options
 	 *
 	 * @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);
 	}
 
 	/**
 	 * Decrypt passwords in the given config options
 	 *
 	 * @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 @@ public static function makeConfigHash($config) {
 				'o' => $config['options'],
 				'p' => $config['priority'] ?? -1,
 				'mo' => $config['mountOptions'] ?? [],
-			]
+			],
+			JSON_THROW_ON_ERROR
 		);
 		return hash('md5', $data);
 	}