Merge "Replace User::isAllowed with PermissionManager."
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 21 Aug 2019 08:00:33 +0000 (08:00 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 21 Aug 2019 08:00:33 +0000 (08:00 +0000)
1  2 
includes/MovePage.php
includes/ServiceWiring.php
includes/Title.php
includes/api/ApiMove.php
includes/block/BlockManager.php
tests/phpunit/includes/block/BlockManagerTest.php

diff --combined includes/MovePage.php
   * @file
   */
  
 +use MediaWiki\Config\ServiceOptions;
  use MediaWiki\MediaWikiServices;
 +use MediaWiki\Page\MovePageFactory;
 +use MediaWiki\Permissions\PermissionManager;
  use MediaWiki\Revision\SlotRecord;
  use Wikimedia\Rdbms\IDatabase;
 +use Wikimedia\Rdbms\LoadBalancer;
  
  /**
   * Handles the backend logic of moving a page from one title
@@@ -45,69 -41,9 +45,69 @@@ class MovePage 
         */
        protected $newTitle;
  
 -      public function __construct( Title $oldTitle, Title $newTitle ) {
 +      /**
 +       * @var ServiceOptions
 +       */
 +      protected $options;
 +
 +      /**
 +       * @var LoadBalancer
 +       */
 +      protected $loadBalancer;
 +
 +      /**
 +       * @var NamespaceInfo
 +       */
 +      protected $nsInfo;
 +
 +      /**
 +       * @var WatchedItemStore
 +       */
 +      protected $watchedItems;
 +
 +      /**
 +       * @var PermissionManager
 +       */
 +      protected $permMgr;
 +
 +      /**
 +       * @var RepoGroup
 +       */
 +      protected $repoGroup;
 +
 +      /**
 +       * Calling this directly is deprecated in 1.34. Use MovePageFactory instead.
 +       *
 +       * @param Title $oldTitle
 +       * @param Title $newTitle
 +       * @param ServiceOptions|null $options
 +       * @param LoadBalancer|null $loadBalancer
 +       * @param NamespaceInfo|null $nsInfo
 +       * @param WatchedItemStore|null $watchedItems
 +       * @param PermissionManager|null $permMgr
 +       */
 +      public function __construct(
 +              Title $oldTitle,
 +              Title $newTitle,
 +              ServiceOptions $options = null,
 +              LoadBalancer $loadBalancer = null,
 +              NamespaceInfo $nsInfo = null,
 +              WatchedItemStore $watchedItems = null,
 +              PermissionManager $permMgr = null,
 +              RepoGroup $repoGroup = null
 +      ) {
                $this->oldTitle = $oldTitle;
                $this->newTitle = $newTitle;
 +              $this->options = $options ??
 +                      new ServiceOptions( MovePageFactory::$constructorOptions,
 +                              MediaWikiServices::getInstance()->getMainConfig() );
 +              $this->loadBalancer =
 +                      $loadBalancer ?? MediaWikiServices::getInstance()->getDBLoadBalancer();
 +              $this->nsInfo = $nsInfo ?? MediaWikiServices::getInstance()->getNamespaceInfo();
 +              $this->watchedItems =
 +                      $watchedItems ?? MediaWikiServices::getInstance()->getWatchedItemStore();
 +              $this->permMgr = $permMgr ?? MediaWikiServices::getInstance()->getPermissionManager();
 +              $this->repoGroup = $repoGroup ?? MediaWikiServices::getInstance()->getRepoGroup();
        }
  
        /**
                $status = new Status();
  
                $errors = wfMergeErrorArrays(
 -                      $this->oldTitle->getUserPermissionsErrors( 'move', $user ),
 -                      $this->oldTitle->getUserPermissionsErrors( 'edit', $user ),
 -                      $this->newTitle->getUserPermissionsErrors( 'move-target', $user ),
 -                      $this->newTitle->getUserPermissionsErrors( 'edit', $user )
 +                      $this->permMgr->getPermissionErrors( 'move', $user, $this->oldTitle ),
 +                      $this->permMgr->getPermissionErrors( 'edit', $user, $this->oldTitle ),
 +                      $this->permMgr->getPermissionErrors( 'move-target', $user, $this->newTitle ),
 +                      $this->permMgr->getPermissionErrors( 'edit', $user, $this->newTitle )
                );
  
                // Convert into a Status object
                }
  
                $tp = $this->newTitle->getTitleProtection();
-               if ( $tp !== false && !$user->isAllowed( $tp['permission'] ) ) {
-                               $status->fatal( 'cantmove-titleprotected' );
+               $permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
+               if ( $tp !== false && !$permissionManager->userHasRight( $user, $tp['permission'] ) ) {
+                       $status->fatal( 'cantmove-titleprotected' );
                }
  
                Hooks::run( 'MovePageCheckPermissions',
         * @return Status
         */
        public function isValidMove() {
 -              global $wgContentHandlerUseDB;
                $status = new Status();
  
                if ( $this->oldTitle->equals( $this->newTitle ) ) {
                        $status->fatal( 'selfmove' );
 +              } elseif ( $this->newTitle->getArticleID() && !$this->isValidMoveTarget() ) {
 +                      // The move is allowed only if (1) the target doesn't exist, or (2) the target is a
 +                      // redirect to the source, and has no history (so we can undo bad moves right after
 +                      // they're done).
 +                      $status->fatal( 'articleexists' );
                }
 -              if ( !$this->oldTitle->isMovable() ) {
 +
 +              // @todo If the old title is invalid, maybe we should check if it somehow exists in the
 +              // database and allow moving it to a valid name? Why prohibit the move from an empty name
 +              // without checking in the database?
 +              if ( $this->oldTitle->getDBkey() == '' ) {
 +                      $status->fatal( 'badarticleerror' );
 +              } elseif ( $this->oldTitle->isExternal() ) {
 +                      $status->fatal( 'immobile-source-namespace-iw' );
 +              } elseif ( !$this->oldTitle->isMovable() ) {
                        $status->fatal( 'immobile-source-namespace', $this->oldTitle->getNsText() );
 +              } elseif ( !$this->oldTitle->exists() ) {
 +                      $status->fatal( 'movepage-source-doesnt-exist' );
                }
 +
                if ( $this->newTitle->isExternal() ) {
                        $status->fatal( 'immobile-target-namespace-iw' );
 -              }
 -              if ( !$this->newTitle->isMovable() ) {
 +              } elseif ( !$this->newTitle->isMovable() ) {
                        $status->fatal( 'immobile-target-namespace', $this->newTitle->getNsText() );
                }
 -
 -              $oldid = $this->oldTitle->getArticleID();
 -
 -              if ( $this->newTitle->getDBkey() === '' ) {
 -                      $status->fatal( 'articleexists' );
 -              }
 -              if (
 -                      ( $this->oldTitle->getDBkey() == '' ) ||
 -                      ( !$oldid ) ||
 -                      ( $this->newTitle->getDBkey() == '' )
 -              ) {
 -                      $status->fatal( 'badarticleerror' );
 -              }
 -
 -              # The move is allowed only if (1) the target doesn't exist, or
 -              # (2) the target is a redirect to the source, and has no history
 -              # (so we can undo bad moves right after they're done).
 -              if ( $this->newTitle->getArticleID() && !$this->isValidMoveTarget() ) {
 -                      $status->fatal( 'articleexists' );
 +              if ( !$this->newTitle->isValid() ) {
 +                      $status->fatal( 'movepage-invalid-target-title' );
                }
  
                // Content model checks
 -              if ( !$wgContentHandlerUseDB &&
 +              if ( !$this->options->get( 'ContentHandlerUseDB' ) &&
                        $this->oldTitle->getContentModel() !== $this->newTitle->getContentModel() ) {
                        // can't move a page if that would change the page's content model
                        $status->fatal(
         */
        protected function isValidFileMove() {
                $status = new Status();
 -              $file = wfLocalFile( $this->oldTitle );
 +
 +              if ( !$this->newTitle->inNamespace( NS_FILE ) ) {
 +                      $status->fatal( 'imagenocrossnamespace' );
 +                      // No need for further errors about the target filename being wrong
 +                      return $status;
 +              }
 +
 +              $file = $this->repoGroup->getLocalRepo()->newFile( $this->oldTitle );
                $file->load( File::READ_LATEST );
                if ( $file->exists() ) {
                        if ( $this->newTitle->getText() != wfStripIllegalFilenameChars( $this->newTitle->getText() ) ) {
                        }
                }
  
 -              if ( !$this->newTitle->inNamespace( NS_FILE ) ) {
 -                      $status->fatal( 'imagenocrossnamespace' );
 -              }
 -
                return $status;
        }
  
        protected function isValidMoveTarget() {
                # Is it an existing file?
                if ( $this->newTitle->inNamespace( NS_FILE ) ) {
 -                      $file = wfLocalFile( $this->newTitle );
 +                      $file = $this->repoGroup->getLocalRepo()->newFile( $this->newTitle );
                        $file->load( File::READ_LATEST );
                        if ( $file->exists() ) {
                                wfDebug( __METHOD__ . ": file exists\n" );
                }
  
                // Check suppressredirect permission
-               if ( !$user->isAllowed( 'suppressredirect' ) ) {
+               $permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
+               if ( !$permissionManager->userHasRight( $user, 'suppressredirect' ) ) {
                        $createRedirect = true;
                }
  
         * @return Status
         */
        private function moveUnsafe( User $user, $reason, $createRedirect, array $changeTags ) {
 -              global $wgCategoryCollation;
 -
                $status = Status::newGood();
                Hooks::run( 'TitleMove', [ $this->oldTitle, $this->newTitle, $user, $reason, &$status ] );
                if ( !$status->isOK() ) {
                        return $status;
                }
  
 -              $dbw = wfGetDB( DB_MASTER );
 +              $dbw = $this->loadBalancer->getConnection( DB_MASTER );
                $dbw->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
  
                Hooks::run( 'TitleMoveStarting', [ $this->oldTitle, $this->newTitle, $user ] );
                        [ 'cl_from' => $pageid ],
                        __METHOD__
                );
 -              $services = MediaWikiServices::getInstance();
 -              $type = $services->getNamespaceInfo()->
 -                      getCategoryLinkType( $this->newTitle->getNamespace() );
 +              $type = $this->nsInfo->getCategoryLinkType( $this->newTitle->getNamespace() );
                foreach ( $prefixes as $prefixRow ) {
                        $prefix = $prefixRow->cl_sortkey_prefix;
                        $catTo = $prefixRow->cl_to;
                                [
                                        'cl_sortkey' => Collation::singleton()->getSortKey(
                                                        $this->newTitle->getCategorySortkey( $prefix ) ),
 -                                      'cl_collation' => $wgCategoryCollation,
 +                                      'cl_collation' => $this->options->get( 'CategoryCollation' ),
                                        'cl_type' => $type,
                                        'cl_timestamp=cl_timestamp' ],
                                [
                # Update watchlists
                $oldtitle = $this->oldTitle->getDBkey();
                $newtitle = $this->newTitle->getDBkey();
 -              $oldsnamespace = $services->getNamespaceInfo()->
 -                      getSubject( $this->oldTitle->getNamespace() );
 -              $newsnamespace = $services->getNamespaceInfo()->
 -                      getSubject( $this->newTitle->getNamespace() );
 +              $oldsnamespace = $this->nsInfo->getSubject( $this->oldTitle->getNamespace() );
 +              $newsnamespace = $this->nsInfo->getSubject( $this->newTitle->getNamespace() );
                if ( $oldsnamespace != $newsnamespace || $oldtitle != $newtitle ) {
 -                      $services->getWatchedItemStore()->duplicateAllAssociatedEntries(
 -                              $this->oldTitle, $this->newTitle );
 +                      $this->watchedItems->duplicateAllAssociatedEntries( $this->oldTitle, $this->newTitle );
                }
  
                // If it is a file then move it last.
                        $oldTitle->getPrefixedText()
                );
  
 -              $file = wfLocalFile( $oldTitle );
 +              $file = $this->repoGroup->getLocalRepo()->newFile( $oldTitle );
                $file->load( File::READ_LATEST );
                if ( $file->exists() ) {
                        $status = $file->move( $newTitle );
                }
  
                // Clear RepoGroup process cache
 -              RepoGroup::singleton()->clearCache( $oldTitle );
 -              RepoGroup::singleton()->clearCache( $newTitle ); # clear false negative cache
 +              $this->repoGroup->clearCache( $oldTitle );
 +              $this->repoGroup->clearCache( $newTitle ); # clear false negative cache
                return $status;
        }
  
                        $comment .= wfMessage( 'colon-separator' )->inContentLanguage()->text() . $reason;
                }
  
 -              $dbw = wfGetDB( DB_MASTER );
 +              $dbw = $this->loadBalancer->getConnection( DB_MASTER );
  
                $oldpage = WikiPage::factory( $this->oldTitle );
                $oldcountable = $oldpage->isCountable();
@@@ -50,7 -50,6 +50,7 @@@ use MediaWiki\Linker\LinkRenderer
  use MediaWiki\Linker\LinkRendererFactory;
  use MediaWiki\Logger\LoggerFactory;
  use MediaWiki\MediaWikiServices;
 +use MediaWiki\Page\MovePageFactory;
  use MediaWiki\Permissions\PermissionManager;
  use MediaWiki\Preferences\PreferencesFactory;
  use MediaWiki\Preferences\DefaultPreferencesFactory;
@@@ -98,7 -97,8 +98,8 @@@ return 
                                BlockManager::$constructorOptions, $services->getMainConfig()
                        ),
                        $context->getUser(),
-                       $context->getRequest()
+                       $context->getRequest(),
+                       $services->getPermissionManager()
                );
        },
  
        },
  
        'LocalServerObjectCache' => function ( MediaWikiServices $services ) : BagOStuff {
 +              $config = $services->getMainConfig();
                $cacheId = \ObjectCache::detectLocalServerCache();
  
 -              return \ObjectCache::newFromId( $cacheId );
 +              return \ObjectCache::newFromParams( $config->get( 'ObjectCaches' )[$cacheId] );
        },
  
        'MagicWordFactory' => function ( MediaWikiServices $services ) : MagicWordFactory {
                );
        },
  
 +      'MessageCache' => function ( MediaWikiServices $services ) : MessageCache {
 +              $mainConfig = $services->getMainConfig();
 +              return new MessageCache(
 +                      $services->getMainWANObjectCache(),
 +                      ObjectCache::getInstance( $mainConfig->get( 'MessageCacheType' ) ),
 +                      $mainConfig->get( 'UseLocalMessageCache' )
 +                              ? $services->getLocalServerObjectCache()
 +                              : new EmptyBagOStuff(),
 +                      $mainConfig->get( 'UseDatabaseMessages' ),
 +                      $mainConfig->get( 'MsgCacheExpiry' ),
 +                      $services->getContentLanguage()
 +              );
 +      },
 +
        'MimeAnalyzer' => function ( MediaWikiServices $services ) : MimeAnalyzer {
                $logger = LoggerFactory::getInstance( 'Mime' );
                $mainConfig = $services->getMainConfig();
                return new MimeAnalyzer( $params );
        },
  
 +      'MovePageFactory' => function ( MediaWikiServices $services ) : MovePageFactory {
 +              return new MovePageFactory(
 +                      new ServiceOptions( MovePageFactory::$constructorOptions, $services->getMainConfig() ),
 +                      $services->getDBLoadBalancer(),
 +                      $services->getNamespaceInfo(),
 +                      $services->getWatchedItemStore(),
 +                      $services->getPermissionManager(),
 +                      $services->getRepoGroup()
 +              );
 +      },
 +
        'NamespaceInfo' => function ( MediaWikiServices $services ) : NamespaceInfo {
                return new NamespaceInfo( new ServiceOptions( NamespaceInfo::$constructorOptions,
                        $services->getMainConfig() ) );
diff --combined includes/Title.php
@@@ -2506,8 -2506,9 +2506,9 @@@ class Title implements LinkTarget, IDBA
                global $wgNamespaceProtection;
  
                if ( isset( $wgNamespaceProtection[$this->mNamespace] ) ) {
+                       $permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
                        foreach ( (array)$wgNamespaceProtection[$this->mNamespace] as $right ) {
-                               if ( $right != '' && !$user->isAllowed( $right ) ) {
+                               if ( !$permissionManager->userHasRight( $user, $right ) ) {
                                        return true;
                                }
                        }
                        return [ [ 'badtitletext' ] ];
                }
  
 -              $mp = new MovePage( $this, $nt );
 +              $mp = MediaWikiServices::getInstance()->getMovePageFactory()->newMovePage( $this, $nt );
                $errors = $mp->isValidMove()->getErrorsArray();
                if ( $auth ) {
                        $errors = wfMergeErrorArrays(
  
                global $wgUser;
  
 -              $mp = new MovePage( $this, $nt );
 +              $mp = MediaWikiServices::getInstance()->getMovePageFactory()->newMovePage( $this, $nt );
                $method = $auth ? 'moveIfAllowed' : 'move';
                $status = $mp->$method( $wgUser, $reason, $createRedirect, $changeTags );
                if ( $status->isOK() ) {
diff --combined includes/api/ApiMove.php
@@@ -63,9 -63,10 +63,10 @@@ class ApiMove extends ApiBase 
                        && !RepoGroup::singleton()->getLocalRepo()->findFile( $toTitle )
                        && MediaWikiServices::getInstance()->getRepoGroup()->findFile( $toTitle )
                ) {
-                       if ( !$params['ignorewarnings'] && $user->isAllowed( 'reupload-shared' ) ) {
+                       if ( !$params['ignorewarnings'] &&
+                                $this->getPermissionManager()->userHasRight( $user, 'reupload-shared' ) ) {
                                $this->dieWithError( 'apierror-fileexists-sharedrepo-perm' );
-                       } elseif ( !$user->isAllowed( 'reupload-shared' ) ) {
+                       } elseif ( !$this->getPermissionManager()->userHasRight( $user, 'reupload-shared' ) ) {
                                $this->dieWithError( 'apierror-cantoverwrite-sharedfile' );
                        }
                }
         * @return Status
         */
        protected function movePage( Title $from, Title $to, $reason, $createRedirect, $changeTags ) {
 -              $mp = new MovePage( $from, $to );
 +              $mp = MediaWikiServices::getInstance()->getMovePageFactory()->newMovePage( $from, $to );
                $valid = $mp->isValidMove();
                if ( !$valid->isOK() ) {
                        return $valid;
                }
  
                // Check suppressredirect permission
-               if ( !$user->isAllowed( 'suppressredirect' ) ) {
+               if ( !$this->getPermissionManager()->userHasRight( $user, 'suppressredirect' ) ) {
                        $createRedirect = true;
                }
  
  namespace MediaWiki\Block;
  
  use DateTime;
 +use DateTimeZone;
  use DeferredUpdates;
  use IP;
  use MediaWiki\Config\ServiceOptions;
+ use MediaWiki\Permissions\PermissionManager;
  use MediaWiki\User\UserIdentity;
  use MWCryptHash;
  use User;
@@@ -46,6 -46,9 +47,9 @@@ class BlockManager 
        /** @var WebRequest */
        private $currentRequest;
  
+       /** @var PermissionManager */
+       private $permissionManager;
        /**
         * TODO Make this a const when HHVM support is dropped (T192166)
         *
         * @param ServiceOptions $options
         * @param User $currentUser
         * @param WebRequest $currentRequest
+        * @param PermissionManager $permissionManager
         */
        public function __construct(
                ServiceOptions $options,
                User $currentUser,
-               WebRequest $currentRequest
+               WebRequest $currentRequest,
+               PermissionManager $permissionManager
        ) {
                $options->assertRequiredOptions( self::$constructorOptions );
                $this->options = $options;
                $this->currentUser = $currentUser;
                $this->currentRequest = $currentRequest;
+               $this->permissionManager = $permissionManager;
        }
  
        /**
                $globalUserName = $sessionUser->isSafeToLoad()
                        ? $sessionUser->getName()
                        : IP::sanitizeIP( $this->currentRequest->getIP() );
-               if ( $user->getName() === $globalUserName && !$user->isAllowed( 'ipblock-exempt' ) ) {
+               if ( $user->getName() === $globalUserName &&
+                        !$this->permissionManager->userHasRight( $user, 'ipblock-exempt' ) ) {
                        $ip = $this->currentRequest->getIP();
                }
  
  
        /**
         * Try to load a block from an ID given in a cookie value. If the block is invalid
 -       * or doesn't exist, remove the cookie.
 +       * doesn't exist, or the cookie value is malformed, remove the cookie.
         *
         * @param UserIdentity $user
         * @param WebRequest $request
                UserIdentity $user,
                WebRequest $request
        ) {
 -              $blockCookieId = $this->getIdFromCookieValue( $request->getCookie( 'BlockID' ) );
 +              $cookieValue = $request->getCookie( 'BlockID' );
 +              if ( is_null( $cookieValue ) ) {
 +                      return false;
 +              }
  
 -              if ( $blockCookieId !== null ) {
 +              $blockCookieId = $this->getIdFromCookieValue( $cookieValue );
 +              if ( !is_null( $blockCookieId ) ) {
                        // TODO: remove dependency on DatabaseBlock
                        $block = DatabaseBlock::newFromID( $blockCookieId );
                        if (
                        ) {
                                return $block;
                        }
 -                      $this->clearBlockCookie( $request->response() );
                }
  
 +              $this->clearBlockCookie( $request->response() );
 +
                return false;
        }
  
                }
  
                // Set the cookie. Reformat the MediaWiki datetime as a Unix timestamp for the cookie.
 -              $expiryValue = DateTime::createFromFormat( 'YmdHis', $expiryTime )->format( 'U' );
 +              $expiryValue = DateTime::createFromFormat(
 +                      'YmdHis',
 +                      $expiryTime,
 +                      new DateTimeZone( 'UTC' )
 +              )->format( 'U' );
                $cookieOptions = [ 'httpOnly' => false ];
                $cookieValue = $this->getCookieValue( $block );
                $response->setCookie( 'BlockID', $cookieValue, $expiryValue, $cookieOptions );
@@@ -4,6 -4,7 +4,6 @@@ use MediaWiki\Block\BlockManager
  use MediaWiki\Block\DatabaseBlock;
  use MediaWiki\Block\CompositeBlock;
  use MediaWiki\Block\SystemBlock;
 -use MediaWiki\Config\ServiceOptions;
  use MediaWiki\MediaWikiServices;
  use Wikimedia\TestingAccessWrapper;
  
@@@ -13,7 -14,6 +13,7 @@@
   * @coversDefaultClass \MediaWiki\Block\BlockManager
   */
  class BlockManagerTest extends MediaWikiTestCase {
 +      use TestAllServiceOptionsUsed;
  
        /** @var User */
        protected $user;
                $this->setMwGlobals( $blockManagerConfig );
                $this->overrideMwServices();
                return [
 -                      new ServiceOptions(
 +                      new LoggedServiceOptions(
 +                              self::$serviceOptionsAccessLog,
                                BlockManager::$constructorOptions,
                                MediaWikiServices::getInstance()->getMainConfig()
                        ),
                        $this->user,
-                       $this->user->getRequest()
+                       $this->user->getRequest(),
+                       MediaWikiServices::getInstance()->getPermissionManager()
                ];
        }
  
                ];
        }
  
 +      /**
 +       * @coversNothing
 +       */
 +      public function testAllServiceOptionsUsed() {
 +              $this->assertAllServiceOptionsUsed( [ 'ApplyIpBlocksToXff', 'SoftBlockRanges' ] );
 +      }
  }