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

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

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

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/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);
 	}

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;
+	}
 }

apps/files_external/lib/Service/EncryptionService.php

@@ -0,0 +1,85 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCA\Files_External\Service;
+
+use OCP\IConfig;
+use OCP\Security\ISecureRandom;
+use phpseclib\Crypt\AES;
+
+class EncryptionService {
+	public function __construct(
+		private IConfig $config,
+		private ISecureRandom $secureRandom,
+	) {
+	}
+
+	/**
+	 * Encrypt passwords in the given config options
+	 *
+	 * @param array $options mount options
+	 * @return array updated options
+	 */
+	public function encryptPasswords(array $options): array {
+		if (isset($options['password'])) {
+			$options['password_encrypted'] = $this->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;
+	}
+}

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();
 	}
 

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();