From 1882c4c9faf1332c5248ae6313bc45fa7ddc73f7 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Mon, 6 Nov 2023 15:50:48 -0100 Subject: [PATCH] disable user unmount Signed-off-by: Maxence Lange --- .../Controller/GlobalStoragesController.php | 19 +++------- .../lib/Controller/StoragesController.php | 17 +++------ .../UserGlobalStoragesController.php | 7 ++-- .../lib/Controller/UserStoragesController.php | 18 ++++++++-- .../lib/Service/BackendService.php | 36 ++++++++++--------- apps/files_external/lib/Settings/Admin.php | 1 + apps/files_external/lib/Settings/Personal.php | 1 + .../GlobalStoragesControllerTest.php | 1 + .../Controller/UserStoragesControllerTest.php | 1 + .../tests/Settings/AdminTest.php | 1 + 10 files changed, 55 insertions(+), 47 deletions(-) diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php index cb785695647f4..cd78a48b6488b 100644 --- a/apps/files_external/lib/Controller/GlobalStoragesController.php +++ b/apps/files_external/lib/Controller/GlobalStoragesController.php @@ -28,6 +28,7 @@ namespace OCA\Files_External\Controller; use OCA\Files_External\NotFoundException; +use OCA\Files_External\Service\BackendService; use OCA\Files_External\Service\GlobalStoragesService; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -42,36 +43,26 @@ * Global storages controller */ class GlobalStoragesController extends StoragesController { - /** - * Creates a new global storages controller. - * - * @param string $AppName application name - * @param IRequest $request request object - * @param IL10N $l10n l10n service - * @param GlobalStoragesService $globalStoragesService storage service - * @param LoggerInterface $logger - * @param IUserSession $userSession - * @param IGroupManager $groupManager - * @param IConfig $config - */ public function __construct( - $AppName, + string $appName, IRequest $request, IL10N $l10n, GlobalStoragesService $globalStoragesService, LoggerInterface $logger, IUserSession $userSession, IGroupManager $groupManager, + BackendService $backendService, IConfig $config ) { parent::__construct( - $AppName, + $appName, $request, $l10n, $globalStoragesService, $logger, $userSession, $groupManager, + $backendService, $config ); } diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php index ead6aa9663a95..7c238a6d5d937 100644 --- a/apps/files_external/lib/Controller/StoragesController.php +++ b/apps/files_external/lib/Controller/StoragesController.php @@ -34,6 +34,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\StoragesService; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -50,26 +51,18 @@ * Base class for storages controllers */ abstract class StoragesController extends Controller { - /** - * Creates a new storages controller. - * - * @param string $AppName application name - * @param IRequest $request request object - * @param IL10N $l10n l10n service - * @param StoragesService $storagesService storage service - * @param LoggerInterface $logger - */ public function __construct( - $AppName, + string $appName, IRequest $request, protected IL10N $l10n, protected StoragesService $service, protected LoggerInterface $logger, protected IUserSession $userSession, protected IGroupManager $groupManager, + protected BackendService $backendService, protected IConfig $config ) { - parent::__construct($AppName, $request); + parent::__construct($appName, $request); } /** @@ -118,7 +111,7 @@ protected function createStorage( $priority ); } catch (\InvalidArgumentException $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); + $this->logger->warning('Invalid backend or authentication mechanism class', ['exception' => $e]); return new DataResponse( [ 'message' => $this->l10n->t('Invalid backend or authentication mechanism class') diff --git a/apps/files_external/lib/Controller/UserGlobalStoragesController.php b/apps/files_external/lib/Controller/UserGlobalStoragesController.php index ba15afb2bdfb1..ce73282b216a8 100644 --- a/apps/files_external/lib/Controller/UserGlobalStoragesController.php +++ b/apps/files_external/lib/Controller/UserGlobalStoragesController.php @@ -33,6 +33,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\DataResponse; @@ -59,23 +60,25 @@ class UserGlobalStoragesController extends StoragesController { * @param IGroupManager $groupManager */ public function __construct( - $AppName, + string $appName, IRequest $request, IL10N $l10n, UserGlobalStoragesService $userGlobalStoragesService, LoggerInterface $logger, IUserSession $userSession, IGroupManager $groupManager, + BackendService $backendService, IConfig $config ) { parent::__construct( - $AppName, + $appName, $request, $l10n, $userGlobalStoragesService, $logger, $userSession, $groupManager, + $backendService, $config ); } diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php index 7c141afcb305e..0f7b5c178d9fb 100644 --- a/apps/files_external/lib/Controller/UserStoragesController.php +++ b/apps/files_external/lib/Controller/UserStoragesController.php @@ -32,6 +32,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\DataResponse; @@ -58,23 +59,25 @@ class UserStoragesController extends StoragesController { * @param IGroupManager $groupManager */ public function __construct( - $AppName, + string $appName, IRequest $request, IL10N $l10n, UserStoragesService $userStoragesService, LoggerInterface $logger, IUserSession $userSession, IGroupManager $groupManager, + BackendService $backendService, IConfig $config ) { parent::__construct( - $AppName, + $appName, $request, $l10n, $userStoragesService, $logger, $userSession, $groupManager, + $backendService, $config ); } @@ -232,6 +235,17 @@ public function update( * {@inheritdoc} */ public function destroy($id) { + if (!$this->backendService->isUserUnmountingAllowed()) { + return new DataResponse( + [ + 'message' => $this->l10n->t( + 'Insufficient right to disconnect this storage' + ) + ], + Http::STATUS_NOT_FOUND + ); + } + return parent::destroy($id); } } diff --git a/apps/files_external/lib/Service/BackendService.php b/apps/files_external/lib/Service/BackendService.php index 056b288a88b04..09c9e0fcbf84e 100644 --- a/apps/files_external/lib/Service/BackendService.php +++ b/apps/files_external/lib/Service/BackendService.php @@ -24,11 +24,11 @@ * along with this program. If not, see * */ + namespace OCA\Files_External\Service; use OCA\Files_External\Config\IConfigHandler; use OCA\Files_External\Lib\Auth\AuthMechanism; - use OCA\Files_External\Lib\Backend\Backend; use OCA\Files_External\Lib\Config\IAuthMechanismProvider; use OCA\Files_External\Lib\Config\IBackendProvider; @@ -52,40 +52,34 @@ class BackendService { /** Priority constants for PriorityTrait */ public const PRIORITY_DEFAULT = 100; - /** @var IConfig */ - protected $config; - - /** @var bool */ - private $userMountingAllowed = true; + private bool $userMountingAllowed = true; /** @var string[] */ - private $userMountingBackends = []; + 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 = []; + private array $configHandlers = []; /** * @param IConfig $config */ public function __construct( - IConfig $config + protected IConfig $config ) { - $this->config = $config; - // Load config values if ($this->config->getAppValue('files_external', 'allow_user_mounting', 'yes') !== 'yes') { $this->userMountingAllowed = false; @@ -276,10 +270,18 @@ public function getAuthMechanism($identifier) { /** * @return bool */ - public function isUserMountingAllowed() { + public function isUserMountingAllowed(): bool { return $this->userMountingAllowed; } + public function isUserUnmountingAllowed(): bool { + return ('yes' === $this->config->getAppValue( + 'files_external', + 'allow_user_unmounting', + 'yes' + )); + } + /** * Check a backend if a user is allowed to mount it * diff --git a/apps/files_external/lib/Settings/Admin.php b/apps/files_external/lib/Settings/Admin.php index 116738b3f42b4..71861df2f0a2f 100644 --- a/apps/files_external/lib/Settings/Admin.php +++ b/apps/files_external/lib/Settings/Admin.php @@ -68,6 +68,7 @@ public function getForm() { 'authMechanisms' => $this->backendService->getAuthMechanisms(), 'dependencies' => \OCA\Files_External\MountConfig::dependencyMessage($this->backendService->getBackends()), 'allowUserMounting' => $this->backendService->isUserMountingAllowed(), + 'allowUserUnmounting' => $this->backendService->isUserUnmountingAllowed(), 'globalCredentials' => $this->globalAuth->getAuth(''), 'globalCredentialsUid' => '', ]; diff --git a/apps/files_external/lib/Settings/Personal.php b/apps/files_external/lib/Settings/Personal.php index c27cb0fce264d..27bfca95fea7a 100644 --- a/apps/files_external/lib/Settings/Personal.php +++ b/apps/files_external/lib/Settings/Personal.php @@ -76,6 +76,7 @@ public function getForm() { 'authMechanisms' => $this->backendService->getAuthMechanisms(), 'dependencies' => \OCA\Files_External\MountConfig::dependencyMessage($this->backendService->getBackends()), 'allowUserMounting' => $this->backendService->isUserMountingAllowed(), + 'allowUserUnmounting' => $this->backendService->isUserUnmountingAllowed(), 'globalCredentials' => $this->globalAuth->getAuth($uid), 'globalCredentialsUid' => $uid, ]; diff --git a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php index 5ce9db68e4035..f6b01fdd48a95 100644 --- a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php @@ -69,6 +69,7 @@ private function createController($allowCreateLocal = true) { $this->createMock(LoggerInterface::class), $session, $this->createMock(IGroupManager::class), + $this->createMock(BackendService::class), $config ); } diff --git a/apps/files_external/tests/Controller/UserStoragesControllerTest.php b/apps/files_external/tests/Controller/UserStoragesControllerTest.php index deb0f6e37bf66..df9bde9fa8032 100644 --- a/apps/files_external/tests/Controller/UserStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/UserStoragesControllerTest.php @@ -77,6 +77,7 @@ private function createController($allowCreateLocal = true) { $this->createMock(LoggerInterface::class), $session, $this->createMock(IGroupManager::class), + $this->createMock(BackendService::class), $config ); } diff --git a/apps/files_external/tests/Settings/AdminTest.php b/apps/files_external/tests/Settings/AdminTest.php index bad5da8516d07..d99101e791f53 100644 --- a/apps/files_external/tests/Settings/AdminTest.php +++ b/apps/files_external/tests/Settings/AdminTest.php @@ -98,6 +98,7 @@ public function testGetForm() { 'authMechanisms' => ['g', 'h', 'i'], 'dependencies' => \OCA\Files_External\MountConfig::dependencyMessage($this->backendService->getBackends()), 'allowUserMounting' => true, + 'allowUserUnmounting' => true, 'globalCredentials' => 'asdf:asdf', 'globalCredentialsUid' => '', ];