diff --git a/.github/workflows/files-external-sftp.yml b/.github/workflows/files-external-sftp.yml index 8d6049fc1384d..22f5e0ac4be0a 100644 --- a/.github/workflows/files-external-sftp.yml +++ b/.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' || '' }} diff --git a/apps/files_external/composer/composer/autoload_classmap.php b/apps/files_external/composer/composer/autoload_classmap.php index ea66a333a15a4..e8bf64ac1688d 100644 --- a/apps/files_external/composer/composer/autoload_classmap.php +++ b/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', diff --git a/apps/files_external/composer/composer/autoload_static.php b/apps/files_external/composer/composer/autoload_static.php index 37b3bb4052449..9b0a1df75b051 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/Command/Verify.php b/apps/files_external/lib/Command/Verify.php index 80ba831e6f5cc..c618291edf879 100644 --- a/apps/files_external/lib/Command/Verify.php +++ b/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(), ) diff --git a/apps/files_external/lib/Config/ConfigAdapter.php b/apps/files_external/lib/Config/ConfigAdapter.php index 6a306c0f11738..ed65b0d860030 100644 --- a/apps/files_external/lib/Config/ConfigAdapter.php +++ b/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'); diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php index fa2fdfaa9a7a5..f350576362405 100644 --- a/apps/files_external/lib/Controller/GlobalStoragesController.php +++ b/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, ); } diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php index 9a61d9bfd6eba..07afd0ceb44e2 100644 --- a/apps/files_external/lib/Controller/StoragesController.php +++ b/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(), ) diff --git a/apps/files_external/lib/Controller/UserGlobalStoragesController.php b/apps/files_external/lib/Controller/UserGlobalStoragesController.php index 5314dd572050a..21fac9fccf099 100644 --- a/apps/files_external/lib/Controller/UserGlobalStoragesController.php +++ b/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, ); } diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php index fcedad3dfdb61..2ed6d4d92d138 100644 --- a/apps/files_external/lib/Controller/UserStoragesController.php +++ b/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, ); } diff --git a/apps/files_external/lib/MountConfig.php b/apps/files_external/lib/MountConfig.php index 3b7d26925e089..d1b3d9eafaa9d 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 @@ 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); } /** @@ -113,15 +54,10 @@ public static function getBackendStatus($class, $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); } /** @@ -129,64 +65,19 @@ public static function encryptPasswords($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); } diff --git a/apps/files_external/lib/Service/BackendService.php b/apps/files_external/lib/Service/BackendService.php index 1da8c618ccadf..c9fc5473acfe6 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; @@ -16,8 +18,11 @@ use OCA\Files_External\Lib\Config\IBackendProvider; use OCP\EventDispatcher\GenericEvent; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\StorageNotAvailableException; use OCP\IAppConfig; use OCP\Server; +use Psr\Container\ContainerExceptionInterface; +use Psr\Log\LoggerInterface; /** * Service class to manage backend definitions @@ -40,24 +45,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 +339,66 @@ protected function loadConfigHandlers():void { /** * @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 0000000000000..c36f6d0613269 --- /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/lib/Service/ImportLegacyStoragesService.php b/apps/files_external/lib/Service/ImportLegacyStoragesService.php index d143f2a4c43ba..a71aec7917611 100644 --- a/apps/files_external/lib/Service/ImportLegacyStoragesService.php +++ b/apps/files_external/lib/Service/ImportLegacyStoragesService.php @@ -11,13 +11,6 @@ class ImportLegacyStoragesService extends LegacyStoragesService { private $data; - /** - * @param BackendService $backendService - */ - public function __construct(BackendService $backendService) { - $this->backendService = $backendService; - } - public function setData($data) { $this->data = $data; } diff --git a/apps/files_external/lib/Service/LegacyStoragesService.php b/apps/files_external/lib/Service/LegacyStoragesService.php index 9f199a89b3f10..08cef135bc71b 100644 --- a/apps/files_external/lib/Service/LegacyStoragesService.php +++ b/apps/files_external/lib/Service/LegacyStoragesService.php @@ -16,9 +16,11 @@ * Read mount config from legacy mount.json */ abstract class LegacyStoragesService { - /** @var BackendService */ - protected $backendService; - + public function __construct( + protected BackendService $backendService, + protected EncryptionService $encryptionService, + ) { + } /** * Read legacy config data * @@ -132,7 +134,7 @@ public function getAllStorages() { $relativeMountPath = rtrim($parts[2], '/'); // note: we cannot do this after the loop because the decrypted config // options might be needed for the config hash - $storageOptions['options'] = MountConfig::decryptPasswords($storageOptions['options']); + $storageOptions['options'] = $this->encryptionService->decryptPasswords($storageOptions['options']); if (!isset($storageOptions['backend'])) { $storageOptions['backend'] = $storageOptions['class']; // legacy compat } diff --git a/apps/files_external/tests/Config/ConfigAdapterTest.php b/apps/files_external/tests/Config/ConfigAdapterTest.php index fe7aa7d30e37e..6a84e913501bf 100644 --- a/apps/files_external/tests/Config/ConfigAdapterTest.php +++ b/apps/files_external/tests/Config/ConfigAdapterTest.php @@ -125,7 +125,12 @@ public function setUp(): void { $this->userStoragesService = Server::get(UserStoragesService::class); $this->userGlobalStoragesService = Server::get(UserGlobalStoragesService::class); - $this->adapter = new ConfigAdapter($this->userStoragesService, $this->userGlobalStoragesService, $this->createMock(ClockInterface::class)); + $this->adapter = new ConfigAdapter( + $this->userStoragesService, + $this->userGlobalStoragesService, + $this->createMock(BackendService::class), + $this->createMock(ClockInterface::class), + ); $this->user = $this->createMock(IUser::class); $this->user->method('getUID')->willReturn('user1'); diff --git a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php index 74a27eb15e4fb..7445c3e16d88d 100644 --- a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php @@ -49,7 +49,8 @@ private function createController(bool $allowCreateLocal = true): GlobalStorages $this->createMock(LoggerInterface::class), $session, $this->createMock(IGroupManager::class), - $config + $config, + $this->createMock(BackendService::class), ); } diff --git a/apps/files_external/tests/Controller/StoragesControllerTestCase.php b/apps/files_external/tests/Controller/StoragesControllerTestCase.php index 12e3e13f943c2..33c8eb26f5f82 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\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/Controller/UserStoragesControllerTest.php b/apps/files_external/tests/Controller/UserStoragesControllerTest.php index 3e8d89ec060c3..e2d5f301b122f 100644 --- a/apps/files_external/tests/Controller/UserStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/UserStoragesControllerTest.php @@ -58,7 +58,8 @@ private function createController(bool $allowCreateLocal = true) { $this->createMock(LoggerInterface::class), $session, $this->createMock(IGroupManager::class), - $config + $config, + $this->createMock(BackendService::class), ); } diff --git a/apps/files_external/tests/Service/BackendServiceTest.php b/apps/files_external/tests/Service/BackendServiceTest.php index 43553db69859d..6fca74124a5fe 100644 --- a/apps/files_external/tests/Service/BackendServiceTest.php +++ b/apps/files_external/tests/Service/BackendServiceTest.php @@ -15,12 +15,15 @@ use OCA\Files_External\Service\BackendService; use OCP\IAppConfig; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; class BackendServiceTest extends \Test\TestCase { protected IAppConfig&MockObject $appConfig; + protected LoggerInterface&MockObject $logger; protected function setUp(): void { $this->appConfig = $this->createMock(IAppConfig::class); + $this->logger = $this->createMock(LoggerInterface::class); } /** @@ -46,7 +49,7 @@ protected function getAuthMechanismMock($class) { } public function testRegisterBackend(): void { - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $backend = $this->getBackendMock('\Foo\Bar'); @@ -72,7 +75,7 @@ public function testRegisterBackend(): void { } public function testBackendProvider(): void { - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $backend1 = $this->getBackendMock('\Foo\Bar'); $backend2 = $this->getBackendMock('\Bar\Foo'); @@ -91,7 +94,7 @@ public function testBackendProvider(): void { } public function testAuthMechanismProvider(): void { - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $backend1 = $this->getAuthMechanismMock('\Foo\Bar'); $backend2 = $this->getAuthMechanismMock('\Bar\Foo'); @@ -110,7 +113,7 @@ public function testAuthMechanismProvider(): void { } public function testMultipleBackendProviders(): void { - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $backend1a = $this->getBackendMock('\Foo\Bar'); $backend1b = $this->getBackendMock('\Bar\Foo'); @@ -147,7 +150,7 @@ public function testUserMountingBackends(): void { ->with('files_external', 'allow_user_mounting') ->willReturn(true); - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); $backendAllowed->expects($this->never()) @@ -171,7 +174,7 @@ public function testUserMountingBackends(): void { } public function testGetAvailableBackends(): void { - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $backendAvailable = $this->getBackendMock('\Backend\Available'); $backendAvailable->expects($this->once()) @@ -212,7 +215,7 @@ public static function invalidConfigPlaceholderProvider(): array { public function testRegisterConfigHandlerInvalid(array $placeholders): void { $this->expectException(\RuntimeException::class); - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $mock = $this->createMock(IConfigHandler::class); $cb = function () use ($mock) { return $mock; @@ -223,7 +226,7 @@ public function testRegisterConfigHandlerInvalid(array $placeholders): void { } public function testConfigHandlers(): void { - $service = new BackendService($this->appConfig); + $service = new BackendService($this->appConfig, $this->logger); $mock = $this->createMock(IConfigHandler::class); $mock->expects($this->exactly(3)) ->method('handle'); diff --git a/apps/files_external/tests/Service/StoragesServiceTestCase.php b/apps/files_external/tests/Service/StoragesServiceTestCase.php index 1ad78a34a6054..5eed8294b8f8c 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\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 @@ protected function setUp(): void { 'datadirectory', \OC::$SERVERROOT . '/data/' ); - MountConfig::$skipTest = true; $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->appConfig = $this->createMock(IAppConfig::class); @@ -140,7 +138,6 @@ protected function setUp(): void { } protected function tearDown(): void { - MountConfig::$skipTest = false; self::$hookCalls = []; if ($this->dbConfig) { $this->dbConfig->clean(); diff --git a/apps/files_external/tests/Storage/SFTP_KeyTest.php b/apps/files_external/tests/Storage/SFTP_KeyTest.php deleted file mode 100644 index 4352580132ca3..0000000000000 --- a/apps/files_external/tests/Storage/SFTP_KeyTest.php +++ /dev/null @@ -1,82 +0,0 @@ -getUniqueID(); - $this->loadConfig(__DIR__ . '/../config.php'); - // Make sure we have an new empty folder to work in - $this->config['sftp_key']['root'] .= '/' . $id; - $this->instance = new SFTP_Key($this->config['sftp_key']); - $this->instance->mkdir('/'); - } - - protected function shouldRunConfig(mixed $config): bool { - return is_array($config) && ($config['sftp_key']['run'] ?? false); - } - - protected function tearDown(): void { - if ($this->instance) { - $this->instance->rmdir('/'); - } - - parent::tearDown(); - } - - - public function testInvalidAddressShouldThrowException(): void { - $this->expectException(\InvalidArgumentException::class); - - // I'd use example.com for this, but someone decided to break the spec and make it resolve - $this->instance->assertHostAddressValid('notarealaddress...'); - } - - public function testValidAddressShouldPass(): void { - $this->assertTrue($this->instance->assertHostAddressValid('localhost')); - } - - - public function testNegativePortNumberShouldThrowException(): void { - $this->expectException(\InvalidArgumentException::class); - - $this->instance->assertPortNumberValid('-1'); - } - - - public function testNonNumericalPortNumberShouldThrowException(): void { - $this->expectException(\InvalidArgumentException::class); - - $this->instance->assertPortNumberValid('a'); - } - - - public function testHighPortNumberShouldThrowException(): void { - $this->expectException(\InvalidArgumentException::class); - - $this->instance->assertPortNumberValid('65536'); - } - - public function testValidPortNumberShouldPass(): void { - $this->assertTrue($this->instance->assertPortNumberValid('22222')); - } -} diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 4b122b002f23b..09b298b7083b8 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1594,14 +1594,6 @@ - - - - - - - - @@ -1614,6 +1606,14 @@ + + + + + + + + - -