From 9ae5d067b064f64a393b636b223630ad67f99883 Mon Sep 17 00:00:00 2001 From: Zoltan Flamis Date: Wed, 17 Mar 2021 02:29:22 +1300 Subject: Check usernames and emails against each other on user create and update (#17296) * check username and email together on user create and update * Update APITest.php --- plugins/UsersManager/API.php | 20 +++++++-- plugins/UsersManager/lang/en.json | 2 + plugins/UsersManager/tests/Integration/APITest.php | 51 ++++++++++++++++++++-- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/plugins/UsersManager/API.php b/plugins/UsersManager/API.php index 1b64000e23..41d91f9bd8 100644 --- a/plugins/UsersManager/API.php +++ b/plugins/UsersManager/API.php @@ -652,15 +652,27 @@ class API extends \Piwik\Plugin\API throw new Exception(Piwik::translate('UsersManager_ExceptionLoginExists', $userLogin)); } + if ($this->userEmailExists($userLogin)) { + throw new Exception(Piwik::translate('UsersManager_ExceptionLoginExistsAsEmail', $userLogin)); + } + Piwik::checkValidLoginString($userLogin); } - private function checkEmail($email) + private function checkEmail($email, $userLogin = null) { if ($this->userEmailExists($email)) { throw new Exception(Piwik::translate('UsersManager_ExceptionEmailExists', $email)); } + if ($userLogin && Common::mb_strtolower($userLogin) !== Common::mb_strtolower($email) && $this->userExists($email)) { + throw new Exception(Piwik::translate('UsersManager_ExceptionEmailExistsAsLogin', $email)); + } + + if (!$userLogin && $this->userExists($email)) { + throw new Exception(Piwik::translate('UsersManager_ExceptionEmailExistsAsLogin', $email)); + } + if (!Piwik::isValidEmailString($email)) { throw new Exception(Piwik::translate('UsersManager_ExceptionInvalidEmail')); } @@ -918,7 +930,7 @@ class API extends \Piwik\Plugin\API $hasEmailChanged = Common::mb_strtolower($email) !== Common::mb_strtolower($userInfo['email']); if ($hasEmailChanged) { - $this->checkEmail($email); + $this->checkEmail($email, $userLogin); $changeShouldRequirePasswordConfirmation = true; } @@ -1336,7 +1348,7 @@ class API extends \Piwik\Plugin\API * sent as a POST parameter. * @param string $description The description for this app specific password, for example your app name. Max 100 characters are allowed * @param string $expireDate Optionally a date when the token should expire - * @param string $expireHours Optionally number of hours for how long the token should be valid before it expires. + * @param string $expireHours Optionally number of hours for how long the token should be valid before it expires. * If expireDate is set and expireHours, then expireDate will be used. * If expireDate is set and expireHours, then expireDate will be used. * @return string @@ -1350,7 +1362,7 @@ class API extends \Piwik\Plugin\API $userLogin = $user['login']; } } - + if (empty($user) || !$this->passwordVerifier->isPasswordCorrect($userLogin, $passwordConfirmation)) { if (empty($user)) { /** diff --git a/plugins/UsersManager/lang/en.json b/plugins/UsersManager/lang/en.json index 91bbdd8b8e..ae74572c50 100644 --- a/plugins/UsersManager/lang/en.json +++ b/plugins/UsersManager/lang/en.json @@ -47,6 +47,8 @@ "ExceptionDeleteOnlyUserWithSuperUserAccess": "Deleting user '%s' is not possible.", "ExceptionEditAnonymous": "The anonymous user cannot be edited or deleted. It is used by Matomo to define a user that has not logged in yet. For example, you can make your statistics public by granting the 'view' access to the 'anonymous' user.", "ExceptionEmailExists": "User with email '%s' already exists.", + "ExceptionEmailExistsAsLogin": "Email '%s' already used as a username.", + "ExceptionLoginExistsAsEmail": "Username '%s' already used as an email.", "ExceptionInvalidEmail": "The email doesn't have a valid format.", "ExceptionInvalidLoginFormat": "The username must be between %1$s and %2$s characters long and contain only letters, numbers, or the characters '_' or '-' or '.' or '@' or '+'", "ExceptionInvalidPassword": "The password length must be greater than %1$s characters.", diff --git a/plugins/UsersManager/tests/Integration/APITest.php b/plugins/UsersManager/tests/Integration/APITest.php index 5ce6ba2bd0..6aec636762 100644 --- a/plugins/UsersManager/tests/Integration/APITest.php +++ b/plugins/UsersManager/tests/Integration/APITest.php @@ -137,7 +137,7 @@ class APITest extends IntegrationTestCase * @var Model */ private $model; - + private $login = 'userLogin'; private $password = 'password'; @@ -159,14 +159,14 @@ class APITest extends IntegrationTestCase Fixture::createWebsite('2014-01-01 00:00:00'); $this->api->addUser($this->login, $this->password, $this->email); } - + public function tearDown(): void { Config::getInstance()->General['enable_update_users_email'] = 1; - parent::tearDown(); + parent::tearDown(); } - + public function test_setUserAccess_ShouldTriggerRemoveSiteAccessEvent_IfAccessToAWebsiteIsRemoved() { $eventTriggered = false; @@ -387,6 +387,49 @@ class APITest extends IntegrationTestCase $this->api->updateUser($this->login, str_pad('foo', UsersManager::PASSWORD_MAX_LENGTH + 1), 'email@example.com', false, $this->password); } + public function test_update_user_fails_if_email_exists_as_other_user_username() + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('UsersManager_ExceptionEmailExistsAsLogin'); + + $user2 = 'existed@example.com'; + $this->api->addUser($user2, 'password', 'userlogin2@password.de'); + + $this->api->updateUser($this->login, $this->password, $user2, false, $this->password); + } + + public function test_update_can_update_user_email_to_own_username() + { + $user2 = 'ownemail@example.com'; + $password = 'password'; + $this->api->addUser($user2, $password, 'ownemail_wrong@example.com'); + + FakeAccess::$identity = $user2; + $this->api->updateUser($user2, $password, $user2, false, $password); + + $user2Array = $this->api->getUser($user2); + $this->assertEquals($user2Array['email'], $user2); + } + + public function test_cannot_create_user_if_email_exists_as_username() + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('UsersManager_ExceptionEmailExistsAsLogin'); + + $user2 = 'existed@example.com'; + $this->api->addUser($user2, 'password', 'email@example.com'); + + $this->api->addUser('user3', 'password', $user2); + } + + public function test_cannot_create_user_if_username_exists_as_email() + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('UsersManager_ExceptionLoginExistsAsEmail'); + + $this->api->addUser($this->email, 'password', 'new_user@example.com'); + } + public function test_getSitesAccessFromUser_forSuperUser() { $user2 = 'userLogin2'; -- cgit v1.2.3