permissions: Avoid potential infinite loop if BlockDisablesLogin = true

Why:

- PermissionManager::getUserPermissions() checks whether the user is
  blocked if $wgBlockDisablesLogin = true, so that it can then limit
  user's permissions to the set of permissions assigned to unregistered
  users if so.
- This causes the GetUserBlock hook to run, which may itself check
  permissions on the user (e.g. in the GlobalBlocking extension),
  causing an infinite loop.
- Since the decision whether the user is blocked isn't yet final by the
  time GetUserBlock runs, any permission checks triggered by
  GetUserBlock handlers should see the user's full set of permissions.

What:

- Stash the user's permissions in PermissionManager's in-memory cache
  before running block checks if BlockDisablesLogin = true.
- Add tests.

Bug: T384197
Change-Id: I3e3804fe518627e9edc2b574cce88f533fd93fe4
(cherry picked from commit 27062b9f8752cc853a65e8a46c9d7d1a9af32c48)

includes/Permissions/PermissionManager.php

@@ -1563,6 +1563,11 @@ class PermissionManager {
 				$userObj->isRegistered()
 				&& $this->options->get( MainConfigNames::BlockDisablesLogin )
 			) {
+				// Stash the permissions as they are before triggering any block checks for BlockDisablesLogin
+				// to avoid a potential infinite loop, since GetUserBlock handlers may themselves check
+				// permissions on this user. (T384197)
+				$this->usersRights[ $rightsCacheKey ] = $rights;
+
 				$isExempt = in_array( 'ipblock-exempt', $rights, true );
 				if ( $this->blockManager->getBlock(
 					$userObj,

tests/phpunit/includes/Permissions/PermissionManagerTest.php

@@ -23,6 +23,7 @@ use MediaWiki\Tests\Unit\MockBlockTrait;
 use MediaWiki\Tests\User\TempUser\TempUserTestTrait;
 use MediaWiki\Title\Title;
 use MediaWiki\User\User;
+use MediaWiki\User\UserIdentity;
 use MediaWiki\User\UserIdentityValue;
 use MediaWikiLangTestCase;
 use stdClass;
@@ -57,6 +58,7 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
 		$localOffset = date( 'Z' ) / 60;
 
 		$this->overrideConfigValues( [
+			MainConfigNames::BlockDisablesLogin => false,
 			MainConfigNames::Localtimezone => $localZone,
 			MainConfigNames::LocalTZoffset => $localOffset,
 			MainConfigNames::ImplicitRights => [
@@ -1528,4 +1530,72 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
 			'getPermissionStatus() preserves ApiMessage objects'
 		);
 	}
+
+	public function testShouldLimitPermissionsForBlockedUserWhenBlockDisablesLogin(): void {
+		$this->overrideConfigValues( [
+			MainConfigNames::BlockDisablesLogin => true,
+			MainConfigNames::GroupPermissions => [
+				'*' => [ 'edit' => true ],
+				'user' => [ 'edit' => true, 'move' => true ],
+				'sysop' => [ 'block' => true ],
+			],
+		] );
+
+		$testUser = $this->getTestUser()->getUserIdentity();
+		$this->blockUser( $testUser );
+
+		$permissions = $this->getServiceContainer()->getPermissionManager()->getUserPermissions( $testUser );
+
+		$this->assertSame( [ 'edit' ], $permissions );
+	}
+
+	public function testShouldLimitPermissionsForBlockedUserShouldAllowPermissionChecksInGetUserBlock(): void {
+		$this->overrideConfigValues( [
+			MainConfigNames::BlockDisablesLogin => true,
+			MainConfigNames::GroupPermissions => [
+				'*' => [ 'edit' => true ],
+				'user' => [ 'edit' => true, 'move' => true ],
+				'sysop' => [ 'block' => true ],
+			],
+		] );
+
+		$testUser = $this->getTestUser()->getUserIdentity();
+		$hookRan = false;
+
+		$this->setTemporaryHook(
+			'GetUserBlock',
+			function ( UserIdentity $user ) use ( $testUser, &$hookRan ): void {
+				if ( $user->equals( $testUser ) ) {
+					// Trigger an arbitrary permissions check to verify that they do not cause an infinite loop
+					// when BlockDisablesLogin = true (T384197).
+					$this->getServiceContainer()->getPermissionManager()
+						->userHasRight( $user, 'test' );
+
+					$hookRan = true;
+				}
+			}
+		);
+
+		$testUser = $this->getTestUser()->getUserIdentity();
+		$this->blockUser( $testUser );
+
+		$permissions = $this->getServiceContainer()->getPermissionManager()->getUserPermissions( $testUser );
+
+		$this->assertSame( [ 'edit' ], $permissions );
+		$this->assertTrue( $hookRan );
+	}
+
+	/**
+	 * Convenience function to block a given user.
+	 * @param UserIdentity $user
+	 * @return void
+	 */
+	private function blockUser( UserIdentity $user ): void {
+		$status = $this->getServiceContainer()
+			->getBlockUserFactory()
+			->newBlockUser( $user, $this->getTestSysop()->getAuthority(), 'infinity' )
+			->placeBlock();
+
+		$this->assertStatusGood( $status );
+	}
 }