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

@@ -969,28 +969,40 @@ class Block {
 
 	/**
 	 * Get/set whether the Block prevents a given action
-	 * @param string $action
+	 *
-	 * @param bool|null $x
+	 * @param string $action Action to check
-	 * @return bool
+	 * @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 );
+				$res = wfSetVar( $this->mDisableUsertalk, $x );
-
+				break;
-			default:
+			case 'read':
-				return null;
+				$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;
 	}
 
 	/**

includes/Title.php

@@ -2271,13 +2271,17 @@ 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' ];
@@ -2434,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

includes/user/User.php

@@ -3147,6 +3147,22 @@ class User implements IDBAccessObject {
 			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 ) );
+
+			// 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();
+			if (
+				$this->isLoggedIn() &&
+				$config->get( 'BlockDisablesLogin' ) &&
+				$this->isBlocked()
+			) {
+				$anon = new User;
+				$this->mRights = array_intersect( $this->mRights, $anon->getRights() );
+			}
 		}
 		return $this->mRights;
 	}