diff --git a/application/Espo/Classes/MassAction/User/MassDelete.php b/application/Espo/Classes/MassAction/User/MassDelete.php index cd470fc401..c6ca174c39 100644 --- a/application/Espo/Classes/MassAction/User/MassDelete.php +++ b/application/Espo/Classes/MassAction/User/MassDelete.php @@ -29,48 +29,36 @@ namespace Espo\Classes\MassAction\User; -use Espo\Core\{ - ApplicationUser, - MassAction\Actions\MassDelete as MassDeleteOriginal, - MassAction\QueryBuilder, - MassAction\Params, - MassAction\Result, - MassAction\Data, - MassAction\MassAction, - Acl, - ORM\EntityManager, - Exceptions\Forbidden, -}; +use Espo\Core\Acl; +use Espo\Core\ApplicationUser; +use Espo\Core\Exceptions\BadRequest; +use Espo\Core\Exceptions\Forbidden; +use Espo\Core\MassAction\Actions\MassDelete as MassDeleteOriginal; +use Espo\Core\MassAction\Data; +use Espo\Core\MassAction\MassAction; +use Espo\Core\MassAction\Params; +use Espo\Core\MassAction\QueryBuilder; +use Espo\Core\MassAction\Result; +use Espo\Core\ORM\EntityManager; -use Espo\{ - Entities\User, - ORM\Entity, -}; +use Espo\Entities\User; +/** + * Extended to forbid removal of own and system users. + */ class MassDelete implements MassAction { - private MassDeleteOriginal $massDeleteOriginal; - private QueryBuilder $queryBuilder; - private EntityManager $entityManager; - private Acl $acl; - private User $user; - public function __construct( - MassDeleteOriginal $massDeleteOriginal, - QueryBuilder $queryBuilder, - EntityManager $entityManager, - Acl $acl, - User $user - ) { - $this->massDeleteOriginal = $massDeleteOriginal; - $this->queryBuilder = $queryBuilder; - $this->entityManager = $entityManager; - $this->acl = $acl; - $this->user = $user; - } + private MassDeleteOriginal $massDeleteOriginal, + private QueryBuilder $queryBuilder, + private EntityManager $entityManager, + private Acl $acl, + private User $user + ) {} /** * @throws Forbidden + * @throws BadRequest */ public function process(Params $params, Data $data): Result { @@ -93,7 +81,7 @@ class MassDelete implements MassAction ->getRDBRepository(User::ENTITY_TYPE) ->clone($query) ->sth() - ->select(['id']) + ->select(['id', 'userName']) ->find(); foreach ($collection as $entity) { @@ -106,9 +94,9 @@ class MassDelete implements MassAction /** * @throws Forbidden */ - protected function checkEntity(Entity $entity): void + private function checkEntity(User $entity): void { - if ($entity->getId() === ApplicationUser::SYSTEM_USER_ID) { + if ($entity->getUserName() === ApplicationUser::SYSTEM_USER_NAME) { throw new Forbidden("Can't delete 'system' user."); } diff --git a/application/Espo/Classes/MassAction/User/MassUpdate.php b/application/Espo/Classes/MassAction/User/MassUpdate.php index 8af89198e3..2a3788b778 100644 --- a/application/Espo/Classes/MassAction/User/MassUpdate.php +++ b/application/Espo/Classes/MassAction/User/MassUpdate.php @@ -30,6 +30,7 @@ namespace Espo\Classes\MassAction\User; use Espo\Core\ApplicationUser; +use Espo\Core\Exceptions\BadRequest; use Espo\Core\MassAction\Actions\MassUpdate as MassUpdateOriginal; use Espo\Core\MassAction\QueryBuilder; use Espo\Core\MassAction\Params; @@ -44,7 +45,6 @@ use Espo\Core\Acl\Table; use Espo\Core\Exceptions\Forbidden; use Espo\Entities\User; -use Espo\ORM\Entity; use Espo\ORM\EntityManager; use Espo\Tools\MassUpdate\Data as MassUpdateData; @@ -75,6 +75,7 @@ class MassUpdate implements MassAction /** * @throws Forbidden + * @throws BadRequest */ public function process(Params $params, Data $data): Result { @@ -102,7 +103,7 @@ class MassUpdate implements MassAction ->getRDBRepository(User::ENTITY_TYPE) ->clone($query) ->sth() - ->select(['id']) + ->select(['id', 'userName']) ->find(); foreach ($collection as $entity) { @@ -131,9 +132,9 @@ class MassUpdate implements MassAction /** * @throws Forbidden */ - private function checkEntity(Entity $entity, MassUpdateData $data): void + private function checkEntity(User $entity, MassUpdateData $data): void { - if ($entity->getId() === ApplicationUser::SYSTEM_USER_ID) { + if ($entity->getUserName() === ApplicationUser::SYSTEM_USER_NAME) { throw new Forbidden("Can't update 'system' user."); } diff --git a/application/Espo/Core/ApplicationUser.php b/application/Espo/Core/ApplicationUser.php index 4421079872..833e424c63 100644 --- a/application/Espo/Core/ApplicationUser.php +++ b/application/Espo/Core/ApplicationUser.php @@ -40,6 +40,7 @@ use RuntimeException; class ApplicationUser { public const SYSTEM_USER_ID = 'system'; + public const SYSTEM_USER_NAME = 'system'; public function __construct( private Container $container, @@ -63,7 +64,7 @@ class ApplicationUser 'lastName', 'deleted', ]) - ->where(['id' => self::SYSTEM_USER_ID]) + ->where(['userName' => self::SYSTEM_USER_NAME]) ->findOne(); if (!$user) { diff --git a/application/Espo/Core/Rebuild/Actions/AddSystemUser.php b/application/Espo/Core/Rebuild/Actions/AddSystemUser.php index 20ef813d70..2a7972d911 100644 --- a/application/Espo/Core/Rebuild/Actions/AddSystemUser.php +++ b/application/Espo/Core/Rebuild/Actions/AddSystemUser.php @@ -44,22 +44,28 @@ class AddSystemUser implements RebuildAction public function process(): void { - $userId = ApplicationUser::SYSTEM_USER_ID; - $repository = $this->entityManager->getRDBRepositoryByClass(User::class); - $user = $repository->getById($userId); + $user = $repository + ->where(['userName' => ApplicationUser::SYSTEM_USER_NAME]) + ->findOne(); if ($user) { return; } + // @todo If a user with the 'system' ID already exists, delete it from DB. + /** @var array $attributes */ $attributes = $this->config->get('systemUserAttributes'); $user = $repository->getNew(); + + $user->set('id', ApplicationUser::SYSTEM_USER_ID); + $user->set('userName', ApplicationUser::SYSTEM_USER_NAME); + $user->set('type', User::TYPE_SYSTEM); $user->set($attributes); - $user->set('id', $userId); + $repository->save($user); } } diff --git a/application/Espo/Entities/Email.php b/application/Espo/Entities/Email.php index 2f1badbf17..d39aec198e 100644 --- a/application/Espo/Entities/Email.php +++ b/application/Espo/Entities/Email.php @@ -169,8 +169,15 @@ class Email extends Entity $this->setInContainer('isUsers', false); } + /** + * @deprecated As of v7.4. As the system user ID may be not constant in the future. + */ public function isManuallyArchived(): bool { + if ($this->getStatus() !== self::STATUS_ARCHIVED) { + return false; + } + return $this->getStatus() === self::STATUS_ARCHIVED && $this->get('createdById') !== ApplicationUser::SYSTEM_USER_ID; } diff --git a/application/Espo/EntryPoints/Avatar.php b/application/Espo/EntryPoints/Avatar.php index 616e0d4d76..28fe9944b8 100644 --- a/application/Espo/EntryPoints/Avatar.php +++ b/application/Espo/EntryPoints/Avatar.php @@ -100,6 +100,7 @@ class Avatar extends Image throw new BadRequest(); } + /** @var ?User $user */ $user = $this->entityManager->getEntityById(User::ENTITY_TYPE, $userId); if (!$user) { @@ -112,6 +113,8 @@ class Avatar extends Image if ($id) { $this->show($response, $id, $size, true); + + return; } $identicon = new Identicon(); @@ -136,7 +139,7 @@ class Avatar extends Image $color = $this->getColor($userId); - if ($hash === ApplicationUser::SYSTEM_USER_ID) { + if ($user->getUserName() === ApplicationUser::SYSTEM_USER_NAME) { $color = $this->metadata->get(['app', 'avatars', 'systemColor']) ?? $this->systemColor; } diff --git a/application/Espo/Resources/defaults/systemConfig.php b/application/Espo/Resources/defaults/systemConfig.php index 95320c2131..6a563aff45 100644 --- a/application/Espo/Resources/defaults/systemConfig.php +++ b/application/Espo/Resources/defaults/systemConfig.php @@ -42,10 +42,7 @@ return [ 'delete' => 'delete', ], 'systemUserAttributes' => [ - 'userName' => 'system', - 'firstName' => '', 'lastName' => 'System', - 'type' => 'system', ], 'systemItems' => [ 'systemItems', diff --git a/application/Espo/Services/Email.php b/application/Espo/Services/Email.php index 28a0a6d2f4..a143dc9bc8 100644 --- a/application/Espo/Services/Email.php +++ b/application/Espo/Services/Email.php @@ -29,6 +29,7 @@ namespace Espo\Services; +use Espo\Core\ApplicationUser; use Espo\Tools\Email\SendService; use Espo\ORM\Entity; use Espo\Entities\User; @@ -211,12 +212,11 @@ class Email extends Record $skipFilter = true; } - if ($entity->isManuallyArchived()) { + if ($this->isEmailManuallyArchived($entity)) { $skipFilter = true; - } else { - if ($entity->isAttributeChanged('dateSent')) { - $entity->set('dateSent', $entity->getFetched('dateSent')); - } + } + else if ($entity->isAttributeChanged('dateSent')) { + $entity->set('dateSent', $entity->getFetched('dateSent')); } if ($entity->getStatus() === EmailEntity::STATUS_DRAFT) { @@ -248,6 +248,30 @@ class Email extends Record } } + private function isEmailManuallyArchived(EmailEntity $email): bool + { + if ($email->getStatus() !== EmailEntity::STATUS_ARCHIVED) { + return false; + } + + $userId = $email->getCreatedBy()?->getId(); + + if (!$userId) { + return false; + } + + /** @var ?User $user */ + $user = $this->entityManager + ->getRDBRepositoryByClass(User::class) + ->getById($userId); + + if (!$user) { + return true; + } + + return $user->getUserName() !== ApplicationUser::SYSTEM_USER_NAME; + } + private function clearEntityForUpdate(EmailEntity $email): void { $fieldDefsList = $this->entityManager diff --git a/application/Espo/Services/User.php b/application/Espo/Services/User.php index 468ff6fe64..5e7ba03e0a 100644 --- a/application/Espo/Services/User.php +++ b/application/Espo/Services/User.php @@ -89,18 +89,18 @@ class User extends Record implements */ public function getEntity(string $id): ?Entity { - if ($id === ApplicationUser::SYSTEM_USER_ID) { - throw new Forbidden(); - } - /** @var ?UserEntity $entity */ $entity = parent::getEntity($id); - if ($entity && $entity->isSuperAdmin() && !$this->user->isSuperAdmin()) { + if (!$entity) { + return null; + } + + if ($entity->isSuperAdmin() && !$this->user->isSuperAdmin()) { throw new Forbidden(); } - if ($entity && $entity->isSystem()) { + if ($entity->isSystem()) { throw new Forbidden(); } @@ -176,10 +176,6 @@ class User extends Record implements public function update(string $id, stdClass $data, UpdateParams $params): Entity { - if ($id === ApplicationUser::SYSTEM_USER_ID) { - throw new Forbidden(); - } - $newPassword = null; if (property_exists($data, 'password')) { @@ -192,7 +188,7 @@ class User extends Record implements $data->password = $this->hashPassword($data->password); } - if ($id == $this->user->getId()) { + if ($id === $this->user->getId()) { unset($data->isActive); unset($data->isPortalUser); unset($data->type); @@ -207,7 +203,7 @@ class User extends Record implements $this->sendPassword($user, $newPassword); } } - catch (Exception $e) {} + catch (Exception) {} } return $user; @@ -281,43 +277,41 @@ class User extends Record implements } /** + * @param UserEntity $entity * @throws Forbidden */ protected function beforeCreateEntity(Entity $entity, $data) { - /** @var UserEntity $entity */ + $userLimit = $this->config->get('userLimit'); if ( - $this->config->get('userLimit') && + $userLimit && !$this->user->isSuperAdmin() && !$entity->isPortal() && !$entity->isApi() ) { $userCount = $this->getInternalUserCount(); - if ($userCount >= $this->config->get('userLimit')) { - throw new Forbidden( - 'User limit '.$this->config->get('userLimit').' is reached.' - ); + if ($userCount >= $userLimit) { + throw new Forbidden("User limit {$userLimit} is reached."); } } + + $portalUserLimit = $this->config->get('portalUserLimit'); + if ( - $this->config->get('portalUserLimit') && + $portalUserLimit && !$this->user->isSuperAdmin() && $entity->isPortal() ) { $portalUserCount = $this->getPortalUserCount(); - if ($portalUserCount >= $this->config->get('portalUserLimit')) { - throw new Forbidden( - 'Portal user limit ' . $this->config->get('portalUserLimit').' is reached.' - ); + if ($portalUserCount >= $portalUserLimit) { + throw new Forbidden("Portal user limit {$portalUserLimit} is reached."); } } if ($entity->isApi()) { - $apiKey = Util::generateApiKey(); - - $entity->set('apiKey', $apiKey); + $entity->set('apiKey', Util::generateApiKey()); if ($entity->getAuthMethod() === Hmac::NAME) { $secretKey = Util::generateSecretKey(); @@ -326,25 +320,27 @@ class User extends Record implements } } - if (!$entity->isSuperAdmin()) { - if ( - $entity->getType() && - !in_array($entity->getType(), $this->allowedUserTypeList) - ) { - throw new Forbidden(); - } + if ( + !$entity->isSuperAdmin() && + $entity->getType() && + !in_array($entity->getType(), $this->allowedUserTypeList) + ) { + throw new Forbidden(); } } /** + * @param UserEntity $entity * @throws Forbidden */ protected function beforeUpdateEntity(Entity $entity, $data) { - /** @var UserEntity $entity */ + $userLimit = $this->config->get('userLimit'); - if ($this->config->get('userLimit') && !$this->user->isSuperAdmin()) { - if ( + if ( + $userLimit && + !$this->user->isSuperAdmin() && + ( ( $entity->isActive() && $entity->isAttributeChanged('isActive') && @@ -364,17 +360,21 @@ class User extends Record implements $entity->getFetched('type') == UserEntity::TYPE_API ) ) - ) { - $userCount = $this->getInternalUserCount(); + ) + ) { + $userCount = $this->getInternalUserCount(); - if ($userCount >= $this->config->get('userLimit')) { - throw new Forbidden('User limit '.$this->config->get('userLimit').' is reached.'); - } + if ($userCount >= $userLimit) { + throw new Forbidden("User limit {$userLimit} is reached."); } } - if ($this->config->get('portalUserLimit') && !$this->user->isSuperAdmin()) { - if ( + $portalUserLimit = $this->config->get('portalUserLimit'); + + if ( + $portalUserLimit && + !$this->user->isSuperAdmin() && + ( ( $entity->isActive() && $entity->isAttributeChanged('isActive') && @@ -384,47 +384,39 @@ class User extends Record implements $entity->isPortal() && $entity->isAttributeChanged('type') ) - ) { - $portalUserCount = $this->getPortalUserCount(); + ) + ) { + $portalUserCount = $this->getPortalUserCount(); - if ($portalUserCount >= $this->config->get('portalUserLimit')) { - throw new Forbidden( - 'Portal user limit '. $this->config->get('portalUserLimit').' is reached.' - ); - } + if ($portalUserCount >= $portalUserLimit) { + throw new Forbidden("Portal user limit {$portalUserLimit} is reached."); } } - if ($entity->isApi()) { - if ( - $entity->isAttributeChanged('authMethod') && - $entity->getAuthMethod() === Hmac::NAME - ) { - $secretKey = Util::generateSecretKey(); + if ( + $entity->isApi() && + $entity->isAttributeChanged('authMethod') && + $entity->getAuthMethod() === Hmac::NAME + ) { + $secretKey = Util::generateSecretKey(); - $entity->set('secretKey', $secretKey); - } + $entity->set('secretKey', $secretKey); } - if (!$entity->isSuperAdmin()) { - if ( - $entity->isAttributeChanged('type') && - $entity->getType() && - !in_array($entity->getType(), $this->allowedUserTypeList) - ) { - throw new Forbidden(); - } + if ( + !$entity->isSuperAdmin() && + $entity->isAttributeChanged('type') && + $entity->getType() && + !in_array($entity->getType(), $this->allowedUserTypeList) + ) { + throw new Forbidden("Can't change type."); } } public function delete(string $id, DeleteParams $params): void { - if ($id === ApplicationUser::SYSTEM_USER_ID) { - throw new Forbidden(); - } - - if ($id == $this->user->getId()) { - throw new Forbidden(); + if ($id === $this->user->getId()) { + throw new Forbidden("Can't delete own user."); } parent::delete($id, $params); diff --git a/application/Espo/Tools/MassUpdate/MassUpdate.php b/application/Espo/Tools/MassUpdate/MassUpdate.php index 6ba0c88ec0..cf6a3ded6a 100644 --- a/application/Espo/Tools/MassUpdate/MassUpdate.php +++ b/application/Espo/Tools/MassUpdate/MassUpdate.php @@ -30,6 +30,8 @@ namespace Espo\Tools\MassUpdate; use Espo\Core\ApplicationUser; +use Espo\Core\Exceptions\BadRequest; +use Espo\Core\Exceptions\Forbidden; use Espo\Core\Exceptions\NotFound; use Espo\Core\MassAction\Params; use Espo\Core\MassAction\Result; @@ -45,29 +47,31 @@ use RuntimeException; */ class MassUpdate { - private MassActionFactory $massActionFactory; - private EntityManager $entityManager; - private const ACTION = 'massUpdate'; - private const DEFAULT_USER_ID = ApplicationUser::SYSTEM_USER_ID; - public function __construct(MassActionFactory $massActionFactory, EntityManager $entityManager) - { - $this->massActionFactory = $massActionFactory; - $this->entityManager = $entityManager; - } + public function __construct( + private MassActionFactory $massActionFactory, + private EntityManager $entityManager + ) {} /** + * Process. + * * @param ?User $user Under what user to perform mass-update. If not specified, the system user will be used. * Access control is applied for the user. * @throws NotFound + * @throws Forbidden + * @throws BadRequest */ public function process(Params $params, Data $data, ?User $user = null): Result { $entityType = $params->getEntityType(); if (!$user) { - $user = $this->entityManager->getEntityById(User::ENTITY_TYPE, self::DEFAULT_USER_ID); + $user = $this->entityManager + ->getRDBRepositoryByClass(User::class) + ->where(['userName' => ApplicationUser::SYSTEM_USER_NAME]) + ->findOne(); } if (!$user) {