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

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';