From: David Barratt <dbarratt@wikimedia.org>
Date: Fri, 19 Jul 2019 01:02:54 +0000 (-0400)
Subject: Allow partially blocked users to tag unrelated revisions
X-Git-Tag: 1.34.0-rc.0~276^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=3224bdabc08f522da81478a63971c949cee40bc6

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 aff01830e0..bb6c580385 100644
--- 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 9ee000d590..ba6cb2ca6b 100644
--- 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 2737e356a0..40fcd74b15 100644
--- 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 70a1bd4783..1dd19694c9 100644
--- 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();