From b4b4924e99712fa0bdbcb019c33a2086a2c7da60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 21 Apr 2026 10:47:24 +0200 Subject: [PATCH 1/8] fix: check for the identifier alias for the storage backend --- apps/files_external/lib/Controller/GlobalStoragesController.php | 2 +- apps/files_external/lib/Controller/UserStoragesController.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php index 9ee3c91783d9..c7609dd0a7a8 100644 --- a/apps/files_external/lib/Controller/GlobalStoragesController.php +++ b/apps/files_external/lib/Controller/GlobalStoragesController.php @@ -88,7 +88,7 @@ public function create( ) { $canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false); - if ($backend === 'local' && $canCreateNewLocalStorage === false) { + if (($backend === 'local' || $backend === '\OC\Files\Storage\Local') && $canCreateNewLocalStorage === false) { return new DataResponse( null, Http::STATUS_FORBIDDEN diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php index 8c7a1e4efaae..7f2232e129ce 100644 --- a/apps/files_external/lib/Controller/UserStoragesController.php +++ b/apps/files_external/lib/Controller/UserStoragesController.php @@ -129,7 +129,7 @@ public function create( ); } $canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false); - if ($backend === 'local' && $canCreateNewLocalStorage === false) { + if (($backend === 'local' || $backend === '\OC\Files\Storage\Local') && $canCreateNewLocalStorage === false) { return new DataResponse( null, Http::STATUS_FORBIDDEN From 834c901e34396e507a3b68d474ca651261a87073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 21 Apr 2026 12:55:40 +0200 Subject: [PATCH 2/8] test: add unit tests to local external storage --- .../GlobalStoragesControllerTest.php | 32 +++++++++++++++++++ .../Controller/UserStoragesControllerTest.php | 32 +++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php index 6e45b6ca927f..3417a3b0695e 100644 --- a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php @@ -111,6 +111,38 @@ public function testCreate() { $this->assertEquals($expectedStorage, $actual); } + public function testCreateLocal() { + $mount = 'randomMount'; + $backend = 'local'; + $auth = 'identifier:\Random\Missing\Auth\Class'; + $backendOpts = [ + 'datadir' => '/tmp', + ]; + $priority = 3; + + // there is already a teardown in the parent class setting this value to false + \OC::$server->getSystemConfig()->setValue('files_external_allow_create_new_local', false); + + $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); + $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); + } + + public function testCreateLocalClassname() { + $mount = 'randomMount'; + $backend = '\OC\Files\Storage\Local'; + $auth = 'identifier:\Random\Missing\Auth\Class'; + $backendOpts = [ + 'datadir' => '/tmp', + ]; + $priority = 3; + + // there is already a teardown in the parent class setting this value to false + \OC::$server->getSystemConfig()->setValue('files_external_allow_create_new_local', false); + + $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); + $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); + } + public function testUpdate() { $mount = 'randomMount'; $backend = 'identifier:\This\Doesnt\Exist'; diff --git a/apps/files_external/tests/Controller/UserStoragesControllerTest.php b/apps/files_external/tests/Controller/UserStoragesControllerTest.php index c20722b73c4f..51df05ebde2b 100644 --- a/apps/files_external/tests/Controller/UserStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/UserStoragesControllerTest.php @@ -195,6 +195,38 @@ public function testCreate() { $this->assertEquals($expectedStorage, $actual); } + public function testCreateLocal() { + $mount = 'randomMount'; + $backend = 'local'; + $auth = 'identifier:\Random\Missing\Auth\Class'; + $backendOpts = [ + 'datadir' => '/tmp', + ]; + $priority = 3; + + // there is already a teardown in the parent class setting this value to false + \OC::$server->getSystemConfig()->setValue('files_external_allow_create_new_local', false); + + $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); + $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); + } + + public function testCreateLocalClassname() { + $mount = 'randomMount'; + $backend = '\OC\Files\Storage\Local'; + $auth = 'identifier:\Random\Missing\Auth\Class'; + $backendOpts = [ + 'datadir' => '/tmp', + ]; + $priority = 3; + + // there is already a teardown in the parent class setting this value to false + \OC::$server->getSystemConfig()->setValue('files_external_allow_create_new_local', false); + + $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); + $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); + } + public function testUpdate() { $mount = 'randomMount'; $backend = 'identifier:\This\Doesnt\Exist'; From 5080fc6714c7fb0cd67e147a8814433e6ad405bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 21 Apr 2026 12:56:21 +0200 Subject: [PATCH 3/8] chore: add changelog entry --- changelog/unreleased/41538 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/41538 diff --git a/changelog/unreleased/41538 b/changelog/unreleased/41538 new file mode 100644 index 000000000000..81ba32f875c9 --- /dev/null +++ b/changelog/unreleased/41538 @@ -0,0 +1,8 @@ +Bugfix: Prevent mounting local storage if not allowed. + +Mounting a local storage was possible if the internal class name was used as +backend, despite local storage not allowed to be mounted. This problem is +fixed and the local storage can't be mounted if is was explicitly disallowed in +the configuration. + +https://github.com/owncloud/core/pull/41538 From b6dd4785e16df88833bc0a3ec1a69c2f215166c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 7 May 2026 14:49:49 +0200 Subject: [PATCH 4/8] fix: move backend checks to a different place --- .../Controller/GlobalStoragesController.php | 9 --- .../lib/Controller/StoragesController.php | 4 +- .../lib/Controller/UserStoragesController.php | 8 --- .../Files/External/StoragesBackendChecker.php | 67 +++++++++++++++++++ .../Files/External/StoragesBackendService.php | 41 +++--------- lib/private/Server.php | 3 +- 6 files changed, 79 insertions(+), 53 deletions(-) create mode 100644 lib/private/Files/External/StoragesBackendChecker.php diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php index c7609dd0a7a8..14abdafa2bc2 100644 --- a/apps/files_external/lib/Controller/GlobalStoragesController.php +++ b/apps/files_external/lib/Controller/GlobalStoragesController.php @@ -86,15 +86,6 @@ public function create( $applicableGroups, $priority ) { - $canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false); - - if (($backend === 'local' || $backend === '\OC\Files\Storage\Local') && $canCreateNewLocalStorage === false) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } - $newStorage = $this->createStorage( $mountPoint, $backend, diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php index 9b4fdf85ab9e..7c899f9f8c30 100644 --- a/apps/files_external/lib/Controller/StoragesController.php +++ b/apps/files_external/lib/Controller/StoragesController.php @@ -183,7 +183,7 @@ protected function validate(IStorageConfig $storage) { $backend->getIdentifier() ]) ], - Http::STATUS_UNPROCESSABLE_ENTITY + Http::STATUS_FORBIDDEN ); } if (!$authMechanism->isVisibleFor($this->service->getVisibilityType())) { @@ -194,7 +194,7 @@ protected function validate(IStorageConfig $storage) { $authMechanism->getIdentifier() ]) ], - Http::STATUS_UNPROCESSABLE_ENTITY + Http::STATUS_FORBIDDEN ); } diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php index 7f2232e129ce..4fb5868014c4 100644 --- a/apps/files_external/lib/Controller/UserStoragesController.php +++ b/apps/files_external/lib/Controller/UserStoragesController.php @@ -128,14 +128,6 @@ public function create( Http::STATUS_FORBIDDEN ); } - $canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false); - if (($backend === 'local' || $backend === '\OC\Files\Storage\Local') && $canCreateNewLocalStorage === false) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } - $newStorage = $this->createStorage( $mountPoint, $backend, diff --git a/lib/private/Files/External/StoragesBackendChecker.php b/lib/private/Files/External/StoragesBackendChecker.php new file mode 100644 index 000000000000..5d493d5147fa --- /dev/null +++ b/lib/private/Files/External/StoragesBackendChecker.php @@ -0,0 +1,67 @@ +config = $config; + } + + /** + * Checks if the regular users are allowed to mount external storages + * @return bool + */ + public function isUserMountingAllowed() { + if ($this->allowUserMounting === null) { + $this->allowUserMounting = $this->config->getAppValue('files_external', 'allow_user_mounting', 'no') === 'yes'; + // if no backend is in the list an empty string is in the array and user mounting is disabled + if ($this->allowedBackendsForUsers() === ['']) { + $this->allowUserMounting = false; + } + } + return $this->allowUserMounting; + } + + private function allowedBackendsForUsers() { + if ($this->userMountingBackends === null) { + $user_mounting_backends = $this->config->getAppValue('files_external', 'user_mounting_backends', ''); + $this->userMountingBackends = \explode( + ',', + $user_mounting_backends + ); + } + return $this->userMountingBackends; + } + + /** + * Checks if the regular users are allowed to mount the specified backend. + * Note that the admin might still mount the backend. + * @return bool + */ + public function isAllowedUserBackend(Backend $backend) { + $blacklistedBackendsForUsers = ['\OC\Files\Storage\Local']; + if (in_array($backend->getStorageClass(), $blacklistedBackendsForUsers, true)) { + return false; + } + + if ($this->isUserMountingAllowed() && + \array_intersect($backend->getIdentifierAliases(), $this->allowedBackendsForUsers())) + { + return true; + } + return false; + } +} diff --git a/lib/private/Files/External/StoragesBackendService.php b/lib/private/Files/External/StoragesBackendService.php index 712a6a286a16..3f176ae88223 100644 --- a/lib/private/Files/External/StoragesBackendService.php +++ b/lib/private/Files/External/StoragesBackendService.php @@ -25,6 +25,7 @@ use OCP\IConfig; +use OC\Files\External\StoragesBackendChecker; use OCP\Files\External\Backend\Backend; use OCP\Files\External\Auth\AuthMechanism; use OCP\Files\External\Config\IBackendProvider; @@ -35,14 +36,8 @@ * Service class to manage backend definitions */ class StoragesBackendService implements IStoragesBackendService { - /** @var IConfig */ - protected $config; - - /** @var bool */ - private $userMountingAllowed = true; - - /** @var string[] */ - private $userMountingBackends = []; + /** @var StoragesBackendChecker */ + protected $storagesBackendChecker; /** @var Backend[] */ private $backends = []; @@ -57,27 +52,12 @@ class StoragesBackendService implements IStoragesBackendService { private $authMechanismProviders = []; /** - * @param IConfig $config + * @param StoragesBackendChecker $storagesBackendChecker */ public function __construct( - IConfig $config + StoragesBackendChecker $storagesBackendChecker ) { - $this->config = $config; - - // Load config values - if ($this->config->getAppValue('files_external', 'allow_user_mounting', 'no') !== 'yes') { - $this->userMountingAllowed = false; - } - $user_mounting_backends = $this->config->getAppValue('files_external', 'user_mounting_backends', ''); - $this->userMountingBackends = \explode( - ',', - $user_mounting_backends - ); - - // if no backend is in the list an empty string is in the array and user mounting is disabled - if ($this->userMountingBackends === ['']) { - $this->userMountingAllowed = false; - } + $this->storagesBackendChecker = $storagesBackendChecker; } /** @@ -244,7 +224,7 @@ public function getAuthMechanism($identifier) { * @return bool */ public function isUserMountingAllowed() { - return $this->userMountingAllowed; + return $this->storagesBackendChecker->isUserMountingAllowed(); } /** @@ -254,12 +234,7 @@ public function isUserMountingAllowed() { * @return bool */ protected function isAllowedUserBackend(Backend $backend) { - if ($this->userMountingAllowed && - \array_intersect($backend->getIdentifierAliases(), $this->userMountingBackends) - ) { - return true; - } - return false; + return $this->storagesBackendChecker->isAllowedUserBackend($backend); } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index 3cc8bfa8cd78..142cb6812d77 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -114,6 +114,7 @@ use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use OC\Files\External\StoragesBackendService; +use OC\Files\External\StoragesBackendChecker; use OC\Files\External\Service\UserStoragesService; use OC\Files\External\Service\UserGlobalStoragesService; use OC\Files\External\Service\GlobalStoragesService; @@ -844,7 +845,7 @@ public function __construct($webRoot, \OC\Config $config) { ); }); $this->registerService('StoragesBackendService', function (Server $c) { - $service = new StoragesBackendService($c->query('AllConfig')); + $service = new StoragesBackendService($c->query(StoragesBackendChecker::class)); // register auth mechanisms provided by core $provider = new \OC\Files\External\Auth\CoreAuthMechanismProvider($c, [ From 88fcb18e07bfb4171a63a07362c55964e447f8e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 7 May 2026 15:50:48 +0200 Subject: [PATCH 5/8] fix: adjust unit tests --- .../Files/External/StoragesBackendChecker.php | 6 +-- .../GlobalStoragesServiceDeleteUserTest.php | 3 +- .../External/StoragesBackendServiceTest.php | 45 ++++++++++--------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/private/Files/External/StoragesBackendChecker.php b/lib/private/Files/External/StoragesBackendChecker.php index 5d493d5147fa..449172fdc4a3 100644 --- a/lib/private/Files/External/StoragesBackendChecker.php +++ b/lib/private/Files/External/StoragesBackendChecker.php @@ -53,13 +53,11 @@ private function allowedBackendsForUsers() { */ public function isAllowedUserBackend(Backend $backend) { $blacklistedBackendsForUsers = ['\OC\Files\Storage\Local']; - if (in_array($backend->getStorageClass(), $blacklistedBackendsForUsers, true)) { + if (\in_array($backend->getStorageClass(), $blacklistedBackendsForUsers, true)) { return false; } - if ($this->isUserMountingAllowed() && - \array_intersect($backend->getIdentifierAliases(), $this->allowedBackendsForUsers())) - { + if ($this->isUserMountingAllowed() && \array_intersect($backend->getIdentifierAliases(), $this->allowedBackendsForUsers())) { return true; } return false; diff --git a/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php b/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php index 67e36a61510f..3fa744dfaeca 100644 --- a/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php +++ b/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php @@ -23,6 +23,7 @@ use OC\Files\Config\UserMountCache; use OC\Files\External\Service\DBConfigService; use OC\Files\External\Service\GlobalStoragesService; +use OC\Files\External\StoragesBackendChecker; use OC\Files\External\StoragesBackendService; use OCP\Files\External\Backend\Backend; use OCP\IUser; @@ -125,7 +126,7 @@ public function providesDeleteAllUser() { * @param $storageParams */ public function testDeleteAllForUser($storageParams, $userId) { - $backendService = new StoragesBackendService(\OC::$server->getConfig()); + $backendService = new StoragesBackendService(new StoragesBackendChecker(\OC::$server->getConfig())); $dbConfigService = new DBConfigService(\OC::$server->getDatabaseConnection(), \OC::$server->getCrypto()); $userManager = \OC::$server->getUserManager(); $userMountCache = new UserMountCache(\OC::$server->getDatabaseConnection(), $userManager, \OC::$server->getLogger()); diff --git a/tests/lib/Files/External/StoragesBackendServiceTest.php b/tests/lib/Files/External/StoragesBackendServiceTest.php index f61dacd8dcda..7d4735cef660 100644 --- a/tests/lib/Files/External/StoragesBackendServiceTest.php +++ b/tests/lib/Files/External/StoragesBackendServiceTest.php @@ -20,22 +20,21 @@ */ namespace Test\Files\External; +use OC\Files\External\StoragesBackendChecker; use OC\Files\External\StoragesBackendService; use OCP\Files\External\IStoragesBackendService; +use OCP\Files\External\Backend\Backend; use OCP\IConfig; use PHPUnit\Framework\MockObject\MockObject; class StoragesBackendServiceTest extends \Test\TestCase { - /** @var \OCP\IConfig */ - protected $config; + /** @var StoragesBackendChecker */ + protected $storagesBackendChecker; protected function setUp(): void { - $this->config = $this->createMock('\OCP\IConfig'); - $this->config - ->method('getAppValue') - ->willReturnMap([ - ['files_external', 'user_mounting_backends', '', ''] - ]); + $this->storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $this->storagesBackendChecker->method('isUserMountingAllowed')->willReturn(false); + $this->storagesBackendChecker->method('isAllowedUserBackend')->willReturn(false); } /** @@ -67,7 +66,7 @@ protected function getAuthMechanismMock($class) { } public function testRegisterBackend() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend = $this->getBackendMock('\Foo\Bar'); @@ -94,7 +93,7 @@ public function testRegisterBackend() { } public function testBackendProvider() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend1 = $this->getBackendMock('\Foo\Bar'); $backend2 = $this->getBackendMock('\Bar\Foo'); @@ -112,7 +111,7 @@ public function testBackendProvider() { } public function testAuthMechanismProvider() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend1 = $this->getAuthMechanismMock('\Foo\Bar'); $backend2 = $this->getAuthMechanismMock('\Bar\Foo'); @@ -130,7 +129,7 @@ public function testAuthMechanismProvider() { } public function testMultipleBackendProviders() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend1a = $this->getBackendMock('\Foo\Bar'); $backend1b = $this->getBackendMock('\Bar\Foo'); @@ -156,15 +155,17 @@ public function testMultipleBackendProviders() { } public function testUserMountingBackends() { - $config = $this->createMock(IConfig::class); - $config->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['files_external', 'allow_user_mounting', 'no', 'yes'], - ['files_external', 'user_mounting_backends', '', 'identifier:\User\Mount\Allowed,identifier_alias'] - ]); - - $service = new StoragesBackendService($config); + $storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $storagesBackendChecker->method('isUserMountingAllowed')->willReturn(true); + $storagesBackendChecker->method('isAllowedUserBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + + $service = new StoragesBackendService($storagesBackendChecker); $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); $backendAllowed->expects($this->never()) @@ -188,7 +189,7 @@ public function testUserMountingBackends() { } public function testGetAvailableBackends() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backendAvailable = $this->getBackendMock('\Backend\Available'); $backendAvailable->expects($this->once()) From 45ffc86016c4b25587389f44e7230c9bac7745b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Mon, 11 May 2026 11:56:16 +0200 Subject: [PATCH 6/8] fix: visibility for local storage for the admin based on flag Without the visibility, the admin won't be able to create local storages, and the previously created local mounts will be hidden and inaccessible --- .../Files/External/StoragesBackendChecker.php | 18 +++- .../Files/External/StoragesBackendService.php | 12 +++ .../External/StoragesBackendServiceTest.php | 83 +++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/External/StoragesBackendChecker.php b/lib/private/Files/External/StoragesBackendChecker.php index 449172fdc4a3..fb0991c18378 100644 --- a/lib/private/Files/External/StoragesBackendChecker.php +++ b/lib/private/Files/External/StoragesBackendChecker.php @@ -24,7 +24,7 @@ public function __construct(IConfig $config) { * Checks if the regular users are allowed to mount external storages * @return bool */ - public function isUserMountingAllowed() { + public function isUserMountingAllowed(): bool { if ($this->allowUserMounting === null) { $this->allowUserMounting = $this->config->getAppValue('files_external', 'allow_user_mounting', 'no') === 'yes'; // if no backend is in the list an empty string is in the array and user mounting is disabled @@ -49,9 +49,10 @@ private function allowedBackendsForUsers() { /** * Checks if the regular users are allowed to mount the specified backend. * Note that the admin might still mount the backend. + * @param Backend $backend * @return bool */ - public function isAllowedUserBackend(Backend $backend) { + public function isAllowedUserBackend(Backend $backend): bool { $blacklistedBackendsForUsers = ['\OC\Files\Storage\Local']; if (\in_array($backend->getStorageClass(), $blacklistedBackendsForUsers, true)) { return false; @@ -62,4 +63,17 @@ public function isAllowedUserBackend(Backend $backend) { } return false; } + + /** + * Checks if the admin is allowed to mount the specified backend. + * @param Backend $backend + * @return bool + */ + public function isAllowedAdminBackend(Backend $backend): bool { + $canCreateNewLocalStorage = $this->config->getSystemValue('files_external_allow_create_new_local', false); + if ($backend->getStorageClass() === '\OC\Files\Storage\Local' && !$canCreateNewLocalStorage) { + return false; + } + return true; + } } diff --git a/lib/private/Files/External/StoragesBackendService.php b/lib/private/Files/External/StoragesBackendService.php index 3f176ae88223..0accca21db33 100644 --- a/lib/private/Files/External/StoragesBackendService.php +++ b/lib/private/Files/External/StoragesBackendService.php @@ -104,6 +104,9 @@ public function registerBackend(Backend $backend) { if (!$this->isAllowedUserBackend($backend)) { $backend->removeVisibility(IStoragesBackendService::VISIBILITY_PERSONAL); } + if (!$this->isAllowedAdminBackend($backend)) { + $backend->removeVisibility(IStoragesBackendService::VISIBILITY_ADMIN); + } foreach ($backend->getIdentifierAliases() as $alias) { $this->backends[$alias] = $backend; } @@ -237,6 +240,15 @@ protected function isAllowedUserBackend(Backend $backend) { return $this->storagesBackendChecker->isAllowedUserBackend($backend); } + /** + * Checks if the admin is allowed to mount the backend + * @param Backend $backend + * @return bool + */ + protected function isAllowedAdminBackend(Backend $backend) { + return $this->storagesBackendChecker->isAllowedAdminBackend($backend); + } + /** * Check an authentication mechanism if a user is allowed to use it * diff --git a/tests/lib/Files/External/StoragesBackendServiceTest.php b/tests/lib/Files/External/StoragesBackendServiceTest.php index 7d4735cef660..9327a4a2ab62 100644 --- a/tests/lib/Files/External/StoragesBackendServiceTest.php +++ b/tests/lib/Files/External/StoragesBackendServiceTest.php @@ -35,6 +35,7 @@ protected function setUp(): void { $this->storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); $this->storagesBackendChecker->method('isUserMountingAllowed')->willReturn(false); $this->storagesBackendChecker->method('isAllowedUserBackend')->willReturn(false); + $this->storagesBackendChecker->method('isAllowedAdminBackend')->willReturn(true); } /** @@ -157,6 +158,7 @@ public function testMultipleBackendProviders() { public function testUserMountingBackends() { $storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); $storagesBackendChecker->method('isUserMountingAllowed')->willReturn(true); + $storagesBackendChecker->method('isAllowedAdminBackend')->willReturn(true); $storagesBackendChecker->method('isAllowedUserBackend')->willReturnCallback(function (Backend $backend) { $backendAliases = $backend->getIdentifierAliases(); if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { @@ -188,6 +190,87 @@ public function testUserMountingBackends() { $service->registerBackend($backendAlias); } + public function testAdminMountingBackends() { + $storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $storagesBackendChecker->method('isUserMountingAllowed')->willReturn(true); + $storagesBackendChecker->method('isAllowedUserBackend')->willReturn(true); + $storagesBackendChecker->method('isAllowedAdminBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + + $service = new StoragesBackendService($storagesBackendChecker); + + $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); + $backendAllowed->expects($this->never()) + ->method('removeVisibility'); + $backendNotAllowed = $this->getBackendMock('\User\Mount\NotAllowed'); + $backendNotAllowed->expects($this->once()) + ->method('removeVisibility') + ->with(IStoragesBackendService::VISIBILITY_ADMIN); + + $backendAlias = $this->getMockBuilder('\OCP\Files\External\Backend\Backend') + ->disableOriginalConstructor() + ->getMock(); + $backendAlias->method('getIdentifierAliases') + ->willReturn(['identifier_real', 'identifier_alias']); + $backendAlias->expects($this->never()) + ->method('removeVisibility'); + + $service->registerBackend($backendAllowed); + $service->registerBackend($backendNotAllowed); + $service->registerBackend($backendAlias); + } + + public function testAdminAndUserMountingBackends() { + $storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $storagesBackendChecker->method('isUserMountingAllowed')->willReturn(true); + $storagesBackendChecker->method('isAllowedUserBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + $storagesBackendChecker->method('isAllowedAdminBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + + $service = new StoragesBackendService($storagesBackendChecker); + + $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); + $backendAllowed->expects($this->never()) + ->method('removeVisibility'); + $backendNotAllowed = $this->getBackendMock('\User\Mount\NotAllowed'); + $backendNotAllowed->expects($this->exactly(2)) + ->method('removeVisibility') + ->with( + $this->logicalOr( + $this->identicalTo(IStoragesBackendService::VISIBILITY_ADMIN), + $this->identicalTo(IStoragesBackendService::VISIBILITY_PERSONAL) + ) + ); + + $backendAlias = $this->getMockBuilder('\OCP\Files\External\Backend\Backend') + ->disableOriginalConstructor() + ->getMock(); + $backendAlias->method('getIdentifierAliases') + ->willReturn(['identifier_real', 'identifier_alias']); + $backendAlias->expects($this->never()) + ->method('removeVisibility'); + + $service->registerBackend($backendAllowed); + $service->registerBackend($backendNotAllowed); + $service->registerBackend($backendAlias); + } + public function testGetAvailableBackends() { $service = new StoragesBackendService($this->storagesBackendChecker); From aee313042f69311b97b44c77d31d8dc976288490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 12 May 2026 14:38:39 +0200 Subject: [PATCH 7/8] fix: review comments --- .../Controller/StoragesControllerTest.php | 2 +- .../Controller/UserStoragesControllerTest.php | 99 +++++++++++++++++-- changelog/unreleased/41538 | 2 +- .../Files/External/StoragesBackendChecker.php | 16 ++- 4 files changed, 104 insertions(+), 15 deletions(-) diff --git a/apps/files_external/tests/Controller/StoragesControllerTest.php b/apps/files_external/tests/Controller/StoragesControllerTest.php index 97cc11a0c3a8..3df57c9d04df 100644 --- a/apps/files_external/tests/Controller/StoragesControllerTest.php +++ b/apps/files_external/tests/Controller/StoragesControllerTest.php @@ -170,7 +170,7 @@ public function testAddStorageWithoutConfig() { $data = $response->getData(); $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); - $this->assertNull($data); + $this->assertEquals(['message' => ''], $data); // message comes from translatable error string, which isn't set here. } public function testUpdateStorage() { diff --git a/apps/files_external/tests/Controller/UserStoragesControllerTest.php b/apps/files_external/tests/Controller/UserStoragesControllerTest.php index 51df05ebde2b..027b4328e6df 100644 --- a/apps/files_external/tests/Controller/UserStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/UserStoragesControllerTest.php @@ -39,6 +39,9 @@ use OCP\Files\External\Service\IUserStoragesService; class UserStoragesControllerTest extends StoragesControllerTest { + /** @var IConfig */ + private $config; + public function setUp(): void { parent::setUp(); $this->service = $this->createMock(IUserStoragesService::class); @@ -115,7 +118,7 @@ public function testAddOrUpdateStorageDisallowedBackend() { null ); - $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); $response = $this->controller->update( 1, @@ -129,7 +132,7 @@ public function testAddOrUpdateStorageDisallowedBackend() { null ); - $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); } public function testCreate() { @@ -195,31 +198,109 @@ public function testCreate() { $this->assertEquals($expectedStorage, $actual); } - public function testCreateLocal() { + public function localBackendNameProvider() { + return [ + ['local'], + ['\OC\Files\Storage\Local'], + ]; + } + + /** + * @dataProvider localBackendNameProvider + */ + public function testCreateLocal($localBackendName) { $mount = 'randomMount'; - $backend = 'local'; + $backend = "identifier:{$localBackendName}"; $auth = 'identifier:\Random\Missing\Auth\Class'; $backendOpts = [ 'datadir' => '/tmp', ]; $priority = 3; - // there is already a teardown in the parent class setting this value to false - \OC::$server->getSystemConfig()->setValue('files_external_allow_create_new_local', false); + $storageConfig = $this->getNewStorageConfigMock([ + 'id' => 30, + 'backendClass' => '\OCA\Files_External\Lib\Backend', + 'backendStorageClass' => '\OC\Files\Storage\Local', + 'authClass' => '\Random\Missing\Auth\Class', + 'mountPoint' => $mount, + 'backendOpts' => $backendOpts, + 'priority' => $priority, + 'type' => IStorageConfig::MOUNT_TYPE_ADMIN, + ]); + + $backendMock = $storageConfig->getBackend(); + $backendMock->method('isVisibleFor')->willReturn(true); + $backendMock->method('validateStorage')->willReturn(true); + + $authMock = $storageConfig->getAuthMechanism(); + $authMock->method('isVisibleFor')->willReturn(true); + $authMock->method('validateStorage')->willReturn(true); + + $this->service->expects($this->once()) + ->method('createStorage') + ->willReturn($storageConfig); + $this->service->expects($this->once()) + ->method('addStorage') + ->will($this->returnArgument(0)); + + $expectedStorage = [ + 'id' => 30, + 'mountPoint' => '/randomMount', + 'backend' => "identifier:\OCA\Files_External\Lib\Backend", + 'authMechanism' => 'identifier:\Random\Missing\Auth\Class', + 'backendOptions' => [ + 'datadir' => '/tmp', + ], + 'priority' => 3, + 'userProvided' => false, + 'type' => 'system', + 'status' => StorageNotAvailableException::STATUS_SUCCESS, // status check for the storage is skipped and always returns this value + ]; $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); - $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); + $actual = $result->getData()->jsonSerialize(); + $this->assertEquals(Http::STATUS_CREATED, $result->getStatus()); + $this->assertEquals($expectedStorage, $actual); } - public function testCreateLocalClassname() { + /** + * @dataProvider localBackendNameProvider + */ + public function testCreateLocalNotAllowed($localBackendName) { $mount = 'randomMount'; - $backend = '\OC\Files\Storage\Local'; + $backend = $localBackendName; $auth = 'identifier:\Random\Missing\Auth\Class'; $backendOpts = [ 'datadir' => '/tmp', ]; $priority = 3; + $storageConfig = $this->getNewStorageConfigMock([ + 'id' => 30, + 'backendClass' => '\OCA\Files_External\Lib\Backend', + 'backendStorageClass' => '\OC\Files\Storage\Local', + 'authClass' => '\Random\Missing\Auth\Class', + 'mountPoint' => $mount, + 'backendOpts' => $backendOpts, + 'priority' => $priority, + 'type' => IStorageConfig::MOUNT_TYPE_ADMIN, + ]); + + $backendMock = $storageConfig->getBackend(); + $backendMock->method('isVisibleFor')->willReturn(false); + $backendMock->method('validateStorage')->willReturn(true); + + $authMock = $storageConfig->getAuthMechanism(); + $authMock->method('isVisibleFor')->willReturn(true); + $authMock->method('validateStorage')->willReturn(true); + + $this->service->expects($this->once()) + ->method('createStorage') + ->willReturn($storageConfig); + $this->service->expects($this->never()) + ->method('addStorage') + ->will($this->returnArgument(0)); + // there is already a teardown in the parent class setting this value to false \OC::$server->getSystemConfig()->setValue('files_external_allow_create_new_local', false); diff --git a/changelog/unreleased/41538 b/changelog/unreleased/41538 index 81ba32f875c9..167725ad0666 100644 --- a/changelog/unreleased/41538 +++ b/changelog/unreleased/41538 @@ -2,7 +2,7 @@ Bugfix: Prevent mounting local storage if not allowed. Mounting a local storage was possible if the internal class name was used as backend, despite local storage not allowed to be mounted. This problem is -fixed and the local storage can't be mounted if is was explicitly disallowed in +fixed and the local storage can't be mounted if it was explicitly disallowed in the configuration. https://github.com/owncloud/core/pull/41538 diff --git a/lib/private/Files/External/StoragesBackendChecker.php b/lib/private/Files/External/StoragesBackendChecker.php index fb0991c18378..3b48bb0b4f87 100644 --- a/lib/private/Files/External/StoragesBackendChecker.php +++ b/lib/private/Files/External/StoragesBackendChecker.php @@ -8,9 +8,11 @@ class StoragesBackendChecker { /** @var IConfig */ private IConfig $config; - /** @var bool */ + /** @var bool|null */ + private $canCreateNewLocalStorage = null; + /** @var bool|null */ private $allowUserMounting = null; - /** @var array */ + /** @var array|null */ private $userMountingBackends = null; /** @@ -53,6 +55,9 @@ private function allowedBackendsForUsers() { * @return bool */ public function isAllowedUserBackend(Backend $backend): bool { + // Allowing users to mount local storage is a security risk, so it's + // blacklisted regardless of admin's decision. The admin can still + // setup a local mount for everyone (or chosen users) if he wants. $blacklistedBackendsForUsers = ['\OC\Files\Storage\Local']; if (\in_array($backend->getStorageClass(), $blacklistedBackendsForUsers, true)) { return false; @@ -70,8 +75,11 @@ public function isAllowedUserBackend(Backend $backend): bool { * @return bool */ public function isAllowedAdminBackend(Backend $backend): bool { - $canCreateNewLocalStorage = $this->config->getSystemValue('files_external_allow_create_new_local', false); - if ($backend->getStorageClass() === '\OC\Files\Storage\Local' && !$canCreateNewLocalStorage) { + if ($this->canCreateNewLocalStorage === null) { + $this->canCreateNewLocalStorage = $this->config->getSystemValue('files_external_allow_create_new_local', false); + } + + if ($backend->getStorageClass() === '\OC\Files\Storage\Local' && !$this->canCreateNewLocalStorage) { return false; } return true; From 1661b69b6dbdf8f35792eef8a1336081746f65f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 14 May 2026 14:46:06 +0200 Subject: [PATCH 8/8] fix: remove user mounting check in controller and rely on validation --- .../lib/Controller/StoragesController.php | 4 +- .../lib/Controller/UserStoragesController.php | 33 ----- .../Controller/UserStoragesControllerTest.php | 36 +---- .../External/StoragesBackendCheckerTest.php | 125 ++++++++++++++++++ .../External/StoragesBackendServiceTest.php | 1 - 5 files changed, 128 insertions(+), 71 deletions(-) create mode 100644 tests/lib/Files/External/StoragesBackendCheckerTest.php diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php index 7c899f9f8c30..21de089ab642 100644 --- a/apps/files_external/lib/Controller/StoragesController.php +++ b/apps/files_external/lib/Controller/StoragesController.php @@ -308,7 +308,7 @@ public function show($id, $testOnly = true) { } catch (NotFoundException $e) { return new DataResponse( [ - 'message' => (string)$this->l10n->t('Storage with id "%i" not found', [$id]) + 'message' => (string)$this->l10n->t('Storage with id "%d" not found', [$id]) ], Http::STATUS_NOT_FOUND ); @@ -335,7 +335,7 @@ public function destroy($id) { } catch (NotFoundException $e) { return new DataResponse( [ - 'message' => (string)$this->l10n->t('Storage with id "%i" not found', [$id]) + 'message' => (string)$this->l10n->t('Storage with id "%d" not found', [$id]) ], Http::STATUS_NOT_FOUND ); diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php index 4fb5868014c4..126483a0dc45 100644 --- a/apps/files_external/lib/Controller/UserStoragesController.php +++ b/apps/files_external/lib/Controller/UserStoragesController.php @@ -31,7 +31,6 @@ use OCP\Files\External\IStorageConfig; use OCP\Files\External\NotFoundException; use OCP\Files\External\Service\IUserStoragesService; -use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; use OCP\IRequest; @@ -45,7 +44,6 @@ class UserStoragesController extends StoragesController { * @var IUserSession */ private $userSession; - private IConfig $config; public function __construct( $AppName, @@ -53,7 +51,6 @@ public function __construct( IL10N $l10n, IUserStoragesService $userStoragesService, IUserSession $userSession, - IConfig $config, ILogger $logger ) { parent::__construct( @@ -64,7 +61,6 @@ public function __construct( $logger ); $this->userSession = $userSession; - $this->config = $config; } protected function manipulateStorageConfig(IStorageConfig $storage) { @@ -82,12 +78,6 @@ protected function manipulateStorageConfig(IStorageConfig $storage) { * @return DataResponse */ public function index() { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } return parent::index(); } @@ -122,12 +112,6 @@ public function create( $backendOptions, $mountOptions ) { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } $newStorage = $this->createStorage( $mountPoint, $backend, @@ -180,12 +164,6 @@ public function update( $mountOptions, $testOnly = true ) { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } $storage = $this->createStorage( $mountPoint, $backend, @@ -233,17 +211,6 @@ public function update( * {@inheritdoc} */ public function destroy($id) { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } - return parent::destroy($id); } - - private function isUserMountingAllowed(): bool { - return $this->config->getAppValue('files_external', 'allow_user_mounting', 'no') === 'yes'; - } } diff --git a/apps/files_external/tests/Controller/UserStoragesControllerTest.php b/apps/files_external/tests/Controller/UserStoragesControllerTest.php index 027b4328e6df..be4bfade78d5 100644 --- a/apps/files_external/tests/Controller/UserStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/UserStoragesControllerTest.php @@ -32,60 +32,28 @@ use OCP\Files\External\Service\IStoragesService; use OCP\Files\StorageNotAvailableException; use OCP\ILogger; -use OCP\IConfig; use OCP\IUserSession; use OCP\IL10N; use OCP\IRequest; use OCP\Files\External\Service\IUserStoragesService; class UserStoragesControllerTest extends StoragesControllerTest { - /** @var IConfig */ - private $config; - public function setUp(): void { parent::setUp(); $this->service = $this->createMock(IUserStoragesService::class); $this->service->method('getVisibilityType') ->willReturn(IStoragesBackendService::VISIBILITY_PERSONAL); - $this->config = $this->createMock(IConfig::class); - $this->config->method('getAppValue')->willReturn('yes'); - $this->controller = new UserStoragesController( 'files_external', $this->createMock(IRequest::class), $this->createMock(IL10N::class), $this->service, $this->createMock(IUserSession::class), - $this->config, $this->createMock(ILogger::class) ); } - public function testApiWhenDisabled(): void { - $config = $this->createMock(IConfig::class); - $config->method('getAppValue')->willReturn('no'); - - $controller = new UserStoragesController( - 'files_external', - $this->createMock(IRequest::class), - $this->createMock(IL10N::class), - $this->service, - $this->createMock(IUserSession::class), - $config, - $this->createMock(ILogger::class) - ); - - $resp = $controller->create( - '', - '', - '', - [], - [], - ); - $this->assertEquals(Http::STATUS_FORBIDDEN, $resp->getStatus()); - } - public function testAddOrUpdateStorageDisallowedBackend() { $backend = $this->getBackendMock(); $backend->method('isVisibleFor') @@ -286,6 +254,7 @@ public function testCreateLocalNotAllowed($localBackendName) { 'type' => IStorageConfig::MOUNT_TYPE_ADMIN, ]); + // if not allowed, the backend must return false for its visibility $backendMock = $storageConfig->getBackend(); $backendMock->method('isVisibleFor')->willReturn(false); $backendMock->method('validateStorage')->willReturn(true); @@ -301,9 +270,6 @@ public function testCreateLocalNotAllowed($localBackendName) { ->method('addStorage') ->will($this->returnArgument(0)); - // there is already a teardown in the parent class setting this value to false - \OC::$server->getSystemConfig()->setValue('files_external_allow_create_new_local', false); - $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); } diff --git a/tests/lib/Files/External/StoragesBackendCheckerTest.php b/tests/lib/Files/External/StoragesBackendCheckerTest.php new file mode 100644 index 000000000000..8197687380af --- /dev/null +++ b/tests/lib/Files/External/StoragesBackendCheckerTest.php @@ -0,0 +1,125 @@ +config = $this->createMock(IConfig::class); + + $this->storagesBackendChecker = new StoragesBackendChecker($this->config); + } + + public function isUserMountingAllowedProvider() { + return [ + ['no', '', false], + ['no', 'local,smb', false], + ['yes', '', false], + ['yes', 'local,smb', true], + ['yes', 'smb', true], + ['', 'local,smb', false], + ['random', 'local,smb', false], + ]; + } + + /** + * @dataProvider isUserMountingAllowedProvider + */ + public function testIsUserMountingAllowed($allowUserMounting, $allowedBackends, $expectedResult) { + $this->config->method('getAppValue')->willReturnCallback(function ($app, $key, $default) use ($allowUserMounting, $allowedBackends) { + $map = [ + 'allow_user_mounting' => $allowUserMounting, + 'user_mounting_backends' => $allowedBackends, + ]; + if ($app === 'files_external') { + if (isset($map[$key])) { + return $map[$key]; + } + } + return $default; + }); + + $this->assertEquals($expectedResult, $this->storagesBackendChecker->isUserMountingAllowed()); + } + + public function isAllowedUserBackendProvider() { + $backend1 = $this->createMock(Backend::class); + $backend1->method('getStorageClass')->willReturn('\OC\Files\Storage\DAV'); + $backend1->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\DAV', 'DAV']); + $backend2 = $this->createMock(Backend::class); + $backend2->method('getStorageClass')->willReturn('\OC\Files\Storage\Local'); + $backend2->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\Local', 'local']); + return [ + ['no', '', $backend1, false], + ['no', '', $backend2, false], + ['no', 'DAV,local', $backend1, false], + ['no', 'DAV,local', $backend2, false], + ['no', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend1, false], + ['no', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend2, false], + ['yes', '', $backend1, false], + ['yes', '', $backend2, false], + ['yes', 'DAV,local', $backend1, true], + ['yes', 'DAV,local', $backend2, false], // local storage always disallowed + ['yes', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend1, true], + ['yes', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend2, false], + ]; + } + + /** + * @dataProvider isAllowedUserBackendProvider + */ + public function testIsAllowedUserBackend($allowUserMounting, $allowedBackends, $backend, $expectedResult) { + $this->config->method('getAppValue')->willReturnCallback(function ($app, $key, $default) use ($allowUserMounting, $allowedBackends) { + $map = [ + 'allow_user_mounting' => $allowUserMounting, + 'user_mounting_backends' => $allowedBackends, + ]; + if ($app === 'files_external') { + if (isset($map[$key])) { + return $map[$key]; + } + } + return $default; + }); + + $this->assertEquals($expectedResult, $this->storagesBackendChecker->isAllowedUserBackend($backend)); + } + + public function isAllowedAdminBackendProvider() { + $backend1 = $this->createMock(Backend::class); + $backend1->method('getStorageClass')->willReturn('\OC\Files\Storage\DAV'); + $backend1->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\DAV', 'DAV']); + $backend2 = $this->createMock(Backend::class); + $backend2->method('getStorageClass')->willReturn('\OC\Files\Storage\Local'); + $backend2->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\Local', 'local']); + return [ + [false, $backend1, true], + [false, $backend2, false], + [true, $backend1, true], + [true, $backend2, true], + ]; + } + + /** + * @dataProvider isAllowedAdminBackendProvider + */ + public function testIsAllowedAdminBackend($isLocalAllowed, $backend, $expectedResult) { + $this->config->method('getSystemValue')->willReturnCallback(function ($key, $default) use ($isLocalAllowed) { + if ($key === 'files_external_allow_create_new_local') { + return $isLocalAllowed; + } + return $default; + }); + + $this->assertEquals($expectedResult, $this->storagesBackendChecker->isAllowedAdminBackend($backend)); + } +} diff --git a/tests/lib/Files/External/StoragesBackendServiceTest.php b/tests/lib/Files/External/StoragesBackendServiceTest.php index 9327a4a2ab62..31e2e5fb4590 100644 --- a/tests/lib/Files/External/StoragesBackendServiceTest.php +++ b/tests/lib/Files/External/StoragesBackendServiceTest.php @@ -24,7 +24,6 @@ use OC\Files\External\StoragesBackendService; use OCP\Files\External\IStoragesBackendService; use OCP\Files\External\Backend\Backend; -use OCP\IConfig; use PHPUnit\Framework\MockObject\MockObject; class StoragesBackendServiceTest extends \Test\TestCase {