From 8eb5f35bee48a81d19dec8b6d6268e096d0bd011 Mon Sep 17 00:00:00 2001
From: Kevin Papst <kpapst@gmx.net>
Date: Wed, 25 Aug 2021 13:53:30 +0200
Subject: [PATCH] prevent email or username becoming non-unique
---
src/Repository/UserRepository.php | 5 +++
src/User/UserService.php | 2 +-
src/Validator/Constraints/User.php | 4 ++
src/Validator/Constraints/UserValidator.php | 47 +++++++++++++--------
translations/validators.de.xlf | 8 ++++
translations/validators.en.xlf | 8 ++++
6 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/src/Repository/UserRepository.php b/src/Repository/UserRepository.php
index 9d6670100..c34aa52bd 100644
--- a/src/Repository/UserRepository.php
+++ b/src/Repository/UserRepository.php
@@ -82,6 +82,11 @@ class UserRepository extends EntityRepository implements UserLoaderInterface
return parent::findOneBy($criteria, $orderBy);
}
+ public function findByUsername($username): ?User
+ {
+ return parent::findOneBy(['username' => $username]);
+ }
+
public function countUser(?bool $enabled = null): int
{
if (null !== $enabled) {
diff --git a/src/User/UserService.php b/src/User/UserService.php
index a979032a7..0237eb354 100644
--- a/src/User/UserService.php
+++ b/src/User/UserService.php
@@ -127,7 +127,7 @@ class UserService
public function findUserByName(string $name): ?User
{
- return $this->repository->findOneBy(['username' => $name]);
+ return $this->repository->findByUsername($name);
}
public function findUserByConfirmationToken(string $token): ?User
diff --git a/src/Validator/Constraints/User.php b/src/Validator/Constraints/User.php
index 77cd5fe8c..142bb9080 100644
--- a/src/Validator/Constraints/User.php
+++ b/src/Validator/Constraints/User.php
@@ -20,10 +20,14 @@ class User extends Constraint
{
public const USER_EXISTING_EMAIL = 'kimai-user-00';
public const USER_EXISTING_NAME = 'kimai-user-01';
+ public const USER_EXISTING_EMAIL_AS_NAME = 'kimai-user-02';
+ public const USER_EXISTING_NAME_AS_EMAIL = 'kimai-user-03';
protected static $errorNames = [
self::USER_EXISTING_EMAIL => 'The email is already used.',
self::USER_EXISTING_NAME => 'The username is already used.',
+ self::USER_EXISTING_EMAIL_AS_NAME => 'An equal username is already used.',
+ self::USER_EXISTING_NAME_AS_EMAIL => 'An equal email is already used.',
];
public $message = 'The user has invalid settings.';
diff --git a/src/Validator/Constraints/UserValidator.php b/src/Validator/Constraints/UserValidator.php
index fa764b9af..f86b57ff8 100644
--- a/src/Validator/Constraints/UserValidator.php
+++ b/src/Validator/Constraints/UserValidator.php
@@ -44,28 +44,41 @@ class UserValidator extends ConstraintValidator
protected function validateUser(UserEntity $user, ExecutionContextInterface $context)
{
+ $matchedEmail = false;
if ($user->getEmail() !== null) {
- $existingByEmail = $this->userService->findUserByEmail($user->getEmail());
-
- if (null !== $existingByEmail && $user->getId() !== $existingByEmail->getId()) {
- $context->buildViolation(User::getErrorName(User::USER_EXISTING_EMAIL))
- ->atPath('email')
- ->setTranslationDomain('validators')
- ->setCode(User::USER_EXISTING_EMAIL)
- ->addViolation();
- }
+ $this->validateEmailExists($user->getId(), $user->getEmail(), 'email', User::USER_EXISTING_EMAIL, $context);
+ $this->validateEmailExists($user->getId(), $user->getUsername(), 'username', User::USER_EXISTING_NAME_AS_EMAIL, $context);
}
if ($user->getUsername() !== null) {
- $existingByName = $this->userService->findUserByName($user->getUsername());
+ $this->validateUsernameExists($user->getId(), $user->getUsername(), 'username', User::USER_EXISTING_NAME, $context);
+ $this->validateUsernameExists($user->getId(), $user->getEmail(), 'email', User::USER_EXISTING_EMAIL_AS_NAME, $context);
+ }
+ }
- if (null !== $existingByName && $user->getId() !== $existingByName->getId()) {
- $context->buildViolation(User::getErrorName(User::USER_EXISTING_NAME))
- ->atPath('username')
- ->setTranslationDomain('validators')
- ->setCode(User::USER_EXISTING_NAME)
- ->addViolation();
- }
+ private function validateEmailExists(?int $userId, string $email, string $path, string $code, ExecutionContextInterface $context): void
+ {
+ $existingByEmail = $this->userService->findUserByEmail($email);
+
+ if (null !== $existingByEmail && $userId !== $existingByEmail->getId()) {
+ $context->buildViolation(User::getErrorName($code))
+ ->atPath($path)
+ ->setTranslationDomain('validators')
+ ->setCode($code)
+ ->addViolation();
+ }
+ }
+
+ private function validateUsernameExists(?int $userId, string $username, string $path, string $code, ExecutionContextInterface $context): void
+ {
+ $existingByName = $this->userService->findUserByName($username);
+
+ if (null !== $existingByName && $userId !== $existingByName->getId()) {
+ $context->buildViolation(User::getErrorName($code))
+ ->atPath($path)
+ ->setTranslationDomain('validators')
+ ->setCode($code)
+ ->addViolation();
}
}
}
diff --git a/translations/validators.de.xlf b/translations/validators.de.xlf
index d32b86473..b13dbeaa1 100644
--- a/translations/validators.de.xlf
+++ b/translations/validators.de.xlf
@@ -18,6 +18,14 @@
<source>The username is already used.</source>
<target>Dieser Benutzername wird bereits verwendet.</target>
</trans-unit>
+ <trans-unit id="An equal username is already used.">
+ <source>An equal username is already used.</source>
+ <target>Eine gleichlautender Benutzername wird bereits verwendet.</target>
+ </trans-unit>
+ <trans-unit id="An equal email is already used.">
+ <source>An equal email is already used.</source>
+ <target>Eine gleichlautende E-Mail-Adresse wird bereits verwendet.</target>
+ </trans-unit>
<trans-unit id="This value is not a valid role.">
<source>This value is not a valid role.</source>
<target>Dieser Wert ist keine gültige Rolle.</target>
diff --git a/translations/validators.en.xlf b/translations/validators.en.xlf
index bd79b2633..60b30965d 100644
--- a/translations/validators.en.xlf
+++ b/translations/validators.en.xlf
@@ -18,6 +18,14 @@
<source>The username is already used.</source>
<target>The username is already used.</target>
</trans-unit>
+ <trans-unit id="An equal username is already used.">
+ <source>An equal username is already used.</source>
+ <target>An equal username is already used.</target>
+ </trans-unit>
+ <trans-unit id="An equal email is already used.">
+ <source>An equal email is already used.</source>
+ <target>An equal email is already used.</target>
+ </trans-unit>
<trans-unit id="This value is not a valid role.">
<source>This value is not a valid role.</source>
<target>This value is not a valid role.</target>