Include bot password app ID in audit data

Authentication audit logs should indicate whether a login is via the
normal password or a bot password (and which one). For failed logins
it could be included in the error message, and it usually is, but
for successful ones there is no message, so we'll send the app ID as
a new AuthManagerLoginAuthenticateAudit parameter.

Bug: T194338
Change-Id: I8aab48177b81a8e80dae118e6476a8f6a32089f1
Depends-On: Id482d2e2205960a0facd334e456d3a23bcad0ece

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index f68875b..00088f2 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;
        }