From df55cb25b96eee60748b5caf9dffa4819ada152f Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Wed, 29 Jun 2016 10:45:25 -0400
Subject: [PATCH] SECURITY: Make $wgBlockDisablesLogin also restrict logged in
 permissions

Does both Title and user related methods, so it catches things that only
call $wgUser->isAllowed( 'read' ), as well as giving a nicer error message
for things that use $title->userCan().

Otherwise, the user can still do stuff and read pages if they have an
ongoing session.

Issue reported by Multichill

Bug: T129738
Change-Id: Ic929a385fa81c27cbc6ac3a0862f51190d3ae993
---
 includes/Block.php     | 38 +++++++++++++++++++++++++-------------
 includes/Title.php     |  8 +++++++-
 includes/user/User.php | 17 +++++++++++++++++
 3 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/includes/Block.php b/includes/Block.php
index 93df004..3d53c8b 100644
--- a/includes/Block.php
+++ b/includes/Block.php
@@ -961,28 +961,40 @@ class Block {
 
 	/**
 	 * Get/set whether the Block prevents a given action
-	 * @param string $action
-	 * @param bool|null $x
-	 * @return bool
+	 *
+	 * @param string $action Action to check
+	 * @param bool|null $x Value for set, or null to just get value
+	 * @return bool|null Null for unrecognized rights.
 	 */
 	public function prevents( $action, $x = null ) {
+		global $wgBlockDisablesLogin;
+		$res = null;
 		switch ( $action ) {
 			case 'edit':
 				# For now... <evil laugh>
-				return true;
-
+				$res = true;
+				break;
 			case 'createaccount':
-				return wfSetVar( $this->mCreateAccount, $x );
-
+				$res = wfSetVar( $this->mCreateAccount, $x );
+				break;
 			case 'sendemail':
-				return wfSetVar( $this->mBlockEmail, $x );
-
+				$res = wfSetVar( $this->mBlockEmail, $x );
+				break;
 			case 'editownusertalk':
-				return wfSetVar( $this->mDisableUsertalk, $x );
-
-			default:
-				return null;
+				$res = wfSetVar( $this->mDisableUsertalk, $x );
+				break;
+			case 'read':
+				$res = false;
+				break;
 		}
+		if ( !$res && $wgBlockDisablesLogin ) {
+			// If a block would disable login, then it should
+			// prevent any action that all users cannot do
+			$anon = new User;
+			$res = $anon->isAllowed( $action ) ? $res : true;
+		}
+
+		return $res;
 	}
 
 	/**
diff --git a/includes/Title.php b/includes/Title.php
index f291a69..73cc13a 100644
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -2270,13 +2270,18 @@ class Title implements LinkTarget {
 	 * @return array List of errors
 	 */
 	private function checkUserBlock( $action, $user, $errors, $rigor, $short ) {
+		global $wgEmailConfirmToEdit, $wgBlockDisablesLogin;
 		// Account creation blocks handled at userlogin.
 		// Unblocking handled in SpecialUnblock
 		if ( $rigor === 'quick' || in_array( $action, [ 'createaccount', 'unblock' ] ) ) {
 			return $errors;
 		}
 
-		global $wgEmailConfirmToEdit;
+		// Optimize for a very common case
+		if ( $action === 'read' && !$wgBlockDisablesLogin ) {
+			return $errors;
+		}
+
 
 		if ( $wgEmailConfirmToEdit && !$user->isEmailConfirmed() ) {
 			$errors[] = [ 'confirmedittext' ];
@@ -2433,6 +2438,7 @@ class Title implements LinkTarget {
 			$checks = [
 				'checkPermissionHooks',
 				'checkReadPermissions',
+				'checkUserBlock', // for wgBlockDisablesLogin
 			];
 		# Don't call checkSpecialsAndNSPermissions or checkCSSandJSPermissions
 		# here as it will lead to duplicate error messages. This is okay to do
diff --git a/includes/user/User.php b/includes/user/User.php
index 40b5b40..18bd39e 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -3260,6 +3260,23 @@ class User implements IDBAccessObject {
 				}
 			}
 
+			// If block disables login, we should also remove any
+			// extra rights blocked users might have, in case the
+			// blocked user has a pre-existing session (T129738).
+			// This is checked here for cases where people only call
+			// $user->isAllowed(). It is also checked in Title::checkUserBlock()
+			// to give a better error message in the common case.
+			$config = RequestContext::getMain()->getConfig();
+			$blkDisablesLogin = $config->get( 'BlockDisablesLogin' );
+			if (
+				$this->isLoggedIn() &&
+				$blkDisablesLogin &&
+				$this->isBlocked()
+			) {
+				$anon = New User;
+				$this->mRights = array_intersect( $this->mRights, $anon->getRights() );
+			}
+
 			Hooks::run( 'UserGetRights', [ $this, &$this->mRights ] );
 			// Force reindexation of rights when a hook has unset one of them
 			$this->mRights = array_values( array_unique( $this->mRights ) );
-- 
2.0.1