From 6a963f39d474998e7cf53eedd40ebd9bc372c97f Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Fri, 21 Apr 2023 13:08:06 +0300 Subject: [PATCH] username unique index --- application/Espo/Hooks/User/DeleteId.php | 61 +++++++++++++++++++ application/Espo/Repositories/User.php | 56 +---------------- .../Resources/metadata/entityAcl/User.json | 3 + .../Resources/metadata/entityDefs/User.json | 18 ++++++ application/Espo/Services/User.php | 58 ++++++++++++++++++ upgrades/7.5/data.json | 2 + upgrades/7.5/scripts/BeforeUpgrade.php | 51 ++++++++++++++++ 7 files changed, 194 insertions(+), 55 deletions(-) create mode 100644 application/Espo/Hooks/User/DeleteId.php create mode 100644 upgrades/7.5/data.json create mode 100644 upgrades/7.5/scripts/BeforeUpgrade.php diff --git a/application/Espo/Hooks/User/DeleteId.php b/application/Espo/Hooks/User/DeleteId.php new file mode 100644 index 0000000000..3735d12932 --- /dev/null +++ b/application/Espo/Hooks/User/DeleteId.php @@ -0,0 +1,61 @@ + + * @implements BeforeSave + */ +class DeleteId implements BeforeRemove, BeforeSave +{ + public function beforeRemove(Entity $entity, RemoveOptions $options): void + { + $entity->set('deleteId', Util::generateId()); + } + + public function beforeSave(Entity $entity, SaveOptions $options): void + { + if (!$entity->isAttributeChanged('deleted')) { + return; + } + + $deleteId = $entity->get('deleted') ? Util::generateId() : '0'; + + $entity->set('deleteId', $deleteId); + } +} diff --git a/application/Espo/Repositories/User.php b/application/Espo/Repositories/User.php index f59013f150..072f8a59e3 100644 --- a/application/Espo/Repositories/User.php +++ b/application/Espo/Repositories/User.php @@ -30,18 +30,13 @@ namespace Espo\Repositories; use Espo\ORM\Entity; - -use Espo\Core\Exceptions\Error; -use Espo\Core\Exceptions\Conflict; use Espo\Core\Repositories\Database; -use Espo\Core\Utils\Json; - use Espo\Repositories\UserData as UserDataRepository; use Espo\Entities\UserData; use Espo\Entities\User as UserEntity; /** - * @extends Database<\Espo\Entities\User> + * @extends Database */ class User extends Database { @@ -51,8 +46,6 @@ class User extends Database * @param UserEntity $entity * @param array $options * @return void - * @throws Conflict - * @throws Error */ protected function beforeSave(Entity $entity, array $options = []) { @@ -91,53 +84,6 @@ class User extends Database $entity->set('defaultTeamId', null); $entity->set('defaultTeamName', null); } - - if ($entity->isNew()) { - $userName = $entity->getUserName(); - - if (empty($userName)) { - throw new Error("Username can't be empty."); - } - - $this->entityManager->getLocker()->lockExclusive($this->entityType); - - $user = $this - ->select(['id']) - ->where([ - 'userName' => $userName, - ]) - ->findOne(); - - if ($user) { - $this->entityManager->getLocker()->rollback(); - - throw new Conflict(Json::encode(['reason' => 'userNameExists'])); - } - } else { - if ($entity->isAttributeChanged('userName')) { - $userName = $entity->getUserName(); - - if (empty($userName)) { - throw new Error("Username can't be empty."); - } - - $this->entityManager->getLocker()->lockExclusive($this->entityType); - - $user = $this - ->select(['id']) - ->where([ - 'userName' => $userName, - 'id!=' => $entity->getId(), - ]) - ->findOne(); - - if ($user) { - $this->entityManager->getLocker()->rollback(); - - throw new Conflict(Json::encode(['reason' => 'userNameExists'])); - } - } - } } /** diff --git a/application/Espo/Resources/metadata/entityAcl/User.json b/application/Espo/Resources/metadata/entityAcl/User.json index 34b8da2f5d..bcb7f811b9 100644 --- a/application/Espo/Resources/metadata/entityAcl/User.json +++ b/application/Espo/Resources/metadata/entityAcl/User.json @@ -61,6 +61,9 @@ }, "userData": { "forbidden": true + }, + "deleteId": { + "forbidden": true } }, "links": { diff --git a/application/Espo/Resources/metadata/entityDefs/User.json b/application/Espo/Resources/metadata/entityDefs/User.json index 0aa2caf173..5a04d73edd 100644 --- a/application/Espo/Resources/metadata/entityDefs/User.json +++ b/application/Espo/Resources/metadata/entityDefs/User.json @@ -391,6 +391,15 @@ "layoutDetailDisabled": true, "directAccessDisabled": true, "exportDisabled": true + }, + "deleteId": { + "type": "varchar", + "maxLength": 17, + "readOnly": true, + "notNull": true, + "default": "0", + "disabled": true, + "customizationDisabled": true } }, "links": { @@ -508,5 +517,14 @@ "orderBy": "userName", "order": "asc", "textFilterFields": ["name", "userName", "emailAddress"] + }, + "indexes": { + "userNameDeleteId": { + "type": "unique", + "columns": [ + "userName", + "deleteId" + ] + } } } diff --git a/application/Espo/Services/User.php b/application/Espo/Services/User.php index 76dd5f4760..23d0849145 100644 --- a/application/Espo/Services/User.php +++ b/application/Espo/Services/User.php @@ -30,6 +30,8 @@ namespace Espo\Services; use Espo\Core\Authentication\Logins\Hmac; +use Espo\Core\Exceptions\Conflict; +use Espo\Core\Exceptions\NotFound; use Espo\Core\Mail\Exceptions\SendingError; use Espo\Entities\Team as TeamEntity; use Espo\Entities\User as UserEntity; @@ -46,6 +48,7 @@ use Espo\Core\Utils\ApiKey as ApiKeyUtil; use Espo\Core\Utils\PasswordHash; use Espo\Core\Utils\Util; use Espo\ORM\Entity; +use Espo\ORM\Query\SelectBuilder; use Espo\Tools\UserSecurity\Password\Checker as PasswordChecker; use Espo\Tools\UserSecurity\Password\Sender as PasswordSender; use Espo\Tools\UserSecurity\Password\Service as PasswordService; @@ -275,9 +278,25 @@ class User extends Record implements ->count(); } + /** + * @throws Conflict + */ + private function processUserExistsChecking(UserEntity $user): void + { + $existing = $this->getRepository() + ->select('id') + ->where(['userName' => $user->getUserName()]) + ->findOne(); + + if ($existing) { + throw new Conflict('userNameExists'); + } + } + /** * @param UserEntity $entity * @throws Forbidden + * @throws Conflict */ protected function beforeCreateEntity(Entity $entity, $data) { @@ -309,6 +328,8 @@ class User extends Record implements } } + $this->processUserExistsChecking($entity); + if ($entity->isApi()) { $entity->set('apiKey', Util::generateApiKey()); @@ -331,6 +352,7 @@ class User extends Record implements /** * @param UserEntity $entity * @throws Forbidden + * @throws Conflict */ protected function beforeUpdateEntity(Entity $entity, $data) { @@ -392,6 +414,10 @@ class User extends Record implements } } + if ($entity->isAttributeChanged('userName')) { + $this->processUserExistsChecking($entity); + } + if ( $entity->isApi() && $entity->isAttributeChanged('authMethod') && @@ -475,6 +501,38 @@ class User extends Record implements } } + /** + * @throws Forbidden + * @throws NotFound + * @throws Conflict + */ + public function restoreDeleted(string $id): void + { + $entity = $this->getRepository() + ->clone( + SelectBuilder::create() + ->from(UserEntity::ENTITY_TYPE) + ->withDeleted() + ->build() + ) + ->where(['id' => $id]) + ->findOne(); + + if ($entity) { + $this->processUserExistsChecking($entity); + } + + parent::restoreDeleted($id); + + $entity = $this->getRepository()->getById($id); + + if ($entity) { + $entity->set('deleteId', '0'); + + $this->getRepository()->save($entity); + } + } + protected function clearRoleCache(UserEntity $user): void { $this->createAclCacheClearer()->clearForUser($user); diff --git a/upgrades/7.5/data.json b/upgrades/7.5/data.json new file mode 100644 index 0000000000..2c63c08510 --- /dev/null +++ b/upgrades/7.5/data.json @@ -0,0 +1,2 @@ +{ +} diff --git a/upgrades/7.5/scripts/BeforeUpgrade.php b/upgrades/7.5/scripts/BeforeUpgrade.php new file mode 100644 index 0000000000..179ced42e0 --- /dev/null +++ b/upgrades/7.5/scripts/BeforeUpgrade.php @@ -0,0 +1,51 @@ +deleteDeletedUsers($container->getByClass(EntityManager::class)); + } + + private function deleteDeletedUsers(EntityManager $entityManager): void + { + $query = DeleteBuilder::create() + ->from(User::ENTITY_TYPE) + ->where(['deleted' => true]) + ->build(); + + $entityManager->getQueryExecutor()->execute($query); + } +}