From: Gergő Tisza <tgr.huwiki@gmail.com>
Date: Thu, 11 Jul 2019 17:22:20 +0000 (+0200)
Subject: Add mechanism for temporary user rights
X-Git-Tag: 1.34.0-rc.0~978^2
X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_ecrire%28%22auteur_infos%22%2C%22id_auteur=%24connect_id_auteur%22%29%20.%20%22?a=commitdiff_plain;h=659db7bddd607ae6e5877f31e54ef8496a7ad336;p=lhc%2Fweb%2Fwiklou.git

Add mechanism for temporary user rights

Add a mechanism for adding temporary user rights that only exist
for the current request. This is occasionally needed to let normal
users act with a bot flag; traditionally the fact that User::$mRights
was public has been abused to do it, but I88992403 broke that.

Bug: T227772
Change-Id: Ife8f9d8affa750701e4e5d646ed8cd153c1d867b
---

diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34
index 69b2088125..089471a70e 100644
--- a/RELEASE-NOTES-1.34
+++ b/RELEASE-NOTES-1.34
@@ -277,7 +277,8 @@ because of Phabricator reports.
   in JavaScript, use mw.log.deprecate() instead.
 * The 'user.groups' module, deprecated in 1.28, was removed.
   Use the 'user' module instead.
-* The ability to override User::$mRights has been removed.
+* The ability to override User::$mRights has been removed. Use
+  PermissionManager::addTemporaryUserRights() instead.
 * Previously, when iterating ResultWrapper with foreach() or a similar
   construct, the range of the index was 1..numRows. This has been fixed to be
   0..(numRows-1).
diff --git a/includes/Permissions/PermissionManager.php b/includes/Permissions/PermissionManager.php
index defcb656de..98a5b17608 100644
--- a/includes/Permissions/PermissionManager.php
+++ b/includes/Permissions/PermissionManager.php
@@ -32,6 +32,7 @@ use RequestContext;
 use SpecialPage;
 use Title;
 use User;
+use Wikimedia\ScopedCallback;
 use WikiPage;
 
 /**
@@ -84,6 +85,12 @@ class PermissionManager {
 	/** @var string[][] Cached user rights */
 	private $usersRights = null;
 
+	/**
+	 * Temporary user rights, valid for the current request only.
+	 * @var string[][][] userid => override group => rights
+	 */
+	private $temporaryUserRights = [];
+
 	/** @var string[] Cached rights for isEveryoneAllowed */
 	private $cachedRights = [];
 
@@ -1223,7 +1230,11 @@ class PermissionManager {
 				);
 			}
 		}
-		return $this->usersRights[ $user->getId() ];
+		$rights = $this->usersRights[ $user->getId() ];
+		foreach ( $this->temporaryUserRights[ $user->getId() ] ?? [] as $overrides ) {
+			$rights = array_values( array_unique( array_merge( $rights, $overrides ) ) );
+		}
+		return $rights;
 	}
 
 	/**
@@ -1391,6 +1402,33 @@ class PermissionManager {
 		return $this->allRights;
 	}
 
+	/**
+	 * Add temporary user rights, only valid for the current scope.
+	 * This is meant for making it possible to programatically trigger certain actions that
+	 * the user wouldn't be able to trigger themselves; e.g. allow users without the bot right
+	 * to make bot-flagged actions through certain special pages.
+	 * Returns a "scope guard" variable; whenever that variable goes out of scope or is consumed
+	 * via ScopedCallback::consume(), the temporary rights are revoked.
+	 * @param UserIdentity $user
+	 * @param string|string[] $rights
+	 * @return ScopedCallback
+	 */
+	public function addTemporaryUserRights( UserIdentity $user, $rights ) {
+		$nextKey = count( $this->temporaryUserRights[$user->getId()] ?? [] );
+		$this->temporaryUserRights[$user->getId()][$nextKey] = (array)$rights;
+		return new ScopedCallback( [ $this, 'revokeTemporaryUserRights' ], [ $user->getId(), $nextKey ] );
+	}
+
+	/**
+	 * Revoke rights added by addTemporaryUserRights().
+	 * @param int $userId
+	 * @param int $rightsGroupKey Key in self::$temporaryUserRights
+	 * @internal For use by addTemporaryUserRights() only.
+	 */
+	public function revokeTemporaryUserRights( $userId, $rightsGroupKey ) {
+		unset( $this->temporaryUserRights[$userId][$rightsGroupKey] );
+	}
+
 	/**
 	 * Overrides user permissions cache
 	 *
diff --git a/tests/phpunit/includes/Permissions/PermissionManagerTest.php b/tests/phpunit/includes/Permissions/PermissionManagerTest.php
index 3da73c148e..03b35b5331 100644
--- a/tests/phpunit/includes/Permissions/PermissionManagerTest.php
+++ b/tests/phpunit/includes/Permissions/PermissionManagerTest.php
@@ -17,6 +17,7 @@ use MediaWiki\Block\Restriction\PageRestriction;
 use MediaWiki\Block\SystemBlock;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Permissions\PermissionManager;
+use Wikimedia\ScopedCallback;
 use Wikimedia\TestingAccessWrapper;
 
 /**
@@ -41,11 +42,6 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
 	 */
 	protected $user, $anonUser, $userUser, $altUser;
 
-	/**
-	 * @var PermissionManager
-	 */
-	protected $permissionManager;
-
 	/** Constant for self::testIsBlockedFrom */
 	const USER_TALK_PAGE = '<user talk page>';
 
@@ -1653,4 +1649,39 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
 		$this->assertFalse( $result );
 	}
 
+	/**
+	 * @covers \MediaWiki\Permissions\PermissionManager::addTemporaryUserRights
+	 * @covers \MediaWiki\Permissions\PermissionManager::revokeTemporaryUserRights
+	 */
+	public function testTemporaryUserRights() {
+		$permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
+		$this->overrideUserPermissions( $this->user, [ 'read', 'edit' ] );
+		// sanity checks
+		$this->assertEquals( [ 'read', 'edit' ], $permissionManager->getUserPermissions( $this->user ) );
+		$this->assertFalse( $permissionManager->userHasRight( $this->user, 'move' ) );
+
+		$scope = $permissionManager->addTemporaryUserRights( $this->user, [ 'move', 'delete' ] );
+		$this->assertEquals( [ 'read', 'edit', 'move', 'delete' ],
+			$permissionManager->getUserPermissions( $this->user ) );
+		$this->assertTrue( $permissionManager->userHasRight( $this->user, 'move' ) );
+
+		$scope2 = $permissionManager->addTemporaryUserRights( $this->user, [ 'delete', 'upload' ] );
+		$this->assertEquals( [ 'read', 'edit', 'move', 'delete', 'upload' ],
+			$permissionManager->getUserPermissions( $this->user ) );
+
+		ScopedCallback::consume( $scope );
+		$this->assertEquals( [ 'read', 'edit', 'delete', 'upload' ],
+			$permissionManager->getUserPermissions( $this->user ) );
+		ScopedCallback::consume( $scope2 );
+		$this->assertEquals( [ 'read', 'edit' ],
+			$permissionManager->getUserPermissions( $this->user ) );
+		$this->assertFalse( $permissionManager->userHasRight( $this->user, 'move' ) );
+
+		( function () use ( $permissionManager ) {
+			$scope = $permissionManager->addTemporaryUserRights( $this->user, 'move' );
+			$this->assertTrue( $permissionManager->userHasRight( $this->user, 'move' ) );
+		} )();
+		$this->assertFalse( $permissionManager->userHasRight( $this->user, 'move' ) );
+	}
+
 }