Inject dependencies into the authentication provider

- Define it with the extension.json key, instead of using the
  registration callback
- Inject the services it needs
- Replace direct User instantiation with UserFactory
- Move log subtypes to extension.json as well

Change-Id: I86a761c7fa844b1f417b974798373622a15f6411

extension.json

@@ -113,12 +113,40 @@
 			"create": [
 				"create"
 			]
+		},
+		"suppress": {
+			"abuselog": [
+				"hide-afl",
+				"unhide-afl"
+			]
+		},
+		"rights": {
+			"blockautopromote": [
+				"blockautopromote"
+			],
+			"restoreautopromote": [
+				"restoreautopromote"
+			]
 		}
 	},
 	"LogRestrictions": {
 		"abusefilter": "abusefilter-view",
 		"abusefilterprivatedetails": "abusefilter-privatedetails-log"
 	},
+	"AuthManagerAutoConfig": {
+		"preauth": {
+			"AbuseFilterPreAuthenticationProvider": {
+				"class": "MediaWiki\\Extension\\AbuseFilter\\AbuseFilterPreAuthenticationProvider",
+				"services": [
+					"AbuseFilterVariableGeneratorFactory",
+					"AbuseFilterRunnerFactory",
+					"StatsdDataFactory",
+					"UserFactory"
+				],
+				"sort": 5
+			}
+		}
+	},
 	"APIModules": {
 		"abusefilterchecksyntax": {
 			"class": "MediaWiki\\Extension\\AbuseFilter\\Api\\CheckSyntax",

includes/AbuseFilterPreAuthenticationProvider.php

@@ -2,21 +2,54 @@
 
 namespace MediaWiki\Extension\AbuseFilter;
 
+use IBufferingStatsdDataFactory;
 use MediaWiki\Auth\AbstractPreAuthenticationProvider;
 use MediaWiki\Auth\AuthenticationRequest;
-use MediaWiki\MediaWikiServices;
+use MediaWiki\Extension\AbuseFilter\VariableGenerator\VariableGeneratorFactory;
+use MediaWiki\User\UserFactory;
 use SpecialPage;
 use StatusValue;
 use User;
 
+/**
+ * AuthenticationProvider used to filter account creations. This runs after normal preauth providers
+ * to keep the log cleaner.
+ */
 class AbuseFilterPreAuthenticationProvider extends AbstractPreAuthenticationProvider {
+	/** @var VariableGeneratorFactory */
+	private $variableGeneratorFactory;
+	/** @var FilterRunnerFactory */
+	private $filterRunnerFactory;
+	/** @var IBufferingStatsdDataFactory */
+	private $statsd;
+	/** @var UserFactory */
+	private $userFactory;
+
+	/**
+	 * @param VariableGeneratorFactory $variableGeneratorFactory
+	 * @param FilterRunnerFactory $filterRunnerFactory
+	 * @param IBufferingStatsdDataFactory $statsd
+	 * @param UserFactory $userFactory
+	 */
+	public function __construct(
+		VariableGeneratorFactory $variableGeneratorFactory,
+		FilterRunnerFactory $filterRunnerFactory,
+		IBufferingStatsdDataFactory $statsd,
+		UserFactory $userFactory
+	) {
+		$this->variableGeneratorFactory = $variableGeneratorFactory;
+		$this->filterRunnerFactory = $filterRunnerFactory;
+		$this->statsd = $statsd;
+		$this->userFactory = $userFactory;
+	}
+
 	/**
 	 * @param User $user
 	 * @param User $creator
 	 * @param AuthenticationRequest[] $reqs
 	 * @return StatusValue
 	 */
-	public function testForAccountCreation( $user, $creator, array $reqs ) {
+	public function testForAccountCreation( $user, $creator, array $reqs ): StatusValue {
 		return $this->testUser( $user, $creator, false );
 	}
 
@@ -26,11 +59,11 @@ class AbuseFilterPreAuthenticationProvider extends AbstractPreAuthenticationProv
 	 * @param array $options
 	 * @return StatusValue
 	 */
-	public function testUserForCreation( $user, $autocreate, array $options = [] ) {
+	public function testUserForCreation( $user, $autocreate, array $options = [] ): StatusValue {
 		// if this is not an autocreation, testForAccountCreation already handled it
 		if ( $autocreate ) {
-			// FIXME Using the constructor directly here a bit hacky but needed for T272244
-			return $this->testUser( $user, new User, true );
+			// Make sure to use an anon as the creator, see T272244
+			return $this->testUser( $user, $this->userFactory->newAnonymous(), true );
 		}
 		return StatusValue::newGood();
 	}
@@ -41,23 +74,21 @@ class AbuseFilterPreAuthenticationProvider extends AbstractPreAuthenticationProv
 	 * @param bool $autocreate Is this an autocreation?
 	 * @return StatusValue
 	 */
-	protected function testUser( $user, $creator, $autocreate ) {
+	private function testUser( $user, $creator, $autocreate ): StatusValue {
 		$startTime = microtime( true );
 		if ( $user->getName() === wfMessage( 'abusefilter-blocker' )->inContentLanguage()->text() ) {
 			return StatusValue::newFatal( 'abusefilter-accountreserved' );
 		}
 
 		$title = SpecialPage::getTitleFor( 'Userlogin' );
-		$builder = AbuseFilterServices::getVariableGeneratorFactory()->newRunGenerator( $creator, $title );
+		$builder = $this->variableGeneratorFactory->newRunGenerator( $creator, $title );
 		$vars = $builder->getAccountCreationVars( $user, $autocreate );
 
-		$runnerFactory = AbuseFilterServices::getFilterRunnerFactory();
 		// pass creator in explicitly to prevent recording the current user on autocreation - T135360
-		$runner = $runnerFactory->newRunner( $creator, $title, $vars, 'default' );
+		$runner = $this->filterRunnerFactory->newRunner( $creator, $title, $vars, 'default' );
 		$status = $runner->run();
 
-		MediaWikiServices::getInstance()->getStatsdDataFactory()
-			->timing( 'timing.createaccountAbuseFilter', microtime( true ) - $startTime );
+		$this->statsd->timing( 'timing.createaccountAbuseFilter', microtime( true ) - $startTime );
 
 		return $status->getStatusValue();
 	}

includes/Hooks/Handlers/RegistrationCallback.php

@@ -2,8 +2,6 @@
 
 namespace MediaWiki\Extension\AbuseFilter\Hooks\Handlers;
 
-use MediaWiki\Extension\AbuseFilter\AbuseFilterPreAuthenticationProvider;
-
 /**
  * This class runs a callback when the extension is registered, right after configuration has been
  * loaded (not really a hook, but almost).
@@ -12,7 +10,7 @@ use MediaWiki\Extension\AbuseFilter\AbuseFilterPreAuthenticationProvider;
 class RegistrationCallback {
 
 	public static function onRegistration(): void {
-		global $wgAuthManagerAutoConfig, $wgActionFilteredLogs, $wgAbuseFilterProfile,
+		global $wgAbuseFilterProfile,
 			   $wgAbuseFilterProfiling, $wgAbuseFilterPrivateLog, $wgAbuseFilterForceSummary,
 			   $wgGroupPermissions, $wgAbuseFilterRestrictions, $wgAbuseFilterDisallowGlobalLocalBlocks,
 			   $wgAbuseFilterActionRestrictions, $wgAbuseFilterLocallyDisabledGlobalActions;
@@ -89,27 +87,6 @@ class RegistrationCallback {
 			wfWarn( '$wgAbuseFilterRestrictions has been renamed to $wgAbuseFilterActionRestrictions.' );
 			$wgAbuseFilterActionRestrictions = $wgAbuseFilterRestrictions;
 		}
-
-		$wgAuthManagerAutoConfig['preauth'][AbuseFilterPreAuthenticationProvider::class] = [
-			'class' => AbuseFilterPreAuthenticationProvider::class,
-			// Run after normal preauth providers to keep the log cleaner
-			'sort' => 5,
-		];
-
-		$wgActionFilteredLogs['suppress'] = array_merge(
-			$wgActionFilteredLogs['suppress'],
-			// Message: log-action-filter-suppress-abuselog
-			[ 'abuselog' => [ 'hide-afl', 'unhide-afl' ] ]
-		);
-		$wgActionFilteredLogs['rights'] = array_merge(
-			$wgActionFilteredLogs['rights'],
-			// Messages: log-action-filter-rights-blockautopromote,
-			// log-action-filter-rights-restoreautopromote
-			[
-				'blockautopromote' => [ 'blockautopromote' ],
-				'restoreautopromote' => [ 'restoreautopromote' ]
-			]
-		);
 	}
 
 }

tests/phpunit/AbuseFilterCreateAccountTestTrait.php

@@ -1,6 +1,8 @@
 <?php
 
 use MediaWiki\Extension\AbuseFilter\AbuseFilterPreAuthenticationProvider;
+use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
+use MediaWiki\MediaWikiServices;
 
 /**
  * This trait can be used to create accounts in integration tests.
@@ -19,14 +21,19 @@ trait AbuseFilterCreateAccountTestTrait {
 		User $creator = null,
 		bool $autocreate = false
 	): StatusValue {
-		$user = User::newFromName( $accountName );
+		$user = MediaWikiServices::getInstance()->getUserFactory()->newFromName( $accountName );
 		// A creatable username must exist to be passed to $logEntry->setPerformer(),
 		// so create the account.
 		$user->addToDatabase();
 
 		$creator = $creator ?? $user;
 
-		$provider = new AbuseFilterPreAuthenticationProvider();
+		$provider = new AbuseFilterPreAuthenticationProvider(
+			AbuseFilterServices::getVariableGeneratorFactory(),
+			AbuseFilterServices::getFilterRunnerFactory(),
+			new NullStatsdDataFactory(),
+			MediaWikiServices::getInstance()->getUserFactory()
+		);
 		$status = $provider->testForAccountCreation( $user, $creator, [] );
 
 		// FIXME This is a bit hacky, but AuthManager doesn't expose any methods for logging