Add PasswordFactory to MediaWikiServices

Instead of having basically every caller do:
 $pf = new PasswordFactory();
 $pf->init( RequestContext::getMain()->getConfig() );
Just create a single PasswordFactory via MediaWikiServices and pass that
around. Things that want to use their own config can still pass settings
via the new constructor.

This will eventually let us remove the init() function, removing the
only hard dependency upon MediaWiki, to make it easier to librarize
(T89742).

Change-Id: I0fc7520dc023b11a7fa66083eff7b88ebfe49c7b

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index 006476f..ae7f68c 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -337,6 +337,8 @@ because of Phabricator reports.
 * The wfUseMW function, soft-deprecated in 1.26, is now hard deprecated.
 * All MagicWord static methods are now deprecated.  Use the MagicWordFactory
   methods instead.
+* PasswordFactory::init is deprecated. To get a password factory with the
+  standard configuration, use MediaWikiServices::getPasswordFactory.
 
 === Other changes in 1.32 ===
 * (T198811) The following tables have had their UNIQUE indexes turned into

diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index 9726e6c..472a453 100644 (file)
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -38,6 +38,7 @@ use MimeAnalyzer;
 use ObjectCache;
 use Parser;
 use ParserCache;
+use PasswordFactory;
 use ProxyLookup;
 use SearchEngine;
 use SearchEngineConfig;
@@ -881,6 +882,14 @@ class MediaWikiServices extends ServiceContainer {
                return $this->getService( 'ContentLanguage' );
        }
 
+       /**
+        * @since 1.32
+        * @return PasswordFactory
+        */
+       public function getPasswordFactory() {
+               return $this->getService( 'PasswordFactory' );
+       }
+
        ///////////////////////////////////////////////////////////////////////////
        // NOTE: When adding a service getter here, don't forget to add a test
        // case for it in MediaWikiServicesTest::provideGetters() and in

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 92b2411..853b06d 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -604,6 +604,14 @@ return [
                return Language::factory( $services->getMainConfig()->get( 'LanguageCode' ) );
        },
 
+       'PasswordFactory' => function ( MediaWikiServices $services ) {
+               $config = $services->getMainConfig();
+               return new PasswordFactory(
+                       $config->get( 'PasswordConfig' ),
+                       $config->get( 'PasswordDefault' )
+               );
+       },
+
        ///////////////////////////////////////////////////////////////////////////
        // NOTE: When adding a service here, don't forget to add a getter function
        // in the MediaWikiServices class. The convenience getter should just call

diff --git a/includes/auth/AbstractPasswordPrimaryAuthenticationProvider.php b/includes/auth/AbstractPasswordPrimaryAuthenticationProvider.php
index 3d26767..4096f19 100644 (file)
--- a/includes/auth/AbstractPasswordPrimaryAuthenticationProvider.php
+++ b/includes/auth/AbstractPasswordPrimaryAuthenticationProvider.php
@@ -53,8 +53,10 @@ abstract class AbstractPasswordPrimaryAuthenticationProvider
         */
        protected function getPasswordFactory() {
                if ( $this->passwordFactory === null ) {
-                       $this->passwordFactory = new PasswordFactory();
-                       $this->passwordFactory->init( $this->config );
+                       $this->passwordFactory = new PasswordFactory(
+                               $this->config->get( 'PasswordConfig' ),
+                               $this->config->get( 'PasswordDefault' )
+                       );
                }
                return $this->passwordFactory;
        }

diff --git a/includes/password/PasswordFactory.php b/includes/password/PasswordFactory.php
index bc37b48..f7b283b 100644 (file)
--- a/includes/password/PasswordFactory.php
+++ b/includes/password/PasswordFactory.php
@@ -36,6 +36,7 @@ final class PasswordFactory {
 
        /**
         * Mapping of password types to classes
+        *
         * @var array
         * @see PasswordFactory::register
         * @see Setup.php
@@ -44,11 +45,31 @@ final class PasswordFactory {
                '' => [ 'type' => '', 'class' => InvalidPassword::class ],
        ];
 
+       /**
+        * Construct a new password factory.
+        * Most of the time you'll want to use MediaWikiServices::getPasswordFactory instead.
+        * @param array $config Mapping of password type => config
+        * @param string $default Default password type
+        * @see PasswordFactory::register
+        * @see PasswordFactory::setDefaultType
+        */
+       public function __construct( array $config = [], $default = '' ) {
+               foreach ( $config as $type => $options ) {
+                       $this->register( $type, $options );
+               }
+
+               if ( $default !== '' ) {
+                       $this->setDefaultType( $default );
+               }
+       }
+
        /**
         * Register a new type of password hash
         *
-        * @param string $type Unique type name for the hash
-        * @param array $config Array of configuration options
+        * @param string $type Unique type name for the hash. Will be prefixed to the password hashes
+        *   to identify what hashing method was used.
+        * @param array $config Array of configuration options. 'class' is required (the Password
+        *   subclass name), everything else is passed to the constructor of that class.
         */
        public function register( $type, array $config ) {
                $config['type'] = $type;
@@ -58,8 +79,11 @@ final class PasswordFactory {
        /**
         * Set the default password type
         *
-        * @throws InvalidArgumentException If the type is not registered
+        * This type will be used for creating new passwords when the type is not specified.
+        * Passwords of a different type will be considered outdated and in need of update.
+        *
         * @param string $type Password hash type
+        * @throws InvalidArgumentException If the type is not registered
         */
        public function setDefaultType( $type ) {
                if ( !isset( $this->types[$type] ) ) {
@@ -78,6 +102,8 @@ final class PasswordFactory {
        }
 
        /**
+        * @deprecated since 1.32 Initialize settings using the constructor
+        *
         * Initialize the internal static variables using the global variables
         *
         * @param Config $config Configuration object to load data from

diff --git a/includes/specials/SpecialBotPasswords.php b/includes/specials/SpecialBotPasswords.php
index 2d3a0cc..2d62d8f 100644 (file)
--- a/includes/specials/SpecialBotPasswords.php
+++ b/includes/specials/SpecialBotPasswords.php
@@ -22,6 +22,7 @@
  */
 
 use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
 
 /**
  * Let users manage bot passwords
@@ -166,8 +167,7 @@ class SpecialBotPasswords extends FormSpecialPage {
 
                } else {
                        $linkRenderer = $this->getLinkRenderer();
-                       $passwordFactory = new PasswordFactory();
-                       $passwordFactory->init( $this->getConfig() );
+                       $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
 
                        $dbr = BotPassword::getDB( DB_REPLICA );
                        $res = $dbr->select(
@@ -321,8 +321,7 @@ class SpecialBotPasswords extends FormSpecialPage {
 
                if ( $this->operation === 'insert' || !empty( $data['resetPassword'] ) ) {
                        $this->password = BotPassword::generatePassword( $this->getConfig() );
-                       $passwordFactory = new PasswordFactory();
-                       $passwordFactory->init( RequestContext::getMain()->getConfig() );
+                       $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
                        $password = $passwordFactory->newFromPlaintext( $this->password );
                } else {
                        $password = null;

diff --git a/includes/user/BotPassword.php b/includes/user/BotPassword.php
index 960a486..2228feb 100644 (file)
--- a/includes/user/BotPassword.php
+++ b/includes/user/BotPassword.php
@@ -250,8 +250,7 @@ class BotPassword implements IDBAccessObject {
                        return PasswordFactory::newInvalidPassword();
                }
 
-               $passwordFactory = new \PasswordFactory();
-               $passwordFactory->init( \RequestContext::getMain()->getConfig() );
+               $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
                try {
                        return $passwordFactory->newFromCiphertext( $password );
                } catch ( PasswordError $ex ) {

diff --git a/maintenance/wrapOldPasswords.php b/maintenance/wrapOldPasswords.php
index 1fc0f37..ef9e46e 100644 (file)
--- a/maintenance/wrapOldPasswords.php
+++ b/maintenance/wrapOldPasswords.php
@@ -1,7 +1,4 @@
 <?php
-
-use MediaWiki\MediaWikiServices;
-
 /**
  * Maintenance script to wrap all old-style passwords in a layered type
  *
@@ -23,8 +20,11 @@ use MediaWiki\MediaWikiServices;
  * @file
  * @ingroup Maintenance
  */
+
 require_once __DIR__ . '/Maintenance.php';
 
+use MediaWiki\MediaWikiServices;
+
 /**
  * Maintenance script to wrap all passwords of a certain type in a specified layered
  * type that wraps around the old type.
@@ -43,8 +43,7 @@ class WrapOldPasswords extends Maintenance {
        }
 
        public function execute() {
-               $passwordFactory = new PasswordFactory();
-               $passwordFactory->init( RequestContext::getMain()->getConfig() );
+               $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
 
                $typeInfo = $passwordFactory->getTypes();
                $layeredType = $this->getOption( 'type' );

diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php
index 92f9752..ae71d9f 100644 (file)
--- a/tests/phpunit/includes/MediaWikiServicesTest.php
+++ b/tests/phpunit/includes/MediaWikiServicesTest.php
@@ -367,6 +367,7 @@ class MediaWikiServicesTest extends MediaWikiTestCase {
                        'ConfigRepository' => [ 'ConfigRepository', \MediaWiki\Config\ConfigRepository::class ],
                        'MagicWordFactory' => [ 'MagicWordFactory', MagicWordFactory::class ],
                        'ContentLanguage' => [ 'ContentLanguage', Language::class ],
+                       'PasswordFactory' => [ 'PasswordFactory', PasswordFactory::class ],
                ];
        }
 

diff --git a/tests/phpunit/includes/TestUser.php b/tests/phpunit/includes/TestUser.php
index 86f4ae7..952a662 100644 (file)
--- a/tests/phpunit/includes/TestUser.php
+++ b/tests/phpunit/includes/TestUser.php
@@ -1,5 +1,7 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
+
 /**
  * Wraps the user object, so we can also retain full access to properties
  * like password if we log in via the API.
@@ -140,8 +142,7 @@ class TestUser {
                        throw new MWException( "Passed User has an ID but is not in the database?" );
                }
 
-               $passwordFactory = new PasswordFactory();
-               $passwordFactory->init( RequestContext::getMain()->getConfig() );
+               $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
                if ( !$passwordFactory->newFromCiphertext( $row->user_password )->equals( $password ) ) {
                        $passwordHash = $passwordFactory->newFromPlaintext( $password );
                        $dbw->update(

diff --git a/tests/phpunit/includes/api/ApiLoginTest.php b/tests/phpunit/includes/api/ApiLoginTest.php
index d382c83..384d779 100644 (file)
--- a/tests/phpunit/includes/api/ApiLoginTest.php
+++ b/tests/phpunit/includes/api/ApiLoginTest.php
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
 use Wikimedia\TestingAccessWrapper;
 
 /**
@@ -233,8 +234,7 @@ class ApiLoginTest extends ApiTestCase {
                $this->assertNotEquals( 0, $centralId, 'sanity check' );
 
                $password = 'ngfhmjm64hv0854493hsj5nncjud2clk';
-               $passwordFactory = new PasswordFactory();
-               $passwordFactory->init( RequestContext::getMain()->getConfig() );
+               $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
                // A is unsalted MD5 (thus fast) ... we don't care about security here, this is test only
                $passwordHash = $passwordFactory->newFromPlaintext( $password );
 

diff --git a/tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php b/tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php
index 1708f1c..8863aa2 100644 (file)
--- a/tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php
+++ b/tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php
@@ -128,11 +128,10 @@ class TemporaryPasswordPrimaryAuthenticationProviderTest extends \MediaWikiTestC
                $user = self::getMutableTestUser()->getUser();
 
                $dbw = wfGetDB( DB_MASTER );
-
-               $passwordFactory = new \PasswordFactory();
-               $passwordFactory->init( \RequestContext::getMain()->getConfig() );
+               $config = MediaWikiServices::getInstance()->getMainConfig();
                // A is unsalted MD5 (thus fast) ... we don't care about security here, this is test only
-               $passwordFactory->setDefaultType( 'A' );
+               $passwordFactory = new \PasswordFactory( $config->get( 'PasswordConfig' ), 'A' );
+
                $pwhash = $passwordFactory->newFromPlaintext( 'password' )->toString();
 
                $provider = $this->getProvider();

diff --git a/tests/phpunit/includes/password/PasswordFactoryTest.php b/tests/phpunit/includes/password/PasswordFactoryTest.php
index 01b0de2..a7b3557 100644 (file)
--- a/tests/phpunit/includes/password/PasswordFactoryTest.php
+++ b/tests/phpunit/includes/password/PasswordFactoryTest.php
@@ -4,6 +4,20 @@
  * @covers PasswordFactory
  */
 class PasswordFactoryTest extends MediaWikiTestCase {
+       public function testConstruct() {
+               $pf = new PasswordFactory();
+               $this->assertEquals( [ '' ], array_keys( $pf->getTypes() ) );
+               $this->assertEquals( '', $pf->getDefaultType() );
+
+               $pf = new PasswordFactory( [
+                       'foo' => [ 'class' => 'FooPassword' ],
+                       'bar' => [ 'class' => 'BarPassword', 'baz' => 'boom' ],
+               ], 'foo' );
+               $this->assertEquals( [ '', 'foo', 'bar' ], array_keys( $pf->getTypes() ) );
+               $this->assertArraySubset( [ 'class' => 'BarPassword', 'baz' => 'boom' ], $pf->getTypes()['bar'] );
+               $this->assertEquals( 'foo', $pf->getDefaultType() );
+       }
+
        public function testRegister() {
                $pf = new PasswordFactory;
                $pf->register( 'foo', [ 'class' => InvalidPassword::class ] );

diff --git a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
index 4767994..2298056 100644 (file)
--- a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
+++ b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
@@ -2,6 +2,7 @@
 
 namespace MediaWiki\Session;
 
+use MediaWiki\MediaWikiServices;
 use Psr\Log\LogLevel;
 use MediaWikiTestCase;
 use Wikimedia\TestingAccessWrapper;
@@ -63,8 +64,7 @@ class BotPasswordSessionProviderTest extends MediaWikiTestCase {
        }
 
        public function addDBDataOnce() {
-               $passwordFactory = new \PasswordFactory();
-               $passwordFactory->init( \RequestContext::getMain()->getConfig() );
+               $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
                $passwordHash = $passwordFactory->newFromPlaintext( 'foobaz' );
 
                $sysop = static::getTestSysop()->getUser();

diff --git a/tests/phpunit/includes/user/BotPasswordTest.php b/tests/phpunit/includes/user/BotPasswordTest.php
index 3bbc2df..0d22b21 100644 (file)
--- a/tests/phpunit/includes/user/BotPasswordTest.php
+++ b/tests/phpunit/includes/user/BotPasswordTest.php
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
 use MediaWiki\Session\SessionManager;
 use Wikimedia\ScopedCallback;
 use Wikimedia\TestingAccessWrapper;
@@ -59,8 +60,7 @@ class BotPasswordTest extends MediaWikiTestCase {
        }
 
        public function addDBData() {
-               $passwordFactory = new \PasswordFactory();
-               $passwordFactory->init( \RequestContext::getMain()->getConfig() );
+               $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
                $passwordHash = $passwordFactory->newFromPlaintext( 'foobaz' );
 
                $dbw = wfGetDB( DB_MASTER );
@@ -350,8 +350,7 @@ class BotPasswordTest extends MediaWikiTestCase {
         * @param string|null $password
         */
        public function testSave( $password ) {
-               $passwordFactory = new \PasswordFactory();
-               $passwordFactory->init( \RequestContext::getMain()->getConfig() );
+               $passwordFactory = MediaWikiServices::getInstance()->getPasswordFactory();
 
                $bp = BotPassword::newUnsaved( [
                        'centralId' => 42,