Turn PasswordReset into a service

My team has plans to work in this area, better make it more testable.

Bug: T232694
Change-Id: I200874ec10db69378ada1743b2a7953b3fa01e3e
(cherry picked from commit 631f56c5766f582f21dd35eff0376b14692aa145)

diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34
index 48fcbaf..3f9ae9d 100644 (file)
--- a/RELEASE-NOTES-1.34
+++ b/RELEASE-NOTES-1.34
@@ -598,6 +598,7 @@ because of Phabricator reports.
   $wgGroupPermissions['sysop']['blockemail'] = true;
 * ApiQueryBase::showHiddenUsersAddBlockInfo() is deprecated. Use
   ApiQueryBlockInfoTrait instead.
+* PasswordReset is now a service, its direct instantiation is deprecated.
 
 === Other changes in 1.34 ===
 * Added option to specify "Various authors" as author in extension credits using

diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index 8f4ddf6..f1fa892 100644 (file)
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -19,6 +19,7 @@ use MediaWiki\Block\BlockRestrictionStore;
 use MediaWiki\FileBackend\FSFile\TempFSFileFactory;
 use MediaWiki\FileBackend\LockManager\LockManagerGroupFactory;
 use MediaWiki\Http\HttpRequestFactory;
+use PasswordReset;
 use Wikimedia\Message\IMessageFormatterFactory;
 use MediaWiki\Page\MovePageFactory;
 use MediaWiki\Permissions\PermissionManager;
@@ -818,6 +819,14 @@ class MediaWikiServices extends ServiceContainer {
                return $this->getService( 'PasswordFactory' );
        }
 
+       /**
+        * @since 1.34
+        * @return PasswordReset
+        */
+       public function getPasswordReset() : PasswordReset {
+               return $this->getService( 'PasswordReset' );
+       }
+
        /**
         * @since 1.32
         * @return StatsdDataFactoryInterface

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 83847d8..2c1725e 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -521,6 +521,17 @@ return [
                );
        },
 
+       'PasswordReset' => function ( MediaWikiServices $services ) : PasswordReset {
+               $options = new ServiceOptions( PasswordReset::$constructorOptions, $services->getMainConfig() );
+               return new PasswordReset(
+                       $options,
+                       AuthManager::singleton(),
+                       $services->getPermissionManager(),
+                       $services->getDBLoadBalancer(),
+                       LoggerFactory::getInstance( 'authentication' )
+               );
+       },
+
        'PerDbNameStatsdDataFactory' =>
        function ( MediaWikiServices $services ) : StatsdDataFactoryInterface {
                $config = $services->getMainConfig();

diff --git a/includes/api/ApiResetPassword.php b/includes/api/ApiResetPassword.php
index 6b1c217..6f13af2 100644 (file)
--- a/includes/api/ApiResetPassword.php
+++ b/includes/api/ApiResetPassword.php
@@ -20,7 +20,6 @@
  * @file
  */
 
-use MediaWiki\Auth\AuthManager;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -64,11 +63,7 @@ class ApiResetPassword extends ApiBase {
 
                $this->requireOnlyOneParameter( $params, 'user', 'email' );
 
-               $passwordReset = new PasswordReset(
-                       $this->getConfig(),
-                       AuthManager::singleton(),
-                       MediaWikiServices::getInstance()->getPermissionManager()
-               );
+               $passwordReset = MediaWikiServices::getInstance()->getPasswordReset();
 
                $status = $passwordReset->isAllowed( $this->getUser() );
                if ( !$status->isOK() ) {

diff --git a/includes/specialpage/LoginSignupSpecialPage.php b/includes/specialpage/LoginSignupSpecialPage.php
index 8be029a..86365c5 100644 (file)
--- a/includes/specialpage/LoginSignupSpecialPage.php
+++ b/includes/specialpage/LoginSignupSpecialPage.php
@@ -977,11 +977,7 @@ abstract class LoginSignupSpecialPage extends AuthManagerSpecialPage {
                        }
                }
                if ( !$this->isSignup() && $this->showExtraInformation() ) {
-                       $passwordReset = new PasswordReset(
-                               $this->getConfig(),
-                               AuthManager::singleton(),
-                               MediaWikiServices::getInstance()->getPermissionManager()
-                       );
+                       $passwordReset = MediaWikiServices::getInstance()->getPasswordReset();
                        if ( $passwordReset->isAllowed( $this->getUser() )->isGood() ) {
                                $fieldDefinitions['passwordReset'] = [
                                        'type' => 'info',

diff --git a/includes/specials/SpecialPasswordReset.php b/includes/specials/SpecialPasswordReset.php
index 2ef96ad..c1d30ee 100644 (file)
--- a/includes/specials/SpecialPasswordReset.php
+++ b/includes/specials/SpecialPasswordReset.php
@@ -21,7 +21,6 @@
  * @ingroup SpecialPage
  */
 
-use MediaWiki\Auth\AuthManager;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -53,11 +52,7 @@ class SpecialPasswordReset extends FormSpecialPage {
 
        private function getPasswordReset() {
                if ( $this->passwordReset === null ) {
-                       $this->passwordReset = new PasswordReset(
-                               $this->getConfig(),
-                               AuthManager::singleton(),
-                               MediaWikiServices::getInstance()->getPermissionManager()
-                       );
+                       $this->passwordReset = MediaWikiServices::getInstance()->getPasswordReset();
                }
                return $this->passwordReset;
        }

diff --git a/includes/user/PasswordReset.php b/includes/user/PasswordReset.php
index 38707de..8ef1d0d 100644 (file)
--- a/includes/user/PasswordReset.php
+++ b/includes/user/PasswordReset.php
@@ -22,10 +22,14 @@
 
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\Auth\TemporaryPasswordAuthenticationRequest;
+use MediaWiki\Config\ServiceOptions;
+use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
 use MediaWiki\Permissions\PermissionManager;
 use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
 use Psr\Log\LoggerInterface;
-use MediaWiki\Logger\LoggerFactory;
+use Wikimedia\Rdbms\ILoadBalancer;
 
 /**
  * Helper class for the password reset functionality shared by the web UI and the API.
@@ -35,17 +39,19 @@ use MediaWiki\Logger\LoggerFactory;
  * functionality) to be enabled.
  */
 class PasswordReset implements LoggerAwareInterface {
-       /** @var Config */
+       use LoggerAwareTrait;
+
+       /** @var ServiceOptions|Config */
        protected $config;
 
        /** @var AuthManager */
        protected $authManager;
 
        /** @var PermissionManager */
-       private $permissionManager;
+       protected $permissionManager;
 
-       /** @var LoggerInterface */
-       protected $logger;
+       /** @var ILoadBalancer */
+       protected $loadBalancer;
 
        /**
         * In-process cache for isAllowed lookups, by username.
@@ -54,26 +60,44 @@ class PasswordReset implements LoggerAwareInterface {
         */
        private $permissionCache;
 
+       public static $constructorOptions = [
+               'EnableEmail',
+               'PasswordResetRoutes',
+       ];
+
+       /**
+        * This class is managed by MediaWikiServices, don't instantiate directly.
+        *
+        * @param ServiceOptions|Config $config
+        * @param AuthManager $authManager
+        * @param PermissionManager $permissionManager
+        * @param ILoadBalancer|null $loadBalancer
+        * @param LoggerInterface|null $logger
+        */
        public function __construct(
-               Config $config,
+               $config,
                AuthManager $authManager,
-               PermissionManager $permissionManager
+               PermissionManager $permissionManager,
+               ILoadBalancer $loadBalancer = null,
+               LoggerInterface $logger = null
        ) {
                $this->config = $config;
                $this->authManager = $authManager;
                $this->permissionManager = $permissionManager;
-               $this->permissionCache = new MapCacheLRU( 1 );
-               $this->logger = LoggerFactory::getInstance( 'authentication' );
-       }
 
-       /**
-        * Set the logger instance to use.
-        *
-        * @param LoggerInterface $logger
-        * @since 1.29
-        */
-       public function setLogger( LoggerInterface $logger ) {
+               if ( !$loadBalancer ) {
+                       wfDeprecated( 'Not passing LoadBalancer to ' . __METHOD__, '1.34' );
+                       $loadBalancer = MediaWikiServices::getInstance()->getDBLoadBalancer();
+               }
+               $this->loadBalancer = $loadBalancer;
+
+               if ( !$logger ) {
+                       wfDeprecated( 'Not passing LoggerInterface to ' . __METHOD__, '1.34' );
+                       $logger = LoggerFactory::getInstance( 'authentication' );
+               }
                $this->logger = $logger;
+
+               $this->permissionCache = new MapCacheLRU( 1 );
        }
 
        /**
@@ -280,7 +304,7 @@ class PasswordReset implements LoggerAwareInterface {
         */
        protected function getUsersByEmail( $email ) {
                $userQuery = User::getQueryInfo();
-               $res = wfGetDB( DB_REPLICA )->select(
+               $res = $this->loadBalancer->getConnectionRef( DB_REPLICA )->select(
                        $userQuery['tables'],
                        $userQuery['fields'],
                        [ 'user_email' => $email ],

diff --git a/tests/phpunit/includes/user/PasswordResetTest.php b/tests/phpunit/includes/user/PasswordResetTest.php
index 4a7aa05..b5677fa 100644 (file)
--- a/tests/phpunit/includes/user/PasswordResetTest.php
+++ b/tests/phpunit/includes/user/PasswordResetTest.php
@@ -4,23 +4,32 @@ use MediaWiki\Auth\AuthManager;
 use MediaWiki\Block\DatabaseBlock;
 use MediaWiki\Block\CompositeBlock;
 use MediaWiki\Block\SystemBlock;
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Permissions\PermissionManager;
+use Psr\Log\NullLogger;
+use Wikimedia\Rdbms\ILoadBalancer;
 
 /**
  * @covers PasswordReset
  * @group Database
  */
 class PasswordResetTest extends MediaWikiTestCase {
+       private function makeConfig( $enableEmail, array $passwordResetRoutes = [] ) {
+               $hash = new HashConfig( [
+                       'EnableEmail' => $enableEmail,
+                       'PasswordResetRoutes' => $passwordResetRoutes,
+               ] );
+
+               return new ServiceOptions( PasswordReset::$constructorOptions, $hash );
+       }
+
        /**
         * @dataProvider provideIsAllowed
         */
        public function testIsAllowed( $passwordResetRoutes, $enableEmail,
                $allowsAuthenticationDataChange, $canEditPrivate, $block, $globalBlock, $isAllowed
        ) {
-               $config = new HashConfig( [
-                       'PasswordResetRoutes' => $passwordResetRoutes,
-                       'EnableEmail' => $enableEmail,
-               ] );
+               $config = $this->makeConfig( $enableEmail, $passwordResetRoutes );
 
                $authManager = $this->getMockBuilder( AuthManager::class )->disableOriginalConstructor()
                        ->getMock();
@@ -39,10 +48,14 @@ class PasswordResetTest extends MediaWikiTestCase {
                        ->with( $user, 'editmyprivateinfo' )
                        ->willReturn( $canEditPrivate );
 
+               $loadBalancer = $this->getMockBuilder( ILoadBalancer::class )->getMock();
+
                $passwordReset = new PasswordReset(
                        $config,
                        $authManager,
-                       $permissionManager
+                       $permissionManager,
+                       $loadBalancer,
+                       new NullLogger()
                );
 
                $this->assertSame( $isAllowed, $passwordReset->isAllowed( $user )->isGood() );
@@ -187,10 +200,7 @@ class PasswordResetTest extends MediaWikiTestCase {
        }
 
        public function testExecute_email() {
-               $config = new HashConfig( [
-                       'PasswordResetRoutes' => [ 'username' => true, 'email' => true ],
-                       'EnableEmail' => true,
-               ] );
+               $config = $this->makeConfig( true, [ 'username' => true, 'email' => true ] );
 
                // Unregister the hooks for proper unit testing
                $this->mergeMwGlobalArrayValue( 'wgHooks', [
@@ -204,6 +214,14 @@ class PasswordResetTest extends MediaWikiTestCase {
                        ->willReturn( Status::newGood() );
                $authManager->expects( $this->exactly( 2 ) )->method( 'changeAuthenticationData' );
 
+               $permissionManager = $this->getMockBuilder( PermissionManager::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $permissionManager->method( 'userHasRight' )->willReturn( true );
+
+               $loadBalancer = $this->getMockBuilder( ILoadBalancer::class )
+                       ->getMock();
+
                $request = new FauxRequest();
                $request->setIP( '1.2.3.4' );
                $performingUser = $this->getMockBuilder( User::class )->getMock();
@@ -228,8 +246,14 @@ class PasswordResetTest extends MediaWikiTestCase {
                $targetUser2->expects( $this->any() )->method( 'getEmail' )->willReturn( 'foo@bar.baz' );
 
                $passwordReset = $this->getMockBuilder( PasswordReset::class )
-                       ->setConstructorArgs( [ $config, $authManager, $permissionManager ] )
                        ->setMethods( [ 'getUsersByEmail' ] )
+                       ->setConstructorArgs( [
+                               $config,
+                               $authManager,
+                               $permissionManager,
+                               $loadBalancer,
+                               new NullLogger()
+                       ] )
                        ->getMock();
                $passwordReset->expects( $this->any() )->method( 'getUsersByEmail' )->with( 'foo@bar.baz' )
                        ->willReturn( [ $targetUser1, $targetUser2 ] );