From 573838efc5b8c14967dfad953d3d7160daf96120 Mon Sep 17 00:00:00 2001
From: Thalia <thalia.e.chan@googlemail.com>
Date: Tue, 30 May 2023 14:37:38 +0300
Subject: [PATCH] Degroup: Return early if user is a temporary user

Treat temporary users the same as IP users. Neither has user groups,
so return early for both.

Bug: T335062
Change-Id: I20b48608cf6ba5f8e8e36a378d66c603d84b032f
---
 includes/Consequences/Consequence/Degroup.php | 12 ++++++-
 includes/Consequences/ConsequencesFactory.php |  9 +++++-
 tests/phpunit/DegroupTest.php                 | 32 +++++++++++++++++++
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/includes/Consequences/Consequence/Degroup.php b/includes/Consequences/Consequence/Degroup.php
index 2349c1fcc..5a626c234 100644
--- a/includes/Consequences/Consequence/Degroup.php
+++ b/includes/Consequences/Consequence/Degroup.php
@@ -11,6 +11,7 @@ use MediaWiki\Extension\AbuseFilter\Variables\UnsetVariableException;
 use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
 use MediaWiki\User\UserGroupManager;
 use MediaWiki\User\UserIdentity;
+use MediaWiki\User\UserNameUtils;
 use MessageLocalizer;
 use TitleValue;
 
@@ -27,6 +28,9 @@ class Degroup extends Consequence implements HookAborterConsequence, ReversibleC
 	/** @var UserGroupManager */
 	private $userGroupManager;
 
+	/** @var UserNameUtils */
+	private $userNameUtils;
+
 	/** @var FilterUser */
 	private $filterUser;
 
@@ -37,6 +41,7 @@ class Degroup extends Consequence implements HookAborterConsequence, ReversibleC
 	 * @param Parameters $params
 	 * @param VariableHolder $vars
 	 * @param UserGroupManager $userGroupManager
+	 * @param UserNameUtils $userNameUtils
 	 * @param FilterUser $filterUser
 	 * @param MessageLocalizer $messageLocalizer
 	 */
@@ -44,12 +49,14 @@ class Degroup extends Consequence implements HookAborterConsequence, ReversibleC
 		Parameters $params,
 		VariableHolder $vars,
 		UserGroupManager $userGroupManager,
+		UserNameUtils $userNameUtils,
 		FilterUser $filterUser,
 		MessageLocalizer $messageLocalizer
 	) {
 		parent::__construct( $params );
 		$this->vars = $vars;
 		$this->userGroupManager = $userGroupManager;
+		$this->userNameUtils = $userNameUtils;
 		$this->filterUser = $filterUser;
 		$this->messageLocalizer = $messageLocalizer;
 	}
@@ -60,7 +67,10 @@ class Degroup extends Consequence implements HookAborterConsequence, ReversibleC
 	public function execute(): bool {
 		$user = $this->parameters->getUser();
 
-		if ( !$user->isRegistered() ) {
+		if (
+			!$user->isRegistered() ||
+			$this->userNameUtils->isTemp( $user->getName() )
+		) {
 			return false;
 		}
 
diff --git a/includes/Consequences/ConsequencesFactory.php b/includes/Consequences/ConsequencesFactory.php
index 3786f2645..6910a53ba 100644
--- a/includes/Consequences/ConsequencesFactory.php
+++ b/includes/Consequences/ConsequencesFactory.php
@@ -177,7 +177,14 @@ class ConsequencesFactory {
 	 * @return Degroup
 	 */
 	public function newDegroup( Parameters $params, VariableHolder $vars ): Degroup {
-		return new Degroup( $params, $vars, $this->userGroupManager, $this->filterUser, $this->messageLocalizer );
+		return new Degroup(
+			$params,
+			$vars,
+			$this->userGroupManager,
+			$this->userNameUtils,
+			$this->filterUser,
+			$this->messageLocalizer
+		);
 	}
 
 	/**
diff --git a/tests/phpunit/DegroupTest.php b/tests/phpunit/DegroupTest.php
index e0070bfdc..cf411feab 100644
--- a/tests/phpunit/DegroupTest.php
+++ b/tests/phpunit/DegroupTest.php
@@ -7,6 +7,7 @@ use MediaWiki\Extension\AbuseFilter\FilterUser;
 use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
 use MediaWiki\User\UserGroupManager;
 use MediaWiki\User\UserIdentityValue;
+use MediaWiki\User\UserNameUtils;
 
 /**
  * @coversDefaultClass \MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Degroup
@@ -47,6 +48,7 @@ class DegroupTest extends MediaWikiIntegrationTestCase {
 			$params,
 			VariableHolder::newFromArray( [ 'user_groups' => [ '*', 'user', 'sysop' ] ] ),
 			$userGroupManager,
+			$this->createMock( UserNameUtils::class ),
 			$filterUser,
 			$this->getMsgLocalizer()
 		);
@@ -68,6 +70,7 @@ class DegroupTest extends MediaWikiIntegrationTestCase {
 			$params,
 			VariableHolder::newFromArray( [ 'user_groups' => [ '*', 'user' ] ] ),
 			$userGroupManager,
+			$this->createMock( UserNameUtils::class ),
 			$this->createMock( FilterUser::class ),
 			$this->getMsgLocalizer()
 		);
@@ -95,6 +98,7 @@ class DegroupTest extends MediaWikiIntegrationTestCase {
 			$params,
 			new VariableHolder(),
 			$userGroupManager,
+			$this->createMock( UserNameUtils::class ),
 			$filterUser,
 			$this->getMsgLocalizer()
 		);
@@ -116,6 +120,32 @@ class DegroupTest extends MediaWikiIntegrationTestCase {
 			$params,
 			$this->createMock( VariableHolder::class ),
 			$userGroupManager,
+			$this->createMock( UserNameUtils::class ),
+			$filterUser,
+			$this->getMsgLocalizer()
+		);
+		$this->assertFalse( $degroup->execute() );
+	}
+
+	/**
+	 * @covers ::execute
+	 */
+	public function testExecute_temp() {
+		$user = new UserIdentityValue( 10, '*12345' );
+		$params = $this->provideGetMessageParameters( $user )->current()[0];
+		$userGroupManager = $this->createMock( UserGroupManager::class );
+		$userGroupManager->expects( $this->never() )->method( $this->anything() );
+		$userNameUtils = $this->createMock( UserNameUtils::class );
+		$userNameUtils->method( 'isTemp' )
+			->willReturn( true );
+		$filterUser = $this->createMock( FilterUser::class );
+		$filterUser->expects( $this->never() )->method( $this->anything() );
+
+		$degroup = new Degroup(
+			$params,
+			$this->createMock( VariableHolder::class ),
+			$userGroupManager,
+			$userNameUtils,
 			$filterUser,
 			$this->getMsgLocalizer()
 		);
@@ -152,6 +182,7 @@ class DegroupTest extends MediaWikiIntegrationTestCase {
 			$params,
 			VariableHolder::newFromArray( [ 'user_groups' => $hadGroups ] ),
 			$userGroupManager,
+			$this->createMock( UserNameUtils::class ),
 			$this->createMock( FilterUser::class ),
 			$this->getMsgLocalizer()
 		);
@@ -172,6 +203,7 @@ class DegroupTest extends MediaWikiIntegrationTestCase {
 			$params,
 			new VariableHolder(),
 			$this->createMock( UserGroupManager::class ),
+			$this->createMock( UserNameUtils::class ),
 			$this->createMock( FilterUser::class ),
 			$this->getMsgLocalizer()
 		);