From 56508fdca683cea881f5524ef5e3efbd559cb0e8 Mon Sep 17 00:00:00 2001 From: Alexandre Emsenhuber Date: Sun, 13 Nov 2011 07:25:56 +0000 Subject: [PATCH] Removed usage of $wgUser in block and unblock processing: * Made HTMLFormField pass the HTMLForm object to the validation and filter callbacks (so that they can get a context) * Added new parameter to SpecialBlock::checkUnblockSelf() to pass the user doing the request * SpecialBlock::processForm() and SpecialUnblock::processUnblock() now require a context as second parameter; added SpecialBlock::processUIForm() and SpecialUnblock::processUIUnblock() as adaptators from HTMLForm as second parameter to context --- includes/HTMLForm.php | 4 +- includes/api/ApiBlock.php | 4 +- includes/api/ApiUnblock.php | 4 +- includes/specials/SpecialBlock.php | 65 ++++++++++++++++------------ includes/specials/SpecialUnblock.php | 21 ++++++--- 5 files changed, 58 insertions(+), 40 deletions(-) diff --git a/includes/HTMLForm.php b/includes/HTMLForm.php index b781534549..6336e5bb91 100644 --- a/includes/HTMLForm.php +++ b/includes/HTMLForm.php @@ -891,7 +891,7 @@ abstract class HTMLFormField { } if ( isset( $this->mValidationCallback ) ) { - return call_user_func( $this->mValidationCallback, $value, $alldata ); + return call_user_func( $this->mValidationCallback, $value, $alldata, $this->mParent ); } return true; @@ -899,7 +899,7 @@ abstract class HTMLFormField { function filter( $value, $alldata ) { if ( isset( $this->mFilterCallback ) ) { - $value = call_user_func( $this->mFilterCallback, $value, $alldata ); + $value = call_user_func( $this->mFilterCallback, $value, $alldata, $this->mParent ); } return $value; diff --git a/includes/api/ApiBlock.php b/includes/api/ApiBlock.php index 56c6289581..0e73c6881a 100644 --- a/includes/api/ApiBlock.php +++ b/includes/api/ApiBlock.php @@ -62,7 +62,7 @@ class ApiBlock extends ApiBase { } # bug 15810: blocked admins should have limited access here if ( $user->isBlocked() ) { - $status = SpecialBlock::checkUnblockSelf( $params['user'] ); + $status = SpecialBlock::checkUnblockSelf( $params['user'], $user ); if ( $status !== true ) { $this->dieUsageMsg( array( $status ) ); } @@ -93,7 +93,7 @@ class ApiBlock extends ApiBase { 'Confirm' => true, ); - $retval = SpecialBlock::processForm( $data ); + $retval = SpecialBlock::processForm( $data, $this->getContext() ); if ( $retval !== true ) { // We don't care about multiple errors, just report one of them $this->dieUsageMsg( $retval ); diff --git a/includes/api/ApiUnblock.php b/includes/api/ApiUnblock.php index 41157271c6..85a972ff0e 100644 --- a/includes/api/ApiUnblock.php +++ b/includes/api/ApiUnblock.php @@ -66,7 +66,7 @@ class ApiUnblock extends ApiBase { } # bug 15810: blocked admins should have limited access here if ( $user->isBlocked() ) { - $status = SpecialBlock::checkUnblockSelf( $params['user'] ); + $status = SpecialBlock::checkUnblockSelf( $params['user'], $user ); if ( $status !== true ) { $this->dieUsageMsg( $status ); } @@ -77,7 +77,7 @@ class ApiUnblock extends ApiBase { 'Reason' => is_null( $params['reason'] ) ? '' : $params['reason'] ); $block = Block::newFromTarget( $data['Target'] ); - $retval = SpecialUnblock::processUnblock( $data ); + $retval = SpecialUnblock::processUnblock( $data, $this->getContext() ); if ( $retval !== true ) { $this->dieUsageMsg( $retval[0] ); } diff --git a/includes/specials/SpecialBlock.php b/includes/specials/SpecialBlock.php index d4625b3162..eedbd470c1 100644 --- a/includes/specials/SpecialBlock.php +++ b/includes/specials/SpecialBlock.php @@ -57,7 +57,8 @@ class SpecialBlock extends SpecialPage { public function execute( $par ) { # Permission check - if( !$this->userCanExecute( $this->getUser() ) ) { + $user = $this->getUser(); + if( !$this->userCanExecute( $user ) ) { $this->displayRestrictionError(); return; } @@ -82,7 +83,7 @@ class SpecialBlock extends SpecialPage { $this->requestedHideUser = $request->getBool( 'wpHideUser' ); # bug 15810: blocked admins should have limited access here - $status = self::checkUnblockSelf( $this->target ); + $status = self::checkUnblockSelf( $this->target, $user ); if ( $status !== true ) { throw new ErrorPageError( 'badaccess', $status ); } @@ -99,7 +100,7 @@ class SpecialBlock extends SpecialPage { $form = new HTMLForm( $fields, $this->getContext() ); $form->setWrapperLegend( wfMsg( 'blockip-legend' ) ); - $form->setSubmitCallback( array( __CLASS__, 'processForm' ) ); + $form->setSubmitCallback( array( __CLASS__, 'processUIForm' ) ); $t = $this->alreadyBlocked ? wfMsg( 'ipb-change-block' ) @@ -492,7 +493,7 @@ class SpecialBlock extends SpecialPage { * @param $alldata Array * @return Message */ - public static function validateTargetField( $value, $alldata = null ) { + public static function validateTargetField( $value, $alldata, $form ) { global $wgBlockCIDRLimit; list( $target, $type ) = self::getTargetAndType( $value ); @@ -500,13 +501,13 @@ class SpecialBlock extends SpecialPage { if( $type == Block::TYPE_USER ){ # TODO: why do we not have a User->exists() method? if( !$target->getId() ){ - return wfMessage( 'nosuchusershort', + return $form->msg( 'nosuchusershort', wfEscapeWikiText( $target->getName() ) ); } - $status = self::checkUnblockSelf( $target ); + $status = self::checkUnblockSelf( $target, $form->getUser() ); if ( $status !== true ) { - return wfMessage( 'badaccess', $status ); + return $form->msg( 'badaccess', $status ); } } elseif( $type == Block::TYPE_RANGE ){ @@ -516,39 +517,48 @@ class SpecialBlock extends SpecialPage { || ( IP::isIPv6( $ip ) && $wgBlockCIDRLimit['IPv6'] == 128 ) ) { # Range block effectively disabled - return wfMessage( 'range_block_disabled' ); + return $form->msg( 'range_block_disabled' ); } if( ( IP::isIPv4( $ip ) && $range > 32 ) || ( IP::isIPv6( $ip ) && $range > 128 ) ) { # Dodgy range - return wfMessage( 'ip_range_invalid' ); + return $form->msg( 'ip_range_invalid' ); } if( IP::isIPv4( $ip ) && $range < $wgBlockCIDRLimit['IPv4'] ) { - return wfMessage( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv4'] ); + return $form->msg( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv4'] ); } if( IP::isIPv6( $ip ) && $range < $wgBlockCIDRLimit['IPv6'] ) { - return wfMessage( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv6'] ); + return $form->msg( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv6'] ); } } elseif( $type == Block::TYPE_IP ){ # All is well } else { - return wfMessage( 'badipaddress' ); + return $form->msg( 'badipaddress' ); } return true; } + /** + * Submit callback for an HTMLForm object, will simply pass + */ + public static function processUIForm( array $data, HTMLForm $form ) { + return self::processForm( $data, $form->getContext() ); + } + /** * Given the form data, actually implement a block * @param $data Array * @return Bool|String */ - public static function processForm( array $data ){ - global $wgUser, $wgBlockAllowsUTEdit; + public static function processForm( array $data, IContextSource $context ){ + global $wgBlockAllowsUTEdit; + + $performer = $context->getUser(); // Handled by field validator callback // self::validateTargetField( $data['Target'] ); @@ -566,7 +576,7 @@ class SpecialBlock extends SpecialPage { # Give admins a heads-up before they go and block themselves. Much messier # to do this for IPs, but it's pretty unlikely they'd ever get the 'block' # permission anyway, although the code does allow for it - if( $target === $wgUser->getName() && + if( $target === $performer->getName() && ( $data['PreviousTarget'] !== $data['Target'] || !$data['Confirm'] ) ) { return array( 'ipb-blockingself' ); @@ -598,7 +608,7 @@ class SpecialBlock extends SpecialPage { } if( $data['HideUser'] ) { - if( !$wgUser->isAllowed('hideuser') ){ + if( !$performer->isAllowed('hideuser') ){ # this codepath is unreachable except by a malicious user spoofing forms, # or by race conditions (user has oversight and sysop, loads block form, # and is de-oversighted before submission); so need to fail completely @@ -624,7 +634,7 @@ class SpecialBlock extends SpecialPage { # Create block object. $block = new Block(); $block->setTarget( $target ); - $block->setBlocker( $wgUser ); + $block->setBlocker( $performer ); $block->mReason = $data['Reason'][0]; $block->mExpiry = self::parseExpiryInput( $data['Expiry'] ); $block->prevents( 'createaccount', $data['CreateAccount'] ); @@ -634,7 +644,7 @@ class SpecialBlock extends SpecialPage { $block->isAutoblocking( $data['AutoBlock'] ); $block->mHideName = $data['HideUser']; - if( !wfRunHooks( 'BlockIp', array( &$block, &$wgUser ) ) ) { + if( !wfRunHooks( 'BlockIp', array( &$block, &$performer ) ) ) { return array( 'hookaborted' ); } @@ -658,7 +668,7 @@ class SpecialBlock extends SpecialPage { # If the name was hidden and the blocking user cannot hide # names, then don't allow any block changes... - if( $currentBlock->mHideName && !$wgUser->isAllowed( 'hideuser' ) ) { + if( $currentBlock->mHideName && !$performer->isAllowed( 'hideuser' ) ) { return array( 'cant-see-hidden-user' ); } @@ -680,7 +690,7 @@ class SpecialBlock extends SpecialPage { $logaction = 'block'; } - wfRunHooks( 'BlockIpComplete', array( $block, $wgUser ) ); + wfRunHooks( 'BlockIpComplete', array( $block, $performer ) ); # Set *_deleted fields if requested if( $data['HideUser'] ) { @@ -689,7 +699,7 @@ class SpecialBlock extends SpecialPage { # Can't watch a rangeblock if( $type != Block::TYPE_RANGE && $data['Watch'] ) { - $wgUser->addWatch( Title::makeTitle( NS_USER, $target ) ); + $performer->addWatch( Title::makeTitle( NS_USER, $target ) ); } # Block constructor sanitizes certain block options on insert @@ -791,24 +801,23 @@ class SpecialBlock extends SpecialPage { * others, and probably shouldn't be able to unblock themselves * either. * @param $user User|Int|String + * @param $performer User user doing the request * @return Bool|String true or error message key */ - public static function checkUnblockSelf( $user ) { - global $wgUser; - + public static function checkUnblockSelf( $user, User $performer ) { if ( is_int( $user ) ) { $user = User::newFromId( $user ); } elseif ( is_string( $user ) ) { $user = User::newFromName( $user ); } - if( $wgUser->isBlocked() ){ - if( $user instanceof User && $user->getId() == $wgUser->getId() ) { + if( $performer->isBlocked() ){ + if( $user instanceof User && $user->getId() == $performer->getId() ) { # User is trying to unblock themselves - if ( $wgUser->isAllowed( 'unblockself' ) ) { + if ( $performer->isAllowed( 'unblockself' ) ) { return true; # User blocked themselves and is now trying to reverse it - } elseif ( $wgUser->blockedBy() === $wgUser->getName() ) { + } elseif ( $performer->blockedBy() === $performer->getName() ) { return true; } else { return 'ipbnounblockself'; diff --git a/includes/specials/SpecialUnblock.php b/includes/specials/SpecialUnblock.php index 3ecbdbd1a2..e38d1c2548 100644 --- a/includes/specials/SpecialUnblock.php +++ b/includes/specials/SpecialUnblock.php @@ -58,7 +58,7 @@ class SpecialUnblock extends SpecialPage { $form = new HTMLForm( $this->getFields(), $this->getContext() ); $form->setWrapperLegend( wfMsg( 'unblockip' ) ); - $form->setSubmitCallback( array( __CLASS__, 'processUnblock' ) ); + $form->setSubmitCallback( array( __CLASS__, 'processUIUnblock' ) ); $form->setSubmitText( wfMsg( 'ipusubmit' ) ); $form->addPreText( wfMsgExt( 'unblockiptext', 'parse' ) ); @@ -142,13 +142,22 @@ class SpecialUnblock extends SpecialPage { return $fields; } + /** + * Submit callback for an HTMLForm object + */ + public static function processUIUnblock( array $data, HTMLForm $form ) { + return self::processUnblock( $data, $form->getContext() ); + } + /** * Process the form + * + * @param $data Array + * @param $context IContextSource * @return Array( Array(message key, parameters) ) on failure, True on success */ - public static function processUnblock( array $data ){ - global $wgUser; - + public static function processUnblock( array $data, IContextSource $context ){ + $performer = $context->getUser(); $target = $data['Target']; $block = Block::newFromTarget( $data['Target'] ); @@ -159,7 +168,7 @@ class SpecialUnblock extends SpecialPage { # bug 15810: blocked admins should have limited access here. This # won't allow sysops to remove autoblocks on themselves, but they # should have ipblock-exempt anyway - $status = SpecialBlock::checkUnblockSelf( $target ); + $status = SpecialBlock::checkUnblockSelf( $target, $performer ); if ( $status !== true ) { throw new ErrorPageError( 'badaccess', $status ); } @@ -174,7 +183,7 @@ class SpecialUnblock extends SpecialPage { # If the name was hidden and the blocking user cannot hide # names, then don't allow any block removals... - if( !$wgUser->isAllowed( 'hideuser' ) && $block->mHideName ) { + if( !$performer->isAllowed( 'hideuser' ) && $block->mHideName ) { return array( 'unblock-hideuser' ); } -- 2.20.1