Password confirmation fix for custom login plugins. (#15945)

* Password confirmation fix for custom login plugins.

* Throw exception if password confirm is incorrect.

* Update API.php

* apply review feedback

* update submodule

* fix tests

* update screenshot and fix test

* update submodule

4 files changed, 32 insertions, 42 deletions

diff --git a/plugins/Login/PasswordVerifier.php b/plugins/Login/PasswordVerifier.php
index d95ba17804..33e267b1ef 100644
--- a/plugins/Login/PasswordVerifier.php
+++ b/plugins/Login/PasswordVerifier.php
@@ -189,7 +189,7 @@ class PasswordVerifier
         $sessionNamespace->setExpirationSeconds(self::VERIFY_VALID_FOR_MINUTES * 60 * 5, 'redirectParams');
 
         if ($this->enableRedirect) {
-            Piwik::redirectToModule('Login', 'confirmPassword');
+            Piwik::redirectToModule(Piwik::getLoginPluginName(), 'confirmPassword');
         }
     }
 }
diff --git a/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthTest.php b/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthTest.php
index 1df67e1e36..78a6c2f055 100644
--- a/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthTest.php
+++ b/plugins/TwoFactorAuth/tests/Integration/TwoFactorAuthTest.php
@@ -73,20 +73,22 @@ class TwoFactorAuthTest extends IntegrationTestCase
     {
         $token = Request::processRequest('UsersManager.createAppSpecificTokenAuth', array(
             'userLogin' => $this->userWithout2Fa,
-            'md5Password' => md5($this->userPassword),
+            'passwordConfirmation' => $this->userPassword,
             'description' => 'twofa test'
         ));
         $this->assertEquals(32, strlen($token));
     }
 
-    public function test_onCreateAppSpecificTokenAuth_returnsRandomTokenWhenNotAuthenticatedEvenWhen2FAenabled()
+    public function test_onCreateAppSpecificTokenAuth_failsWhenNotAuthenticatedEvenWhen2FAenabled()
     {
-        $token = Request::processRequest('UsersManager.createAppSpecificTokenAuth', array(
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('UsersManager_CurrentPasswordNotCorrect');
+
+        Request::processRequest('UsersManager.createAppSpecificTokenAuth', array(
             'userLogin' => $this->userWith2Fa,
-            'md5Password' => md5('invalidPAssword'),
+            'passwordConfirmation' => 'invalidPAssword',
             'description' => 'twofa test'
         ));
-        $this->assertEquals(32, strlen($token));
     }
 
     public function test_onCreateAppSpecificTokenAuth_throwsErrorWhenMissingTokenWhenUsing2FaAndAuthenticatedCorrectly()
@@ -97,7 +99,7 @@ class TwoFactorAuthTest extends IntegrationTestCase
         Request::processRequest('UsersManager.createAppSpecificTokenAuth', array(
 
             'userLogin' => $this->userWith2Fa,
-            'md5Password' => md5($this->userPassword),
+            'passwordConfirmation' => $this->userPassword,
             'description' => 'twofa test'
         ));
     }
@@ -110,7 +112,7 @@ class TwoFactorAuthTest extends IntegrationTestCase
         $_GET['authCode'] = '111222';
         Request::processRequest('UsersManager.createAppSpecificTokenAuth', array(
             'userLogin' => $this->userWith2Fa,
-            'md5Password' => md5($this->userPassword),
+            'passwordConfirmation' => $this->userPassword,
             'description' => 'twofa test'
         ));
     }
@@ -120,7 +122,7 @@ class TwoFactorAuthTest extends IntegrationTestCase
         $_GET['authCode'] = $this->generateValidAuthCode($this->user2faSecret);
         $token = Request::processRequest('UsersManager.createAppSpecificTokenAuth', array(
             'userLogin' => $this->userWith2Fa,
-            'md5Password' => md5($this->userPassword),
+            'passwordConfirmation' => $this->userPassword,
             'description' => 'twofa test'
         ));
         $this->assertEquals(32, strlen($token));
diff --git a/plugins/UsersManager/API.php b/plugins/UsersManager/API.php
index 3bbf50b42b..697dfa5566 100644
--- a/plugins/UsersManager/API.php
+++ b/plugins/UsersManager/API.php
@@ -699,7 +699,8 @@ class API extends \Piwik\Plugin\API
      * @param string   $userLogin          the user login.
      * @param bool|int $hasSuperUserAccess true or '1' to grant Super User access, false or '0' to remove Super User
      *                                     access.
-     * @param string $passwordConfirmation the current user's password.
+     * @param string $passwordConfirmation the current user's password. For security purposes, this value should be
+     *                                     sent as a POST parameter.
      * @throws \Exception
      */
     public function setSuperUserAccess($userLogin, $hasSuperUserAccess, $passwordConfirmation = null)
@@ -1297,7 +1298,8 @@ class API extends \Piwik\Plugin\API
      * If the username/password combination is incorrect an invalid token will be returned.
      *
      * @param string $userLogin Login or Email address
-     * @param string $md5Password hashed string of the password (using current hash function; MD5-named for historical reasons)
+     * @param string $passwordConfirmation the current user's password. For security purposes, this value should be
+     *                                     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. 
@@ -1305,10 +1307,8 @@ class API extends \Piwik\Plugin\API
      *                            If expireDate is set and expireHours, then expireDate will be used.
      * @return string
      */
-    public function createAppSpecificTokenAuth($userLogin, $md5Password, $description, $expireDate = null, $expireHours = 0)
+    public function createAppSpecificTokenAuth($userLogin, $passwordConfirmation, $description, $expireDate = null, $expireHours = 0)
     {
-        UsersManager::checkPasswordHash($md5Password, Piwik::translate('UsersManager_ExceptionPasswordMD5HashExpected'));
-
         $user = $this->model->getUser($userLogin);
         if (empty($user) && Piwik::isValidEmailString($userLogin)) {
             $user = $this->model->getUserByEmail($userLogin);
@@ -1317,19 +1317,16 @@ class API extends \Piwik\Plugin\API
             }
         }
         
-        if (empty($user) || !$this->password->verify($md5Password, $user['password'])) {
-            /**
-             * @ignore
-             * @internal
-             */
-            Piwik::postEvent('Login.authenticate.failed', array($userLogin));
-
-            return md5($userLogin . microtime(true) . Common::generateUniqId());
-        }
+        if (empty($user) || !$this->passwordVerifier->isPasswordCorrect($userLogin, $passwordConfirmation)) {
+            if (empty($user)) {
+                /**
+                 * @ignore
+                 * @internal
+                 */
+                Piwik::postEvent('Login.authenticate.failed', array($userLogin));
+            }
 
-        if ($this->password->needsRehash($user['password'])) {
-            $userUpdater = new UserUpdater();
-            $userUpdater->updateUserWithoutCurrentPassword($userLogin, $this->password->hash($md5Password));
+            throw new \Exception(Piwik::translate('UsersManager_CurrentPasswordNotCorrect'));
         }
 
         if (empty($expireDate) && !empty($expireHours) && is_numeric($expireHours)) {
diff --git a/plugins/UsersManager/tests/System/ApiTest.php b/plugins/UsersManager/tests/System/ApiTest.php
index a4d6bbb74a..e96dbf9016 100644
--- a/plugins/UsersManager/tests/System/ApiTest.php
+++ b/plugins/UsersManager/tests/System/ApiTest.php
@@ -86,7 +86,7 @@ class ApiTest extends SystemTestCase
     public function test_createAppSpecificTokenAuth()
     {
         $this->model->deleteAllTokensForUser('login1');
-        $token = $this->api->createAppSpecificTokenAuth('login1', md5('password'), 'test');
+        $token = $this->api->createAppSpecificTokenAuth('login1', 'password', 'test');
         $this->assertMd5($token);
 
         $user = $this->model->getUserByTokenAuth($token);
@@ -96,35 +96,26 @@ class ApiTest extends SystemTestCase
     public function test_createAppSpecificTokenAuth_canLoginByEmail()
     {
         $this->model->deleteAllTokensForUser('login1');
-        $token = $this->api->createAppSpecificTokenAuth('login1@example.com', md5('password'), 'test');
+        $token = $this->api->createAppSpecificTokenAuth('login1@example.com', 'password', 'test');
         $this->assertMd5($token);
 
         $user = $this->model->getUserByTokenAuth($token);
         $this->assertSame('login1', $user['login']);
     }
 
-    public function test_createAppSpecificTokenAuth_notValidPasswordFormat()
+    public function test_createAppSpecificTokenAuth_failsWhenPasswordNotValid()
     {
         $this->expectException(\Exception::class);
-        $this->expectExceptionMessage('is expecting a MD5-hashed password');
+        $this->expectExceptionMessage('The current password you entered is not correct.');
 
-        $this->api->createAppSpecificTokenAuth('login1', 'foobar', 'test');
-    }
-
-    public function test_createAppSpecificTokenAuth_generatesRandomTokenWhenPasswordNotValid()
-    {
         $this->model->deleteAllTokensForUser('login1');
-        $token = $this->api->createAppSpecificTokenAuth('login1', md5('foooooo'), 'test');
-        $this->assertMd5($token);
-
-        $user = $this->model->getUserByTokenAuth($token);
-        $this->assertEmpty($user);
+        $this->api->createAppSpecificTokenAuth('login1', 'foooooo', 'test');
     }
 
     public function test_createAppSpecificTokenAuth_withExpireDate()
     {
         $this->model->deleteAllTokensForUser('login1');
-        $token = $this->api->createAppSpecificTokenAuth('login1', md5('password'), 'test', '2026-01-02 03:04:05');
+        $token = $this->api->createAppSpecificTokenAuth('login1', 'password', 'test', '2026-01-02 03:04:05');
         $this->assertMd5($token);
 
         $tokens = $this->model->getAllNonSystemTokensForLogin('login1');
@@ -138,7 +129,7 @@ class ApiTest extends SystemTestCase
     {
         $expireInHours = 48;
         $this->model->deleteAllTokensForUser('login1');
-        $token = $this->api->createAppSpecificTokenAuth('login1', md5('password'), 'test', null, $expireInHours);
+        $token = $this->api->createAppSpecificTokenAuth('login1', 'password', 'test', null, $expireInHours);
         $this->assertMd5($token);
 
         $tokens = $this->model->getAllNonSystemTokensForLogin('login1');