From: jenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Date: Tue, 10 Sep 2019 19:00:47 +0000 (+0000)
Subject: Merge "Allow partially blocked users to tag unrelated revisions"
X-Git-Tag: 1.34.0-rc.0~276
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=5a296a1113d1243962c6459281a0307dd3efb80f;hp=2f127bb114f73951c14887710aec34c87de132ba

Merge "Allow partially blocked users to tag unrelated revisions"
---

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 3f9c491576..5ac5f82c51 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();