From 080fafe63a980f6a485027fd4216864adf764e1e Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Wed, 19 Aug 2015 21:13:16 +0100 Subject: AjaxController uses RSA auth mechanism --- apps/files_external/controller/ajaxcontroller.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'apps/files_external/controller') diff --git a/apps/files_external/controller/ajaxcontroller.php b/apps/files_external/controller/ajaxcontroller.php index cb2de432286..c285cd34e70 100644 --- a/apps/files_external/controller/ajaxcontroller.php +++ b/apps/files_external/controller/ajaxcontroller.php @@ -25,19 +25,19 @@ namespace OCA\Files_External\Controller; use OCP\AppFramework\Controller; use OCP\IRequest; use OCP\AppFramework\Http\JSONResponse; -use phpseclib\Crypt\RSA; +use OCA\Files_External\Lib\Auth\PublicKey\RSA; class AjaxController extends Controller { - public function __construct($appName, IRequest $request) { + /** @var RSA */ + private $rsaMechanism; + + public function __construct($appName, IRequest $request, RSA $rsaMechanism) { parent::__construct($appName, $request); + $this->rsaMechanism = $rsaMechanism; } private function generateSshKeys() { - $rsa = new RSA(); - $rsa->setPublicKeyFormat(RSA::PUBLIC_FORMAT_OPENSSH); - $rsa->setPassword(\OC::$server->getConfig()->getSystemValue('secret', '')); - - $key = $rsa->createKey(); + $key = $this->rsaMechanism->createKey(); // Replace the placeholder label with a more meaningful one $key['publicKey'] = str_replace('phpseclib-generated-key', gethostname(), $key['publickey']); -- cgit v1.2.3 From cc88c5f4b84da57c425cbdb7dc8b391b1942b503 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Fri, 28 Aug 2015 15:52:29 +0100 Subject: Implement more fine-grained external storage permissions model VisibilityTrait -> PermissionsTrait PermissionsTrait stores two sets of data, $permissions and $allowedPermissions (analogous to $visibility and $allowedVisibility of VisibilityTrait). Each set is a map of user type ('admin' or 'personal') to permissions (mounting permission, create permission). The result is that a backend can now be restricted for creation, while still allowing it to be mounted. This is useful for deprecating backends or auth mechanisms, preventing new storages being created, while still allowing existing storages to be mounted. --- .../controller/userstoragescontroller.php | 2 +- apps/files_external/lib/auth/authmechanism.php | 6 +- apps/files_external/lib/backend/backend.php | 6 +- apps/files_external/lib/backend/local.php | 2 +- apps/files_external/lib/permissionstrait.php | 164 +++++++++++++++++++++ apps/files_external/lib/visibilitytrait.php | 136 ----------------- apps/files_external/personal.php | 8 +- apps/files_external/service/backendservice.php | 68 ++------- apps/files_external/settings.php | 16 +- apps/files_external/templates/settings.php | 2 +- .../tests/controller/storagescontrollertest.php | 8 +- .../controller/userstoragescontrollertest.php | 4 +- .../tests/service/backendservicetest.php | 28 +--- 13 files changed, 212 insertions(+), 238 deletions(-) create mode 100644 apps/files_external/lib/permissionstrait.php delete mode 100644 apps/files_external/lib/visibilitytrait.php (limited to 'apps/files_external/controller') diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index 0d41e088a76..9baac3a8031 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -79,7 +79,7 @@ class UserStoragesController extends StoragesController { // Prevent non-admin users from mounting local storage and other disabled backends /** @var Backend */ $backend = $storage->getBackend(); - if (!$backend->isVisibleFor(BackendService::VISIBILITY_PERSONAL)) { + if (!$backend->isPermitted(BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT)) { return new DataResponse( array( 'message' => (string)$this->l10n->t('Admin-only storage backend "%s"', [ diff --git a/apps/files_external/lib/auth/authmechanism.php b/apps/files_external/lib/auth/authmechanism.php index 11d99bb330d..ddc0c6a4dca 100644 --- a/apps/files_external/lib/auth/authmechanism.php +++ b/apps/files_external/lib/auth/authmechanism.php @@ -22,7 +22,7 @@ namespace OCA\Files_External\Lib\Auth; use \OCA\Files_External\Lib\StorageConfig; -use \OCA\Files_External\Lib\VisibilityTrait; +use \OCA\Files_External\Lib\PermissionsTrait; use \OCA\Files_External\Lib\IdentifierTrait; use \OCA\Files_External\Lib\FrontendDefinitionTrait; use \OCA\Files_External\Lib\StorageModifierTrait; @@ -40,7 +40,7 @@ use \OCA\Files_External\Lib\StorageModifierTrait; * scheme, which are provided from the authentication mechanism. * * This class uses the following traits: - * - VisibilityTrait + * - PermissionsTrait * Restrict usage to admin-only/none * - FrontendDefinitionTrait * Specify configuration parameters and other definitions @@ -58,7 +58,7 @@ class AuthMechanism implements \JsonSerializable { const SCHEME_PUBLICKEY = 'publickey'; const SCHEME_OPENSTACK = 'openstack'; - use VisibilityTrait; + use PermissionsTrait; use FrontendDefinitionTrait; use StorageModifierTrait; use IdentifierTrait; diff --git a/apps/files_external/lib/backend/backend.php b/apps/files_external/lib/backend/backend.php index 90d5d38ed94..2a2add3ac59 100644 --- a/apps/files_external/lib/backend/backend.php +++ b/apps/files_external/lib/backend/backend.php @@ -22,7 +22,7 @@ namespace OCA\Files_External\Lib\Backend; use \OCA\Files_External\Lib\StorageConfig; -use \OCA\Files_External\Lib\VisibilityTrait; +use \OCA\Files_External\Lib\PermissionsTrait; use \OCA\Files_External\Lib\FrontendDefinitionTrait; use \OCA\Files_External\Lib\PriorityTrait; use \OCA\Files_External\Lib\DependencyTrait; @@ -43,7 +43,7 @@ use \OCA\Files_External\Lib\Auth\AuthMechanism; * scheme, which are provided from the authentication mechanism. * * This class uses the following traits: - * - VisibilityTrait + * - PermissionsTrait * Restrict usage to admin-only/none * - FrontendDefinitionTrait * Specify configuration parameters and other definitions @@ -56,7 +56,7 @@ use \OCA\Files_External\Lib\Auth\AuthMechanism; */ class Backend implements \JsonSerializable { - use VisibilityTrait; + use PermissionsTrait; use FrontendDefinitionTrait; use PriorityTrait; use DependencyTrait; diff --git a/apps/files_external/lib/backend/local.php b/apps/files_external/lib/backend/local.php index a80b437fab7..a6635491b6e 100644 --- a/apps/files_external/lib/backend/local.php +++ b/apps/files_external/lib/backend/local.php @@ -39,7 +39,7 @@ class Local extends Backend { ->addParameters([ (new DefinitionParameter('datadir', $l->t('Location'))), ]) - ->setAllowedVisibility(BackendService::VISIBILITY_ADMIN) + ->setAllowedPermissions(BackendService::USER_PERSONAL, BackendService::PERMISSION_NONE) ->setPriority(BackendService::PRIORITY_DEFAULT + 50) ->addAuthScheme(AuthMechanism::SCHEME_NULL) ->setLegacyAuthMechanism($legacyAuth) diff --git a/apps/files_external/lib/permissionstrait.php b/apps/files_external/lib/permissionstrait.php new file mode 100644 index 00000000000..8509a01e422 --- /dev/null +++ b/apps/files_external/lib/permissionstrait.php @@ -0,0 +1,164 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\Files_External\Lib; + +use \OCA\Files_External\Service\BackendService; + +/** + * Trait to implement backend and auth mechanism permissions + * + * For user type constants, see BackendService::USER_* + * For permission constants, see BackendService::PERMISSION_* + */ +trait PermissionsTrait { + + /** @var array [user type => permissions] */ + protected $permissions = [ + BackendService::USER_PERSONAL => BackendService::PERMISSION_DEFAULT, + BackendService::USER_ADMIN => BackendService::PERMISSION_DEFAULT, + ]; + + /** @var array [user type => allowed permissions] */ + protected $allowedPermissions = [ + BackendService::USER_PERSONAL => BackendService::PERMISSION_DEFAULT, + BackendService::USER_ADMIN => BackendService::PERMISSION_DEFAULT, + ]; + + /** + * @param string $userType + * @return int + */ + public function getPermissions($userType) { + if (isset($this->permissions[$userType])) { + return $this->permissions[$userType]; + } + return BackendService::PERMISSION_NONE; + } + + /** + * Check if the user type has permission + * + * @param string $userType + * @param int $permission + * @return bool + */ + public function isPermitted($userType, $permission) { + if ($this->getPermissions($userType) & $permission) { + return true; + } + return false; + } + + /** + * @param string $userType + * @param int $permissions + * @return self + */ + public function setPermissions($userType, $permissions) { + $this->permissions[$userType] = $permissions; + $this->allowedPermissions[$userType] = + $this->getAllowedPermissions($userType) | $permissions; + return $this; + } + + /** + * @param string $userType + * @param int $permission + * @return self + */ + public function addPermission($userType, $permission) { + return $this->setPermissions($userType, + $this->getPermissions($userType) | $permission + ); + } + + /** + * @param string $userType + * @param int $permission + * @return self + */ + public function removePermission($userType, $permission) { + return $this->setPermissions($userType, + $this->getPermissions($userType) & ~$permission + ); + } + + /** + * @param string $userType + * @return int + */ + public function getAllowedPermissions($userType) { + if (isset($this->allowedPermissions[$userType])) { + return $this->allowedPermissions[$userType]; + } + return BackendService::PERMISSION_NONE; + } + + /** + * Check if the user type has an allowed permission + * + * @param string $userType + * @param int $permission + * @return bool + */ + public function isAllowedPermitted($userType, $permission) { + if ($this->getAllowedPermissions($userType) & $permission) { + return true; + } + return false; + } + + /** + * @param string $userType + * @param int $permissions + * @return self + */ + public function setAllowedPermissions($userType, $permissions) { + $this->allowedPermissions[$userType] = $permissions; + $this->permissions[$userType] = + $this->getPermissions($userType) & $permissions; + return $this; + } + + /** + * @param string $userType + * @param int $permission + * @return self + */ + public function addAllowedPermission($userType, $permission) { + return $this->setAllowedPermissions($userType, + $this->getAllowedPermissions($userType) | $permission + ); + } + + /** + * @param string $userType + * @param int $permission + * @return self + */ + public function removeAllowedPermission($userType, $permission) { + return $this->setAllowedPermissions($userType, + $this->getAllowedPermissions($userType) & ~$permission + ); + } + +} diff --git a/apps/files_external/lib/visibilitytrait.php b/apps/files_external/lib/visibilitytrait.php deleted file mode 100644 index dfd2d323ca6..00000000000 --- a/apps/files_external/lib/visibilitytrait.php +++ /dev/null @@ -1,136 +0,0 @@ - - * - * @copyright Copyright (c) 2015, ownCloud, Inc. - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ - -namespace OCA\Files_External\Lib; - -use \OCA\Files_External\Service\BackendService; - -/** - * Trait to implement visibility mechanics for a configuration class - * - * The standard visibility defines which users/groups can use or see the - * object. The allowed visibility defines the maximum visibility allowed to be - * set on the object. The standard visibility is often set dynamically by - * stored configuration parameters that can be modified by the administrator, - * while the allowed visibility is set directly by the object and cannot be - * modified by the administrator. - */ -trait VisibilityTrait { - - /** @var int visibility */ - protected $visibility = BackendService::VISIBILITY_DEFAULT; - - /** @var int allowed visibilities */ - protected $allowedVisibility = BackendService::VISIBILITY_DEFAULT; - - /** - * @return int - */ - public function getVisibility() { - return $this->visibility; - } - - /** - * Check if the backend is visible for a user type - * - * @param int $visibility - * @return bool - */ - public function isVisibleFor($visibility) { - if ($this->visibility & $visibility) { - return true; - } - return false; - } - - /** - * @param int $visibility - * @return self - */ - public function setVisibility($visibility) { - $this->visibility = $visibility; - $this->allowedVisibility |= $visibility; - return $this; - } - - /** - * @param int $visibility - * @return self - */ - public function addVisibility($visibility) { - return $this->setVisibility($this->visibility | $visibility); - } - - /** - * @param int $visibility - * @return self - */ - public function removeVisibility($visibility) { - return $this->setVisibility($this->visibility & ~$visibility); - } - - /** - * @return int - */ - public function getAllowedVisibility() { - return $this->allowedVisibility; - } - - /** - * Check if the backend is allowed to be visible for a user type - * - * @param int $allowedVisibility - * @return bool - */ - public function isAllowedVisibleFor($allowedVisibility) { - if ($this->allowedVisibility & $allowedVisibility) { - return true; - } - return false; - } - - /** - * @param int $allowedVisibility - * @return self - */ - public function setAllowedVisibility($allowedVisibility) { - $this->allowedVisibility = $allowedVisibility; - $this->visibility &= $allowedVisibility; - return $this; - } - - /** - * @param int $allowedVisibility - * @return self - */ - public function addAllowedVisibility($allowedVisibility) { - return $this->setAllowedVisibility($this->allowedVisibility | $allowedVisibility); - } - - /** - * @param int $allowedVisibility - * @return self - */ - public function removeAllowedVisibility($allowedVisibility) { - return $this->setAllowedVisibility($this->allowedVisibility & ~$allowedVisibility); - } - -} diff --git a/apps/files_external/personal.php b/apps/files_external/personal.php index 8717d91d4f1..d47f983b357 100644 --- a/apps/files_external/personal.php +++ b/apps/files_external/personal.php @@ -34,8 +34,12 @@ $userStoragesService = $appContainer->query('OCA\Files_external\Service\UserStor OCP\Util::addScript('files_external', 'settings'); OCP\Util::addStyle('files_external', 'settings'); -$backends = $backendService->getBackendsVisibleFor(BackendService::VISIBILITY_PERSONAL); -$authMechanisms = $backendService->getAuthMechanismsVisibleFor(BackendService::VISIBILITY_PERSONAL); +$backends = array_filter($backendService->getAvailableBackends(), function($backend) { + return $backend->isPermitted(BackendService::USER_PERSONAL, BackendService::PERMISSION_CREATE); +}); +$authMechanisms = array_filter($backendService->getAuthMechanisms(), function($authMechanism) { + return $authMechanism->isPermitted(BackendService::USER_PERSONAL, BackendService::PERMISSION_CREATE); +}); foreach ($backends as $backend) { if ($backend->getCustomJs()) { \OCP\Util::addScript('files_external', $backend->getCustomJs()); diff --git a/apps/files_external/service/backendservice.php b/apps/files_external/service/backendservice.php index 382834b4c19..d9a3e316ce4 100644 --- a/apps/files_external/service/backendservice.php +++ b/apps/files_external/service/backendservice.php @@ -31,13 +31,16 @@ use \OCA\Files_External\Lib\Auth\AuthMechanism; */ class BackendService { - /** Visibility constants for VisibilityTrait */ - const VISIBILITY_NONE = 0; - const VISIBILITY_PERSONAL = 1; - const VISIBILITY_ADMIN = 2; - //const VISIBILITY_ALIENS = 4; + /** Permission constants for PermissionsTrait */ + const PERMISSION_NONE = 0; + const PERMISSION_MOUNT = 1; + const PERMISSION_CREATE = 2; - const VISIBILITY_DEFAULT = 3; // PERSONAL | ADMIN + const PERMISSION_DEFAULT = 3; // MOUNT | CREATE + + /** User contants */ + const USER_ADMIN = 'admin'; + const USER_PERSONAL = 'personal'; /** Priority constants for PriorityTrait */ const PRIORITY_DEFAULT = 100; @@ -81,7 +84,7 @@ class BackendService { */ public function registerBackend(Backend $backend) { if (!$this->isAllowedUserBackend($backend)) { - $backend->removeVisibility(BackendService::VISIBILITY_PERSONAL); + $backend->removePermission(self::USER_PERSONAL, self::PERMISSION_CREATE | self::PERMISSION_MOUNT); } foreach ($backend->getIdentifierAliases() as $alias) { $this->backends[$alias] = $backend; @@ -103,7 +106,7 @@ class BackendService { */ public function registerAuthMechanism(AuthMechanism $authMech) { if (!$this->isAllowedAuthMechanism($authMech)) { - $authMech->removeVisibility(BackendService::VISIBILITY_PERSONAL); + $authMech->removePermission(self::USER_PERSONAL, self::PERMISSION_CREATE | self::PERMISSION_MOUNT); } foreach ($authMech->getIdentifierAliases() as $alias) { $this->authMechanisms[$alias] = $authMech; @@ -144,30 +147,6 @@ class BackendService { }); } - /** - * Get backends visible for $visibleFor - * - * @param int $visibleFor - * @return Backend[] - */ - public function getBackendsVisibleFor($visibleFor) { - return array_filter($this->getAvailableBackends(), function($backend) use ($visibleFor) { - return $backend->isVisibleFor($visibleFor); - }); - } - - /** - * Get backends allowed to be visible for $visibleFor - * - * @param int $visibleFor - * @return Backend[] - */ - public function getBackendsAllowedVisibleFor($visibleFor) { - return array_filter($this->getAvailableBackends(), function($backend) use ($visibleFor) { - return $backend->isAllowedVisibleFor($visibleFor); - }); - } - /** * @param string $identifier * @return Backend|null @@ -205,31 +184,6 @@ class BackendService { }); } - /** - * Get authentication mechanisms visible for $visibleFor - * - * @param int $visibleFor - * @return AuthMechanism[] - */ - public function getAuthMechanismsVisibleFor($visibleFor) { - return array_filter($this->getAuthMechanisms(), function($authMechanism) use ($visibleFor) { - return $authMechanism->isVisibleFor($visibleFor); - }); - } - - /** - * Get authentication mechanisms allowed to be visible for $visibleFor - * - * @param int $visibleFor - * @return AuthMechanism[] - */ - public function getAuthMechanismsAllowedVisibleFor($visibleFor) { - return array_filter($this->getAuthMechanisms(), function($authMechanism) use ($visibleFor) { - return $authMechanism->isAllowedVisibleFor($visibleFor); - }); - } - - /** * @param string $identifier * @return AuthMechanism|null diff --git a/apps/files_external/settings.php b/apps/files_external/settings.php index 29c0553158f..840f1325fb5 100644 --- a/apps/files_external/settings.php +++ b/apps/files_external/settings.php @@ -41,8 +41,12 @@ OCP\Util::addStyle('files_external', 'settings'); \OC_Util::addVendorScript('select2/select2'); \OC_Util::addVendorStyle('select2/select2'); -$backends = $backendService->getBackendsVisibleFor(BackendService::VISIBILITY_ADMIN); -$authMechanisms = $backendService->getAuthMechanismsVisibleFor(BackendService::VISIBILITY_ADMIN); +$backends = array_filter($backendService->getAvailableBackends(), function($backend) { + return $backend->isPermitted(BackendService::USER_ADMIN, BackendService::PERMISSION_CREATE); +}); +$authMechanisms = array_filter($backendService->getAuthMechanisms(), function($authMechanism) { + return $authMechanism->isPermitted(BackendService::USER_ADMIN, BackendService::PERMISSION_CREATE); +}); foreach ($backends as $backend) { if ($backend->getCustomJs()) { \OCP\Util::addScript('files_external', $backend->getCustomJs()); @@ -54,13 +58,19 @@ foreach ($authMechanisms as $authMechanism) { } } +$userBackends = array_filter($backendService->getAvailableBackends(), function($backend) { + return $backend->isAllowedPermitted( + BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT + ); +}); + $tmpl = new OCP\Template('files_external', 'settings'); $tmpl->assign('encryptionEnabled', \OC::$server->getEncryptionManager()->isEnabled()); $tmpl->assign('isAdminPage', true); $tmpl->assign('storages', $globalStoragesService->getAllStorages()); $tmpl->assign('backends', $backends); $tmpl->assign('authMechanisms', $authMechanisms); -$tmpl->assign('userBackends', $backendService->getBackendsAllowedVisibleFor(BackendService::VISIBILITY_PERSONAL)); +$tmpl->assign('userBackends', $userBackends); $tmpl->assign('dependencies', OC_Mount_Config::dependencyMessage($backendService->getBackends())); $tmpl->assign('allowUserMounting', $backendService->isUserMountingAllowed()); return $tmpl->fetchPage(); diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index 5b79edf6cf7..63a3a19de2f 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -197,7 +197,7 @@

class="hidden"> t('Allow users to mount the following external storage')); ?>
- isVisibleFor(BackendService::VISIBILITY_PERSONAL)) print_unescaped(' checked="checked"'); ?> /> + isPermitted(BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT)) print_unescaped(' checked="checked"'); ?> />
diff --git a/apps/files_external/tests/controller/storagescontrollertest.php b/apps/files_external/tests/controller/storagescontrollertest.php index 2b0ee7a14ea..5a2cff99244 100644 --- a/apps/files_external/tests/controller/storagescontrollertest.php +++ b/apps/files_external/tests/controller/storagescontrollertest.php @@ -78,7 +78,7 @@ abstract class StoragesControllerTest extends \Test\TestCase { $backend = $this->getBackendMock(); $backend->method('validateStorage') ->willReturn(true); - $backend->method('isVisibleFor') + $backend->method('isPermitted') ->willReturn(true); $storageConfig = new StorageConfig(1); @@ -117,7 +117,7 @@ abstract class StoragesControllerTest extends \Test\TestCase { $backend = $this->getBackendMock(); $backend->method('validateStorage') ->willReturn(true); - $backend->method('isVisibleFor') + $backend->method('isPermitted') ->willReturn(true); $storageConfig = new StorageConfig(1); @@ -248,7 +248,7 @@ abstract class StoragesControllerTest extends \Test\TestCase { $backend = $this->getBackendMock(); $backend->method('validateStorage') ->willReturn(true); - $backend->method('isVisibleFor') + $backend->method('isPermitted') ->willReturn(true); $storageConfig = new StorageConfig(255); @@ -332,7 +332,7 @@ abstract class StoragesControllerTest extends \Test\TestCase { $backend = $this->getBackendMock(); $backend->method('validateStorage') ->willReturn($backendValidate); - $backend->method('isVisibleFor') + $backend->method('isPermitted') ->willReturn(true); $authMech = $this->getAuthMechMock(); diff --git a/apps/files_external/tests/controller/userstoragescontrollertest.php b/apps/files_external/tests/controller/userstoragescontrollertest.php index 9f1a8df8d2e..b9668064e33 100644 --- a/apps/files_external/tests/controller/userstoragescontrollertest.php +++ b/apps/files_external/tests/controller/userstoragescontrollertest.php @@ -50,8 +50,8 @@ class UserStoragesControllerTest extends StoragesControllerTest { public function testAddOrUpdateStorageDisallowedBackend() { $backend = $this->getBackendMock(); - $backend->method('isVisibleFor') - ->with(BackendService::VISIBILITY_PERSONAL) + $backend->method('isPermitted') + ->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT) ->willReturn(false); $authMech = $this->getAuthMechMock(); diff --git a/apps/files_external/tests/service/backendservicetest.php b/apps/files_external/tests/service/backendservicetest.php index 08f6b9bf988..b37b5e9b466 100644 --- a/apps/files_external/tests/service/backendservicetest.php +++ b/apps/files_external/tests/service/backendservicetest.php @@ -83,11 +83,11 @@ class BackendServiceTest extends \Test\TestCase { $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); $backendAllowed->expects($this->never()) - ->method('removeVisibility'); + ->method('removePermission'); $backendNotAllowed = $this->getBackendMock('\User\Mount\NotAllowed'); $backendNotAllowed->expects($this->once()) - ->method('removeVisibility') - ->with(BackendService::VISIBILITY_PERSONAL); + ->method('removePermission') + ->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_CREATE | BackendService::PERMISSION_MOUNT); $backendAlias = $this->getMockBuilder('\OCA\Files_External\Lib\Backend\Backend') ->disableOriginalConstructor() @@ -126,27 +126,5 @@ class BackendServiceTest extends \Test\TestCase { $this->assertArrayNotHasKey('identifier:\Backend\NotAvailable', $availableBackends); } - public function testGetUserBackends() { - $service = new BackendService($this->config, $this->l10n); - - $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); - $backendAllowed->expects($this->once()) - ->method('isVisibleFor') - ->with(BackendService::VISIBILITY_PERSONAL) - ->will($this->returnValue(true)); - $backendNotAllowed = $this->getBackendMock('\User\Mount\NotAllowed'); - $backendNotAllowed->expects($this->once()) - ->method('isVisibleFor') - ->with(BackendService::VISIBILITY_PERSONAL) - ->will($this->returnValue(false)); - - $service->registerBackend($backendAllowed); - $service->registerBackend($backendNotAllowed); - - $userBackends = $service->getBackendsVisibleFor(BackendService::VISIBILITY_PERSONAL); - $this->assertArrayHasKey('identifier:\User\Mount\Allowed', $userBackends); - $this->assertArrayNotHasKey('identifier:\User\Mount\NotAllowed', $userBackends); - } - } -- cgit v1.2.3 From f0c8cfa9a6a5db7134a2490cc562ff2623ce685d Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Fri, 28 Aug 2015 16:15:21 +0100 Subject: Validate permissions for created admin storages, auth mechanism Backend and auth mechanism permissions are checked on storage creation, both for personal storages and for admin storages --- .../controller/globalstoragescontroller.php | 11 ++++++ .../controller/storagescontroller.php | 34 +++++++++++++++++- .../controller/userstoragescontroller.php | 42 ++++++---------------- .../tests/controller/storagescontrollertest.php | 8 +++++ .../controller/userstoragescontrollertest.php | 2 +- 5 files changed, 63 insertions(+), 34 deletions(-) (limited to 'apps/files_external/controller') diff --git a/apps/files_external/controller/globalstoragescontroller.php b/apps/files_external/controller/globalstoragescontroller.php index 756a34fc5d4..32408420039 100644 --- a/apps/files_external/controller/globalstoragescontroller.php +++ b/apps/files_external/controller/globalstoragescontroller.php @@ -32,6 +32,7 @@ use \OCP\AppFramework\Http; use \OCA\Files_external\Service\GlobalStoragesService; use \OCA\Files_external\NotFoundException; use \OCA\Files_external\Lib\StorageConfig; +use \OCA\Files_External\Service\BackendService; /** * Global storages controller @@ -178,4 +179,14 @@ class GlobalStoragesController extends StoragesController { } + /** + * Get the user type for this controller, used in validation + * + * @return string BackendService::USER_* constants + */ + protected function getUserType() { + return BackendService::USER_ADMIN; + } + + } diff --git a/apps/files_external/controller/storagescontroller.php b/apps/files_external/controller/storagescontroller.php index 613f22c0331..d99b8b5f2c5 100644 --- a/apps/files_external/controller/storagescontroller.php +++ b/apps/files_external/controller/storagescontroller.php @@ -36,6 +36,7 @@ use \OCA\Files_External\Lib\Backend\Backend; use \OCA\Files_External\Lib\Auth\AuthMechanism; use \OCP\Files\StorageNotAvailableException; use \OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; +use \OCA\Files_External\Service\BackendService; /** * Base class for storages controllers @@ -157,12 +158,36 @@ abstract class StoragesController extends Controller { return new DataResponse( array( 'message' => (string)$this->l10n->t('Invalid storage backend "%s"', [ - $storage->getBackend()->getIdentifier() + $backend->getIdentifier() ]) ), Http::STATUS_UNPROCESSABLE_ENTITY ); } + + if (!$backend->isPermitted($this->getUserType(), BackendService::PERMISSION_CREATE)) { + // not permitted to use backend + return new DataResponse( + array( + 'message' => (string)$this->l10n->t('Not permitted to use backend "%s"', [ + $backend->getIdentifier() + ]) + ), + Http::STATUS_UNPROCESSABLE_ENTITY + ); + } + if (!$authMechanism->isPermitted($this->getUserType(), BackendService::PERMISSION_CREATE)) { + // not permitted to use auth mechanism + return new DataResponse( + array( + 'message' => (string)$this->l10n->t('Not permitted to use authentication mechanism "%s"', [ + $authMechanism->getIdentifier() + ]) + ), + Http::STATUS_UNPROCESSABLE_ENTITY + ); + } + if (!$backend->validateStorage($storage)) { // unsatisfied parameters return new DataResponse( @@ -185,6 +210,13 @@ abstract class StoragesController extends Controller { return null; } + /** + * Get the user type for this controller, used in validation + * + * @return string BackendService::USER_* constants + */ + abstract protected function getUserType(); + /** * Check whether the given storage is available / valid. * diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index 9baac3a8031..585ff8eeb00 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -61,38 +61,6 @@ class UserStoragesController extends StoragesController { ); } - /** - * Validate storage config - * - * @param StorageConfig $storage storage config - * - * @return DataResponse|null returns response in case of validation error - */ - protected function validate(StorageConfig $storage) { - $result = parent::validate($storage); - - if ($result !== null) { - return $result; - } - - // Verify that the mount point applies for the current user - // Prevent non-admin users from mounting local storage and other disabled backends - /** @var Backend */ - $backend = $storage->getBackend(); - if (!$backend->isPermitted(BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT)) { - return new DataResponse( - array( - 'message' => (string)$this->l10n->t('Admin-only storage backend "%s"', [ - $storage->getBackend()->getIdentifier() - ]) - ), - Http::STATUS_UNPROCESSABLE_ENTITY - ); - } - - return null; - } - /** * Return storage * @@ -218,4 +186,14 @@ class UserStoragesController extends StoragesController { public function destroy($id) { return parent::destroy($id); } + + /** + * Get the user type for this controller, used in validation + * + * @return string BackendService::USER_* constants + */ + protected function getUserType() { + return BackendService::USER_PERSONAL; + } + } diff --git a/apps/files_external/tests/controller/storagescontrollertest.php b/apps/files_external/tests/controller/storagescontrollertest.php index 5a2cff99244..c43761f3bcb 100644 --- a/apps/files_external/tests/controller/storagescontrollertest.php +++ b/apps/files_external/tests/controller/storagescontrollertest.php @@ -75,6 +75,8 @@ abstract class StoragesControllerTest extends \Test\TestCase { $authMech = $this->getAuthMechMock(); $authMech->method('validateStorage') ->willReturn(true); + $authMech->method('isPermitted') + ->willReturn(true); $backend = $this->getBackendMock(); $backend->method('validateStorage') ->willReturn(true); @@ -114,6 +116,8 @@ abstract class StoragesControllerTest extends \Test\TestCase { $authMech = $this->getAuthMechMock(); $authMech->method('validateStorage') ->willReturn(true); + $authMech->method('isPermitted') + ->willReturn(true); $backend = $this->getBackendMock(); $backend->method('validateStorage') ->willReturn(true); @@ -245,6 +249,8 @@ abstract class StoragesControllerTest extends \Test\TestCase { $authMech = $this->getAuthMechMock(); $authMech->method('validateStorage') ->willReturn(true); + $authMech->method('isPermitted') + ->willReturn(true); $backend = $this->getBackendMock(); $backend->method('validateStorage') ->willReturn(true); @@ -338,6 +344,8 @@ abstract class StoragesControllerTest extends \Test\TestCase { $authMech = $this->getAuthMechMock(); $authMech->method('validateStorage') ->will($this->returnValue($authMechValidate)); + $authMech->method('isPermitted') + ->willReturn(true); $storageConfig = new StorageConfig(); $storageConfig->setMountPoint('mount'); diff --git a/apps/files_external/tests/controller/userstoragescontrollertest.php b/apps/files_external/tests/controller/userstoragescontrollertest.php index b9668064e33..720e59cff93 100644 --- a/apps/files_external/tests/controller/userstoragescontrollertest.php +++ b/apps/files_external/tests/controller/userstoragescontrollertest.php @@ -51,7 +51,7 @@ class UserStoragesControllerTest extends StoragesControllerTest { public function testAddOrUpdateStorageDisallowedBackend() { $backend = $this->getBackendMock(); $backend->method('isPermitted') - ->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT) + ->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_CREATE) ->willReturn(false); $authMech = $this->getAuthMechMock(); -- cgit v1.2.3 From d2e3c17c0000bc0020f1ff641190452f370434de Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Fri, 28 Aug 2015 16:50:10 +0100 Subject: Introduce MODIFY permission for external storages --- .../files_external/controller/globalstoragescontroller.php | 4 ++-- apps/files_external/controller/storagescontroller.php | 7 ++++--- apps/files_external/controller/userstoragescontroller.php | 4 ++-- apps/files_external/service/backendservice.php | 3 ++- .../tests/controller/userstoragescontrollertest.php | 14 +++++++++++--- 5 files changed, 21 insertions(+), 11 deletions(-) (limited to 'apps/files_external/controller') diff --git a/apps/files_external/controller/globalstoragescontroller.php b/apps/files_external/controller/globalstoragescontroller.php index 32408420039..7d97fdbb4f4 100644 --- a/apps/files_external/controller/globalstoragescontroller.php +++ b/apps/files_external/controller/globalstoragescontroller.php @@ -98,7 +98,7 @@ class GlobalStoragesController extends StoragesController { return $newStorage; } - $response = $this->validate($newStorage); + $response = $this->validate($newStorage, BackendService::PERMISSION_CREATE); if (!empty($response)) { return $response; } @@ -154,7 +154,7 @@ class GlobalStoragesController extends StoragesController { } $storage->setId($id); - $response = $this->validate($storage); + $response = $this->validate($storage, BackendService::PERMISSION_MODIFY); if (!empty($response)) { return $response; } diff --git a/apps/files_external/controller/storagescontroller.php b/apps/files_external/controller/storagescontroller.php index d99b8b5f2c5..46202c8ba4a 100644 --- a/apps/files_external/controller/storagescontroller.php +++ b/apps/files_external/controller/storagescontroller.php @@ -125,10 +125,11 @@ abstract class StoragesController extends Controller { * Validate storage config * * @param StorageConfig $storage storage config + * @param int $permissionCheck permission to check * * @return DataResponse|null returns response in case of validation error */ - protected function validate(StorageConfig $storage) { + protected function validate(StorageConfig $storage, $permissionCheck = BackendService::PERMISSION_CREATE) { $mountPoint = $storage->getMountPoint(); if ($mountPoint === '' || $mountPoint === '/') { return new DataResponse( @@ -165,7 +166,7 @@ abstract class StoragesController extends Controller { ); } - if (!$backend->isPermitted($this->getUserType(), BackendService::PERMISSION_CREATE)) { + if (!$backend->isPermitted($this->getUserType(), $permissionCheck)) { // not permitted to use backend return new DataResponse( array( @@ -176,7 +177,7 @@ abstract class StoragesController extends Controller { Http::STATUS_UNPROCESSABLE_ENTITY ); } - if (!$authMechanism->isPermitted($this->getUserType(), BackendService::PERMISSION_CREATE)) { + if (!$authMechanism->isPermitted($this->getUserType(), $permissionCheck)) { // not permitted to use auth mechanism return new DataResponse( array( diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index 585ff8eeb00..801c9ab0aae 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -103,7 +103,7 @@ class UserStoragesController extends StoragesController { return $newStorage; } - $response = $this->validate($newStorage); + $response = $this->validate($newStorage, BackendService::PERMISSION_CREATE); if (!empty($response)) { return $response; } @@ -151,7 +151,7 @@ class UserStoragesController extends StoragesController { } $storage->setId($id); - $response = $this->validate($storage); + $response = $this->validate($storage, BackendService::PERMISSION_MODIFY); if (!empty($response)) { return $response; } diff --git a/apps/files_external/service/backendservice.php b/apps/files_external/service/backendservice.php index d9a3e316ce4..70cb9291660 100644 --- a/apps/files_external/service/backendservice.php +++ b/apps/files_external/service/backendservice.php @@ -35,8 +35,9 @@ class BackendService { const PERMISSION_NONE = 0; const PERMISSION_MOUNT = 1; const PERMISSION_CREATE = 2; + const PERMISSION_MODIFY = 4; - const PERMISSION_DEFAULT = 3; // MOUNT | CREATE + const PERMISSION_DEFAULT = 7; // MOUNT | CREATE | MODIFY /** User contants */ const USER_ADMIN = 'admin'; diff --git a/apps/files_external/tests/controller/userstoragescontrollertest.php b/apps/files_external/tests/controller/userstoragescontrollertest.php index 720e59cff93..b61174b0797 100644 --- a/apps/files_external/tests/controller/userstoragescontrollertest.php +++ b/apps/files_external/tests/controller/userstoragescontrollertest.php @@ -49,15 +49,21 @@ class UserStoragesControllerTest extends StoragesControllerTest { } public function testAddOrUpdateStorageDisallowedBackend() { - $backend = $this->getBackendMock(); - $backend->method('isPermitted') + $backend1 = $this->getBackendMock(); + $backend1->expects($this->once()) + ->method('isPermitted') ->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_CREATE) ->willReturn(false); + $backend2 = $this->getBackendMock(); + $backend2->expects($this->once()) + ->method('isPermitted') + ->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_MODIFY) + ->willReturn(false); $authMech = $this->getAuthMechMock(); $storageConfig = new StorageConfig(1); $storageConfig->setMountPoint('mount'); - $storageConfig->setBackend($backend); + $storageConfig->setBackend($backend1); $storageConfig->setAuthMechanism($authMech); $storageConfig->setBackendOptions([]); @@ -82,6 +88,8 @@ class UserStoragesControllerTest extends StoragesControllerTest { $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $storageConfig->setBackend($backend2); + $response = $this->controller->update( 1, 'mount', -- cgit v1.2.3