Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable20] dont offer to edit external config settings if we can't edit them #25132

Merged
merged 4 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions apps/files_external/js/statusmanager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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'), () => {});
icewind1991 marked this conversation as resolved.
Show resolved Hide resolved
}
} 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) {
Expand Down
12 changes: 10 additions & 2 deletions apps/files_external/lib/Controller/GlobalStoragesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@
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
Expand All @@ -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
);
}

Expand Down
24 changes: 22 additions & 2 deletions apps/files_external/lib/Controller/StoragesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@
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
Expand All @@ -68,6 +70,16 @@ abstract class StoragesController extends Controller {
*/
protected $logger;

/**
* @var IUserSession
*/
protected $userSession;

/**
* @var IGroupManager
*/
protected $groupManager;

/**
* Creates a new storages controller.
*
Expand All @@ -82,12 +94,16 @@ public function __construct(
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;
}

/**
Expand Down Expand Up @@ -337,8 +353,12 @@ public function show($id, $testOnly = true) {
);
}

$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
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand All @@ -46,36 +47,35 @@
* User global storages controller
*/
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;
}

/**
Expand Down Expand Up @@ -133,8 +133,12 @@ public function show($id, $testOnly = true) {

$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
);
}
Expand Down
17 changes: 8 additions & 9 deletions apps/files_external/lib/Controller/UserStoragesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
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;
Expand All @@ -44,37 +45,35 @@
* User storages controller
*/
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -42,12 +46,18 @@ protected function setUp(): void {
$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),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,9 @@ public function testGetStorage() {
$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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -52,13 +55,18 @@ protected function setUp(): void {
$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)
);
}

Expand Down