Replace $wgUser with RequestContext::getUser in User::getBlockedStatus

$wgUser is not guaranteed to exist until MediaWiki has been fully
initialized; block status needs to be checked early on for
authentication-related permission checks.

Bug: T218608
Change-Id: I16315c071855024bc0412d5360c95f843420d9a9

diff --git a/includes/user/User.php b/includes/user/User.php
index 76691ea..86bb27b 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -1775,7 +1775,7 @@ class User implements IDBAccessObject, UserIdentity {
         *   Check when actually saving should be done against master.
         */
        private function getBlockedStatus( $bFromSlave = true ) {
-               global $wgProxyWhitelist, $wgUser, $wgApplyIpBlocksToXff, $wgSoftBlockRanges;
+               global $wgProxyWhitelist, $wgApplyIpBlocksToXff, $wgSoftBlockRanges;
 
                if ( -1 != $this->mBlockedby ) {
                        return;
@@ -1795,11 +1795,12 @@ class User implements IDBAccessObject, UserIdentity {
                # know which IP address they're actually coming from
                $ip = null;
                if ( !$this->isAllowed( 'ipblock-exempt' ) ) {
-                       // $wgUser->getName() only works after the end of Setup.php. Until
-                       // then, assume it's a logged-out user.
-                       $globalUserName = $wgUser->isSafeToLoad()
-                               ? $wgUser->getName()
-                               : IP::sanitizeIP( $wgUser->getRequest()->getIP() );
+                       $sessionUser = RequestContext::getMain()->getUser();
+                       // the session user is set up towards the end of Setup.php. Until then,
+                       // assume it's a logged-out user.
+                       $globalUserName = $sessionUser->isSafeToLoad()
+                               ? $sessionUser->getName()
+                               : IP::sanitizeIP( $sessionUser->getRequest()->getIP() );
                        if ( $this->getName() === $globalUserName ) {
                                $ip = $this->getRequest()->getIP();
                        }
@@ -1881,9 +1882,9 @@ class User implements IDBAccessObject, UserIdentity {
                }
 
                // Avoid PHP 7.1 warning of passing $this by reference
-               $user = $this;
+               $thisUser = $this;
                // Extensions
-               Hooks::run( 'GetBlockedStatus', [ &$user ] );
+               Hooks::run( 'GetBlockedStatus', [ &$thisUser ] );
        }
 
        /**

diff --git a/tests/phpunit/includes/auth/CheckBlocksSecondaryAuthenticationProviderTest.php b/tests/phpunit/includes/auth/CheckBlocksSecondaryAuthenticationProviderTest.php
index 81cdc9d..e8b61c5 100644 (file)
--- a/tests/phpunit/includes/auth/CheckBlocksSecondaryAuthenticationProviderTest.php
+++ b/tests/phpunit/includes/auth/CheckBlocksSecondaryAuthenticationProviderTest.php
@@ -165,6 +165,7 @@ class CheckBlocksSecondaryAuthenticationProviderTest extends \MediaWikiTestCase
                        $user->saveSettings();
                }
                $this->setMwGlobals( [ 'wgUser' => $user ] );
+               \RequestContext::getMain()->setUser( $user );
                $newuser = \User::newFromName( 'RandomUser' );
 
                $provider = new CheckBlocksSecondaryAuthenticationProvider(

diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index e819d35..ebfecbc 100644 (file)
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -795,30 +795,36 @@ class UserTest extends MediaWikiTestCase {
        }
 
        public function testSoftBlockRanges() {
-               global $wgUser;
-
-               $this->setMwGlobals( [
-                       'wgSoftBlockRanges' => [ '10.0.0.0/8' ],
-                       'wgUser' => null,
-               ] );
+               $setSessionUser = function ( User $user, WebRequest $request ) {
+                       $this->setMwGlobals( 'wgUser', $user );
+                       RequestContext::getMain()->setUser( $user );
+                       RequestContext::getMain()->setRequest( $request );
+                       TestingAccessWrapper::newFromObject( $user )->mRequest = $request;
+                       $request->getSession()->setUser( $user );
+               };
+               $this->setMwGlobals( 'wgSoftBlockRanges', [ '10.0.0.0/8' ] );
 
                // IP isn't in $wgSoftBlockRanges
+               $wgUser = new User();
                $request = new FauxRequest();
                $request->setIP( '192.168.0.1' );
-               $wgUser = User::newFromSession( $request );
+               $setSessionUser( $wgUser, $request );
                $this->assertNull( $wgUser->getBlock() );
 
                // IP is in $wgSoftBlockRanges
+               $wgUser = new User();
                $request = new FauxRequest();
                $request->setIP( '10.20.30.40' );
-               $wgUser = User::newFromSession( $request );
+               $setSessionUser( $wgUser, $request );
                $block = $wgUser->getBlock();
                $this->assertInstanceOf( Block::class, $block );
                $this->assertSame( 'wgSoftBlockRanges', $block->getSystemBlockType() );
 
                // Make sure the block is really soft
-               $request->getSession()->setUser( $this->getTestUser()->getUser() );
-               $wgUser = User::newFromSession( $request );
+               $wgUser = $this->getTestUser()->getUser();
+               $request = new FauxRequest();
+               $request->setIP( '10.20.30.40' );
+               $setSessionUser( $wgUser, $request );
                $this->assertFalse( $wgUser->isAnon(), 'sanity check' );
                $this->assertNull( $wgUser->getBlock() );
        }