Allow putting the app ID in the password for bot passwords

Bot passwords allow backwards-compatible login (with grants, for API
usage only) with "<real username>@<botname>" for username plus a
random-generated password.
This doesn't work well with some bot frameworks (including Pywikibot,
the most popular one) which assume that the text that goes into the
username field of the login API is the username that they will be
logged in with afterwards (and so the @-postfix causes all kinds of
errors).

Since the goal of bot passwords is compatibility with old unmaintained
API clients, this patch adds an alternative format which does not
cause problems with old bots: use the username normally, and use
"<botname>@<random-generated password>" as password. Since this is
technically a valid normal password, there is some ambiguity, but
bot passwords have a distintive format so it's easy to check and it is
extremely unlikely that someone would use the exact same format for
their normal password; and if the bot password login fails we can
simply retry it as a normal password, just in case.

Bug: T142304
Change-Id: Ib59a6fbe0e65d80d5e7d19ff37cec5e011c00539

diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php
index 28937f7..5f6e34a 100644 (file)
--- a/includes/api/ApiLogin.php
+++ b/includes/api/ApiLogin.php
@@ -102,17 +102,18 @@ class ApiLogin extends ApiBase {
                }
 
                // Try bot passwords
-               if ( $authRes === false && $this->getConfig()->get( 'EnableBotPasswords' ) &&
-                       strpos( $params['name'], BotPassword::getSeparator() ) !== false
+               if (
+                       $authRes === false && $this->getConfig()->get( 'EnableBotPasswords' ) &&
+                       ( $botLoginData = BotPassword::canonicalizeLoginData( $params['name'], $params['password'] ) )
                ) {
                        $status = BotPassword::login(
-                               $params['name'], $params['password'], $this->getRequest()
+                               $botLoginData[0], $botLoginData[1], $this->getRequest()
                        );
                        if ( $status->isOK() ) {
                                $session = $status->getValue();
                                $authRes = 'Success';
                                $loginType = 'BotPassword';
-                       } else {
+                       } elseif ( !$botLoginData[2] ) {
                                $authRes = 'Failed';
                                $message = $status->getMessage();
                                LoggerFactory::getInstance( 'authentication' )->info(

diff --git a/includes/specials/SpecialBotPasswords.php b/includes/specials/SpecialBotPasswords.php
index 7e330aa..f2ea3e4 100644 (file)
--- a/includes/specials/SpecialBotPasswords.php
+++ b/includes/specials/SpecialBotPasswords.php
@@ -290,9 +290,7 @@ class SpecialBotPasswords extends FormSpecialPage {
                ] );
 
                if ( $this->operation === 'insert' || !empty( $data['resetPassword'] ) ) {
-                       $this->password = PasswordFactory::generateRandomPasswordString(
-                               max( 32, $this->getConfig()->get( 'MinimalPasswordLength' ) )
-                       );
+                       $this->password = BotPassword::generatePassword( $this->getConfig() );
                        $passwordFactory = new PasswordFactory();
                        $passwordFactory->init( RequestContext::getMain()->getConfig() );
                        $password = $passwordFactory->newFromPlaintext( $this->password );
@@ -335,7 +333,9 @@ class SpecialBotPasswords extends FormSpecialPage {
                        $out->addWikiMsg(
                                'botpasswords-newpassword',
                                htmlspecialchars( $username . $sep . $this->par ),
-                               htmlspecialchars( $this->password )
+                               htmlspecialchars( $this->password ),
+                               htmlspecialchars( $username ),
+                               htmlspecialchars( $this->par . $sep . $this->password )
                        );
                        $this->password = null;
                }

diff --git a/includes/user/BotPassword.php b/includes/user/BotPassword.php
index 49a7163..df1cb77 100644 (file)
--- a/includes/user/BotPassword.php
+++ b/includes/user/BotPassword.php
@@ -388,6 +388,44 @@ class BotPassword implements IDBAccessObject {
                return (bool)$dbw->affectedRows();
        }
 
+       /**
+        * Returns a (raw, unhashed) random password string.
+        * @param Config $config
+        * @return string
+        */
+       public static function generatePassword( $config ) {
+               return PasswordFactory::generateRandomPasswordString(
+                       max( 32, $config->get( 'MinimalPasswordLength' ) ) );
+       }
+
+       /**
+        * There are two ways to login with a bot password: "username@appId", "password" and
+        * "username", "appId@password". Transform it so it is always in the first form.
+        * Returns [bot username, bot password, could be normal password?] where the last one is a flag
+        * meaning this could either be a bot password or a normal password, it cannot be decided for
+        * certain (although in such cases it almost always will be a bot password).
+        * If this cannot be a bot password login just return false.
+        * @param string $username
+        * @param string $password
+        * @return array|false
+        */
+       public static function canonicalizeLoginData( $username, $password ) {
+               $sep = BotPassword::getSeparator();
+               if ( strpos( $username, $sep ) !== false ) {
+                       // the separator is not valid in usernames so this must be a bot login
+                       return [ $username, $password, false ];
+               } elseif ( strlen( $password ) > 32 && strpos( $password, $sep ) !== false ) {
+                       // the strlen check helps minimize the password information obtainable from timing
+                       $segments = explode( $sep, $password );
+                       $password = array_pop( $segments );
+                       $appId = implode( $sep, $segments );
+                       if ( preg_match( '/^[0-9a-w]{32,}$/', $password ) ) {
+                               return [ $username . $sep . $appId, $password, true ];
+                       }
+               }
+               return false;
+       }
+
        /**
         * Try to log the user in
         * @param string $username Combined user name and app ID

diff --git a/languages/i18n/en.json b/languages/i18n/en.json
index e1c37c8..7b424ee 100644 (file)
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -571,7 +571,7 @@
        "botpasswords-updated-body": "The bot password for bot name \"$1\" of user \"$2\" was updated.",
        "botpasswords-deleted-title": "Bot password deleted",
        "botpasswords-deleted-body": "The bot password for bot name \"$1\" of user \"$2\" was deleted.",
-       "botpasswords-newpassword": "The new password to log in with <strong>$1</strong> is <strong>$2</strong>. <em>Please record this for future reference.</em>",
+       "botpasswords-newpassword": "The new password to log in with <strong>$1</strong> is <strong>$2</strong>. <em>Please record this for future reference.</em> <br> (For old bots which require the login name to be the same as the eventual username, you can also use <strong>$3</strong> as username and <strong>$4</strong> as password.)",
        "botpasswords-no-provider": "BotPasswordsSessionProvider is not available.",
        "botpasswords-restriction-failed": "Bot password restrictions prevent this login.",
        "botpasswords-invalid-name": "The username specified does not contain the bot password separator (\"$1\").",

diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index 6a3d604..e027618 100644 (file)
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -754,7 +754,7 @@
        "botpasswords-updated-body": "Success message when a bot password is updated. Parameters:\n* $1 - Bot name\n* $2 - User name",
        "botpasswords-deleted-title": "Title of the success page when a bot password is deleted.",
        "botpasswords-deleted-body": "Success message when a bot password is deleted. Parameters:\n* $1 - Bot name\n* $2 - User name",
-       "botpasswords-newpassword": "Success message to display the new password when a bot password is created or updated. Parameters:\n* $1 - User name to be used for login.\n* $2 - Password to be used for login.",
+       "botpasswords-newpassword": "Success message to display the new password when a bot password is created or updated. Parameters:\n* $1 - User name to be used for login.\n* $2 - Password to be used for login.\n* $3, $4 - an alternative version of the user name and password, respectively, which is less preferred, but more compatible with old bots.",
        "botpasswords-no-provider": "Error message when login is attempted but the BotPasswordsSessionProvider is not included in <code>$wgSessionProviders</code>.",
        "botpasswords-restriction-failed": "Error message when login is rejected because the configured restrictions were not satisfied.",
        "botpasswords-invalid-name": "Error message when a username lacking the separator character is passed to BotPassword. Parameters:\n* $1 - The separator character.",

diff --git a/tests/phpunit/includes/user/BotPasswordTest.php b/tests/phpunit/includes/user/BotPasswordTest.php
index cfd5f78..d637704 100644 (file)
--- a/tests/phpunit/includes/user/BotPasswordTest.php
+++ b/tests/phpunit/includes/user/BotPasswordTest.php
@@ -228,6 +228,33 @@ class BotPasswordTest extends MediaWikiTestCase {
                $this->assertNotNull( BotPassword::newFromCentralId( 43, 'BotPassword' ) );
        }
 
+       /**
+        * @dataProvider provideCanonicalizeLoginData
+        */
+       public function testCanonicalizeLoginData( $username, $password, $expectedResult ) {
+               $result = BotPassword::canonicalizeLoginData( $username, $password );
+               if ( is_array( $expectedResult ) ) {
+                       $this->assertArrayEquals( $expectedResult, $result, true, true );
+               } else {
+                       $this->assertSame( $expectedResult, $result );
+               }
+       }
+
+       public function provideCanonicalizeLoginData() {
+               return [
+                       [ 'user', 'pass', false ],
+                       [ 'user', 'abc@def', false ],
+                       [ 'user@bot', '12345678901234567890123456789012',
+                               [ 'user@bot', '12345678901234567890123456789012', false ] ],
+                       [ 'user', 'bot@12345678901234567890123456789012',
+                               [ 'user@bot', '12345678901234567890123456789012', true ] ],
+                       [ 'user', 'bot@12345678901234567890123456789012345',
+                               [ 'user@bot', '12345678901234567890123456789012345', true ] ],
+                       [ 'user', 'bot@x@12345678901234567890123456789012',
+                               [ 'user@bot@x', '12345678901234567890123456789012', true ] ],
+               ];
+       }
+
        public function testLogin() {
                // Test failure when bot passwords aren't enabled
                $this->setMwGlobals( 'wgEnableBotPasswords', false );