Add a service to retrieve the filter user

Unfortunately, this isn't using DI completely, because of the
User::newSystemUser call. I'm not even sure if we really need to call it
or we can just stick to new UserIdentityValue, but leaving like this for
now.
Also, the types were weakened to UserIdentity, so the transition is
going to be easy anyway.

Change-Id: I08f8fae0fcc622ff0ac3f86771476d06d1c18549

extension.json

@@ -169,6 +169,7 @@
 		"MediaWiki\\Extension\\AbuseFilter\\ChangeTagger": "includes/ChangeTagger.php",
 		"MediaWiki\\Extension\\AbuseFilter\\ChangeTagsManager": "includes/ChangeTagsManager.php",
 		"MediaWiki\\Extension\\AbuseFilter\\BlockAutopromoteStore": "includes/BlockAutopromoteStore.php",
+		"MediaWiki\\Extension\\AbuseFilter\\FilterUser": "includes/FilterUser.php",
 		"AFComputedVariable": "includes/AFComputedVariable.php",
 		"AFPData": "includes/parser/AFPData.php",
 		"AFPException": "includes/parser/AFPException.php",

includes/AbuseFilter.php

@@ -566,36 +566,6 @@ class AbuseFilter {
 		return $value[$group] ?? $value['default'];
 	}
 
-	/**
-	 * @return User
-	 */
-	public static function getFilterUser() : User {
-		$username = wfMessage( 'abusefilter-blocker' )->inContentLanguage()->text();
-		$user = User::newSystemUser( $username, [ 'steal' => true ] );
-
-		if ( !$user ) {
-			// User name is invalid. Don't throw because this is a system message, easy
-			// to change and make wrong either by mistake or intentionally to break the site.
-			wfWarn(
-				'The AbuseFilter user\'s name is invalid. Please change it in ' .
-				'MediaWiki:abusefilter-blocker'
-			);
-			// Use the default name to avoid breaking other stuff. This should have no harm,
-			// aside from blocks temporarily attributed to another user.
-			$defaultName = wfMessage( 'abusefilter-blocker' )->inLanguage( 'en' )->text();
-			$user = User::newSystemUser( $defaultName, [ 'steal' => true ] );
-		}
-		'@phan-var User $user';
-
-		// Promote user to 'sysop' so it doesn't look
-		// like an unprivileged account is blocking users
-		if ( !in_array( 'sysop', $user->getGroups() ) ) {
-			$user->addGroup( 'sysop' );
-		}
-
-		return $user;
-	}
-
 	/**
 	 * Check whether a filter is allowed to use a tag
 	 *

includes/AbuseFilterRunner.php

@@ -8,6 +8,7 @@ use MediaWiki\Extension\AbuseFilter\VariableGenerator\VariableGenerator;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Session\SessionManager;
+use MediaWiki\User\UserIdentity;
 use Wikimedia\IPUtils;
 use Wikimedia\Rdbms\IDatabase;
 
@@ -74,6 +75,9 @@ class AbuseFilterRunner {
 	/** @var ChangeTagger */
 	private $changeTagger;
 
+	/** @var UserIdentity */
+	private $filterUser;
+
 	/**
 	 * @param User $user The user who performed the action being filtered
 	 * @param Title $title The title where the action being filtered was performed
@@ -99,6 +103,7 @@ class AbuseFilterRunner {
 		$this->hookRunner = AbuseFilterHookRunner::getRunner();
 		$this->filterProfiler = AbuseFilterServices::getFilterProfiler();
 		$this->changeTagger = AbuseFilterServices::getChangeTagger();
+		$this->filterUser = AbuseFilterServices::getFilterUser()->getUser();
 	}
 
 	/**
@@ -811,7 +816,7 @@ class AbuseFilterRunner {
 
 					// TODO Core should provide a logging method
 					$logEntry = new ManualLogEntry( 'rights', 'rights' );
-					$logEntry->setPerformer( AbuseFilter::getFilterUser() );
+					$logEntry->setPerformer( $this->filterUser );
 					$logEntry->setTarget( $this->user->getUserPage() );
 					$logEntry->setComment(
 						wfMessage(
@@ -935,7 +940,6 @@ class AbuseFilterRunner {
 		$preventEditOwnUserTalk
 	) {
 		$blockUserFactory = MediaWikiServices::getInstance()->getBlockUserFactory();
-		$filterUser = AbuseFilter::getFilterUser();
 		$reason = wfMessage(
 			'abusefilter-blockreason',
 			$ruleDesc, $ruleNumber
@@ -943,7 +947,7 @@ class AbuseFilterRunner {
 
 		$blockUserFactory->newBlockUser(
 			$target,
-			$filterUser,
+			User::newFromIdentity( $this->filterUser ),
 			$expiry,
 			$reason,
 			[

includes/AbuseFilterServices.php

@@ -48,4 +48,11 @@ class AbuseFilterServices {
 	public static function getBlockAutopromoteStore() : BlockAutopromoteStore {
 		return MediaWikiServices::getInstance()->getService( BlockAutopromoteStore::SERVICE_NAME );
 	}
+
+	/**
+	 * @return FilterUser
+	 */
+	public static function getFilterUser() : FilterUser {
+		return MediaWikiServices::getInstance()->getService( FilterUser::SERVICE_NAME );
+	}
 }

includes/BlockAutopromoteStore.php

@@ -2,7 +2,6 @@
 
 namespace MediaWiki\Extension\AbuseFilter;
 
-use AbuseFilter;
 use BagOStuff;
 use ManualLogEntry;
 use MediaWiki\User\UserIdentity;
@@ -26,13 +25,18 @@ class BlockAutopromoteStore {
 	 */
 	private $logger;
 
+	/** @var FilterUser */
+	private $filterUser;
+
 	/**
 	 * @param BagOStuff $store
 	 * @param LoggerInterface $logger
+	 * @param FilterUser $filterUser
 	 */
-	public function __construct( BagOStuff $store, LoggerInterface $logger ) {
+	public function __construct( BagOStuff $store, LoggerInterface $logger, FilterUser $filterUser ) {
 		$this->store = $store;
 		$this->logger = $logger;
+		$this->filterUser = $filterUser;
 	}
 
 	/**
@@ -71,8 +75,7 @@ class BlockAutopromoteStore {
 		}
 
 		$logEntry = new ManualLogEntry( 'rights', 'blockautopromote' );
-		$performer = AbuseFilter::getFilterUser();
-		$logEntry->setPerformer( $performer );
+		$logEntry->setPerformer( $this->filterUser->getUser() );
 		$logEntry->setTarget( new TitleValue( NS_USER, $target->getName() ) );
 
 		$logEntry->setParameters( [

includes/FilterUser.php

@@ -0,0 +1,67 @@
+<?php
+
+namespace MediaWiki\Extension\AbuseFilter;
+
+use MediaWiki\User\UserGroupManager;
+use MediaWiki\User\UserIdentity;
+use MessageLocalizer;
+use Psr\Log\LoggerInterface;
+use User;
+
+class FilterUser {
+	public const SERVICE_NAME = 'AbuseFilterFilterUser';
+
+	/** @var MessageLocalizer */
+	private $messageLocalizer;
+	/** @var UserGroupManager */
+	private $userGroupManager;
+	/** @var LoggerInterface */
+	private $logger;
+
+	/**
+	 * @param MessageLocalizer $messageLocalizer
+	 * @param UserGroupManager $userGroupManager
+	 * @param LoggerInterface $logger
+	 */
+	public function __construct(
+		MessageLocalizer $messageLocalizer,
+		UserGroupManager $userGroupManager,
+		LoggerInterface $logger
+	) {
+		$this->messageLocalizer = $messageLocalizer;
+		$this->userGroupManager = $userGroupManager;
+		$this->logger = $logger;
+	}
+
+	/**
+	 * @todo Core should provide an alternative way to create system users, possibly returning just UserIdentity.
+	 *   Or can we just use `new UserIdentityValue` here?
+	 * @return UserIdentity
+	 */
+	public function getUser() : UserIdentity {
+		$username = $this->messageLocalizer->msg( 'abusefilter-blocker' )->inContentLanguage()->text();
+		$user = User::newSystemUser( $username, [ 'steal' => true ] );
+
+		if ( !$user ) {
+			// User name is invalid. Don't throw because this is a system message, easy
+			// to change and make wrong either by mistake or intentionally to break the site.
+			$this->logger->warning(
+				'The AbuseFilter user\'s name is invalid. Please change it in ' .
+				'MediaWiki:abusefilter-blocker'
+			);
+			// Use the default name to avoid breaking other stuff. This should have no harm,
+			// aside from blocks temporarily attributed to another user.
+			$defaultName = $this->messageLocalizer->msg( 'abusefilter-blocker' )->inLanguage( 'en' )->text();
+			$user = User::newSystemUser( $defaultName, [ 'steal' => true ] );
+		}
+		'@phan-var User $user';
+
+		// Promote user to 'sysop' so it doesn't look
+		// like an unprivileged account is blocking users
+		if ( !in_array( 'sysop', $this->userGroupManager->getUserGroups( $user ) ) ) {
+			$this->userGroupManager->addUserToGroup( $user, 'sysop' );
+		}
+
+		return $user;
+	}
+}

includes/ServiceWiring.php

@@ -6,6 +6,7 @@ use MediaWiki\Extension\AbuseFilter\BlockAutopromoteStore;
 use MediaWiki\Extension\AbuseFilter\ChangeTagger;
 use MediaWiki\Extension\AbuseFilter\ChangeTagsManager;
 use MediaWiki\Extension\AbuseFilter\FilterProfiler;
+use MediaWiki\Extension\AbuseFilter\FilterUser;
 use MediaWiki\Extension\AbuseFilter\Hooks\AbuseFilterHookRunner;
 use MediaWiki\Extension\AbuseFilter\KeywordsManager;
 use MediaWiki\Logger\LoggerFactory;
@@ -50,6 +51,15 @@ return [
 	BlockAutopromoteStore::SERVICE_NAME => function ( MediaWikiServices $services ): BlockAutopromoteStore {
 		return new BlockAutopromoteStore(
 			ObjectCache::getInstance( 'db-replicated' ),
+			LoggerFactory::getInstance( 'AbuseFilter' ),
+			$services->get( FilterUser::SERVICE_NAME )
+		);
+	},
+	FilterUser::SERVICE_NAME => function ( MediaWikiServices $services ): FilterUser {
+		return new FilterUser(
+			// TODO We need a proper MessageLocalizer, see T247127
+			RequestContext::getMain(),
+			$services->getUserGroupManager(),
 			LoggerFactory::getInstance( 'AbuseFilter' )
 		);
 	},

includes/Views/AbuseFilterViewRevert.php

@@ -292,7 +292,8 @@ class AbuseFilterViewRevert extends AbuseFilterView {
 		switch ( $action ) {
 			case 'block':
 				$block = DatabaseBlock::newFromTarget( $result['user'] );
-				if ( !( $block && $block->getBy() === AbuseFilter::getFilterUser()->getId() ) ) {
+				$filterUser = AbuseFilterServices::getFilterUser()->getUser();
+				if ( !( $block && $block->getBy() === $filterUser->getId() ) ) {
 					// Not blocked by abuse filter
 					return false;
 				}

maintenance/normalizeThrottleParameters.php

@@ -18,6 +18,9 @@
  *
  * @ingroup Maintenance
  */
+
+use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
+
 if ( getenv( 'MW_INSTALL_PATH' ) ) {
 	$IP = getenv( 'MW_INSTALL_PATH' );
 } else {
@@ -144,7 +147,7 @@ class NormalizeThrottleParameters extends LoggedUpdateMaintenance {
 	 * @return int Amount of normalized rows
 	 */
 	protected function normalizeParameters() {
-		$user = AbuseFilter::getFilterUser();
+		$user = AbuseFilterServices::getFilterUser()->getUser();
 		$dryRun = $this->hasOption( 'dry-run' );
 
 		// IDs of filters with invalid rate (count or period)

tests/phpunit/AbuseFilterFilterUserTest.php

@@ -0,0 +1,56 @@
+<?php
+
+use MediaWiki\Extension\AbuseFilter\FilterUser;
+use MediaWiki\MediaWikiServices;
+use Psr\Log\NullLogger;
+
+/**
+ * @group Test
+ * @group AbuseFilter
+ * @coversDefaultClass \MediaWiki\Extension\AbuseFilter\FilterUser
+ * @covers ::__construct()
+ * @todo Make a unit test once DI is possible for User::newSystemUser
+ */
+class AbuseFilterFilterUserTest extends MediaWikiIntegrationTestCase {
+	/**
+	 * @covers ::getUser
+	 */
+	public function testGetUser() {
+		$name = 'AbuseFilter blocker user';
+		$ml = $this->createMock( MessageLocalizer::class );
+		$ml->method( 'msg' )->willReturn( $this->getMockMessage( $name ) );
+		$ugm = MediaWikiServices::getInstance()->getUserGroupManager();
+		$filterUser = new FilterUser( $ml, $ugm, new NullLogger() );
+
+		$actual = $filterUser->getUser();
+		$this->assertSame( $name, $actual->getName(), 'name' );
+		$this->assertContains( 'sysop', $ugm->getUserGroups( $actual ), 'sysop' );
+	}
+
+	/**
+	 * @covers ::getUser
+	 */
+	public function testGetUser_invalidName() {
+		$name = 'Foobar filter user';
+		$ml = $this->createMock( MessageLocalizer::class );
+		$msg = $this->createMock( Message::class );
+		$msg->method( 'inContentLanguage' )->willReturn( $this->getMockMessage( '' ) );
+		$msg->method( 'inLanguage' )->willReturn( $this->getMockMessage( $name ) );
+		$ml->method( 'msg' )->willReturn( $msg );
+		$ugm = MediaWikiServices::getInstance()->getUserGroupManager();
+		$logger = new TestLogger();
+		$logger->setCollect( true );
+		$filterUser = new FilterUser( $ml, $ugm, $logger );
+
+		$actual = $filterUser->getUser();
+		$this->assertSame( $name, $actual->getName(), 'name' );
+		$found = false;
+		foreach ( $logger->getBuffer() as $msg ) {
+			if ( strpos( $msg[1], 'MediaWiki:abusefilter-blocker' ) !== false ) {
+				$found = true;
+				break;
+			}
+		}
+		$this->assertTrue( $found, 'Invalid name not logged' );
+	}
+}