From a574f4c065e787166613dae3c7e5849b38d614c5 Mon Sep 17 00:00:00 2001
From: David Barratt <dbarratt@wikimedia.org>
Date: Thu, 21 Feb 2019 12:54:35 -0500
Subject: [PATCH] Block Special pages only if the user is sitewide blocked

Update the default implementation of FormSpecialPage::checkExecutePermissions()
so that a Special page is only blocked if the user has a sitewide block.

This change allows the user to continue performing critical functions (like
resetting their password) even if they are partially blocked.

Bug: T209097
Change-Id: I5190297b7b235b6ebbdfa522323ce9bbd46b6729
---
 includes/specialpage/FormSpecialPage.php      |  6 +-
 tests/common/TestsAutoLoader.php              |  1 +
 .../specialpage/FormSpecialPageTestCase.php   | 79 +++++++++++++++++++
 .../specials/SpecialPasswordResetTest.php     | 10 +++
 4 files changed, 94 insertions(+), 2 deletions(-)
 create mode 100644 tests/phpunit/includes/specialpage/FormSpecialPageTestCase.php
 create mode 100644 tests/phpunit/includes/specials/SpecialPasswordResetTest.php

diff --git a/includes/specialpage/FormSpecialPage.php b/includes/specialpage/FormSpecialPage.php
index 81a0036e83..d1c6aea294 100644
--- a/includes/specialpage/FormSpecialPage.php
+++ b/includes/specialpage/FormSpecialPage.php
@@ -203,9 +203,11 @@ abstract class FormSpecialPage extends SpecialPage {
 	protected function checkExecutePermissions( User $user ) {
 		$this->checkPermissions();
 
-		if ( $this->requiresUnblock() && $user->isBlocked() ) {
+		if ( $this->requiresUnblock() ) {
 			$block = $user->getBlock();
-			throw new UserBlockedError( $block );
+			if ( $block && $block->isSitewide() ) {
+				throw new UserBlockedError( $block );
+			}
 		}
 
 		if ( $this->requiresWrite() ) {
diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php
index 024557248d..cf786fbe77 100644
--- a/tests/common/TestsAutoLoader.php
+++ b/tests/common/TestsAutoLoader.php
@@ -154,6 +154,7 @@ $wgAutoloadClasses += [
 	# tests/phpunit/includes/specialpage
 	'SpecialPageTestHelper' => "$testDir/phpunit/includes/specialpage/SpecialPageTestHelper.php",
 	'AbstractChangesListSpecialPageTestCase' => "$testDir/phpunit/includes/specialpage/AbstractChangesListSpecialPageTestCase.php",
+	'FormSpecialPageTestCase' => "$testDir/phpunit/includes/specialpage/FormSpecialPageTestCase.php",
 
 	# tests/phpunit/includes/specials
 	'SpecialPageTestBase' => "$testDir/phpunit/includes/specials/SpecialPageTestBase.php",
diff --git a/tests/phpunit/includes/specialpage/FormSpecialPageTestCase.php b/tests/phpunit/includes/specialpage/FormSpecialPageTestCase.php
new file mode 100644
index 0000000000..a3b5adb858
--- /dev/null
+++ b/tests/phpunit/includes/specialpage/FormSpecialPageTestCase.php
@@ -0,0 +1,79 @@
+<?php
+/**
+ * Factory for handling the special page list and generating SpecialPage objects.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @group SpecialPage
+ */
+abstract class FormSpecialPageTestCase extends SpecialPageTestBase {
+
+	/**
+	 * @covers FormSpecialPage::checkExecutePermissions
+	 */
+	public function testCheckExecutePermissionsSitewideBlock() {
+		$special = $this->newSpecialPage();
+		$checkExecutePermissions = $this->getMethod( $special, 'checkExecutePermissions' );
+
+		$user = clone $this->getTestUser()->getUser();
+		$user->mBlockedby = $user->getName();
+		$user->mBlock = new Block( [
+			'address' => '127.0.8.1',
+			'by' => $user->getId(),
+			'reason' => 'sitewide block',
+			'timestamp' => time(),
+			'sitewide' => true,
+			'expiry' => 10,
+		] );
+
+		$this->expectException( UserBlockedError::class );
+		$checkExecutePermissions( $user );
+	}
+
+	/**
+	 * @covers FormSpecialPage::checkExecutePermissions
+	 */
+	public function testCheckExecutePermissionsPartialBlock() {
+		$special = $this->newSpecialPage();
+		$checkExecutePermissions = $this->getMethod( $special, 'checkExecutePermissions' );
+
+		$user = clone $this->getTestUser()->getUser();
+		$user->mBlockedby = $user->getName();
+		$user->mBlock = new Block( [
+			'address' => '127.0.8.1',
+			'by' => $user->getId(),
+			'reason' => 'partial block',
+			'timestamp' => time(),
+			'sitewide' => false,
+			'expiry' => 10,
+		] );
+
+		$this->assertNull( $checkExecutePermissions( $user ) );
+	}
+
+	/**
+	 * Get a protected/private method.
+	 *
+	 * @param object $obj
+	 * @param string $name
+	 * @return callable
+	 */
+	protected function getMethod( $obj, $name ) {
+		$method = new ReflectionMethod( $obj, $name );
+		$method->setAccessible( true );
+		return $method->getClosure( $obj );
+	}
+}
diff --git a/tests/phpunit/includes/specials/SpecialPasswordResetTest.php b/tests/phpunit/includes/specials/SpecialPasswordResetTest.php
new file mode 100644
index 0000000000..273b428f4a
--- /dev/null
+++ b/tests/phpunit/includes/specials/SpecialPasswordResetTest.php
@@ -0,0 +1,10 @@
+<?php
+
+class SpecialPasswordResetTest extends FormSpecialPageTestCase {
+	/**
+	 * {@inheritdoc}
+	 */
+	protected function newSpecialPage() {
+		return new SpecialPasswordReset();
+	}
+}
-- 
2.20.1