diff --git a/src/Models/Entities/UserEntity.php b/src/Models/Entities/UserEntity.php index 493c5e46..c55ad50b 100644 --- a/src/Models/Entities/UserEntity.php +++ b/src/Models/Entities/UserEntity.php @@ -66,20 +66,7 @@ final class UserEntity extends AbstractEntity implements IValidatable $otherUser = UserModel::tryGetByName($userName); if ($otherUser !== null and $otherUser->getId() != $this->getId()) { - if (!$otherUser->getConfirmedEmail() - and isset($config->registration->needEmailForRegistering) - and $config->registration->needEmailForRegistering) - { - throw new SimpleException('User with this name is already registered and awaits e-mail confirmation'); - } - - if (!$otherUser->isStaffConfirmed() - and $config->registration->staffActivation) - { - throw new SimpleException('User with this name is already registered and awaits staff confirmation'); - } - - throw new SimpleException('User with this name is already registered'); + $this->throwDuplicateUserError($otherUser, 'name'); } $userNameMinLength = intval($config->registration->userNameMinLength); @@ -96,7 +83,7 @@ final class UserEntity extends AbstractEntity implements IValidatable throw new SimpleException('User name contains invalid characters'); } - public function validatePassword() + private function validatePassword() { if (empty($this->getPasswordHash())) throw new Exception('Trying to save user with no password into database'); @@ -117,7 +104,7 @@ final class UserEntity extends AbstractEntity implements IValidatable throw new SimpleException('Password contains invalid characters'); } - public function validateAccessRank() + private function validateAccessRank() { $this->accessRank->validate(); @@ -125,7 +112,7 @@ final class UserEntity extends AbstractEntity implements IValidatable throw new Exception('Cannot set special access rank "%s"', $this->accessRank->toString()); } - public function validateEmails() + private function validateEmails() { $this->validateEmail($this->getUnconfirmedEmail()); $this->validateEmail($this->getConfirmedEmail()); @@ -135,6 +122,38 @@ final class UserEntity extends AbstractEntity implements IValidatable { if (!empty($email) and !TextHelper::isValidEmail($email)) throw new SimpleException('E-mail address appears to be invalid'); + + $otherUser = UserModel::tryGetByEmail($email); + if ($otherUser !== null and $otherUser->getId() != $this->getId()) + { + $this->throwDuplicateUserError($otherUser, 'e-mail'); + } + } + + private function throwDuplicateUserError($otherUser, $reason) + { + $config = getConfig(); + + if (!$otherUser->getConfirmedEmail() + and isset($config->registration->needEmailForRegistering) + and $config->registration->needEmailForRegistering) + { + throw new SimpleException( + 'User with this %s is already registered and awaits e-mail confirmation', + $reason); + } + + if (!$otherUser->isStaffConfirmed() + and $config->registration->staffActivation) + { + throw new SimpleException( + 'User with this %s is already registered and awaits staff confirmation', + $reason); + } + + throw new SimpleException( + 'User with this %s is already registered', + $reason); } public function isBanned() diff --git a/tests/JobTests/ActivateUserEmailJobTest.php b/tests/JobTests/ActivateUserEmailJobTest.php index 3eceb5d1..5e66894a 100644 --- a/tests/JobTests/ActivateUserEmailJobTest.php +++ b/tests/JobTests/ActivateUserEmailJobTest.php @@ -108,42 +108,4 @@ class ActivateUserEmailJobTest extends AbstractTest ]); }, 'This token was already used'); } - - public function testTokensTwoUsersSameMail() - { - getConfig()->registration->needEmailForRegistering = true; - Mailer::mockSending(); - - $user1 = $this->mockUser(); - $user2 = $this->mockUser(); - $user1->setUnconfirmedEmail('godzilla@whitestar.gov'); - $user2->setUnconfirmedEmail('godzilla@whitestar.gov'); - UserModel::save($user1); - UserModel::save($user2); - - Api::run( - new ActivateUserEmailJob(), - [ - JobArgs::ARG_USER_NAME => $user1->getName(), - ]); - - Api::run( - new ActivateUserEmailJob(), - [ - JobArgs::ARG_USER_NAME => $user2->getName(), - ]); - - $tokens1 = Mailer::getMailsSent()[0]->tokens; - $tokens2 = Mailer::getMailsSent()[1]->tokens; - $token1text = $tokens1['token']; - $token2text = $tokens2['token']; - $this->assert->areNotEqual($token1text, $token2text); - - $token1 = TokenModel::getByToken($token1text); - $token2 = TokenModel::getByToken($token2text); - - $this->assert->areEqual($user1->getId(), $token1->getUser()->getId()); - $this->assert->areEqual($user2->getId(), $token2->getUser()->getId()); - $this->assert->areNotEqual($token1->getUserId(), $token2->getUserId()); - } } diff --git a/tests/JobTests/AddUserJobTest.php b/tests/JobTests/AddUserJobTest.php index 6caf6b4f..9e60b743 100644 --- a/tests/JobTests/AddUserJobTest.php +++ b/tests/JobTests/AddUserJobTest.php @@ -282,7 +282,7 @@ class AddUserJobTest extends AbstractTest ]); }); - $user2 = $this->assert->doesNotThrow(function() + $user2 = $this->assert->throws(function() { return Api::run( new AddUserJob(), @@ -291,18 +291,7 @@ class AddUserJobTest extends AbstractTest JobArgs::ARG_NEW_PASSWORD => 'sekai', JobArgs::ARG_NEW_EMAIL => 'godzilla@whitestar.gov', ]); - }); - - $this->assert->areEqual(2, Mailer::getMailCounter()); - $token1text = Mailer::getMailsSent()[0]->tokens['token']; - $token2text = Mailer::getMailsSent()[1]->tokens['token']; - $this->assert->areNotEqual($token1text, $token2text); - - $token1 = TokenModel::getByToken($token1text); - $token2 = TokenModel::getByToken($token2text); - - $this->assert->areEqual($user1->getId(), $token1->getUser()->getId()); - $this->assert->areEqual($user2->getId(), $token2->getUser()->getId()); + }, 'User with this e-mail is already registered'); } public function testLogBuffering() diff --git a/tests/ModelTests/UserModelTest.php b/tests/ModelTests/UserModelTest.php new file mode 100644 index 00000000..b62c714f --- /dev/null +++ b/tests/ModelTests/UserModelTest.php @@ -0,0 +1,120 @@ +registration->needEmailForRegistering = false; + list ($user1, $user2) = $this->prepareTwoUsersWithSameName(); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this name is already registered'); + } + + public function testSavingTwoUsersSameNameEmailActivation() + { + getConfig()->registration->needEmailForRegistering = true; + list ($user1, $user2) = $this->prepareTwoUsersWithSameName(); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this name is already registered and awaits e-mail'); + } + + public function testSavingTwoUsersSameNameStaffActivation() + { + getConfig()->registration->staffActivation = true; + list ($user1, $user2) = $this->prepareTwoUsersWithSameName(); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this name is already registered and awaits staff'); + } + + public function testSavingTwoUsersSameEmailNoActivation() + { + getConfig()->registration->needEmailForRegistering = false; + list ($user1, $user2) = $this->prepareTwoUsersWithSameEmail(false, false); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this e-mail is already registered'); + } + + public function testSavingTwoUsersSameEmailEmailActivation() + { + getConfig()->registration->needEmailForRegistering = true; + list ($user1, $user2) = $this->prepareTwoUsersWithSameEmail(false, false); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this e-mail is already registered and awaits e-mail'); + } + + public function testSavingTwoUsersSameEmailStaffActivation() + { + getConfig()->registration->staffActivation = true; + list ($user1, $user2) = $this->prepareTwoUsersWithSameEmail(false, false); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this e-mail is already registered and awaits staff'); + } + + public function testSavingTwoUsersSameEmailDifferConfirm1() + { + list ($user1, $user2) = $this->prepareTwoUsersWithSameEmail(true, false); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this e-mail is already registered'); + } + + public function testSavingTwoUsersSameEmailDifferConfirm2() + { + list ($user1, $user2) = $this->prepareTwoUsersWithSameEmail(false, true); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this e-mail is already registered'); + } + + public function testSavingTwoUsersSameEmailDifferConfirm3() + { + list ($user1, $user2) = $this->prepareTwoUsersWithSameEmail(true, true); + $this->assert->throws(function() use ($user2) + { + UserModel::save($user2); + }, 'User with this e-mail is already registered'); + } + + private function prepareTwoUsersWithSameName() + { + $user1 = $this->mockUser(); + $user2 = $this->mockUser(); + $user1->setName('pikachu'); + $user2->setName('pikachu'); + UserModel::save($user1); + return [$user1, $user2]; + } + + private function prepareTwoUsersWithSameEmail($confirmFirst, $confirmSecond) + { + $user1 = $this->mockUser(); + $user2 = $this->mockUser(); + $mail = 'godzilla@whitestar.gov'; + + if ($confirmFirst) + $user1->setConfirmedEmail($mail); + else + $user1->setUnconfirmedEmail($mail); + + if ($confirmFirst) + $user2->setConfirmedEmail($mail); + else + $user2->setUnconfirmedEmail($mail); + + UserModel::save($user1); + return [$user1, $user2]; + } +}