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 69b2088..089471a 100644 (file)
--- 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 defcb65..98a5b17 100644 (file)
--- 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 3da73c1..03b35b5 100644 (file)
--- 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' ) );
+       }
+
 }