fix: check for the identifier alias for the storage backend

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' && $canCreateNewLocalStorage === false) {
-			return new DataResponse(
-				null,
-				Http::STATUS_FORBIDDEN
-			);
-		}
-
 		$newStorage = $this->createStorage(
 			$mountPoint,
 			$backend,

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

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,15 +44,13 @@ class UserStoragesController extends StoragesController {
 	 * @var IUserSession
 	 */
 	private $userSession;
-	private IConfig $config;
 
 	public function __construct(
 		$AppName,
 		IRequest $request,
 		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,20 +112,6 @@ public function create(
 		$backendOptions,
 		$mountOptions
 	) {
-		if (!$this->isUserMountingAllowed()) {
-			return new DataResponse(
-				null,
-				Http::STATUS_FORBIDDEN
-			);
-		}
-		$canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false);
-		if ($backend === 'local' && $canCreateNewLocalStorage === false) {
-			return new DataResponse(
-				null,
-				Http::STATUS_FORBIDDEN
-			);
-		}
-
 		$newStorage = $this->createStorage(
 			$mountPoint,
 			$backend,
@@ -188,12 +164,6 @@ public function update(
 		$mountOptions,
 		$testOnly = true
 	) {
-		if (!$this->isUserMountingAllowed()) {
-			return new DataResponse(
-				null,
-				Http::STATUS_FORBIDDEN
-			);
-		}
 		$storage = $this->createStorage(
 			$mountPoint,
 			$backend,
@@ -241,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';
-	}
 }

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

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

apps/files_external/tests/Controller/UserStoragesControllerTest.php

@@ -32,7 +32,6 @@
 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;
@@ -45,44 +44,16 @@ public function setUp(): void {
 		$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')
@@ -115,7 +86,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 +100,7 @@ public function testAddOrUpdateStorageDisallowedBackend() {
 			null
 		);
 
-		$this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus());
+		$this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus());
 	}
 
 	public function testCreate() {
@@ -195,6 +166,114 @@ public function testCreate() {
 		$this->assertEquals($expectedStorage, $actual);
 	}
 
+	public function localBackendNameProvider() {
+		return [
+			['local'],
+			['\OC\Files\Storage\Local'],
+		];
+	}
+
+	/**
+	 * @dataProvider localBackendNameProvider
+	 */
+	public function testCreateLocal($localBackendName) {
+		$mount = 'randomMount';
+		$backend = "identifier:{$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(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);
+		$actual = $result->getData()->jsonSerialize();
+		$this->assertEquals(Http::STATUS_CREATED, $result->getStatus());
+		$this->assertEquals($expectedStorage, $actual);
+	}
+
+	/**
+	 * @dataProvider localBackendNameProvider
+	 */
+	public function testCreateLocalNotAllowed($localBackendName) {
+		$mount = 'randomMount';
+		$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,
+		]);
+
+		// if not allowed, the backend must return false for its visibility
+		$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));
+
+		$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';

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 it was explicitly disallowed in
+the configuration.
+
+https://github.com/owncloud/core/pull/41538