Inject AuthManager into some api classes

- ApiAMCreateAccount
- ApiClientLogin
- ApiChangeAuthenticationData
- ApiLinkAccount
- ApiLogin
- ApiRemoveAuthenticationData

Now that ApiLogin needs services injected,
update ApiModuleManagerTest to use ApiRsd
as the example api module that doesn't need
any services.

Bug: T259960
Change-Id: If78457f1d859d3d575f2016ef4b212263473cea6

includes/api/ApiAMCreateAccount.php

@@ -22,7 +22,6 @@
 
 use MediaWiki\Auth\AuthenticationResponse;
 use MediaWiki\Auth\AuthManager;
-use MediaWiki\MediaWikiServices;
 
 /**
  * Create an account with AuthManager
@@ -31,8 +30,21 @@ use MediaWiki\MediaWikiServices;
  */
 class ApiAMCreateAccount extends ApiBase {
 
-	public function __construct( ApiMain $main, $action ) {
+	/** @var AuthManager */
+	private $authManager;
+
+	/**
+	 * @param ApiMain $main
+	 * @param string $action
+	 * @param AuthManager $authManager
+	 */
+	public function __construct(
+		ApiMain $main,
+		$action,
+		AuthManager $authManager
+	) {
 		parent::__construct( $main, $action, 'create' );
+		$this->authManager = $authManager;
 	}
 
 	public function getFinalDescription() {
@@ -64,11 +76,10 @@ class ApiAMCreateAccount extends ApiBase {
 			}
 		}
 
-		$manager = MediaWikiServices::getInstance()->getAuthManager();
-		$helper = new ApiAuthManagerHelper( $this, $manager );
+		$helper = new ApiAuthManagerHelper( $this, $this->authManager );
 
 		// Make sure it's possible to create accounts
-		if ( !$manager->canCreateAccounts() ) {
+		if ( !$this->authManager->canCreateAccounts() ) {
 			$this->getResult()->addValue( null, 'createaccount', $helper->formatAuthenticationResponse(
 				AuthenticationResponse::newFail(
 					$this->msg( 'userlogin-cannot-' . AuthManager::ACTION_CREATE )
@@ -82,7 +93,7 @@ class ApiAMCreateAccount extends ApiBase {
 		// Perform the create step
 		if ( $params['continue'] ) {
 			$reqs = $helper->loadAuthenticationRequests( AuthManager::ACTION_CREATE_CONTINUE );
-			$res = $manager->continueAccountCreation( $reqs );
+			$res = $this->authManager->continueAccountCreation( $reqs );
 		} else {
 			$reqs = $helper->loadAuthenticationRequests( AuthManager::ACTION_CREATE );
 			if ( $params['preservestate'] ) {
@@ -91,7 +102,11 @@ class ApiAMCreateAccount extends ApiBase {
 					$reqs[] = $req;
 				}
 			}
-			$res = $manager->beginAccountCreation( $this->getAuthority(), $reqs, $params['returnurl'] );
+			$res = $this->authManager->beginAccountCreation(
+				$this->getAuthority(),
+				$reqs,
+				$params['returnurl']
+			);
 		}
 
 		$this->getResult()->addValue( null, 'createaccount',

includes/api/ApiChangeAuthenticationData.php

@@ -31,13 +31,17 @@ class ApiChangeAuthenticationData extends ApiBase {
 	/** @var AuthManager */
 	private $authManager;
 
+	/**
+	 * @param ApiMain $main
+	 * @param string $action
+	 * @param AuthManager $authManager
+	 */
 	public function __construct(
 		ApiMain $main,
 		$action,
 		AuthManager $authManager
 	) {
 		parent::__construct( $main, $action, 'changeauth' );
-
 		$this->authManager = $authManager;
 	}
 

includes/api/ApiClientLogin.php

@@ -23,7 +23,6 @@
 use MediaWiki\Auth\AuthenticationResponse;
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\Auth\CreateFromLoginAuthenticationRequest;
-use MediaWiki\MediaWikiServices;
 
 /**
  * Log in to the wiki with AuthManager
@@ -32,8 +31,21 @@ use MediaWiki\MediaWikiServices;
  */
 class ApiClientLogin extends ApiBase {
 
-	public function __construct( ApiMain $main, $action ) {
+	/** @var AuthManager */
+	private $authManager;
+
+	/**
+	 * @param ApiMain $main
+	 * @param string $action
+	 * @param AuthManager $authManager
+	 */
+	public function __construct(
+		ApiMain $main,
+		$action,
+		AuthManager $authManager
+	) {
 		parent::__construct( $main, $action, 'login' );
+		$this->authManager = $authManager;
 	}
 
 	public function getFinalDescription() {
@@ -65,11 +77,10 @@ class ApiClientLogin extends ApiBase {
 			}
 		}
 
-		$manager = MediaWikiServices::getInstance()->getAuthManager();
-		$helper = new ApiAuthManagerHelper( $this, $manager );
+		$helper = new ApiAuthManagerHelper( $this, $this->authManager );
 
 		// Make sure it's possible to log in
-		if ( !$manager->canAuthenticateNow() ) {
+		if ( !$this->authManager->canAuthenticateNow() ) {
 			$this->getResult()->addValue( null, 'clientlogin', $helper->formatAuthenticationResponse(
 				AuthenticationResponse::newFail( $this->msg( 'userlogin-cannot-' . AuthManager::ACTION_LOGIN ) )
 			) );
@@ -80,7 +91,7 @@ class ApiClientLogin extends ApiBase {
 		// Perform the login step
 		if ( $params['continue'] ) {
 			$reqs = $helper->loadAuthenticationRequests( AuthManager::ACTION_LOGIN_CONTINUE );
-			$res = $manager->continueAuthentication( $reqs );
+			$res = $this->authManager->continueAuthentication( $reqs );
 		} else {
 			$reqs = $helper->loadAuthenticationRequests( AuthManager::ACTION_LOGIN );
 			if ( $params['preservestate'] ) {
@@ -89,7 +100,7 @@ class ApiClientLogin extends ApiBase {
 					$reqs[] = $req;
 				}
 			}
-			$res = $manager->beginAuthentication( $reqs, $params['returnurl'] );
+			$res = $this->authManager->beginAuthentication( $reqs, $params['returnurl'] );
 		}
 
 		// Remove CreateFromLoginAuthenticationRequest from $res->neededRequests.

includes/api/ApiLinkAccount.php

@@ -22,7 +22,6 @@
 
 use MediaWiki\Auth\AuthenticationResponse;
 use MediaWiki\Auth\AuthManager;
-use MediaWiki\MediaWikiServices;
 
 /**
  * Link an account with AuthManager
@@ -31,8 +30,21 @@ use MediaWiki\MediaWikiServices;
  */
 class ApiLinkAccount extends ApiBase {
 
-	public function __construct( ApiMain $main, $action ) {
+	/** @var AuthManager */
+	private $authManager;
+
+	/**
+	 * @param ApiMain $main
+	 * @param string $action
+	 * @param AuthManager $authManager
+	 */
+	public function __construct(
+		ApiMain $main,
+		$action,
+		AuthManager $authManager
+	) {
 		parent::__construct( $main, $action, 'link' );
+		$this->authManager = $authManager;
 	}
 
 	public function getFinalDescription() {
@@ -68,14 +80,13 @@ class ApiLinkAccount extends ApiBase {
 			}
 		}
 
-		$manager = MediaWikiServices::getInstance()->getAuthManager();
-		$helper = new ApiAuthManagerHelper( $this, $manager );
+		$helper = new ApiAuthManagerHelper( $this, $this->authManager );
 
 		// Check security-sensitive operation status
 		$helper->securitySensitiveOperation( 'LinkAccounts' );
 
 		// Make sure it's possible to link accounts
-		if ( !$manager->canLinkAccounts() ) {
+		if ( !$this->authManager->canLinkAccounts() ) {
 			$this->getResult()->addValue( null, 'linkaccount', $helper->formatAuthenticationResponse(
 				AuthenticationResponse::newFail( $this->msg( 'userlogin-cannot-' . AuthManager::ACTION_LINK ) )
 			) );
@@ -85,10 +96,10 @@ class ApiLinkAccount extends ApiBase {
 		// Perform the link step
 		if ( $params['continue'] ) {
 			$reqs = $helper->loadAuthenticationRequests( AuthManager::ACTION_LINK_CONTINUE );
-			$res = $manager->continueAccountLink( $reqs );
+			$res = $this->authManager->continueAccountLink( $reqs );
 		} else {
 			$reqs = $helper->loadAuthenticationRequests( AuthManager::ACTION_LINK );
-			$res = $manager->beginAccountLink( $this->getUser(), $reqs, $params['returnurl'] );
+			$res = $this->authManager->beginAccountLink( $this->getUser(), $reqs, $params['returnurl'] );
 		}
 
 		$this->getResult()->addValue( null, 'linkaccount',

includes/api/ApiLogin.php

@@ -25,7 +25,6 @@ use MediaWiki\Auth\AuthenticationRequest;
 use MediaWiki\Auth\AuthenticationResponse;
 use MediaWiki\Auth\AuthManager;
 use MediaWiki\Logger\LoggerFactory;
-use MediaWiki\MediaWikiServices;
 
 /**
  * Unit to authenticate log-in attempts to the current wiki.
@@ -34,8 +33,21 @@ use MediaWiki\MediaWikiServices;
  */
 class ApiLogin extends ApiBase {
 
-	public function __construct( ApiMain $main, $action ) {
+	/** @var AuthManager */
+	private $authManager;
+
+	/**
+	 * @param ApiMain $main
+	 * @param string $action
+	 * @param AuthManager $authManager
+	 */
+	public function __construct(
+		ApiMain $main,
+		$action,
+		AuthManager $authManager
+	) {
 		parent::__construct( $main, $action, 'lg' );
+		$this->authManager = $authManager;
 	}
 
 	protected function getExtendedDescription() {
@@ -149,9 +161,11 @@ class ApiLogin extends ApiBase {
 
 		if ( $authRes === false ) {
 			// Simplified AuthManager login, for backwards compatibility
-			$manager = MediaWikiServices::getInstance()->getAuthManager();
 			$reqs = AuthenticationRequest::loadRequestsFromSubmission(
-				$manager->getAuthenticationRequests( AuthManager::ACTION_LOGIN, $this->getUser() ),
+				$this->authManager->getAuthenticationRequests(
+					AuthManager::ACTION_LOGIN,
+					$this->getUser()
+				),
 				[
 					'username' => $params['name'],
 					'password' => $params['password'],
@@ -159,7 +173,7 @@ class ApiLogin extends ApiBase {
 					'rememberMe' => true,
 				]
 			);
-			$res = $manager->beginAuthentication( $reqs, 'null:' );
+			$res = $this->authManager->beginAuthentication( $reqs, 'null:' );
 			switch ( $res->status ) {
 				case AuthenticationResponse::PASS:
 					if ( $this->getConfig()->get( 'EnableBotPasswords' ) ) {

includes/api/ApiMain.php

@@ -60,19 +60,49 @@ class ApiMain extends ApiBase {
 	 * List of available modules: action name => module class
 	 */
 	private const MODULES = [
-		'login' => ApiLogin::class,
-		'clientlogin' => ApiClientLogin::class,
+		'login' => [
+			'class' => ApiLogin::class,
+			'services' => [
+				'AuthManager',
+			],
+		],
+		'clientlogin' => [
+			'class' => ApiClientLogin::class,
+			'services' => [
+				'AuthManager',
+			],
+		],
 		'logout' => ApiLogout::class,
-		'createaccount' => ApiAMCreateAccount::class,
-		'linkaccount' => ApiLinkAccount::class,
-		'unlinkaccount' => ApiRemoveAuthenticationData::class,
+		'createaccount' => [
+			'class' => ApiAMCreateAccount::class,
+			'services' => [
+				'AuthManager',
+			],
+		],
+		'linkaccount' => [
+			'class' => ApiLinkAccount::class,
+			'services' => [
+				'AuthManager',
+			],
+		],
+		'unlinkaccount' => [
+			'class' => ApiRemoveAuthenticationData::class,
+			'services' => [
+				'AuthManager',
+			],
+		],
 		'changeauthenticationdata' => [
 			'class' => ApiChangeAuthenticationData::class,
 			'services' => [
 				'AuthManager',
-			]
+			],
+		],
+		'removeauthenticationdata' => [
+			'class' => ApiRemoveAuthenticationData::class,
+			'services' => [
+				'AuthManager',
+			],
 		],
-		'removeauthenticationdata' => ApiRemoveAuthenticationData::class,
 		'resetpassword' => [
 			'class' => ApiResetPassword::class,
 			'services' => [

includes/api/ApiRemoveAuthenticationData.php

@@ -22,7 +22,6 @@
 
 use MediaWiki\Auth\AuthenticationRequest;
 use MediaWiki\Auth\AuthManager;
-use MediaWiki\MediaWikiServices;
 
 /**
  * Remove authentication data from AuthManager
@@ -34,7 +33,19 @@ class ApiRemoveAuthenticationData extends ApiBase {
 	private $authAction;
 	private $operation;
 
-	public function __construct( ApiMain $main, $action ) {
+	/** @var AuthManager */
+	private $authManager;
+
+	/**
+	 * @param ApiMain $main
+	 * @param string $action
+	 * @param AuthManager $authManager
+	 */
+	public function __construct(
+		ApiMain $main,
+		$action,
+		AuthManager $authManager
+	) {
 		parent::__construct( $main, $action );
 
 		$this->authAction = $action === 'unlinkaccount'
@@ -43,6 +54,8 @@ class ApiRemoveAuthenticationData extends ApiBase {
 		$this->operation = $action === 'unlinkaccount'
 			? 'UnlinkAccount'
 			: 'RemoveCredentials';
+
+		$this->authManager = $authManager;
 	}
 
 	public function execute() {
@@ -51,10 +64,9 @@ class ApiRemoveAuthenticationData extends ApiBase {
 		}
 
 		$params = $this->extractRequestParams();
-		$manager = MediaWikiServices::getInstance()->getAuthManager();
 
 		// Check security-sensitive operation status
-		ApiAuthManagerHelper::newForModule( $this, $manager )
+		ApiAuthManagerHelper::newForModule( $this, $this->authManager )
 			->securitySensitiveOperation( $this->operation );
 
 		// Fetch the request. No need to load from the request, so don't use
@@ -63,7 +75,7 @@ class ApiRemoveAuthenticationData extends ApiBase {
 			? array_flip( $this->getConfig()->get( 'RemoveCredentialsBlacklist' ) )
 			: [];
 		$reqs = array_filter(
-			$manager->getAuthenticationRequests( $this->authAction, $this->getUser() ),
+			$this->authManager->getAuthenticationRequests( $this->authAction, $this->getUser() ),
 			static function ( AuthenticationRequest $req ) use ( $params, $remove ) {
 				return $req->getUniqueId() === $params['request'] &&
 					!isset( $remove[get_class( $req )] );
@@ -75,12 +87,12 @@ class ApiRemoveAuthenticationData extends ApiBase {
 		$req = reset( $reqs );
 
 		// Perform the removal
-		$status = $manager->allowsAuthenticationDataChange( $req, true );
+		$status = $this->authManager->allowsAuthenticationDataChange( $req, true );
 		$this->getHookRunner()->onChangeAuthenticationDataAudit( $req, $status );
 		if ( !$status->isGood() ) {
 			$this->dieStatus( $status );
 		}
-		$manager->changeAuthenticationData( $req );
+		$this->authManager->changeAuthenticationData( $req );
 
 		$this->getResult()->addValue( null, $this->getModuleName(), [ 'status' => 'success' ] );
 	}

tests/phpunit/unit/includes/api/ApiModuleManagerTest.php

@@ -30,53 +30,53 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 		);
 	}
 
-	public function newApiLogin( $main, $action ) {
-		return new ApiLogin( $main, $action );
+	public function newApiRsd( $main, $action ) {
+		return new ApiRsd( $main, $action );
 	}
 
 	public function addModuleProvider() {
 		return [
 			'plain class' => [
-				'login',
+				'rsd',
 				'action',
-				ApiLogin::class,
+				ApiRsd::class,
 				null,
 			],
 
 			'with class and factory' => [
-				'login',
+				'rsd',
 				'action',
-				ApiLogin::class,
-				[ $this, 'newApiLogin' ],
+				ApiRsd::class,
+				[ $this, 'newApiRsd' ],
 			],
 
 			'with spec (class only)' => [
-				'login',
+				'rsd',
 				'action',
 				[
-					'class' => ApiLogin::class
+					'class' => ApiRsd::class
 				],
 				null,
 			],
 
 			'with spec' => [
-				'login',
+				'rsd',
 				'action',
 				[
-					'class' => ApiLogin::class,
-					'factory' => [ $this, 'newApiLogin' ],
+					'class' => ApiRsd::class,
+					'factory' => [ $this, 'newApiRsd' ],
 				],
 				null,
 			],
 
 			'with spec (using services)' => [
-				'logout',
+				'rsd',
 				'action',
 				[
-					'class' => ApiLogout::class,
+					'class' => ApiRsd::class,
 					'factory' => static function ( ApiMain $main, $action, UserFactory $userFactory ) {
 						// we don't actually need the UserFactory, just demonstrating
-						return new ApiLogout( $main, $action );
+						return new ApiRsd( $main, $action );
 					},
 					'services' => [
 						'UserFactory'
@@ -113,7 +113,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 
 			'simple' => [
 				[
-					'login' => ApiLogin::class,
+					'rsd' => ApiRsd::class,
 					'logout' => ApiLogout::class,
 				],
 				'action',
@@ -121,9 +121,9 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 
 			'with factories' => [
 				[
-					'login' => [
-						'class' => ApiLogin::class,
-						'factory' => [ $this, 'newApiLogin' ],
+					'rsd' => [
+						'class' => ApiRsd::class,
+						'factory' => [ $this, 'newApiRsd' ],
 					],
 					'logout' => [
 						'class' => ApiLogout::class,
@@ -156,9 +156,9 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 		$modules = [
 			'disabled' => ApiDisabled::class,
 			'disabled2' => [ 'class' => ApiDisabled::class ],
-			'login' => [
-				'class' => ApiLogin::class,
-				'factory' => [ $this, 'newApiLogin' ],
+			'rsd' => [
+				'class' => ApiRsd::class,
+				'factory' => [ $this, 'newApiRsd' ],
 			],
 			'logout' => [
 				'class' => ApiLogout::class,
@@ -183,8 +183,8 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 
 			'with factory' => [
 				$modules,
-				'login',
-				ApiLogin::class,
+				'rsd',
+				ApiRsd::class,
 			],
 
 			'with closure' => [
@@ -225,7 +225,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetModule_null() {
 		$modules = [
-			'login' => ApiLogin::class,
+			'rsd' => ApiRsd::class,
 			'logout' => ApiLogout::class,
 		];
 
@@ -241,7 +241,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetNames() {
 		$fooModules = [
-			'login' => ApiLogin::class,
+			'rsd' => ApiRsd::class,
 			'logout' => ApiLogout::class,
 		];
 
@@ -267,7 +267,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetNamesWithClasses() {
 		$fooModules = [
-			'login' => ApiLogin::class,
+			'rsd' => ApiRsd::class,
 			'logout' => ApiLogout::class,
 		];
 
@@ -296,7 +296,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetModuleGroup() {
 		$fooModules = [
-			'login' => ApiLogin::class,
+			'rsd' => ApiRsd::class,
 			'logout' => ApiLogout::class,
 		];
 
@@ -309,7 +309,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 		$moduleManager->addModules( $fooModules, 'foo' );
 		$moduleManager->addModules( $barModules, 'bar' );
 
-		$this->assertEquals( 'foo', $moduleManager->getModuleGroup( 'login' ) );
+		$this->assertEquals( 'foo', $moduleManager->getModuleGroup( 'rsd' ) );
 		$this->assertEquals( 'bar', $moduleManager->getModuleGroup( 'feedrecentchanges' ) );
 		$this->assertNull( $moduleManager->getModuleGroup( 'quux' ) );
 	}
@@ -319,7 +319,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetGroups() {
 		$fooModules = [
-			'login' => ApiLogin::class,
+			'rsd' => ApiRsd::class,
 			'logout' => ApiLogout::class,
 		];
 
@@ -341,7 +341,7 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetClassName() {
 		$fooModules = [
-			'login' => ApiLogin::class,
+			'rsd' => ApiRsd::class,
 			'logout' => ApiLogout::class,
 		];
 
@@ -355,8 +355,8 @@ class ApiModuleManagerTest extends MediaWikiUnitTestCase {
 		$moduleManager->addModules( $barModules, 'bar' );
 
 		$this->assertEquals(
-			ApiLogin::class,
-			$moduleManager->getClassName( 'login' )
+			ApiRsd::class,
+			$moduleManager->getClassName( 'rsd' )
 		);
 		$this->assertEquals(
 			ApiLogout::class,