Merge "Include bot password app ID in audit data"

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index 882e72d..5b6f6b2 100644 (file)
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -37,6 +37,8 @@ production.
 * (T96041) __EXPECTUNUSEDCATEGORY__ on a category page causes the category
   to be hidden on Special:UnusedCategories.
 * Add PasswordPolicy to check the password isn't in the large blacklist.
+* The AuthManagerLoginAuthenticateAudit hook has a new parameter for
+  additional information about the authentication event.
 * …
 
 === External library changes in 1.33 ===

diff --git a/docs/hooks.txt b/docs/hooks.txt
index a7ff453..28065fc 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -786,7 +786,11 @@ $response: The MediaWiki\Auth\AuthenticationResponse in either a PASS or FAIL
 $user: The User object being authenticated against, or null if authentication
   failed before getting that far.
 $username: A guess at the user name being authenticated, or null if we can't
-  even determine that.
+  even determine that. When $user is not null, it can be in the form of
+  <username>@<more info> (e.g. for bot passwords).
+$extraData: An array (string => string) with extra information, intended to be
+  added to log contexts. Fields it might include:
+  - appId: the application ID, only if the login was with a bot password
 
 'AuthPluginAutoCreate': DEPRECATED since 1.27! Use the 'LocalUserCreated' hook
 instead. Called when creating a local account for an user logged in from an

diff --git a/includes/auth/AuthManager.php b/includes/auth/AuthManager.php
index aae5a83..e82f2b8 100644 (file)
--- a/includes/auth/AuthManager.php
+++ b/includes/auth/AuthManager.php
@@ -336,7 +336,7 @@ class AuthManager implements LoggerAwareInterface {
                        $this->setSessionDataForUser( $user );
                        $this->callMethodOnProviders( 7, 'postAuthentication', [ $user, $ret ] );
                        $session->remove( 'AuthManager::authnState' );
-                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, $user, $user->getName() ] );
+                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, $user, $user->getName(), [] ] );
                        return $ret;
                }
 
@@ -352,7 +352,7 @@ class AuthManager implements LoggerAwareInterface {
                                $this->callMethodOnProviders( 7, 'postAuthentication',
                                        [ User::newFromName( $guessUserName ) ?: null, $ret ]
                                );
-                               \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, null, $guessUserName ] );
+                               \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, null, $guessUserName, [] ] );
                                return $ret;
                        }
                }
@@ -468,7 +468,7 @@ class AuthManager implements LoggerAwareInterface {
                                                                [ User::newFromName( $guessUserName ) ?: null, $res ]
                                                        );
                                                        $session->remove( 'AuthManager::authnState' );
-                                                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $res, null, $guessUserName ] );
+                                                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $res, null, $guessUserName, [] ] );
                                                        return $res;
                                                case AuthenticationResponse::ABSTAIN;
                                                        // Continue loop
@@ -534,7 +534,7 @@ class AuthManager implements LoggerAwareInterface {
                                                        [ User::newFromName( $guessUserName ) ?: null, $res ]
                                                );
                                                $session->remove( 'AuthManager::authnState' );
-                                               \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $res, null, $guessUserName ] );
+                                               \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $res, null, $guessUserName, [] ] );
                                                return $res;
                                        case AuthenticationResponse::REDIRECT;
                                        case AuthenticationResponse::UI;
@@ -625,7 +625,7 @@ class AuthManager implements LoggerAwareInterface {
                                        );
                                        $this->callMethodOnProviders( 7, 'postAuthentication', [ $user, $ret ] );
                                        $session->remove( 'AuthManager::authnState' );
-                                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, $user, $user->getName() ] );
+                                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, $user, $user->getName(), [] ] );
                                        return $ret;
                                }
                        }
@@ -658,7 +658,7 @@ class AuthManager implements LoggerAwareInterface {
                                                $this->logger->debug( "Login failed in secondary authentication by $id" );
                                                $this->callMethodOnProviders( 7, 'postAuthentication', [ $user, $res ] );
                                                $session->remove( 'AuthManager::authnState' );
-                                               \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $res, $user, $user->getName() ] );
+                                               \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $res, $user, $user->getName(), [] ] );
                                                return $res;
                                        case AuthenticationResponse::REDIRECT;
                                        case AuthenticationResponse::UI;
@@ -694,7 +694,7 @@ class AuthManager implements LoggerAwareInterface {
                        $this->callMethodOnProviders( 7, 'postAuthentication', [ $user, $ret ] );
                        $session->remove( 'AuthManager::authnState' );
                        $this->removeAuthenticationSessionData( null );
-                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, $user, $user->getName() ] );
+                       \Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $ret, $user, $user->getName(), [] ] );
                        return $ret;
                } catch ( \Exception $ex ) {
                        $session->remove( 'AuthManager::authnState' );

diff --git a/includes/user/BotPassword.php b/includes/user/BotPassword.php
index 187c0a9..b83c209 100644 (file)
--- a/includes/user/BotPassword.php
+++ b/includes/user/BotPassword.php
@@ -459,14 +459,14 @@ class BotPassword implements IDBAccessObject {
                // Split name into name+appId
                $sep = self::getSeparator();
                if ( strpos( $username, $sep ) === false ) {
-                       return self::loginHook( $username, Status::newFatal( 'botpasswords-invalid-name', $sep ) );
+                       return self::loginHook( $username, null, Status::newFatal( 'botpasswords-invalid-name', $sep ) );
                }
                list( $name, $appId ) = explode( $sep, $username, 2 );
 
                // Find the named user
                $user = User::newFromName( $name );
                if ( !$user || $user->isAnon() ) {
-                       return self::loginHook( $user ?: $name, Status::newFatal( 'nosuchuser', $name ) );
+                       return self::loginHook( $user ?: $name, null, Status::newFatal( 'nosuchuser', $name ) );
                }
 
                if ( $user->isLocked() ) {
@@ -483,39 +483,39 @@ class BotPassword implements IDBAccessObject {
                        $result = $throttle->increase( $user->getName(), $request->getIP(), __METHOD__ );
                        if ( $result ) {
                                $msg = wfMessage( 'login-throttled' )->durationParams( $result['wait'] );
-                               return self::loginHook( $user, Status::newFatal( $msg ) );
+                               return self::loginHook( $user, null, Status::newFatal( $msg ) );
                        }
                }
 
                // Get the bot password
                $bp = self::newFromUser( $user, $appId );
                if ( !$bp ) {
-                       return self::loginHook( $user, Status::newFatal( 'botpasswords-not-exist', $name, $appId ) );
+                       return self::loginHook( $user, $bp,
+                               Status::newFatal( 'botpasswords-not-exist', $name, $appId ) );
                }
 
                // Check restrictions
                $status = $bp->getRestrictions()->check( $request );
                if ( !$status->isOK() ) {
-                       return self::loginHook( $user, Status::newFatal( 'botpasswords-restriction-failed' ) );
+                       return self::loginHook( $user, $bp, Status::newFatal( 'botpasswords-restriction-failed' ) );
                }
 
                // Check the password
                $passwordObj = $bp->getPassword();
                if ( $passwordObj instanceof InvalidPassword ) {
-                       return self::loginHook( $user, Status::newFatal( 'botpasswords-needs-reset', $name, $appId ) );
+                       return self::loginHook( $user, $bp,
+                               Status::newFatal( 'botpasswords-needs-reset', $name, $appId ) );
                }
                if ( !$passwordObj->equals( $password ) ) {
-                       return self::loginHook( $user, Status::newFatal( 'wrongpassword' ) );
+                       return self::loginHook( $user, $bp, Status::newFatal( 'wrongpassword' ) );
                }
 
                // Ok! Create the session.
                if ( $throttle ) {
                        $throttle->clear( $user->getName(), $request->getIP() );
                }
-               return self::loginHook(
-                       $user,
-                       Status::newGood( $provider->newSessionForRequest( $user, $bp, $request ) )
-               );
+               return self::loginHook( $user, $bp,
+                       Status::newGood( $provider->newSessionForRequest( $user, $bp, $request ) ) );
        }
 
        /**
@@ -525,12 +525,17 @@ class BotPassword implements IDBAccessObject {
         * AuthManager, call the AuthManagerLoginAuthenticateAudit hook.
         *
         * @param User|string $user User being logged in
+        * @param BotPassword|null $bp Bot sub-account, if it can be identified
         * @param Status $status Login status
         * @return Status The passed-in status
         */
-       private static function loginHook( $user, Status $status ) {
+       private static function loginHook( $user, $bp, Status $status ) {
+               $extraData = [];
                if ( $user instanceof User ) {
                        $name = $user->getName();
+                       if ( $bp ) {
+                               $extraData['appId'] = $name . self::getSeparator() . $bp->getAppId();
+                       }
                } else {
                        $name = $user;
                        $user = null;
@@ -541,7 +546,7 @@ class BotPassword implements IDBAccessObject {
                } else {
                        $response = AuthenticationResponse::newFail( $status->getMessage() );
                }
-               Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $response, $user, $name ] );
+               Hooks::run( 'AuthManagerLoginAuthenticateAudit', [ $response, $user, $name, $extraData ] );
 
                return $status;
        }