From e0d85ed9a867df2ef110cdaa588261d7173d73fb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 11 Dec 2020 16:40:29 +0100 Subject: [PATCH 1/4] dont offer to edit external config settings if we can't edit them Signed-off-by: Robin Appelman --- apps/files_external/js/statusmanager.js | 7 ++++-- .../Controller/GlobalStoragesController.php | 12 ++++++++-- .../lib/Controller/StoragesController.php | 24 +++++++++++++++++-- .../UserGlobalStoragesController.php | 22 ++++++++++------- .../lib/Controller/UserStoragesController.php | 17 +++++++------ 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/apps/files_external/js/statusmanager.js b/apps/files_external/js/statusmanager.js index e8273dca0d..c394599181 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 d42facb18a..3f218c84b7 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 6be833687b..35e0e85ab0 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 @@ -68,6 +70,16 @@ abstract class StoragesController extends Controller { */ protected $logger; + /** + * @var IUserSession + */ + protected $userSession; + + /** + * @var IGroupManager + */ + protected $groupManager; + /** * Creates a new storages controller. * @@ -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 1f7eb59570..f26b4dd67f 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; @@ -46,11 +47,6 @@ use OCP\IUserSession; * User global storages controller */ class UserGlobalStoragesController extends StoragesController { - /** - * @var IUserSession - */ - private $userSession; - /** * Creates a new user global storages controller. * @@ -58,24 +54,28 @@ class UserGlobalStoragesController extends StoragesController { * @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 8c588fc967..b6174a4cf5 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; @@ -44,11 +45,6 @@ use OCP\IUserSession; * User storages controller */ class UserStoragesController extends StoragesController { - /** - * @var IUserSession - */ - private $userSession; - /** * Creates a new user storages controller. * @@ -56,25 +52,28 @@ class UserStoragesController extends StoragesController { * @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) { From 6404627d0c6d7a9b89a63fb2280d70844521bc29 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 8 Jan 2021 16:14:44 +0100 Subject: [PATCH 2/4] adjust tests Signed-off-by: Robin Appelman --- .../Controller/GlobalStoragesControllerTest.php | 12 +++++++++++- .../tests/Controller/StoragesControllerTest.php | 4 +++- .../tests/Controller/UserStoragesControllerTest.php | 12 ++++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php index b829525b51..dc296ee120 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 7572da2575..10b38e99a8 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 109a033514..53feb127f9 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) ); } From 590ef229354893fa1de92f13d5440218b01b1177 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2021 16:56:11 +0000 Subject: [PATCH 3/4] Update apps/files_external/js/statusmanager.js Co-authored-by: Morris Jobke Signed-off-by: Robin Appelman --- apps/files_external/js/statusmanager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/js/statusmanager.js b/apps/files_external/js/statusmanager.js index c394599181..9d25fef0c3 100644 --- a/apps/files_external/js/statusmanager.js +++ b/apps/files_external/js/statusmanager.js @@ -190,7 +190,7 @@ OCA.Files_External.StatusManager = { } }); } 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'), () => {}); + 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'), function(){}); } } 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) { From e7805e500976e8552e8b01084b3090b671c0ada4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Feb 2021 15:56:26 +0100 Subject: [PATCH 4/4] fix test Signed-off-by: Robin Appelman --- .../tests/Controller/GlobalStoragesControllerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php index dc296ee120..1c723ced42 100644 --- a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php @@ -57,7 +57,7 @@ class GlobalStoragesControllerTest extends StoragesControllerTest { $this->service, $this->createMock(ILogger::class), $session, - $this->createMock(IGroupManager::class), + $this->createMock(IGroupManager::class) ); } }