diff --git a/application/Espo/Core/Authentication/Authentication.php b/application/Espo/Core/Authentication/Authentication.php index 58d794f993..b78b450eb0 100644 --- a/application/Espo/Core/Authentication/Authentication.php +++ b/application/Espo/Core/Authentication/Authentication.php @@ -480,8 +480,7 @@ class Authentication $request->getHeader(self::HEADER_CREATE_TOKEN_SECRET) === 'true' && !$this->configDataProvider->isAuthTokenSecretDisabled(); - /** @var ?string $password */ - $password = $user->get('password'); + $password = $user->getPassword(); $ipAddress = $this->util->obtainIpFromRequest($request); $authTokenData = AuthTokenData::create([ diff --git a/application/Espo/Core/Authentication/Ldap/LdapLogin.php b/application/Espo/Core/Authentication/Ldap/LdapLogin.php index eb3e181da7..19fcc55822 100644 --- a/application/Espo/Core/Authentication/Ldap/LdapLogin.php +++ b/application/Espo/Core/Authentication/Ldap/LdapLogin.php @@ -317,7 +317,7 @@ class LdapLogin implements Login private function adminLogin(string $username, #[SensitiveParameter] string $password): ?User { $user = $this->entityManager - ->getRDBRepository(User::ENTITY_TYPE) + ->getRDBRepositoryByClass(User::class) ->where([ 'userName' => $username, 'type' => [User::TYPE_ADMIN, User::TYPE_SUPER_ADMIN], @@ -328,7 +328,7 @@ class LdapLogin implements Login return null; } - if (!$this->passwordHash->verify($password, $user->get('password'))) { + if (!$this->passwordHash->verify($password, $user->getPassword())) { return null; } diff --git a/application/Espo/Core/Authentication/Logins/Espo.php b/application/Espo/Core/Authentication/Logins/Espo.php index 6e73345ab7..d5ecac0ccd 100644 --- a/application/Espo/Core/Authentication/Logins/Espo.php +++ b/application/Espo/Core/Authentication/Logins/Espo.php @@ -65,7 +65,7 @@ class Espo implements Login } else { $user = $this->userFinder->find($username); - if ($user && !$this->passwordHash->verify($password, $user->get('password'))) { + if ($user && !$this->passwordHash->verify($password, $user->getPassword())) { $user = null; } } diff --git a/application/Espo/Entities/User.php b/application/Espo/Entities/User.php index 8c60c4bd3e..eccbec32a1 100644 --- a/application/Espo/Entities/User.php +++ b/application/Espo/Entities/User.php @@ -309,6 +309,14 @@ class User extends Person return $this->get('avatarColor'); } + /** + * Get a password hash. + */ + public function getPassword(): string + { + return $this->get('password') ?? ''; + } + private function getNameInternal(): ?string { if (!$this->hasInContainer(Field::NAME) || !$this->getFromContainer(Field::NAME)) { diff --git a/application/Espo/Services/User.php b/application/Espo/Services/User.php index 4378ba7193..77a0301ec0 100644 --- a/application/Espo/Services/User.php +++ b/application/Espo/Services/User.php @@ -50,6 +50,7 @@ use Espo\Tools\UserSecurity\Password\Checker as PasswordChecker; use Espo\Tools\UserSecurity\Password\Generator as PasswordGenerator; use Espo\Tools\UserSecurity\Password\Sender as PasswordSender; use Espo\Tools\UserSecurity\Password\Service as PasswordService; +use SensitiveParameter; use stdClass; use Exception; @@ -83,7 +84,7 @@ class User extends Record implements LogAware return $entity; } - private function hashPassword(string $password): string + private function hashPassword(#[SensitiveParameter] string $password): string { $passwordHash = $this->injectableFactory->create(PasswordHash::class); @@ -108,7 +109,7 @@ class User extends Record implements LogAware /** * @throws BadRequest */ - private function fetchPassword(stdClass $data): ?string + private function fetchPassword(#[SensitiveParameter] stdClass $data): ?string { $password = $data->password ?? null; diff --git a/application/Espo/Tools/UserSecurity/Api/PutPassword.php b/application/Espo/Tools/UserSecurity/Api/PutPassword.php index e53174bcc4..cc7f77afc1 100644 --- a/application/Espo/Tools/UserSecurity/Api/PutPassword.php +++ b/application/Espo/Tools/UserSecurity/Api/PutPassword.php @@ -36,6 +36,7 @@ use Espo\Core\Api\ResponseComposer; use Espo\Core\Exceptions\BadRequest; use Espo\Entities\User; use Espo\Tools\UserSecurity\Password\Service; +use SensitiveParameter; /** * Changes own user password. @@ -47,7 +48,7 @@ class PutPassword implements Action private User $user ) {} - public function process(Request $request): Response + public function process(#[SensitiveParameter] Request $request): Response { $data = $request->getParsedBody(); diff --git a/application/Espo/Tools/UserSecurity/Password/Checker.php b/application/Espo/Tools/UserSecurity/Password/Checker.php index 909e68a275..4baff5d9a1 100644 --- a/application/Espo/Tools/UserSecurity/Password/Checker.php +++ b/application/Espo/Tools/UserSecurity/Password/Checker.php @@ -29,6 +29,8 @@ namespace Espo\Tools\UserSecurity\Password; +use SensitiveParameter; + class Checker { private const SPECIAL_CHARACTERS = "'-!\"#$%&()*,./:;?@[]^_`{|}~+<=>"; @@ -37,7 +39,7 @@ class Checker private ConfigProvider $configProvider, ) {} - public function checkStrength(string $password): bool + public function checkStrength(#[SensitiveParameter] string $password): bool { $minLength = $this->configProvider->getStrengthLength(); diff --git a/application/Espo/Tools/UserSecurity/Password/Service.php b/application/Espo/Tools/UserSecurity/Password/Service.php index ca775d79e6..dacd1a5f4e 100644 --- a/application/Espo/Tools/UserSecurity/Password/Service.php +++ b/application/Espo/Tools/UserSecurity/Password/Service.php @@ -43,6 +43,7 @@ use Espo\Core\Utils\Config; use Espo\Core\Utils\PasswordHash; use Espo\Entities\User; use Espo\ORM\EntityManager; +use SensitiveParameter; class Service { @@ -107,7 +108,7 @@ class Service * @throws Forbidden * @throws NotFound */ - public function changePasswordByRecovery(string $requestId, string $password): ?string + public function changePasswordByRecovery(string $requestId, #[SensitiveParameter] string $password): ?string { $request = $this->recovery->getRequest($requestId); @@ -124,8 +125,12 @@ class Service * @throws NotFound * @throws Error */ - public function changePasswordWithCheck(string $userId, string $password, string $currentPassword): void - { + public function changePasswordWithCheck( + string $userId, + #[SensitiveParameter] string $password, + #[SensitiveParameter] string $currentPassword + ): void { + $this->changePasswordInternal($userId, $password, true, $currentPassword); } @@ -136,7 +141,7 @@ class Service * @throws NotFound * @throws Error */ - private function changePassword(string $userId, string $password): void + private function changePassword(string $userId, #[SensitiveParameter] string $password): void { $this->changePasswordInternal($userId, $password); } @@ -148,9 +153,9 @@ class Service */ private function changePasswordInternal( string $userId, - string $password, + #[SensitiveParameter] string $password, bool $checkCurrentPassword = false, - ?string $currentPassword = null + #[SensitiveParameter] ?string $currentPassword = null ): void { /** @var ?User $user */ @@ -178,15 +183,15 @@ class Service } if ($checkCurrentPassword) { - $u = $this->entityManager - ->getRDBRepository(User::ENTITY_TYPE) - ->where([ - 'id' => $user->getId(), - 'password' => $this->passwordHash->hash($currentPassword ?? ''), - ]) - ->findOne(); + $userFound = $this->entityManager + ->getRDBRepositoryByClass(User::class) + ->getById($user->getId()); - if (!$u) { + if (!$userFound) { + throw new NotFound("User not found"); + } + + if (!$this->passwordHash->verify($currentPassword ?? '', $userFound->getPassword())) { throw new Forbidden("Wrong password."); } } @@ -291,14 +296,14 @@ class Service $this->savePassword($user, $password); } - private function savePassword(User $user, string $password): void + private function savePassword(User $user, #[SensitiveParameter] string $password): void { $user->set('password', $this->passwordHash->hash($password)); $this->entityManager->saveEntity($user); } - private function savePasswordSilent(User $user, string $password): void + private function savePasswordSilent(User $user, #[SensitiveParameter] string $password): void { $user->set('password', $this->passwordHash->hash($password));