Improve handling for changing email of invited users (#22644)

plugins/UsersManager/API.php

@@ -953,7 +953,15 @@ public function updateUser(
 
         Cache::deleteTrackerCache();
 
-        if ($hasEmailChanged && $isEmailNotificationOnInConfig) {
+        if ($hasEmailChanged && $this->model->isPendingUser($userLogin)) {
+            // If the email of a user is changed, who was invited and did not yet accept the invitation
+            // we send a new invite to the new address.
+            // this will indirectly invalidate the invitation sent to the previous address
+            $this->userRepository->reInviteUser(
+                $userLogin,
+                (int) Config\GeneralConfig::getConfigValue('default_invite_user_token_expiry_days')
+            );
+        } elseif ($hasEmailChanged && $isEmailNotificationOnInConfig) {
             $this->sendEmailChangedEmail($userInfo, $email);
         }
 

plugins/UsersManager/Model.php

@@ -277,7 +277,7 @@ public function hashTokenAuth($tokenAuth)
         return hash(self::TOKEN_HASH_ALGO, $tokenAuth . $salt);
     }
 
-    public function generateRandomInviteToken()
+    public function generateRandomInviteToken(): string
     {
         $count = 0;
 
@@ -618,15 +618,15 @@ public function addUser($userLogin, $hashedPassword, $email, $dateRegistered)
         $db->insert($this->userTable, $user);
     }
 
-    public function attachInviteToken($userLogin, $token, $expiryInDays = 7)
+    public function attachInviteToken(string $userLogin, string $token, int $expiryInDays): void
     {
         $this->updateUserFields($userLogin, [
           'invite_token'      => $this->hashTokenAuth($token),
           'invite_expired_at' => Date::now()->addDay($expiryInDays)->getDatetime()
         ]);
     }
 
-    public function attachInviteLinkToken($userLogin, $token, $expiryInDays = 7)
+    public function attachInviteLinkToken(string $userLogin, string $token, int $expiryInDays): void
     {
         $this->updateUserFields($userLogin, [
             'invite_link_token' => $this->hashTokenAuth($token),

plugins/UsersManager/Repository/UserRepository.php

@@ -109,15 +109,15 @@ public function inviteUser(string $userLogin, string $email, ?int $initialIdSite
         $this->sendInvitationEmail($user, $generatedToken, $expiryInDays);
     }
 
-    public function reInviteUser(string $userLogin, $expiryInDays = null): void
+    public function reInviteUser(string $userLogin, int $expiryInDays): void
     {
         $user = $this->model->getUser($userLogin);
         $generatedToken = $this->model->generateRandomInviteToken();
         $this->model->attachInviteToken($userLogin, $generatedToken, $expiryInDays);
         $this->sendInvitationEmail($user, $generatedToken, $expiryInDays);
     }
 
-    public function generateInviteToken(string $userLogin, $expiryInDays = null): string
+    public function generateInviteToken(string $userLogin, int $expiryInDays): string
     {
         $generatedToken = $this->model->generateRandomInviteToken();
         $this->model->attachInviteLinkToken($userLogin, $generatedToken, $expiryInDays);

plugins/UsersManager/UsersManager.php

@@ -421,5 +421,6 @@ public function getClientSideTranslationKeys(&$translationKeys)
         $translationKeys[] = 'UsersManager_YourUsernameCannotBeChanged';
         $translationKeys[] = 'UsersManager_YourVisitsAreIgnoredOnDomain';
         $translationKeys[] = 'UsersManager_YourVisitsAreNotIgnored';
+        $translationKeys[] = 'UsersManager_InviteEmailChange';
     }
 }

plugins/UsersManager/lang/en.json

@@ -212,6 +212,7 @@
         "FirstWebsitePermission": "First website permission",
         "InviteSuccessNotification": "The invited user will receive an email to accept the invite. This invite is valid for %1$s days. You can also resend and delete the invite on the user management page.",
         "InviteConfirmMessage": "You can resend the invitation by copying the invite link and sharing directly with %1$s, or resend an invite email to %2$s.",
+        "InviteEmailChange": "This user has not yet confirmed their account. Changing the email will invalidate the previous invitation and send an invitation to the new email address.",
         "Status": "Status",
         "Pending": "Pending",
         "Active": "Active",

plugins/UsersManager/stylesheets/usersManager.less

@@ -82,6 +82,11 @@
       margin-top: 3em !important;
     }
 
+    .email-input .form-help {
+      background-color: none;
+      padding: 0;
+      border: none;
+    }
   }
 
 

plugins/UsersManager/tests/Integration/APITest.php

@@ -24,6 +24,8 @@
 use Piwik\NoAccessException;
 use Piwik\Option;
 use Piwik\Piwik;
+use Piwik\Plugins\CoreAdminHome\Emails\UserCreatedEmail;
+use Piwik\Plugins\UsersManager\Emails\UserInviteEmail;
 use Piwik\Plugins\UsersManager\SystemSettings;
 use Piwik\Plugins\SitesManager\API as SitesManagerAPI;
 use Piwik\Plugins\UsersManager\API;
@@ -1369,6 +1371,11 @@ public function testInviteUserInitialIdSiteError()
     public function testInviteUserAsSuperUser()
     {
         $eventWasFired = false;
+        $capturedMails = [];
+
+        Piwik::addAction('Mail.send', function (Mail $mail) use (&$capturedMails) {
+            $capturedMails[] = $mail;
+        });
 
         EventDispatcher::getInstance()->addObserver(
             'UsersManager.inviteUser.end',
@@ -1380,9 +1387,40 @@ function ($userLogin, $email) use (&$eventWasFired) {
         );
 
         $this->api->inviteUser('pendingLoginTest', 'pendingLoginTest@matomo.org', 1);
-        $user = $this->model->isPendingUser('pendingLoginTest');
-        self::assertTrue($user);
+        $isPending = $this->model->isPendingUser('pendingLoginTest');
+        self::assertTrue($isPending);
         self::assertTrue($eventWasFired);
+
+        self::assertCount(2, $capturedMails);
+        self::assertInstanceOf(UserCreatedEmail::class, $capturedMails[0]);
+        self::assertInstanceOf(UserInviteEmail::class, $capturedMails[1]);
+    }
+
+    public function testChangingEmailOfInvitedUserShouldResendInvitation()
+    {
+        Fixture::createSuperUser();
+        $this->api->inviteUser('pendingLoginTest', 'pendingLoginTest@matomo.org', 1);
+        $isPending = $this->model->isPendingUser('pendingLoginTest');
+        self::assertTrue($isPending);
+
+        $eventWasFired = false;
+        $capturedMails = [];
+
+        Piwik::addAction('Mail.send', function (Mail $mail) use (&$capturedMails) {
+            $capturedMails[] = $mail;
+        });
+
+        EventDispatcher::getInstance()->addObserver(
+            'UsersManager.inviteUser.end',
+            function ($userLogin, $email) use (&$eventWasFired) {
+                $eventWasFired = true;
+            }
+        );
+
+        $this->api->updateUser('pendingLoginTest', false, 'pendingLoginTest2@matomo.org', false, Fixture::ADMIN_USER_PASSWORD);
+        self::assertFalse($eventWasFired); // event should not be fired on email change
+        self::assertCount(1, $capturedMails);
+        self::assertInstanceOf(UserInviteEmail::class, $capturedMails[0]);
     }
 
     public function testInviteUserAsAdmin()

plugins/UsersManager/tests/UI/UsersManager_spec.js

@@ -249,7 +249,25 @@ describe("UsersManager", function () {
         expect(await page.screenshotSelector('.usersManager')).to.matchImage('user_created');
     });
 
+    it('should warn about invitation resend when changing email', async function () {
+        await page.evaluate(() => $('.userEditForm #user_email').val('theuser2@email.com'));
+        await page.evaluate(() => $('.userEditForm .matomo-save-button input').click());
+        await page.waitForTimeout(100);
+        await page.waitForFunction(() => $('.modal.open:visible').length)
+        const modal = await page.jQuery('.modal.open:visible');
+        await page.focus('.modal.open #currentUserPassword');
+        await page.waitForTimeout(250);
+        expect(await modal.screenshot()).to.matchImage({
+          imageName: 'user_invited_change',
+          comparisonThreshold: 0.025
+        });
+    });
+
     it('should show the permissions edit when the permissions tab is clicked', async function () {
+        // close modal from previous step
+        await (await page.jQuery('.confirm-password-modal .modal-close.modal-no:visible')).click();
+        await page.waitForNetworkIdle();
+
         await page.click('.userEditForm .menuPermissions');
         await page.mouse.move(0, 0);
 
@@ -483,7 +501,7 @@ describe("UsersManager", function () {
 
     it('should fail to set superuser access if password is wrong', async function () {
         await page.type('.modal.open #currentUserPassword', 'wrongpassword');
-        await (await page.jQuery('.modal.open .modal-close:not(.modal-no):visible')).click();
+        await page.evaluate(() => $('.modal.open .modal-close:not(.modal-no):visible').click());
         await page.waitForNetworkIdle();
 
         await page.waitForSelector('.notification-error', { visible: true });