AuthManager: Don't invalidate BotPasswords if a password reset email is sent

There's a difference between addition of credentials, which doesn't
need to invaliate BotPasswords, and changing or removal of credentials,
which does.

It seems most straightforward for the caller of
AuthManager::changeAuthenticationData() to know which is intended, so
let's add a flag there.

Bug: T199809
Change-Id: Ib8405734e605b94f3f0b66596ad95784cb365e4f

diff --git a/includes/auth/AuthManager.php b/includes/auth/AuthManager.php
index 611a8cd..f54224b 100644 (file)
--- a/includes/auth/AuthManager.php
+++ b/includes/auth/AuthManager.php
@@ -882,8 +882,11 @@ class AuthManager implements LoggerAwareInterface {
         * returned success.
         *
         * @param AuthenticationRequest $req
+        * @param bool $isAddition Set true if this represents an addition of
+        *  credentials rather than a change. The main difference is that additions
+        *  should not invalidate BotPasswords. If you're not sure, leave it false.
         */
-       public function changeAuthenticationData( AuthenticationRequest $req ) {
+       public function changeAuthenticationData( AuthenticationRequest $req, $isAddition = false ) {
                $this->logger->info( 'Changing authentication data for {user} class {what}', [
                        'user' => is_string( $req->username ) ? $req->username : '<no name>',
                        'what' => get_class( $req ),
@@ -893,7 +896,9 @@ class AuthManager implements LoggerAwareInterface {
 
                // When the main account's authentication data is changed, invalidate
                // all BotPasswords too.
-               \BotPassword::invalidateAllPasswordsForUser( $req->username );
+               if ( !$isAddition ) {
+                       \BotPassword::invalidateAllPasswordsForUser( $req->username );
+               }
        }
 
        /**@}*/

diff --git a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php
index 7f121cd..c2d730c 100644 (file)
--- a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php
+++ b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php
@@ -119,7 +119,9 @@ class ConfirmLinkSecondaryAuthenticationProvider extends AbstractSecondaryAuthen
                                $status = $this->manager->allowsAuthenticationDataChange( $req );
                                $statuses[] = [ $req, $status ];
                                if ( $status->isGood() ) {
-                                       $this->manager->changeAuthenticationData( $req );
+                                       // We're not changing credentials, just adding a new link
+                                       // to an already-known user.
+                                       $this->manager->changeAuthenticationData( $req, /* $isAddition */ true );
                                } else {
                                        $anyFailed = true;
                                }

diff --git a/includes/user/PasswordReset.php b/includes/user/PasswordReset.php
index b5f7f6a..a1638ea 100644 (file)
--- a/includes/user/PasswordReset.php
+++ b/includes/user/PasswordReset.php
@@ -238,7 +238,9 @@ class PasswordReset implements LoggerAwareInterface {
                }
 
                foreach ( $reqs as $req ) {
-                       $this->authManager->changeAuthenticationData( $req );
+                       // This is adding a new temporary password, not intentionally changing anything
+                       // (even though it might technically invalidate an old temporary password).
+                       $this->authManager->changeAuthenticationData( $req, /* $isAddition */ true );
                }
 
                $this->logger->info(