Merge pull request #24661 from nextcloud/external-storage-cant-edit

dont offer to edit external config settings if we can't edit them

8 files changed, 82 insertions, 28 deletions

diff --git a/apps/files_external/js/statusmanager.js b/apps/files_external/js/statusmanager.js
index e8273dca0df..c3945991810 100644
--- a/apps/files_external/js/statusmanager.js
+++ b/apps/files_external/js/statusmanager.js
@@ -94,7 +94,8 @@ OCA.Files_External.StatusManager = {
 							id: mountData.id,
 							error: statusMessage,
 							userProvided: response.userProvided,
-							authMechanism: response.authMechanism
+							authMechanism: response.authMechanism,
+							canEdit: response.can_edit,
 						};
 					}
 					afterCallback(mountData, self.mountStatus[mountData.mount_point]);
@@ -182,12 +183,14 @@ OCA.Files_External.StatusManager = {
 					if (mountData.userProvided || mountData.authMechanism === 'password::global::user') {
 						// personal mount whit credentials problems
 						this.showCredentialsDialog(name, mountData);
-					} else {
+					} else if (mountData.canEdit) {
 						OC.dialogs.confirm(t('files_external', 'There was an error with message: ') + mountData.error + '. Do you want to review mount point config in admin settings page?', t('files_external', 'External mount error'), function (e) {
 							if (e === true) {
 								OC.redirect(OC.generateUrl('/settings/admin/externalstorages'));
 							}
 						});
+					} else {
+						OC.dialogs.info(t('files_external', 'There was an error with message: ') + mountData.error + '. Please contact your system administrator.', t('files_external', 'External mount error'), () => {});
 					}
 				} else {
 					OC.dialogs.confirm(t('files_external', 'There was an error with message: ') + mountData.error + '. Do you want to review mount point config in personal settings page?', t('files_external', 'External mount error'), function (e) {
diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php
index 4d8b6c4bc8f..7ce2c84b8a9 100644
--- a/apps/files_external/lib/Controller/GlobalStoragesController.php
+++ b/apps/files_external/lib/Controller/GlobalStoragesController.php
@@ -32,9 +32,11 @@ use OCA\Files_External\NotFoundException;
 use OCA\Files_External\Service\GlobalStoragesService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
+use OCP\IUserSession;
 
 /**
  * Global storages controller
@@ -48,20 +50,26 @@ class GlobalStoragesController extends StoragesController {
 	 * @param IL10N $l10n l10n service
 	 * @param GlobalStoragesService $globalStoragesService storage service
 	 * @param ILogger $logger
+	 * @param IUserSession $userSession
+	 * @param IGroupManager $groupManager
 	 */
 	public function __construct(
 		$AppName,
 		IRequest $request,
 		IL10N $l10n,
 		GlobalStoragesService $globalStoragesService,
-		ILogger $logger
+		ILogger $logger,
+		IUserSession $userSession,
+		IGroupManager $groupManager
 	) {
 		parent::__construct(
 			$AppName,
 			$request,
 			$l10n,
 			$globalStoragesService,
-			$logger
+			$logger,
+			$userSession,
+			$groupManager
 		);
 	}
 
diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php
index 5d9ffedf885..d58d61e3939 100644
--- a/apps/files_external/lib/Controller/StoragesController.php
+++ b/apps/files_external/lib/Controller/StoragesController.php
@@ -40,9 +40,11 @@ use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\Files\StorageNotAvailableException;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
+use OCP\IUserSession;
 
 /**
  * Base class for storages controllers
@@ -69,6 +71,16 @@ abstract class StoragesController extends Controller {
 	protected $logger;
 
 	/**
+	 * @var IUserSession
+	 */
+	protected $userSession;
+
+	/**
+	 * @var IGroupManager
+	 */
+	protected $groupManager;
+
+	/**
 	 * Creates a new storages controller.
 	 *
 	 * @param string $AppName application name
@@ -82,12 +94,16 @@ abstract class StoragesController extends Controller {
 		IRequest $request,
 		IL10N $l10n,
 		StoragesService $storagesService,
-		ILogger $logger
+		ILogger $logger,
+		IUserSession $userSession,
+		IGroupManager $groupManager
 	) {
 		parent::__construct($AppName, $request);
 		$this->l10n = $l10n;
 		$this->service = $storagesService;
 		$this->logger = $logger;
+		$this->userSession = $userSession;
+		$this->groupManager = $groupManager;
 	}
 
 	/**
@@ -337,8 +353,12 @@ abstract class StoragesController extends Controller {
 			);
 		}
 
+		$data = $this->formatStorageForUI($storage)->jsonSerialize();
+		$isAdmin = $this->groupManager->isAdmin($this->userSession->getUser()->getUID());
+		$data['can_edit'] = $storage->getType() === StorageConfig::MOUNT_TYPE_PERSONAl || $isAdmin;
+
 		return new DataResponse(
-			$this->formatStorageForUI($storage),
+			$data,
 			Http::STATUS_OK
 		);
 	}
diff --git a/apps/files_external/lib/Controller/UserGlobalStoragesController.php b/apps/files_external/lib/Controller/UserGlobalStoragesController.php
index 1f7eb595701..f26b4dd67f9 100644
--- a/apps/files_external/lib/Controller/UserGlobalStoragesController.php
+++ b/apps/files_external/lib/Controller/UserGlobalStoragesController.php
@@ -37,6 +37,7 @@ use OCA\Files_External\NotFoundException;
 use OCA\Files_External\Service\UserGlobalStoragesService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
@@ -47,35 +48,34 @@ use OCP\IUserSession;
  */
 class UserGlobalStoragesController extends StoragesController {
 	/**
-	 * @var IUserSession
-	 */
-	private $userSession;
-
-	/**
 	 * Creates a new user global storages controller.
 	 *
 	 * @param string $AppName application name
 	 * @param IRequest $request request object
 	 * @param IL10N $l10n l10n service
 	 * @param UserGlobalStoragesService $userGlobalStoragesService storage service
+	 * @param ILogger $logger
 	 * @param IUserSession $userSession
+	 * @param IGroupManager $groupManager
 	 */
 	public function __construct(
 		$AppName,
 		IRequest $request,
 		IL10N $l10n,
 		UserGlobalStoragesService $userGlobalStoragesService,
+		ILogger $logger,
 		IUserSession $userSession,
-		ILogger $logger
+		IGroupManager $groupManager
 	) {
 		parent::__construct(
 			$AppName,
 			$request,
 			$l10n,
 			$userGlobalStoragesService,
-			$logger
+			$logger,
+			$userSession,
+			$groupManager
 		);
-		$this->userSession = $userSession;
 	}
 
 	/**
@@ -133,8 +133,12 @@ class UserGlobalStoragesController extends StoragesController {
 
 		$this->sanitizeStorage($storage);
 
+		$data = $this->formatStorageForUI($storage)->jsonSerialize();
+		$isAdmin = $this->groupManager->isAdmin($this->userSession->getUser()->getUID());
+		$data['can_edit'] = $storage->getType() === StorageConfig::MOUNT_TYPE_PERSONAl || $isAdmin;
+
 		return new DataResponse(
-			$this->formatStorageForUI($storage),
+			$data,
 			Http::STATUS_OK
 		);
 	}
diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php
index c297d3db38e..8686921c90c 100644
--- a/apps/files_external/lib/Controller/UserStoragesController.php
+++ b/apps/files_external/lib/Controller/UserStoragesController.php
@@ -35,6 +35,7 @@ use OCA\Files_External\NotFoundException;
 use OCA\Files_External\Service\UserStoragesService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
@@ -45,36 +46,34 @@ use OCP\IUserSession;
  */
 class UserStoragesController extends StoragesController {
 	/**
-	 * @var IUserSession
-	 */
-	private $userSession;
-
-	/**
 	 * Creates a new user storages controller.
 	 *
 	 * @param string $AppName application name
 	 * @param IRequest $request request object
 	 * @param IL10N $l10n l10n service
 	 * @param UserStoragesService $userStoragesService storage service
-	 * @param IUserSession $userSession
 	 * @param ILogger $logger
+	 * @param IUserSession $userSession
+	 * @param IGroupManager $groupManager
 	 */
 	public function __construct(
 		$AppName,
 		IRequest $request,
 		IL10N $l10n,
 		UserStoragesService $userStoragesService,
+		ILogger $logger,
 		IUserSession $userSession,
-		ILogger $logger
+		IGroupManager $groupManager
 	) {
 		parent::__construct(
 			$AppName,
 			$request,
 			$l10n,
 			$userStoragesService,
-			$logger
+			$logger,
+			$userSession,
+			$groupManager
 		);
-		$this->userSession = $userSession;
 	}
 
 	protected function manipulateStorageConfig(StorageConfig $storage) {
diff --git a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php
index b2e0a656836..9314488aa08 100644
--- a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php
+++ b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php
@@ -26,11 +26,15 @@
 
 namespace OCA\Files_External\Tests\Controller;
 
+use OC\User\User;
 use OCA\Files_External\Controller\GlobalStoragesController;
 use OCA\Files_External\Service\BackendService;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
+use OCP\IUserSession;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 class GlobalStoragesControllerTest extends StoragesControllerTest {
 	protected function setUp(): void {
@@ -42,12 +46,18 @@ class GlobalStoragesControllerTest extends StoragesControllerTest {
 		$this->service->method('getVisibilityType')
 			->willReturn(BackendService::VISIBILITY_ADMIN);
 
+		$session = $this->createMock(IUserSession::class);
+		$session->method('getUser')
+			->willReturn(new User('test', null, $this->createMock(EventDispatcherInterface::class)));
+
 		$this->controller = new GlobalStoragesController(
 			'files_external',
 			$this->createMock(IRequest::class),
 			$this->createMock(IL10N::class),
 			$this->service,
-			$this->createMock(ILogger::class)
+			$this->createMock(ILogger::class),
+			$session,
+			$this->createMock(IGroupManager::class),
 		);
 	}
 }
diff --git a/apps/files_external/tests/Controller/StoragesControllerTest.php b/apps/files_external/tests/Controller/StoragesControllerTest.php
index fffac01507a..80dd01d3684 100644
--- a/apps/files_external/tests/Controller/StoragesControllerTest.php
+++ b/apps/files_external/tests/Controller/StoragesControllerTest.php
@@ -338,7 +338,9 @@ abstract class StoragesControllerTest extends \Test\TestCase {
 		$response = $this->controller->show(1);
 
 		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-		$this->assertEquals($storageConfig, $response->getData());
+		$expected = $storageConfig->jsonSerialize();
+		$expected['can_edit'] = false;
+		$this->assertEquals($expected, $response->getData());
 	}
 
 	public function validateStorageProvider() {
diff --git a/apps/files_external/tests/Controller/UserStoragesControllerTest.php b/apps/files_external/tests/Controller/UserStoragesControllerTest.php
index ed2d46b6149..7ceb18b633b 100644
--- a/apps/files_external/tests/Controller/UserStoragesControllerTest.php
+++ b/apps/files_external/tests/Controller/UserStoragesControllerTest.php
@@ -27,14 +27,17 @@
 
 namespace OCA\Files_External\Tests\Controller;
 
+use OC\User\User;
 use OCA\Files_External\Controller\UserStoragesController;
 use OCA\Files_External\Lib\StorageConfig;
 use OCA\Files_External\Service\BackendService;
 use OCP\AppFramework\Http;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IUserSession;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 class UserStoragesControllerTest extends StoragesControllerTest {
 
@@ -52,13 +55,18 @@ class UserStoragesControllerTest extends StoragesControllerTest {
 		$this->service->method('getVisibilityType')
 			->willReturn(BackendService::VISIBILITY_PERSONAL);
 
+		$session = $this->createMock(IUserSession::class);
+		$session->method('getUser')
+			->willReturn(new User('test', null, $this->createMock(EventDispatcherInterface::class)));
+
 		$this->controller = new UserStoragesController(
 			'files_external',
 			$this->createMock(IRequest::class),
 			$this->createMock(IL10N::class),
 			$this->service,
-			$this->createMock(IUserSession::class),
-			$this->createMock(ILogger::class)
+			$this->createMock(ILogger::class),
+			$session,
+			$this->createMock(IGroupManager::class)
 		);
 	}