From cb55cec3f0e5a153c4d869565614ee8515a875d2 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Wed, 24 Jun 2020 12:48:08 +0300 Subject: [PATCH] acl manager code improvements --- application/Espo/Core/Acl.php | 7 +- .../Espo/Core/Acl/GlobalRestricton.php | 14 +-- application/Espo/Core/Acl/Table.php | 88 +++++++++---------- application/Espo/Core/AclManager.php | 61 ++++++------- application/Espo/Core/AclPortal/Table.php | 26 +++--- application/Espo/Core/Loaders/AclManager.php | 23 +++-- .../Loaders/PortalAclManagerContainer.php | 49 +++++++++++ application/Espo/Core/Portal/AclManager.php | 28 +++--- .../Espo/Core/Portal/AclManagerContainer.php | 69 +++++++++++++++ application/Espo/Core/Portal/Container.php | 2 + .../Espo/Core/Portal/Loaders/AclManager.php | 25 ++++-- .../Portal/Loaders/InternalAclManager.php | 26 ++++-- application/Espo/Services/Stream.php | 6 +- 13 files changed, 291 insertions(+), 133 deletions(-) create mode 100644 application/Espo/Core/Loaders/PortalAclManagerContainer.php create mode 100644 application/Espo/Core/Portal/AclManagerContainer.php diff --git a/application/Espo/Core/Acl.php b/application/Espo/Core/Acl.php index 1d75388dd7..087fdb6950 100644 --- a/application/Espo/Core/Acl.php +++ b/application/Espo/Core/Acl.php @@ -29,9 +29,12 @@ namespace Espo\Core; -use \Espo\ORM\Entity; -use \Espo\Entities\User; +use Espo\ORM\Entity; +use Espo\Entities\User; +/** + * A wrapper for AclManager. To check access for a current user. + */ class Acl { private $user; diff --git a/application/Espo/Core/Acl/GlobalRestricton.php b/application/Espo/Core/Acl/GlobalRestricton.php index 2934a75952..3730199fb5 100644 --- a/application/Espo/Core/Acl/GlobalRestricton.php +++ b/application/Espo/Core/Acl/GlobalRestricton.php @@ -29,6 +29,12 @@ namespace Espo\Core\Acl; +use Espo\Core\{ + Utils\Metadata, + Utils\File\Manager as FileManager, + Utils\FieldManagerUtil, +}; + class GlobalRestricton { protected $fieldTypeList = [ @@ -58,12 +64,8 @@ class GlobalRestricton private $data; public function __construct( - \Espo\Core\Utils\Metadata $metadata, - \Espo\Core\Utils\File\Manager $fileManager, - \Espo\Core\Utils\FieldManagerUtil $fieldManagerUtil, - bool $useCache = true - ) - { + Metadata $metadata, FileManager $fileManager, FieldManagerUtil $fieldManagerUtil, bool $useCache = true + ) { $this->metadata = $metadata; $this->fileManager = $fileManager; $this->fieldManagerUtil = $fieldManagerUtil; diff --git a/application/Espo/Core/Acl/Table.php b/application/Espo/Core/Acl/Table.php index 26e0276971..a9acecef7d 100644 --- a/application/Espo/Core/Acl/Table.php +++ b/application/Espo/Core/Acl/Table.php @@ -29,15 +29,15 @@ namespace Espo\Core\Acl; -use \Espo\Core\Exceptions\Error; +use Espo\Core\Exceptions\Error; -use \Espo\ORM\Entity; -use \Espo\Entities\User; +use Espo\ORM\Entity; +use Espo\Entities\User; -use \Espo\Core\Utils\Config; -use \Espo\Core\Utils\Metadata; -use \Espo\Core\Utils\FieldManagerUtil; -use \Espo\Core\Utils\File\Manager as FileManager; +use Espo\Core\Utils\Config; +use Espo\Core\Utils\Metadata; +use Espo\Core\Utils\FieldManagerUtil; +use Espo\Core\Utils\File\Manager as FileManager; class Table { @@ -59,7 +59,7 @@ class Table protected $fieldLevelList = ['yes', 'no']; - protected $valuePermissionHighestLevels = array(); + protected $valuePermissionHighestLevels = []; protected $valuePermissionList = []; @@ -69,16 +69,18 @@ class Table private $fieldManager; - protected $forbiddenAttributesCache = array(); + protected $forbiddenAttributesCache = []; - protected $forbiddenFieldsCache = array(); + protected $forbiddenFieldsCache = []; protected $isStrictModeForced = false; protected $isStrictMode = false; - public function __construct(User $user, Config $config = null, FileManager $fileManager = null, Metadata $metadata = null, FieldManagerUtil $fieldManager = null) - { + public function __construct( + User $user, Config $config = null, FileManager $fileManager = null, Metadata $metadata = null, + FieldManagerUtil $fieldManagerUtil = null + ) { $this->data = (object) [ 'table' => (object) [], 'fieldTable' => (object) [], @@ -95,8 +97,8 @@ class Table $this->metadata = $metadata; - if ($fieldManager) { - $this->fieldManager = $fieldManager; + if ($fieldManagerUtil) { + $this->fieldManager = $fieldManagerUtil; } if (!$this->user->isFetched()) { @@ -220,7 +222,6 @@ class Table $this->applyDisabled($aclTable, $fieldTable); $this->applyMandatory($aclTable, $fieldTable); $this->applyAdditional($aclTable, $fieldTable, $valuePermissionLists); - $this->applyReadOnlyFields($fieldTable); } else { $aclTable = (object) []; foreach ($this->getScopeList() as $scope) { @@ -260,7 +261,10 @@ class Table $permissionsDefaultsGroupName = 'permissionsStrictDefaults'; } foreach ($this->valuePermissionList as $permission) { - $this->data->$permission = $this->mergeValueList($valuePermissionLists->$permission, $this->metadata->get(['app', $this->type, $permissionsDefaultsGroupName, $permission, 'yes'])); + $this->data->$permission = $this->mergeValueList( + $valuePermissionLists->$permission, + $this->metadata->get(['app', $this->type, $permissionsDefaultsGroupName, $permission, 'yes']) + ); if ($this->metadata->get('app.'.$this->type.'.mandatory.' . $permission)) { $this->data->$permission = $this->metadata->get('app.'.$this->type.'.mandatory.' . $permission); } @@ -312,7 +316,10 @@ class Table $fieldTableQuickAccess = $this->data->fieldTableQuickAccess; - if (!isset($fieldTableQuickAccess->$scope) || !isset($fieldTableQuickAccess->$scope->attributes) || !isset($fieldTableQuickAccess->$scope->attributes->$action)) { + if ( + !isset($fieldTableQuickAccess->$scope) || !isset($fieldTableQuickAccess->$scope->attributes) || + !isset($fieldTableQuickAccess->$scope->attributes->$action) + ) { $this->forbiddenAttributesCache[$key] = []; return []; } @@ -348,7 +355,10 @@ class Table $fieldTableQuickAccess = $this->data->fieldTableQuickAccess; - if (!isset($fieldTableQuickAccess->$scope) || !isset($fieldTableQuickAccess->$scope->fields) || !isset($fieldTableQuickAccess->$scope->fields->$action)) { + if ( + !isset($fieldTableQuickAccess->$scope) || !isset($fieldTableQuickAccess->$scope->fields) || + !isset($fieldTableQuickAccess->$scope->fields->$action) + ) { $this->forbiddenFieldsCache[$key] = []; return []; } @@ -446,7 +456,8 @@ class Table $fieldList = array_keys($this->getMetadata()->get("entityDefs.{$scope}.fields", [])); - $defaultScopeFieldData = $this->metadata->get('app.'.$this->type.'.'.$defaultsGroupName.'.scopeFieldLevel.' . $scope, []); + $defaultScopeFieldData = $this->metadata->get( + 'app.'.$this->type.'.'.$defaultsGroupName.'.scopeFieldLevel.' . $scope, []); foreach (array_merge($defaultFieldData, $defaultScopeFieldData) as $field => $f) { if (!in_array($field, $fieldList)) continue; @@ -480,7 +491,10 @@ class Table if ($this->isStrictMode) { $paramDefaultsName = 'scopeLevelTypesStrictDefaults'; } - $defaultValue = $this->metadata->get(['app', $this->type, $paramDefaultsName, $aclType], $this->metadata->get(['app', $this->type, $paramDefaultsName, 'record'])); + $defaultValue = $this->metadata->get( + ['app', $this->type, $paramDefaultsName, $aclType], + $this->metadata->get(['app', $this->type, $paramDefaultsName, 'record']) + ); if (is_array($defaultValue)) { $defaultValue = (object) $defaultValue; } @@ -651,7 +665,9 @@ class Table if (!isset($data->$scope->$action)) { $data->$scope->$action = $level; } else { - if (array_search($data->$scope->$action, $this->levelList) > array_search($level, $this->levelList)) { + if ( + array_search($data->$scope->$action, $this->levelList) > array_search($level, $this->levelList) + ) { $data->$scope->$action = $level; } } @@ -709,7 +725,12 @@ class Table if (!isset($data->$scope->$field->$action)) { $data->$scope->$field->$action = $level; } else { - if (array_search($data->$scope->$field->$action, $this->fieldLevelList) > array_search($level, $this->fieldLevelList)) { + if ( + array_search( + $data->$scope->$field->$action, + $this->fieldLevelList + ) > array_search($level, $this->fieldLevelList) + ) { $data->$scope->$field->$action = $level; } } @@ -726,27 +747,4 @@ class Table $this->fileManager->putPhpContents($this->cacheFilePath, $this->data, true); } - protected function applyReadOnlyFields(&$fieldTable) - { - // TODO Enable in 5.4.0 - return; - $scopeList = $this->getScopeWithAclList(); - foreach ($scopeList as $scope) { - if (!property_exists($fieldTable, $scope)) continue; - $fieldList = array_keys($this->getMetadata()->get(['entityDefs', $scope, 'fields'], [])); - foreach ($fieldList as $field) { - if ($this->getMetadata()->get(['entityDefs', $scope, 'fields', $field, 'readOnly'])) { - if (property_exists($fieldTable->$scope, $field)) { - $fieldTable->$scope->$field->edit = 'no'; - } else { - $fieldTable->$scope->$field = (object) []; - foreach ($this->fieldActionList as $action) { - $fieldTable->$scope->$field->$action = 'yes'; - } - $fieldTable->$scope->$field->edit = 'no'; - } - } - } - } - } } diff --git a/application/Espo/Core/AclManager.php b/application/Espo/Core/AclManager.php index e579eb9efb..7dee2d3295 100644 --- a/application/Espo/Core/AclManager.php +++ b/application/Espo/Core/AclManager.php @@ -37,12 +37,17 @@ use Espo\Core\Utils\Util; use Espo\Core\Acl\GlobalRestricton; +use Espo\Core\{ + Utils\ClassFinder, + Utils\Config, + ORM\EntityManager, +}; + +/** + * Used to check access for a specific user. + */ class AclManager { - private $container; - - private $metadata; - private $implementationHashMap = []; private $tableHashMap = []; @@ -55,33 +60,28 @@ class AclManager protected $globalRestricton; - public function __construct(Container $container) - { - $this->container = $container; - $this->metadata = $container->get('metadata'); + protected $injectableFactory; + protected $classFinder; + protected $config; + protected $entityManager; - $this->globalRestricton = new GlobalRestricton( - $container->get('metadata'), - $container->get('fileManager'), - $container->get('fieldManagerUtil'), - $container->get('config')->get('useCache') - ); - } + public function __construct( + InjectableFactory $injectableFactory, ClassFinder $classFinder, Config $config, EntityManager $entityManager + ) { + $this->injectableFactory = $injectableFactory; + $this->classFinder = $classFinder; + $this->config = $config; + $this->entityManager = $entityManager; - protected function getContainer() - { - return $this->container; - } - - protected function getMetadata() - { - return $this->metadata; + $this->globalRestricton = $this->injectableFactory->createWith(GlobalRestricton::class, [ + 'useCache' => $config->get('useCache'), + ]); } public function getImplementation(string $scope) { if (empty($this->implementationHashMap[$scope])) { - $className = $this->getContainer()->get('classFinder')->find('Acl', $scope); + $className = $this->classFinder->find('Acl', $scope); if (!$className) { $className = $this->baseImplementationClassName; @@ -91,7 +91,7 @@ class AclManager throw new Error("{$className} does not exist."); } - $acl = $this->getContainer()->get('injectableFactory')->createWith($className, [ + $acl = $this->injectableFactory->createWith($className, [ 'scope' => $scope, ]); @@ -109,12 +109,9 @@ class AclManager } if (empty($this->tableHashMap[$key])) { - $config = $this->getContainer()->get('config'); - $fileManager = $this->getContainer()->get('fileManager'); - $metadata = $this->getContainer()->get('metadata'); - $fieldManager = $this->getContainer()->get('fieldManagerUtil'); - - $this->tableHashMap[$key] = new $this->tableClassName($user, $config, $fileManager, $metadata, $fieldManager); + $this->tableHashMap[$key] = $this->injectableFactory->createWith($this->tableClassName, [ + 'user' => $user, + ]); } return $this->tableHashMap[$key]; @@ -342,7 +339,7 @@ class AclManager if ($permission === 'team') { $teamIdList = $user->getLinkMultipleIdList('teams'); - if (!$this->getContainer()->get('entityManager')->getRepository('User')->checkBelongsToAnyOfTeams($userId, $teamIdList)) { + if (!$this->entityManager->getRepository('User')->checkBelongsToAnyOfTeams($userId, $teamIdList)) { return false; } } diff --git a/application/Espo/Core/AclPortal/Table.php b/application/Espo/Core/AclPortal/Table.php index 7c1d908adf..9538e8e683 100644 --- a/application/Espo/Core/AclPortal/Table.php +++ b/application/Espo/Core/AclPortal/Table.php @@ -29,16 +29,16 @@ namespace Espo\Core\AclPortal; -use \Espo\Core\Exceptions\Error; +use Espo\Core\Exceptions\Error; -use \Espo\ORM\Entity; -use \Espo\Entities\User; -use \Espo\Entities\Portal; +use Espo\ORM\Entity; +use Espo\Entities\User; +use Espo\Entities\Portal; -use \Espo\Core\Utils\Config; -use \Espo\Core\Utils\Metadata; -use \Espo\Core\Utils\FieldManagerUtil; -use \Espo\Core\Utils\File\Manager as FileManager; +use Espo\Core\Utils\Config; +use Espo\Core\Utils\Metadata; +use Espo\Core\Utils\FieldManagerUtil; +use Espo\Core\Utils\File\Manager as FileManager; class Table extends \Espo\Core\Acl\Table { @@ -52,13 +52,16 @@ class Table extends \Espo\Core\Acl\Table protected $isStrictModeForced = true; - public function __construct(User $user, Portal $portal, Config $config = null, FileManager $fileManager = null, Metadata $metadata = null, FieldManagerUtil $fieldManager = null) - { + public function __construct( + User $user, Portal $portal, Config $config = null, FileManager $fileManager = null, + Metadata $metadata = null, FieldManagerUtil $fieldManagerUtil = null + ) { if (empty($portal)) { throw new Error("No portal was passed to AclPortal\\Table constructor."); } $this->portal = $portal; - parent::__construct($user, $config, $fileManager, $metadata, $fieldManager); + + parent::__construct($user, $config, $fileManager, $metadata, $fieldManagerUtil); } protected function getPortal() @@ -132,4 +135,3 @@ class Table extends \Espo\Core\Acl\Table { } } - diff --git a/application/Espo/Core/Loaders/AclManager.php b/application/Espo/Core/Loaders/AclManager.php index edc235a5a6..450601414a 100644 --- a/application/Espo/Core/Loaders/AclManager.php +++ b/application/Espo/Core/Loaders/AclManager.php @@ -30,20 +30,31 @@ namespace Espo\Core\Loaders; use Espo\Core\{ - Container, + InjectableFactory, + Utils\ClassFinder, + Utils\Config, + ORM\EntityManager, + AclManager as AclManagerService, }; class AclManager implements Loader { - protected $container; + protected $injectableFactory; + protected $classFinder; + protected $config; + protected $entityManager; - public function __construct(Container $container) - { - $this->container = $container; + public function __construct( + InjectableFactory $injectableFactory, ClassFinder $classFinder, Config $config, EntityManager $entityManager + ) { + $this->injectableFactory = $injectableFactory; + $this->classFinder = $classFinder; + $this->config = $config; + $this->entityManager = $entityManager; } public function load() { - return new \Espo\Core\AclManager($this->container); + return new AclManagerService($this->injectableFactory, $this->classFinder, $this->config, $this->entityManager); } } diff --git a/application/Espo/Core/Loaders/PortalAclManagerContainer.php b/application/Espo/Core/Loaders/PortalAclManagerContainer.php new file mode 100644 index 0000000000..cf332b9962 --- /dev/null +++ b/application/Espo/Core/Loaders/PortalAclManagerContainer.php @@ -0,0 +1,49 @@ +injectableFactory = $injectableFactory; + } + + public function load() + { + return new PortalAclManagerContainerService($this->injectableFactory); + } +} diff --git a/application/Espo/Core/Portal/AclManager.php b/application/Espo/Core/Portal/AclManager.php index 542f3f10c3..19bcc7b1ef 100644 --- a/application/Espo/Core/Portal/AclManager.php +++ b/application/Espo/Core/Portal/AclManager.php @@ -33,6 +33,8 @@ use Espo\ORM\Entity; use Espo\Entities\User; use Espo\Core\Utils\Util; +use Espo\Entities\Portal; + class AclManager extends \Espo\Core\AclManager { protected $tableClassName = 'Espo\\Core\\AclPortal\\Table'; @@ -48,7 +50,7 @@ class AclManager extends \Espo\Core\AclManager public function getImplementation(string $scope) { if (empty($this->implementationHashMap[$scope])) { - $className = $this->getContainer()->get('classFinder')->find('AclPortal', $scope); + $className = $this->classFinder->find('AclPortal', $scope); if (!$className) { $className = $this->baseImplementationClassName; @@ -58,7 +60,7 @@ class AclManager extends \Espo\Core\AclManager throw new Error("{$className} does not exist."); } - $acl = $this->getContainer()->get('injectableFactory')->createWith($className, [ + $acl = $this->injectableFactory->createWith($className, [ 'scope' => $scope, ]); @@ -78,17 +80,14 @@ class AclManager extends \Espo\Core\AclManager return $this->mainManager; } - public function setPortal($portal) + public function setPortal(Portal $portal) { $this->portal = $portal; } - protected function getPortal() + protected function getPortal() : Portal { - if ($this->portal) { - return $this->portal; - } - return $this->getContainer()->get('portal'); + return $this->portal ?? null; } protected function getTable(User $user) @@ -99,14 +98,10 @@ class AclManager extends \Espo\Core\AclManager } if (empty($this->tableHashMap[$key])) { - $config = $this->getContainer()->get('config'); - $fileManager = $this->getContainer()->get('fileManager'); - $metadata = $this->getContainer()->get('metadata'); - $fieldManager = $this->getContainer()->get('fieldManagerUtil'); - $portal = $this->getPortal(); - - $this->tableHashMap[$key] = new $this->tableClassName( - $user, $portal, $config, $fileManager, $metadata, $fieldManager); + $this->tableHashMap[$key] = $this->injectableFactory->createWith($this->tableClassName, [ + 'user' => $user, + 'portal' => $this->getPortal(), + ]); } return $this->tableHashMap[$key]; @@ -259,5 +254,4 @@ class AclManager extends \Espo\Core\AclManager { return !$user->isPortal(); } - } diff --git a/application/Espo/Core/Portal/AclManagerContainer.php b/application/Espo/Core/Portal/AclManagerContainer.php new file mode 100644 index 0000000000..ecb3bd325c --- /dev/null +++ b/application/Espo/Core/Portal/AclManagerContainer.php @@ -0,0 +1,69 @@ +injectableFactory = $injectableFactory; + } + + public function get(Portal $portal) + { + $id = $portal->id; + + if (!$id) throw new Error("AclManagerContainer: portal should have ID."); + + if (!isset($this->data[$id])) { + $aclManager = $this->injectableFactory->create(AclManager::class); + $aclManager->setPortal($portal); + + $this->data[$id] = $aclManager; + } + + return $this->data[$id]; + } +} diff --git a/application/Espo/Core/Portal/Container.php b/application/Espo/Core/Portal/Container.php index e7e40622ab..d7113f35ce 100644 --- a/application/Espo/Core/Portal/Container.php +++ b/application/Espo/Core/Portal/Container.php @@ -75,5 +75,7 @@ class Container extends BaseContainer foreach ($data as $attribute => $value) { $this->get('config')->set($attribute, $value, true); } + + $this->get('aclManager')->setPortal($portal); } } diff --git a/application/Espo/Core/Portal/Loaders/AclManager.php b/application/Espo/Core/Portal/Loaders/AclManager.php index 5a9607cb54..55b6cb8ea0 100644 --- a/application/Espo/Core/Portal/Loaders/AclManager.php +++ b/application/Espo/Core/Portal/Loaders/AclManager.php @@ -30,22 +30,37 @@ namespace Espo\Core\Portal\Loaders; use Espo\Core\{ - Container, + InjectableFactory, + Utils\ClassFinder, + Utils\Config, + ORM\EntityManager, AclManager as InternalAclManager, Loaders\Loader, + Portal\AclManager as PortalAclManager, }; class AclManager implements Loader { - public function __construct(Container $container, InternalAclManager $internalAclManager) - { - $this->container = $container; + protected $injectableFactory; + protected $classFinder; + protected $config; + protected $entityManager; + protected $internalAclManager; + + public function __construct( + InjectableFactory $injectableFactory, ClassFinder $classFinder, Config $config, EntityManager $entityManager, + InternalAclManager $internalAclManager + ) { + $this->injectableFactory = $injectableFactory; + $this->classFinder = $classFinder; + $this->config = $config; + $this->entityManager = $entityManager; $this->internalAclManager = $internalAclManager; } public function load() { - $aclMenager = new \Espo\Core\Portal\AclManager($this->container); + $aclMenager = new PortalAclManager($this->injectableFactory, $this->classFinder, $this->config, $this->entityManager); $aclMenager->setMainManager($this->internalAclManager); return $aclMenager; } diff --git a/application/Espo/Core/Portal/Loaders/InternalAclManager.php b/application/Espo/Core/Portal/Loaders/InternalAclManager.php index e8c498ed56..f1ad283f7e 100644 --- a/application/Espo/Core/Portal/Loaders/InternalAclManager.php +++ b/application/Espo/Core/Portal/Loaders/InternalAclManager.php @@ -30,19 +30,35 @@ namespace Espo\Core\Portal\Loaders; use Espo\Core\{ - Container, Loaders\Loader, }; +use Espo\Core\{ + InjectableFactory, + Utils\ClassFinder, + Utils\Config, + ORM\EntityManager, + AclManager as InternalAclManagerService, +}; + class InternalAclManager implements Loader { - public function __construct(Container $container) - { - $this->container = $container; + protected $injectableFactory; + protected $classFinder; + protected $config; + protected $entityManager; + + public function __construct( + InjectableFactory $injectableFactory, ClassFinder $classFinder, Config $config, EntityManager $entityManager + ) { + $this->injectableFactory = $injectableFactory; + $this->classFinder = $classFinder; + $this->config = $config; + $this->entityManager = $entityManager; } public function load() { - return new \Espo\Core\AclManager($this->container); + return new InternalAclManagerService($this->injectableFactory, $this->classFinder, $this->config, $this->entityManager); } } diff --git a/application/Espo/Services/Stream.php b/application/Espo/Services/Stream.php index 0e9e787b29..e1043ff207 100644 --- a/application/Espo/Services/Stream.php +++ b/application/Espo/Services/Stream.php @@ -55,7 +55,8 @@ class Stream extends \Espo\Core\Services\Base 'metadata', 'acl', 'aclManager', - 'container' + 'container', + 'portalAclManagerContainer', ]); } @@ -1725,10 +1726,9 @@ class Stream extends \Espo\Core\Services\Base $aclManager = $this->getAclManager(); if ($user->isPortal() && !$this->getUser()->isPortal()) { - $aclManager = new \Espo\Core\Portal\AclManager($this->getInjection('container')); $portals = $user->get('portals'); if (count($portals)) { - $aclManager->setPortal($portals[0]); + $aclManager = $this->getInjection('portalAclManagerContainer')->get($portals[0]); } else { $aclManager = null; }