fix: check for the identifier alias for the storage backend

apps/files_external/lib/Controller/GlobalStoragesController.php

@@ -86,15 +86,10 @@ 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
-			);
-		}
-
+		// NOTE: files_external_allow_create_new_local is checked during backend
+		// registration, and it will remove the related storage visibility
+		// if applicable. If the storage isn't visible, the `validate` method
+		// will fail.
 		$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,11 @@ 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
-			);
-		}
-
+		// NOTE: local backend isn't allowed for regular users regardless
+		// of the value of files_external_allow_create_new_local. The backend
+		// won't be visible for regular users, so the `validate` method will fail.
+		// Only admins can create local storages (if files_external_allow_create_new_local
+		// is true)
 		$newStorage = $this->createStorage(
 			$mountPoint,
 			$backend,
@@ -188,12 +169,6 @@ public function update(
 		$mountOptions,
 		$testOnly = true
 	) {
-		if (!$this->isUserMountingAllowed()) {
-			return new DataResponse(
-				null,
-				Http::STATUS_FORBIDDEN
-			);
-		}
 		$storage = $this->createStorage(
 			$mountPoint,
 			$backend,
@@ -241,17 +216,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,55 @@ 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(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';

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