From: Kunal Mehta Date: Sun, 2 Oct 2016 07:41:55 +0000 (-0700) Subject: Add PasswordFactory to MediaWikiServices X-Git-Tag: 1.34.0-rc.0~4577 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=fb73286fba73c399e119ef50ff036255dd6a1096 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 006476f7e8..ae7f68cd16 100644 --- 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 9726e6c171..472a453f6d 100644 --- 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 92b2411c43..853b06deba 100644 --- 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 3d26767a0e..4096f1971a 100644 --- 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 bc37b48404..f7b283be71 100644 --- 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 2d3a0ccbb5..2d62d8feea 100644 --- 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 960a48644d..2228feb3fe 100644 --- 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 1fc0f37ad8..ef9e46ed3a 100644 --- a/maintenance/wrapOldPasswords.php +++ b/maintenance/wrapOldPasswords.php @@ -1,7 +1,4 @@ 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 92f9752f74..ae71d9fac2 100644 --- 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 86f4ae789d..952a662fcd 100644 --- a/tests/phpunit/includes/TestUser.php +++ b/tests/phpunit/includes/TestUser.php @@ -1,5 +1,7 @@ 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 d382c83c18..384d7794e3 100644 --- a/tests/phpunit/includes/api/ApiLoginTest.php +++ b/tests/phpunit/includes/api/ApiLoginTest.php @@ -1,5 +1,6 @@ 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 1708f1c063..8863aa2d3f 100644 --- 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 01b0de2c25..a7b3557516 100644 --- 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 476799406f..2298056265 100644 --- 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 3bbc2dfaf2..0d22b21592 100644 --- a/tests/phpunit/includes/user/BotPasswordTest.php +++ b/tests/phpunit/includes/user/BotPasswordTest.php @@ -1,5 +1,6 @@ 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,