Allow partially blocked users to tag unrelated revisions

Currently, partially blocked users cannot add, modify, or remove tags from
revisions that they are not blocked from. This change allows them to modify
the tags on those pages.

Bug: T221444
Change-Id: I737ed82f9c5139b45922cb4ea9111b4fdc7c1c61

diff --git a/includes/api/ApiTag.php b/includes/api/ApiTag.php
index aff0183..bb6c580 100644 (file)
--- a/includes/api/ApiTag.php
+++ b/includes/api/ApiTag.php
@@ -20,7 +20,6 @@
  */
 
 use MediaWiki\MediaWikiServices;
-use MediaWiki\Revision\RevisionStore;
 
 /**
  * @ingroup API
@@ -28,7 +27,9 @@ use MediaWiki\Revision\RevisionStore;
  */
 class ApiTag extends ApiBase {
 
-       /** @var RevisionStore */
+       use ApiBlockInfoTrait;
+
+       /** @var \MediaWiki\Revision\RevisionStore */
        private $revisionStore;
 
        public function execute() {
@@ -40,9 +41,9 @@ class ApiTag extends ApiBase {
                // make sure the user is allowed
                $this->checkUserRightsAny( 'changetags' );
 
-               // @TODO Use PermissionManager::isBlockedFrom() instead.
+               // Fail early if the user is sitewide blocked.
                $block = $user->getBlock();
-               if ( $block ) {
+               if ( $block && $block->isSitewide() ) {
                        $this->dieBlocked( $block );
                }
 
@@ -85,6 +86,7 @@ class ApiTag extends ApiBase {
        }
 
        protected function processIndividual( $type, $params, $id ) {
+               $user = $this->getUser();
                $idResult = [ $type => $id ];
 
                // validate the ID
@@ -92,9 +94,30 @@ class ApiTag extends ApiBase {
                switch ( $type ) {
                        case 'rcid':
                                $valid = RecentChange::newFromId( $id );
+                               if ( $valid && $this->getPermissionManager()->isBlockedFrom( $user, $valid->getTitle() ) ) {
+                                       $idResult['status'] = 'error';
+                                       $idResult += $this->getErrorFormatter()->formatMessage( ApiMessage::create(
+                                               'apierror-blocked',
+                                               'blocked',
+                                               [ 'blockinfo' => $this->getBlockDetails( $user->getBlock() ) ]
+                                       ) );
+                                       return $idResult;
+                               }
                                break;
                        case 'revid':
                                $valid = $this->revisionStore->getRevisionById( $id );
+                               if (
+                                       $valid &&
+                                       $this->getPermissionManager()->isBlockedFrom( $user, $valid->getPageAsLinkTarget() )
+                               ) {
+                                       $idResult['status'] = 'error';
+                                       $idResult += $this->getErrorFormatter()->formatMessage( ApiMessage::create(
+                                                       'apierror-blocked',
+                                                       'blocked',
+                                                       [ 'blockinfo' => $this->getBlockDetails( $user->getBlock() ) ]
+                                       ) );
+                                       return $idResult;
+                               }
                                break;
                        case 'logid':
                                $valid = self::validateLogId( $id );

diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php
index 9ee000d..ba6cb2c 100644 (file)
--- a/includes/changetags/ChangeTags.php
+++ b/includes/changetags/ChangeTags.php
@@ -524,9 +524,7 @@ class ChangeTags {
                                        ->userHasRight( $user, 'applychangetags' )
                        ) {
                                return Status::newFatal( 'tags-apply-no-permission' );
-                       } elseif ( $user->getBlock() ) {
-                               // @TODO Ensure that the block does not apply to the `applychangetags`
-                               //       right.
+                       } elseif ( $user->getBlock() && $user->getBlock()->isSitewide() ) {
                                return Status::newFatal( 'tags-apply-blocked', $user->getName() );
                        }
                }
@@ -601,9 +599,7 @@ class ChangeTags {
                                        ->userHasRight( $user, 'changetags' )
                        ) {
                                return Status::newFatal( 'tags-update-no-permission' );
-                       } elseif ( $user->getBlock() ) {
-                               // @TODO Ensure that the block does not apply to the `changetags`
-                               //       right.
+                       } elseif ( $user->getBlock() && $user->getBlock()->isSitewide() ) {
                                return Status::newFatal( 'tags-update-blocked', $user->getName() );
                        }
                }
@@ -1023,9 +1019,7 @@ class ChangeTags {
                                        ->userHasRight( $user, 'managechangetags' )
                        ) {
                                return Status::newFatal( 'tags-manage-no-permission' );
-                       } elseif ( $user->getBlock() ) {
-                               // @TODO Ensure that the block does not apply to the `managechangetags`
-                               //       right.
+                       } elseif ( $user->getBlock() && $user->getBlock()->isSitewide() ) {
                                return Status::newFatal( 'tags-manage-blocked', $user->getName() );
                        }
                }
@@ -1099,9 +1093,7 @@ class ChangeTags {
                                        ->userHasRight( $user, 'managechangetags' )
                        ) {
                                return Status::newFatal( 'tags-manage-no-permission' );
-                       } elseif ( $user->getBlock() ) {
-                               // @TODO Ensure that the block does not apply to the `managechangetags`
-                               //       right.
+                       } elseif ( $user->getBlock() && $user->getBlock()->isSitewide() ) {
                                return Status::newFatal( 'tags-manage-blocked', $user->getName() );
                        }
                }
@@ -1200,9 +1192,7 @@ class ChangeTags {
                                        ->userHasRight( $user, 'managechangetags' )
                        ) {
                                return Status::newFatal( 'tags-manage-no-permission' );
-                       } elseif ( $user->getBlock() ) {
-                               // @TODO Ensure that the block does not apply to the `managechangetags`
-                               //       right.
+                       } elseif ( $user->getBlock() && $user->getBlock()->isSitewide() ) {
                                return Status::newFatal( 'tags-manage-blocked', $user->getName() );
                        }
                }
@@ -1322,9 +1312,7 @@ class ChangeTags {
                                        ->userHasRight( $user, 'deletechangetags' )
                        ) {
                                return Status::newFatal( 'tags-delete-no-permission' );
-                       } elseif ( $user->getBlock() ) {
-                               // @TODO Ensure that the block does not apply to the `deletechangetags`
-                               //       right.
+                       } elseif ( $user->getBlock() && $user->getBlock()->isSitewide() ) {
                                return Status::newFatal( 'tags-manage-blocked', $user->getName() );
                        }
                }

diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php
index 2737e35..40fcd74 100644 (file)
--- a/includes/specialpage/SpecialPageFactory.php
+++ b/includes/specialpage/SpecialPageFactory.php
@@ -192,7 +192,12 @@ class SpecialPageFactory {
                'ApiHelp' => \SpecialApiHelp::class,
                'Blankpage' => \SpecialBlankpage::class,
                'Diff' => \SpecialDiff::class,
-               'EditTags' => \SpecialEditTags::class,
+               'EditTags' => [
+                       'class' => \SpecialEditTags::class,
+                       'services' => [
+                               'PermissionManager',
+                       ],
+               ],
                'Emailuser' => \SpecialEmailUser::class,
                'Movepage' => \MovePageForm::class,
                'Mycontributions' => \SpecialMycontributions::class,

diff --git a/includes/specials/SpecialEditTags.php b/includes/specials/SpecialEditTags.php
index 70a1bd4..1dd1969 100644 (file)
--- a/includes/specials/SpecialEditTags.php
+++ b/includes/specials/SpecialEditTags.php
@@ -19,6 +19,8 @@
  * @ingroup SpecialPage
  */
 
+use MediaWiki\Permissions\PermissionManager;
+
 /**
  * Special page for adding and removing change tags to individual revisions.
  * A lot of this is copied out of SpecialRevisiondelete.
@@ -51,8 +53,18 @@ class SpecialEditTags extends UnlistedSpecialPage {
        /** @var string */
        private $reason;
 
-       public function __construct() {
+       /** @var PermissionManager */
+       private $permissionManager;
+
+       /**
+        * @inheritDoc
+        *
+        * @param PermissionManager $permissionManager
+        */
+       public function __construct( PermissionManager $permissionManager ) {
                parent::__construct( 'EditTags', 'changetags' );
+
+               $this->permissionManager = $permissionManager;
        }
 
        public function doesWrites() {
@@ -67,13 +79,6 @@ class SpecialEditTags extends UnlistedSpecialPage {
                $user = $this->getUser();
                $request = $this->getRequest();
 
-               // Check blocks
-               // @TODO Use PermissionManager::isBlockedFrom() instead.
-               $block = $user->getBlock();
-               if ( $block ) {
-                       throw new UserBlockedError( $block );
-               }
-
                $this->setHeaders();
                $this->outputHeader();
 
@@ -132,6 +137,12 @@ class SpecialEditTags extends UnlistedSpecialPage {
                        $output->addWikiMsg( 'undelete-header' );
                        return;
                }
+
+               // Check blocks
+               if ( $this->permissionManager->isBlockedFrom( $user, $this->targetObj ) ) {
+                       throw new UserBlockedError( $user->getBlock() );
+               }
+
                // Give a link to the logs/hist for this page
                $this->showConvenienceLinks();