fix: disallow http api requests for user external storages in case disabled (#41250)

(cherry picked from commit 32e12ef)

Author: DeepDiver1975

apps/files_external/lib/Controller/UserStoragesController.php

@@ -31,6 +31,7 @@
 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;
@@ -44,23 +45,15 @@ class UserStoragesController extends StoragesController {
 	 * @var IUserSession
 	 */
 	private $userSession;
+	private IConfig $config;
 
-	/**
-	 * Creates a new user storages controller.
-	 *
-	 * @param string $AppName application name
-	 * @param IRequest $request request object
-	 * @param IL10N $l10n l10n service
-	 * @param IUserStoragesService $userStoragesService storage service
-	 * @param IUserSession $userSession
-	 * @param ILogger $logger
-	 */
 	public function __construct(
 		$AppName,
 		IRequest $request,
 		IL10N $l10n,
 		IUserStoragesService $userStoragesService,
 		IUserSession $userSession,
+		IConfig $config,
 		ILogger $logger
 	) {
 		parent::__construct(
@@ -71,6 +64,7 @@ public function __construct(
 			$logger
 		);
 		$this->userSession = $userSession;
+		$this->config = $config;
 	}
 
 	protected function manipulateStorageConfig(IStorageConfig $storage) {
@@ -88,6 +82,12 @@ protected function manipulateStorageConfig(IStorageConfig $storage) {
 	 * @return DataResponse
 	 */
 	public function index() {
+		if (!$this->isUserMountingAllowed()) {
+			return new DataResponse(
+				null,
+				Http::STATUS_FORBIDDEN
+			);
+		}
 		return parent::index();
 	}
 
@@ -122,8 +122,13 @@ 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,
@@ -183,6 +188,12 @@ public function update(
 		$mountOptions,
 		$testOnly = true
 	) {
+		if (!$this->isUserMountingAllowed()) {
+			return new DataResponse(
+				null,
+				Http::STATUS_FORBIDDEN
+			);
+		}
 		$storage = $this->createStorage(
 			$mountPoint,
 			$backend,
@@ -230,6 +241,17 @@ 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', 'yes') === 'yes';
+	}
 }

apps/files_external/tests/Controller/StoragesControllerTest.php

@@ -31,6 +31,7 @@
 use OCP\Files\External\Service\IGlobalStoragesService;
 use OCP\Files\External\Service\IStoragesService;
 use OCP\Files\StorageNotAvailableException;
+use PHPUnit\Framework\MockObject\MockObject;
 
 abstract class StoragesControllerTest extends \Test\TestCase {
 	/**
@@ -39,7 +40,7 @@ abstract class StoragesControllerTest extends \Test\TestCase {
 	protected $controller;
 
 	/**
-	 * @var IGlobalStoragesService
+	 * @var IGlobalStoragesService | MockObject
 	 */
 	protected $service;
 

apps/files_external/tests/Controller/UserStoragesControllerTest.php

@@ -31,28 +31,56 @@
 use OCP\Files\External\IStorageConfig;
 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 array
-	 */
-	private $oldAllowedBackends;
-
 	public function setUp(): void {
 		parent::setUp();
-		$this->service = $this->createMock('\OCP\Files\External\Service\IUserStoragesService');
-
+		$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('\OCP\IRequest'),
-			$this->createMock('\OCP\IL10N'),
+			$this->createMock(IRequest::class),
+			$this->createMock(IL10N::class),
 			$this->service,
-			$this->createMock('\OCP\IUserSession'),
-			$this->createMock('\OCP\ILogger')
+			$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() {