From a656d03597d29dedf37a962ec0fddf552609a8ef Mon Sep 17 00:00:00 2001 From: Martin Urbanec Date: Thu, 23 Apr 2020 21:33:56 +0200 Subject: [PATCH] Introduce backend class for blocking users Rather than having to do DatabaseBlock calls directly, and then ManualLogEntry calls to facilitate logging, let's create a BlockUser service, capable of blocking users and logging, optionally with permission checking. This should make blocking users easier for developers, for instance, AbuseFilter or CheckUser can easily benefit from this commit. Bug: T189073 Change-Id: Ifdced735b694b85116cb0e43dadbfa8e4cdb8cab --- RELEASE-NOTES-1.36 | 9 +- includes/MediaWikiServices.php | 9 + includes/ServiceWiring.php | 10 +- includes/api/ApiBlock.php | 157 ++--- includes/api/ApiMain.php | 3 + includes/block/AbstractBlock.php | 1 + includes/block/BlockUser.php | 659 ++++++++++++++++++ includes/block/BlockUserFactory.php | 53 ++ includes/block/UserBlockCommandFactory.php | 91 ++- includes/specials/SpecialBlock.php | 370 ++-------- resources/src/mediawiki.special.block.js | 2 +- tests/phpunit/includes/api/ApiBlockTest.php | 2 +- .../phpunit/includes/block/BlockUserTest.php | 72 ++ .../includes/specials/SpecialBlockTest.php | 60 +- 14 files changed, 1080 insertions(+), 418 deletions(-) create mode 100644 includes/block/BlockUser.php create mode 100644 includes/block/BlockUserFactory.php create mode 100644 tests/phpunit/includes/block/BlockUserTest.php diff --git a/RELEASE-NOTES-1.36 b/RELEASE-NOTES-1.36 index a91cefb79fd..b8af2883014 100644 --- a/RELEASE-NOTES-1.36 +++ b/RELEASE-NOTES-1.36 @@ -47,9 +47,10 @@ For notes on 1.35.x and older releases, see HISTORY. * … === New developer features in 1.36 === -* A new UnblockUser service was introduced that can be used for - unblocking users. This was done to expose code previously present - in SpecialUnblock to other parts of the code, or to extensions. +* To expose code previously present in SpecialBlock/SpecialUnblock + to other parts of the code, or to extensions, + the new BlockUser and UnblockUser command objects were added. + Use the BlockUserFactory and UnblockUserFactory services to create them. * … === External library changes in 1.36 === @@ -260,6 +261,8 @@ because of Phabricator reports. BlockPermissionChecker::checkEmailPermissions instead. * SpecialBlock::checkUnblockSelf has been deprecated. Please use BlockPermissionChecker::checkBlockPermissions instead. +* SpecialBlock::parseExpiryInput was deprecated - use + BlockUser::parseExpiryInput instead. * … === Other changes in 1.36 === diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index b9aa97ae8fd..9c29871b31e 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -30,6 +30,7 @@ use MediaWiki\Block\BlockErrorFormatter; use MediaWiki\Block\BlockManager; use MediaWiki\Block\BlockPermissionCheckerFactory; use MediaWiki\Block\BlockRestrictionStore; +use MediaWiki\Block\BlockUserFactory; use MediaWiki\Block\DatabaseBlockStore; use MediaWiki\Block\UnblockUserFactory; use MediaWiki\Cache\LinkBatchFactory; @@ -555,6 +556,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'BlockRestrictionStore' ); } + /** + * @since 1.36 + * @return BlockUserFactory + */ + public function getBlockUserFactory() : BlockUserFactory { + return $this->getService( 'BlockUserFactory' ); + } + /** * Returns the Config object containing the bootstrap configuration. * Bootstrap configuration would typically include database credentials diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 22dd85da834..7051302b568 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -51,6 +51,7 @@ use MediaWiki\Block\BlockErrorFormatter; use MediaWiki\Block\BlockManager; use MediaWiki\Block\BlockPermissionCheckerFactory; use MediaWiki\Block\BlockRestrictionStore; +use MediaWiki\Block\BlockUserFactory; use MediaWiki\Block\DatabaseBlockStore; use MediaWiki\Block\UnblockUserFactory; use MediaWiki\Block\UserBlockCommandFactory; @@ -200,6 +201,10 @@ return [ ); }, + 'BlockUserFactory' => function ( MediaWikiServices $services ) : BlockUserFactory { + return $services->getService( '_UserBlockCommandFactory' ); + }, + 'ChangeTagDefStore' => function ( MediaWikiServices $services ) : NameTableStore { return $services->getNameTableStoreFactory()->getChangeTagDef(); }, @@ -1495,9 +1500,12 @@ return [ '_UserBlockCommandFactory' => function ( MediaWikiServices $services ) : UserBlockCommandFactory { return new UserBlockCommandFactory( + new ServiceOptions( UserBlockCommandFactory::CONSTRUCTOR_OPTIONS, $services->getMainConfig() ), + $services->getHookContainer(), $services->getBlockPermissionCheckerFactory(), $services->getDatabaseBlockStore(), - $services->getHookContainer() + $services->getBlockRestrictionStore(), + LoggerFactory::getInstance( 'BlockManager' ) ); }, diff --git a/includes/api/ApiBlock.php b/includes/api/ApiBlock.php index 63d37cacb30..46297f4fdb5 100644 --- a/includes/api/ApiBlock.php +++ b/includes/api/ApiBlock.php @@ -22,9 +22,13 @@ use MediaWiki\Block\AbstractBlock; use MediaWiki\Block\BlockPermissionCheckerFactory; +use MediaWiki\Block\BlockUserFactory; use MediaWiki\Block\DatabaseBlock; -use MediaWiki\MediaWikiServices; +use MediaWiki\Block\Restriction\NamespaceRestriction; +use MediaWiki\Block\Restriction\PageRestriction; use MediaWiki\ParamValidator\TypeDef\UserDef; +use MediaWiki\User\UserFactory; +use MediaWiki\User\UserIdentity; /** * API module that facilitates the blocking of users. Requires API write mode @@ -39,19 +43,37 @@ class ApiBlock extends ApiBase { /** @var BlockPermissionCheckerFactory */ private $blockPermissionCheckerFactory; + /** @var BlockUserFactory */ + private $blockUserFactory; + + /** @var TitleFactory */ + private $titleFactory; + + /** @var UserFactory */ + private $userFactory; + /** * @param ApiMain $main * @param string $action * @param BlockPermissionCheckerFactory $blockPermissionCheckerFactory + * @param BlockUserFactory $blockUserFactory + * @param TitleFactory $titleFactory + * @param UserFactory $userFactory */ public function __construct( ApiMain $main, $action, - BlockPermissionCheckerFactory $blockPermissionCheckerFactory + BlockPermissionCheckerFactory $blockPermissionCheckerFactory, + BlockUserFactory $blockUserFactory, + TitleFactory $titleFactory, + UserFactory $userFactory ) { parent::__construct( $main, $action ); $this->blockPermissionCheckerFactory = $blockPermissionCheckerFactory; + $this->blockUserFactory = $blockUserFactory; + $this->titleFactory = $titleFactory; + $this->userFactory = $userFactory; } /** @@ -62,66 +84,26 @@ class ApiBlock extends ApiBase { */ public function execute() { $this->checkUserRightsAny( 'block' ); - - $user = $this->getUser(); $params = $this->extractRequestParams(); - $this->requireOnlyOneParameter( $params, 'user', 'userid' ); - # T17810: blocked admins should have limited access here - $block = $user->getBlock(); - if ( $block ) { - $status = SpecialBlock::checkUnblockSelf( $params['user'], $user ); - if ( $status !== true ) { - $this->dieWithError( - $status, - null, - [ 'blockinfo' => $this->getBlockDetails( $block ) ] - ); - } - } - - $editingRestriction = $params['partial'] ? 'partial' : 'sitewide'; - $pageRestrictions = implode( "\n", (array)$params['pagerestrictions'] ); - $namespaceRestrictions = implode( "\n", (array)$params['namespacerestrictions'] ); - - if ( $params['userid'] !== null ) { - $username = User::whoIs( $params['userid'] ); - - if ( $username === false ) { - $this->dieWithError( [ 'apierror-nosuchuserid', $params['userid'] ], 'nosuchuserid' ); - } else { - $params['user'] = $username; - } + // Make sure $target contains a parsed target + if ( $params['user'] !== null ) { + $target = $params['user']; } else { - list( $target, $type ) = AbstractBlock::parseTarget( $params['user'] ); - - // T40633 - if the target is a user (not an IP address), but it - // doesn't exist or is unusable, error. - if ( $type === DatabaseBlock::TYPE_USER && - ( $target->isAnon() /* doesn't exist */ || - !MediaWikiServices::getInstance()->getUserNameUtils()->isUsable( $params['user'] ) ) - ) { - $this->dieWithError( [ 'nosuchusershort', $params['user'] ], 'nosuchuser' ); + if ( User::whoIs( $params['userid'] ) === false ) { + $this->dieWithError( [ 'apierror-nosuchuserid', $params['userid'] ], 'nosuchuserid' ); } - } - if ( $params['tags'] ) { - $ableToTag = ChangeTags::canAddTagsAccompanyingChange( $params['tags'], $user ); - if ( !$ableToTag->isOK() ) { - $this->dieStatus( $ableToTag ); - } + $target = $this->userFactory->newFromId( $params['userid'] ); } + list( $target, $targetType ) = AbstractBlock::parseTarget( $target ); - if ( $params['hidename'] && - !$this->getPermissionManager()->userHasRight( $user, 'hideuser' ) ) { - $this->dieWithError( 'apierror-canthide' ); - } if ( $params['noemail'] && !$this->blockPermissionCheckerFactory ->newBlockPermissionChecker( - $params['user'], + $target, $this->getUser() ) ->checkEmailPermissions() @@ -129,44 +111,57 @@ class ApiBlock extends ApiBase { $this->dieWithError( 'apierror-cantblock-email' ); } - $data = [ - 'PreviousTarget' => $params['user'], - 'Target' => $params['user'], - 'Reason' => [ - $params['reason'], - 'other', - $params['reason'] - ], - 'Expiry' => $params['expiry'], - 'HardBlock' => !$params['anononly'], - 'CreateAccount' => $params['nocreate'], - 'AutoBlock' => $params['autoblock'], - 'DisableEmail' => $params['noemail'], - 'HideUser' => $params['hidename'], - 'DisableUTEdit' => !$params['allowusertalk'], - 'Reblock' => $params['reblock'], - 'Watch' => $params['watchuser'], - 'Confirm' => true, - 'Tags' => $params['tags'], - 'EditingRestriction' => $editingRestriction, - 'PageRestrictions' => $pageRestrictions, - 'NamespaceRestrictions' => $namespaceRestrictions, - ]; + $editingRestriction = $params['partial'] ? 'partial' : 'sitewide'; + $pageRestrictions = array_map( function ( $text ) { + $title = $this->titleFactory->newFromText( $text ); + $restriction = new PageRestriction( 0, $title->getArticleID() ); + $restriction->setTitle( $title ); + return $restriction; + }, (array)$params['pagerestrictions'] ); + $namespaceRestrictions = array_map( function ( $id ) { + return new NamespaceRestriction( 0, $id ); + }, (array)$params['namespacerestrictions'] ); + $restrictions = array_merge( $pageRestrictions, $namespaceRestrictions ); + if ( $restrictions === null ) { + $restrictions = []; + } + + $status = $this->blockUserFactory->newBlockUser( + $target, + $this->getUser(), + $params['expiry'], + $params['reason'], + [ + 'isCreateAccountBlocked' => $params['nocreate'], + 'isEmailBlocked' => $params['noemail'], + 'isHardBlock' => !$params['anononly'], + 'isAutoblocking' => $params['autoblock'], + 'isUserTalkEditBlocked' => !$params['allowusertalk'], + 'isHideUser' => $params['hidename'] + ], + $restrictions, + $params['tags'] + )->placeBlock( $params['reblock'] ); + + if ( $params['watchuser'] && $targetType !== AbstractBlock::TYPE_RANGE ) { + WatchAction::doWatch( + Title::makeTitle( NS_USER, $target ), + $this->getUser(), + User::IGNORE_USER_RIGHTS + ); + } - $status = SpecialBlock::validateTarget( $params['user'], $user ); if ( !$status->isOK() ) { $this->dieStatus( $status ); } - $retval = SpecialBlock::processForm( $data, $this->getContext() ); - if ( $retval !== true ) { - $this->dieStatus( $this->errorArrayToStatus( $retval ) ); - } - $res = []; - $res['user'] = $params['user']; - list( $target, /*...*/ ) = AbstractBlock::parseTarget( $params['user'] ); + if ( $target instanceof UserIdentity ) { + $res['user'] = $target->getName(); + } else { + $res['user'] = $target; + } $res['userID'] = $target instanceof User ? $target->getId() : 0; $block = DatabaseBlock::newFromTarget( $target, null, true ); diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index 5bef9719027..a91113bef8d 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -97,6 +97,9 @@ class ApiMain extends ApiBase { 'class' => ApiBlock::class, 'services' => [ 'BlockPermissionCheckerFactory', + 'BlockUserFactory', + 'TitleFactory', + 'UserFactory', ] ], 'unblock' => [ diff --git a/includes/block/AbstractBlock.php b/includes/block/AbstractBlock.php index cc853649540..fa5b964b11b 100644 --- a/includes/block/AbstractBlock.php +++ b/includes/block/AbstractBlock.php @@ -85,6 +85,7 @@ abstract class AbstractBlock { protected $isSitewide = true; # TYPE constants + # Do not introduce negative constants without changing BlockUser command object. public const TYPE_USER = 1; public const TYPE_IP = 2; public const TYPE_RANGE = 3; diff --git a/includes/block/BlockUser.php b/includes/block/BlockUser.php new file mode 100644 index 00000000000..65cd2a19549 --- /dev/null +++ b/includes/block/BlockUser.php @@ -0,0 +1,659 @@ +isPartial(). + */ + private $isPartialRaw = false; + + /** @var AbstractRestriction[] */ + private $blockRestrictions = []; + + /** @var string[] */ + private $tags = []; + + /** + * @param ServiceOptions $options + * @param BlockRestrictionStore $blockRestrictionStore + * @param BlockPermissionCheckerFactory $blockPermissionCheckerFactory + * @param HookContainer $hookContainer + * @param DatabaseBlockStore $databaseBlockStore + * @param LoggerInterface $logger + * @param string|UserIdentity $target Target of the block + * @param User $performer Performer of the block + * @param string $expiry Expiry of the block (timestamp or 'infinity') + * @param string $reason Reason of the block + * @param bool[] $blockOptions Block options + * Valid options: + * - isCreateAccountBlocked : Are acount creations prevented? + * - isEmailBlocked : Is emailing other users prevented? + * - isHardBlock : Are registered users prevented from editing? + * - isAutoblocking : Should this block spread to others to + * limit block evasion? + * - isUserTalkEditBlocked : Is editing blocked user's own talkpage allowed? + * - isHideUser : Should blocked user's name be hiden (needs hideuser)? + * - isPartial : Is this block partial? This is ignored when + * blockRestrictions is not an empty array. + * @param array $blockRestrictions Block restrictions + * @param string[] $tags Tags that should be assigned to the log entry + */ + public function __construct( + ServiceOptions $options, + BlockRestrictionStore $blockRestrictionStore, + BlockPermissionCheckerFactory $blockPermissionCheckerFactory, + HookContainer $hookContainer, + DatabaseBlockStore $databaseBlockStore, + LoggerInterface $logger, + $target, + User $performer, + string $expiry, + string $reason, + array $blockOptions, + array $blockRestrictions, + array $tags + ) { + // Process dependencies + $options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS ); + $this->options = $options; + $this->blockRestrictionStore = $blockRestrictionStore; + $this->blockPermissionChecker = $blockPermissionCheckerFactory + ->newBlockPermissionChecker( + $target, + $performer + ); + $this->hookRunner = new HookRunner( $hookContainer ); + $this->databaseBlockStore = $databaseBlockStore; + $this->logger = $logger; + + // Process block target + list( $this->target, $rawTargetType ) = AbstractBlock::parseTarget( $target ); + if ( $rawTargetType !== null ) { // Guard against invalid targets + $this->targetType = $rawTargetType; + } else { + $this->targetType = -1; + } + + // Process other block parameters + $this->performer = $performer; + $this->rawExpiry = $expiry; + $this->expiryTime = self::parseExpiryInput( $this->rawExpiry ); + $this->reason = $reason; + $this->blockRestrictions = $blockRestrictions; + $this->tags = $tags; + + // Process blockOptions + foreach ( [ + 'isCreateAccountBlocked', + 'isEmailBlocked', + 'isHardBlock', + 'isAutoblocking', + ] as $possibleBlockOption ) { + if ( isset( $blockOptions[ $possibleBlockOption ] ) ) { + $this->$possibleBlockOption = $blockOptions[ $possibleBlockOption ]; + } + } + + // This needs to be called after $this->blockRestrictions is set, as + // $this->isPartial() makes use of it. + if ( + isset( $blockOptions['isPartial'] ) && + !$this->isPartial() + ) { + $this->isPartialRaw = $blockOptions['isPartial']; + } + + if ( + !$this->isPartial() || + in_array( NS_USER_TALK, $this->getNamespaceRestrictions() ) + ) { + + // It is possible to block user talk edit. User talk edit is: + // - always blocked if the config says so; + // - otherwise blocked/unblocked if the option was passed in; + // - otherwise defaults to not blocked. + if ( !$this->options->get( 'BlockAllowsUTEdit' ) ) { + $this->isUserTalkEditBlocked = true; + } else { + $this->isUserTalkEditBlocked = $blockOptions['isUserTalkEditBlocked'] ?? false; + } + + } else { + + // It is not possible to block user talk edit. If the option + // was passed, an error will be thrown in ::placeBlockUnsafe. + // Otherwise, set to not blocked. + if ( !isset( $blockOptions['isUserTalkEditBlocked'] ) || !$blockOptions['isUserTalkEditBlocked'] ) { + $this->isUserTalkEditBlocked = false; + } + + } + + if ( + isset( $blockOptions['isHideUser'] ) && + $this->targetType === AbstractBlock::TYPE_USER + ) { + $this->isHideUser = $blockOptions['isHideUser']; + } + } + + /** + * Convert a submitted expiry time, which may be relative ("2 weeks", etc) or absolute + * ("24 May 2034", etc), into an absolute timestamp we can put into the database. + * + * @todo strtotime() only accepts English strings. This means the expiry input + * can only be specified in English. + * @see https://www.php.net/manual/en/function.strtotime.php + * + * @param string $expiry Whatever was typed into the form + * + * @return string|bool Timestamp or 'infinity' or false on error. + */ + public static function parseExpiryInput( string $expiry ) { + if ( wfIsInfinity( $expiry ) ) { + return 'infinity'; + } + + $expiry = strtotime( $expiry ); + + if ( $expiry < 0 || $expiry === false ) { + return false; + } + + return wfTimestamp( TS_MW, $expiry ); + } + + /** + * Is the to-be-placed block partial? + * + * @return bool + */ + private function isPartial() : bool { + return $this->blockRestrictions !== [] || $this->isPartialRaw; + } + + /** + * Configure DatabaseBlock according to class properties + * + * @param DatabaseBlock|null $sourceBlock Copy any options from this block, + * null to construct a new one. + * + * @return DatabaseBlock + */ + private function configureBlock( $sourceBlock = null ) : DatabaseBlock { + if ( $sourceBlock === null ) { + $block = new DatabaseBlock(); + } else { + $block = clone $sourceBlock; + } + + $isSitewide = !$this->isPartial(); + + $block->setTarget( $this->target ); + $block->setBlocker( $this->performer ); + $block->setReason( $this->reason ); + $block->setExpiry( $this->expiryTime ); + $block->isCreateAccountBlocked( $this->isCreateAccountBlocked ); + $block->isEmailBlocked( $this->isEmailBlocked ); + $block->isHardblock( $this->isHardBlock ); + $block->isAutoblocking( $this->isAutoblocking ); + $block->isSitewide( $isSitewide ); + $block->isUsertalkEditAllowed( !$this->isUserTalkEditBlocked ); + $block->setHideName( $this->isHideUser ); + + if ( $block->getId() === null ) { + // Block wasn't inserted into the DB yet + $block->setRestrictions( $this->blockRestrictions ); + } else { + // Block is in the DB, we need to set restrictions through a service + $block->setRestrictions( + $this->blockRestrictionStore->setBlockId( + $block->getId(), + $this->blockRestrictions + ) + ); + } + + return $block; + } + + /** + * Places a block with checking permissions + * + * @param bool $reblock Should this reblock? + * + * @return Status + */ + public function placeBlock( bool $reblock = false ) : Status { + $priorBlock = DatabaseBlock::newFromTarget( $this->target ); + $priorHideUser = $priorBlock instanceof DatabaseBlock && $priorBlock->getHideName(); + if ( + $this->blockPermissionChecker + ->checkBasePermissions( + $this->isHideUser || $priorHideUser + ) !== true + ) { + return Status::newFatal( $priorHideUser ? 'cant-see-hidden-user' : 'badaccess-group0' ); + } + + $blockCheckResult = $this->blockPermissionChecker->checkBlockPermissions(); + if ( $blockCheckResult !== true ) { + return Status::newFatal( $blockCheckResult ); + } + + if ( + $this->isEmailBlocked && + !$this->blockPermissionChecker->checkEmailPermissions() + ) { + // TODO: Maybe not ignore the error here? + $this->isEmailBlocked = false; + } + + if ( $this->tags !== [] ) { + // TODO: Use DI, see T245964 + $status = ChangeTags::canAddTagsAccompanyingChange( + $this->tags, + $this->performer + ); + + if ( !$status->isOK() ) { + return $status; + } + } + + return $this->placeBlockUnsafe( $reblock ); + } + + /** + * Places a block without any sort of permissions checks. + * + * @param bool $reblock Should this reblock? + * + * @return Status + */ + public function placeBlockUnsafe( bool $reblock = false ) : Status { + // TODO get rid of SpecialBlock call here (T263189) + $status = SpecialBlock::validateTarget( $this->target, $this->performer ); + + if ( !$status->isOK() ) { + return $status; + } + + if ( $this->isUserTalkEditBlocked === null ) { + return Status::newFatal( 'ipb-prevent-user-talk-edit' ); + } + + if ( + // There should be some expiry + strlen( $this->rawExpiry ) === 0 || + // can't be a larger string as 50 (it should be a time format in any way) + strlen( $this->rawExpiry ) > 50 || + // the time can't be parsed + !$this->expiryTime + ) { + return Status::newFatal( 'ipb_expiry_invalid' ); + } + + if ( $this->expiryTime < wfTimestampNow() ) { + return Status::newFatal( 'ipb_expiry_old' ); + } + + if ( $this->isHideUser ) { + if ( $this->isPartial() ) { + return Status::newFatal( 'ipb_hide_partial' ); + } + + if ( !wfIsInfinity( $this->rawExpiry ) ) { + return Status::newFatal( 'ipb_expiry_temp' ); + } + + $hideUserContribLimit = $this->options->get( 'HideUserContribLimit' ); + if ( + $hideUserContribLimit !== false && + $this->target->getEditCount() > $hideUserContribLimit + ) { + return Status::newFatal( 'ipb_hide_invalid', Message::numParam( $hideUserContribLimit ) ); + } + } + + if ( $this->isPartial() ) { + if ( + $this->blockRestrictions === [] && + !$this->isEmailBlocked && + !$this->isCreateAccountBlocked && + !$this->isUserTalkEditBlocked + ) { + return Status::newFatal( 'ipb-empty-block' ); + } + } + + return $this->placeBlockInternal( $reblock ); + } + + /** + * Places a block without any sort of sanity/permission checks, hooks can still + * abort the block through, as well as already existing block. + * + * @param bool $reblock Should this reblock? + * + * @return Status + */ + private function placeBlockInternal( bool $reblock = true ) : Status { + $block = $this->configureBlock(); + + $denyReason = [ 'hookaborted' ]; + if ( !$this->hookRunner->onBlockIp( $block, $this->performer, $denyReason ) ) { + $status = Status::newGood(); + foreach ( $denyReason as $key ) { + $status->fatal( $key ); + } + return $status; + } + + // Try to insert block. Is there a conflicting block? + $insertStatus = $this->databaseBlockStore->insertBlock( $block ); + $priorBlock = DatabaseBlock::newFromTarget( $this->target ); + $isReblock = false; + if ( !$insertStatus ) { + // Reblock if the caller wants so + if ( $reblock ) { + + if ( $priorBlock === null ) { + $this->logger->warning( 'Block could not be inserted. No existing block was found.' ); + return Status::newFatal( 'ipb-block-not-found', $block->getTarget() ); + } + + if ( $block->equals( $priorBlock ) ) { + // Block settings are equal => user is already blocked + return Status::newFatal( 'ipb_already_blocked', $this->target->getUserPage() ); + } + + $currentBlock = $this->configureBlock( $priorBlock ); + $this->databaseBlockStore->updateBlock( $currentBlock ); // TODO handle failure + $isReblock = true; + $block = $currentBlock; + } else { + return Status::newFatal( 'ipb_already_blocked', $this->target->getUserPage() ); + } + } + + // Set *_deleted fields if requested + if ( $this->isHideUser ) { + RevisionDeleteUser::suppressUserName( $this->target, $this->target->getId() ); + } + + $this->hookRunner->onBlockIpComplete( $block, $this->performer, $priorBlock ); + + // DatabaseBlock constructor sanitizes certain block options on insert + $this->isEmailBlocked = $block->isEmailBlocked(); + $this->isAutoblocking = $block->isAutoblocking(); + + $this->log( $block, $isReblock ); + + return Status::newGood(); + } + + /** + * Build namespace restrictions array from $this->blockRestrictions + * + * Returns an array of namespace IDs. + * + * @return int[] + */ + private function getNamespaceRestrictions() : array { + $namespaceRestrictions = []; + foreach ( $this->blockRestrictions as $restriction ) { + if ( $restriction instanceof NamespaceRestriction ) { + $namespaceRestrictions[] = $restriction->getValue(); + } + } + return $namespaceRestrictions; + } + + /** + * Build an array of page restrictions from $this->blockRestrictions + * + * Returns an array of stringified full page titles. + * + * @return string[] + */ + private function getPageRestrictions() : array { + $pageRestrictions = []; + foreach ( $this->blockRestrictions as $restriction ) { + if ( $restriction instanceof PageRestriction ) { + $pageRestrictions[] = $restriction->getTitle()->getFullText(); + } + } + return $pageRestrictions; + } + + /** + * Prepare $logParams + * + * Helper method for $this->log() + * + * @return array + */ + private function constructLogParams() : array { + $logExpiry = wfIsInfinity( $this->rawExpiry ) ? 'infinity' : $this->rawExpiry; + $logParams = [ + '5::duration' => $logExpiry, + '6::flags' => $this->blockLogFlags(), + 'sitewide' => !$this->isPartial() + ]; + + if ( $this->isPartial() ) { + $pageRestrictions = $this->getPageRestrictions(); + $namespaceRestrictions = $this->getNamespaceRestrictions(); + + if ( count( $pageRestrictions ) > 0 ) { + $logParams['7::restrictions']['pages'] = $pageRestrictions; + } + if ( count( $namespaceRestrictions ) > 0 ) { + $logParams['7::restrictions']['namespaces'] = $namespaceRestrictions; + } + } + return $logParams; + } + + /** + * Log the block to Special:Log + * + * @param DatabaseBlock $block + * @param bool $isReblock + */ + private function log( DatabaseBlock $block, bool $isReblock ) { + $logType = $this->isHideUser ? 'suppress' : 'block'; + $logAction = $isReblock ? 'reblock' : 'block'; + + $logEntry = new ManualLogEntry( $logType, $logAction ); + $logEntry->setTarget( Title::makeTitle( NS_USER, $this->target ) ); + $logEntry->setComment( $this->reason ); + $logEntry->setPerformer( $this->performer ); + $logEntry->setParameters( $this->constructLogParams() ); + // Relate log ID to block ID (T27763) + $logEntry->setRelations( [ 'ipb_id' => $block->getId() ] ); + $logEntry->addTags( $this->tags ); + $logId = $logEntry->insert(); + $logEntry->publish( $logId ); + } + + /** + * Return a comma-delimited list of flags to be passed to the log + * reader for this block, to provide more information in the logs. + * + * @return string + */ + private function blockLogFlags() : string { + $flags = []; + + if ( $this->targetType != AbstractBlock::TYPE_USER && !$this->isHardBlock ) { + // For grepping: message block-log-flags-anononly + $flags[] = 'anononly'; + } + + if ( $this->isCreateAccountBlocked ) { + // For grepping: message block-log-flags-nocreate + $flags[] = 'nocreate'; + } + + if ( $this->targetType == AbstractBlock::TYPE_USER && !$this->isAutoblocking ) { + // For grepping: message block-log-flags-noautoblock + $flags[] = 'noautoblock'; + } + + if ( $this->isEmailBlocked ) { + // For grepping: message block-log-flags-noemail + $flags[] = 'noemail'; + } + + if ( $this->options->get( 'BlockAllowsUTEdit' ) && $this->isUserTalkEditBlocked ) { + // For grepping: message block-log-flags-nousertalk + $flags[] = 'nousertalk'; + } + + if ( $this->isHideUser ) { + // For grepping: message block-log-flags-hiddenname + $flags[] = 'hiddenname'; + } + + return implode( ',', $flags ); + } +} diff --git a/includes/block/BlockUserFactory.php b/includes/block/BlockUserFactory.php new file mode 100644 index 00000000000..42529453d2a --- /dev/null +++ b/includes/block/BlockUserFactory.php @@ -0,0 +1,53 @@ +assertRequiredOptions( self::CONSTRUCTOR_OPTIONS ); + + $this->options = $options; + $this->hookContainer = $hookContainer; $this->blockPermissionCheckerFactory = $blockPermissionCheckerFactory; $this->blockStore = $blockStore; - $this->hookContainer = $hookContainer; + $this->blockRestrictionStore = $blockRestrictionStore; + $this->logger = $logger; + } + + /** + * Create BlockUser + * + * @param string|UserIdentity $target Target of the block + * @param User $performer Performer of the block + * @param string $expiry Expiry of the block (timestamp or 'infinity') + * @param string $reason Reason of the block + * @param array $blockOptions Block options + * @param array $blockRestrictions Block restrictions + * @param array|null $tags Tags that should be assigned to the log entry + * + * @return BlockUser + */ + public function newBlockUser( + $target, + User $performer, + string $expiry, + string $reason = '', + array $blockOptions = [], + array $blockRestrictions = [], + $tags = [] + ) : BlockUser { + if ( $tags === null ) { + $tags = []; + } + + return new BlockUser( + $this->options, + $this->blockRestrictionStore, + $this->blockPermissionCheckerFactory, + $this->hookContainer, + $this->blockStore, + $this->logger, + $target, + $performer, + $expiry, + $reason, + $blockOptions, + $blockRestrictions, + $tags + ); } /** diff --git a/includes/specials/SpecialBlock.php b/includes/specials/SpecialBlock.php index 441bf8d7319..4f212ba05b0 100644 --- a/includes/specials/SpecialBlock.php +++ b/includes/specials/SpecialBlock.php @@ -22,10 +22,10 @@ */ use MediaWiki\Block\AbstractBlock; +use MediaWiki\Block\BlockUser; use MediaWiki\Block\DatabaseBlock; use MediaWiki\Block\Restriction\NamespaceRestriction; use MediaWiki\Block\Restriction\PageRestriction; -use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; use MediaWiki\Permissions\PermissionManager; use MediaWiki\User\UserIdentity; @@ -82,7 +82,13 @@ class SpecialBlock extends FormSpecialPage { protected function checkExecutePermissions( User $user ) { parent::checkExecutePermissions( $user ); # T17810: blocked admins should have limited access here - $status = self::checkUnblockSelf( $this->target, $user ); + $blockUserValidator = MediaWikiServices::getInstance() + ->getBlockPermissionCheckerFactory() + ->newBlockPermissionChecker( + $this->target, + $user + ); + $status = $blockUserValidator->checkBlockPermissions(); if ( $status !== true ) { throw new ErrorPageError( 'badaccess', $status ); } @@ -743,8 +749,9 @@ class SpecialBlock extends FormSpecialPage { } /** - * Given the form data, actually implement a block. This is also called from ApiBlock. + * Given the form data, actually implement a block. * + * @deprecated since 1.36, use BlockUserFactory service instead * @param array $data * @param IContextSource $context * @return bool|array @@ -758,6 +765,12 @@ class SpecialBlock extends FormSpecialPage { # can come from it $data['Confirm'] = !in_array( $data['Confirm'], [ '', '0', null, false ], true ); + # If the user has done the form 'properly', they won't even have been given the + # option to suppress-block unless they have the 'hideuser' permission + if ( !isset( $data['HideUser'] ) ) { + $data['HideUser'] = false; + } + /** @var User $target */ list( $target, $type ) = AbstractBlock::parseTarget( $data['Target'] ); if ( $type == DatabaseBlock::TYPE_USER ) { @@ -777,6 +790,10 @@ class SpecialBlock extends FormSpecialPage { ) { return [ 'ipb-blockingself', 'ipb-confirmaction' ]; } + + if ( $data['HideUser'] && !$data['Confirm'] ) { + return [ 'ipb-confirmhideuser', 'ipb-confirmaction' ]; + } } elseif ( $type == DatabaseBlock::TYPE_RANGE ) { $user = null; $userId = 0; @@ -794,123 +811,6 @@ class SpecialBlock extends FormSpecialPage { // @phan-suppress-next-line PhanPluginDuplicateConditionalNullCoalescing $blockReason = isset( $data['Reason'][0] ) ? $data['Reason'][0] : ''; - $expiryTime = self::parseExpiryInput( $data['Expiry'] ); - - if ( - // an expiry time is needed - ( strlen( $data['Expiry'] ) == 0 ) || - // can't be a larger string as 50 (it should be a time format in any way) - ( strlen( $data['Expiry'] ) > 50 ) || - // check, if the time could be parsed - !$expiryTime - ) { - return [ 'ipb_expiry_invalid' ]; - } - - // an expiry time should be in the future, not in the - // past (wouldn't make any sense) - bug T123069 - if ( $expiryTime < wfTimestampNow() ) { - return [ 'ipb_expiry_old' ]; - } - - if ( !isset( $data['DisableEmail'] ) ) { - $data['DisableEmail'] = false; - } - - # If the user has done the form 'properly', they won't even have been given the - # option to suppress-block unless they have the 'hideuser' permission - if ( !isset( $data['HideUser'] ) ) { - $data['HideUser'] = false; - } - - if ( $data['HideUser'] ) { - if ( !MediaWikiServices::getInstance() - ->getPermissionManager() - ->userHasRight( $performer, 'hideuser' ) - ) { - # this codepath is unreachable except by a malicious user spoofing forms, - # or by race conditions (user has hideuser and block rights, loads block form, - # and loses hideuser rights before submission); so need to fail completely - # rather than just silently disable hiding - return [ 'badaccess-group0' ]; - } - - if ( $isPartialBlock ) { - return [ 'ipb_hide_partial' ]; - } - - # Recheck params here... - $hideUserContribLimit = $context->getConfig()->get( 'HideUserContribLimit' ); - if ( $type != DatabaseBlock::TYPE_USER ) { - $data['HideUser'] = false; # IP users should not be hidden - } elseif ( !wfIsInfinity( $data['Expiry'] ) ) { - # Bad expiry. - return [ 'ipb_expiry_temp' ]; - } elseif ( $hideUserContribLimit !== false - /** @phan-suppress-next-line PhanNonClassMethodCall */ - && $user->getEditCount() > $hideUserContribLimit - ) { - # Typically, the user should have a handful of edits. - # Disallow hiding users with many edits for performance. - return [ [ 'ipb_hide_invalid', - Message::numParam( $hideUserContribLimit ) ] ]; - } elseif ( !$data['Confirm'] ) { - return [ 'ipb-confirmhideuser', 'ipb-confirmaction' ]; - } - } - - // Check whether the user can edit their own user talk page. - $blockAllowsUTEdit = $context->getConfig()->get( 'BlockAllowsUTEdit' ); - $isUserTalkNamespaceBlock = !$isPartialBlock || - in_array( NS_USER_TALK, explode( "\n", $data['NamespaceRestrictions'] ) ); - if ( $isUserTalkNamespaceBlock ) { - // If the block blocks the user talk namespace, disallow own user talk edit if - // the global config disallows it; otherwise use the form field value. - $userTalkEditAllowed = $blockAllowsUTEdit ? !$data['DisableUTEdit'] : false; - } else { - // If the block doesn't block the user talk namespace, then it can't block own - // user talk edit, regardless of the config or field (T210475). Return error - // message if the field tries to disallow own user talk edit. - if ( isset( $data['DisableUTEdit'] ) && $data['DisableUTEdit'] ) { - return [ 'ipb-prevent-user-talk-edit' ]; - } - $userTalkEditAllowed = true; - } - - // A block is empty if it is a partial block, the page restrictions are empty, the - // namespace restrictions are empty, and none of the actions are enabled - if ( $isPartialBlock && - !( isset( $data['PageRestrictions'] ) && $data['PageRestrictions'] !== '' ) && - !( isset( $data['NamespaceRestrictions'] ) && $data['NamespaceRestrictions'] !== '' ) && - $data['DisableEmail'] === false && - ( $userTalkEditAllowed || !$blockAllowsUTEdit ) && - !$data['CreateAccount'] - ) { - return [ 'ipb-empty-block' ]; - } - - # Create block object. - $block = new DatabaseBlock(); - $block->setTarget( $target ); - $block->setBlocker( $performer ); - $block->setReason( $blockReason ); - $block->setExpiry( $expiryTime ); - $block->isCreateAccountBlocked( $data['CreateAccount'] ); - $block->isUsertalkEditAllowed( $userTalkEditAllowed ); - $block->isEmailBlocked( $data['DisableEmail'] ); - $block->isHardblock( $data['HardBlock'] ); - $block->isAutoblocking( $data['AutoBlock'] ); - $block->setHideName( $data['HideUser'] ); - - if ( $isPartialBlock ) { - $block->isSitewide( false ); - } - - $reason = [ 'hookaborted' ]; - if ( !Hooks::runner()->onBlockIp( $block, $performer, $reason ) ) { - return $reason; - } - $pageRestrictions = []; $namespaceRestrictions = []; if ( isset( $data['PageRestrictions'] ) && $data['PageRestrictions'] !== '' ) { @@ -930,93 +830,50 @@ class SpecialBlock extends FormSpecialPage { } $restrictions = ( array_merge( $pageRestrictions, $namespaceRestrictions ) ); - $block->setRestrictions( $restrictions ); - $priorBlock = null; - # Try to insert block. Is there a conflicting block? - $blockStore = MediaWikiServices::getInstance()->getDatabaseBlockStore(); - $status = $blockStore->insertBlock( $block ); - if ( !$status ) { - # Indicates whether the user is confirming the block and is aware of - # the conflict (did not change the block target in the meantime) - $blockNotConfirmed = !$data['Confirm'] || ( array_key_exists( 'PreviousTarget', $data ) - && $data['PreviousTarget'] !== $target ); - - # Special case for API - T34434 - $reblockNotAllowed = ( array_key_exists( 'Reblock', $data ) && !$data['Reblock'] ); - - # Show form unless the user is already aware of this... - if ( $blockNotConfirmed || $reblockNotAllowed ) { - return [ [ 'ipb_already_blocked', $block->getTarget() ] ]; - # Otherwise, try to update the block... - } else { - # This returns direct blocks before autoblocks/rangeblocks, since we should - # be sure the user is blocked by now it should work for our purposes - $currentBlock = DatabaseBlock::newFromTarget( $target ); - if ( !$currentBlock instanceof DatabaseBlock ) { - $logger = LoggerFactory::getInstance( 'BlockManager' ); - $logger->warning( 'Block could not be inserted. No existing block was found.' ); - return [ [ 'ipb-block-not-found', $block->getTarget() ] ]; - } - if ( $block->equals( $currentBlock ) ) { - return [ [ 'ipb_already_blocked', $block->getTarget() ] ]; - } - # If the name was hidden and the blocking user cannot hide - # names, then don't allow any block changes... - if ( $currentBlock->getHideName() && !MediaWikiServices::getInstance() - ->getPermissionManager() - ->userHasRight( $performer, 'hideuser' ) - ) { - return [ 'cant-see-hidden-user' ]; - } - - $priorBlock = clone $currentBlock; - $currentBlock->setBlocker( $performer ); - $currentBlock->isHardblock( $block->isHardblock() ); - $currentBlock->isCreateAccountBlocked( $block->isCreateAccountBlocked() ); - $currentBlock->setExpiry( $block->getExpiry() ); - $currentBlock->isAutoblocking( $block->isAutoblocking() ); - $currentBlock->setHideName( $block->getHideName() ); - $currentBlock->isEmailBlocked( $block->isEmailBlocked() ); - $currentBlock->isUsertalkEditAllowed( $block->isUsertalkEditAllowed() ); - $currentBlock->setReason( $block->getReasonComment() ); - - // Maintain the sitewide status. If partial blocks is not enabled, - // saving the block will result in a sitewide block. - $currentBlock->isSitewide( $block->isSitewide() ); - - // Set the block id of the restrictions. - $blockRestrictionStore = MediaWikiServices::getInstance()->getBlockRestrictionStore(); - $currentBlock->setRestrictions( - $blockRestrictionStore->setBlockId( $currentBlock->getId(), $restrictions ) - ); - - $blockStore->updateBlock( $currentBlock ); - // TODO handle failure - - $logaction = 'reblock'; - - # Unset _deleted fields if requested - if ( $currentBlock->getHideName() && !$data['HideUser'] ) { - RevisionDeleteUser::unsuppressUserName( $target, $userId ); - } - - # If hiding/unhiding a name, this should go in the private logs - if ( (bool)$currentBlock->getHideName() ) { - $data['HideUser'] = true; - } - - $block = $currentBlock; - } - } else { - $logaction = 'block'; + if ( !isset( $data['Tags'] ) ) { + $data['Tags'] = []; } - Hooks::runner()->onBlockIpComplete( $block, $performer, $priorBlock ); + $blockOptions = [ + 'isCreateAccountBlocked' => $data['CreateAccount'], + 'isEmailBlocked' => $data['DisableEmail'], + 'isHardBlock' => $data['HardBlock'], + 'isAutoblocking' => $data['AutoBlock'], + 'isHideUser' => $data['HideUser'], + 'isPartial' => $isPartialBlock, + ]; - # Set *_deleted fields if requested - if ( $data['HideUser'] ) { - RevisionDeleteUser::suppressUserName( $target, $userId ); + if ( isset( $data['DisableUTEdit'] ) ) { + $blockOptions['isUserTalkEditBlocked'] = $data['DisableUTEdit']; + } + if ( isset( $data['DisableEmail'] ) ) { + $blockOptions['isEmailBlocked'] = $data['DisableEmail']; + } + + $blockUser = MediaWikiServices::getInstance()->getBlockUserFactory()->newBlockUser( + $target, + $context->getUser(), + $data['Expiry'], + $blockReason, + $blockOptions, + $restrictions, + $data['Tags'] + ); + + # Indicates whether the user is confirming the block and is aware of + # the conflict (did not change the block target in the meantime) + $blockNotConfirmed = !$data['Confirm'] || ( array_key_exists( 'PreviousTarget', $data ) + && $data['PreviousTarget'] !== $target ); + + # Special case for API - T34434 + $reblockNotAllowed = ( array_key_exists( 'Reblock', $data ) && !$data['Reblock'] ); + + $doReblock = !$blockNotConfirmed && !$reblockNotAllowed; + + $status = $blockUser->placeBlock( $doReblock ); + if ( !$status->isOK() ) { + return $status; } # Can't watch a rangeblock @@ -1028,47 +885,6 @@ class SpecialBlock extends FormSpecialPage { ); } - # DatabaseBlock constructor sanitizes certain block options on insert - $data['BlockEmail'] = $block->isEmailBlocked(); - $data['AutoBlock'] = $block->isAutoblocking(); - - # Prepare log parameters - $logParams = []; - - $rawExpiry = $data['Expiry']; - $logExpiry = wfIsInfinity( $rawExpiry ) ? 'infinity' : $rawExpiry; - - $logParams['5::duration'] = $logExpiry; - $logParams['6::flags'] = self::blockLogFlags( $data, $type ); - $logParams['sitewide'] = $block->isSitewide(); - - if ( !$block->isSitewide() ) { - if ( $data['PageRestrictions'] !== '' ) { - $logParams['7::restrictions']['pages'] = explode( "\n", $data['PageRestrictions'] ); - } - - if ( $data['NamespaceRestrictions'] !== '' ) { - $logParams['7::restrictions']['namespaces'] = explode( "\n", $data['NamespaceRestrictions'] ); - } - } - - # Make log entry, if the name is hidden, put it in the suppression log - $log_type = $data['HideUser'] ? 'suppress' : 'block'; - $logEntry = new ManualLogEntry( $log_type, $logaction ); - $logEntry->setTarget( Title::makeTitle( NS_USER, $target ) ); - $logEntry->setComment( $blockReason ); - $logEntry->setPerformer( $performer ); - $logEntry->setParameters( $logParams ); - # Relate log ID to block ID (T27763) - $logEntry->setRelations( [ 'ipb_id' => $block->getId() ] ); - $logId = $logEntry->insert(); - - if ( !empty( $data['Tags'] ) ) { - $logEntry->addTags( $data['Tags'] ); - } - - $logEntry->publish( $logId ); - return true; } @@ -1106,25 +922,13 @@ class SpecialBlock extends FormSpecialPage { * Convert a submitted expiry time, which may be relative ("2 weeks", etc) or absolute * ("24 May 2034", etc), into an absolute timestamp we can put into the database. * - * @todo strtotime() only accepts English strings. This means the expiry input - * can only be specified in English. - * @see https://www.php.net/manual/en/function.strtotime.php + * @deprecated since 1.36, use BlockUser::parseExpiryInput instead * * @param string $expiry Whatever was typed into the form * @return string|bool Timestamp or 'infinity' or false on error. */ public static function parseExpiryInput( $expiry ) { - if ( wfIsInfinity( $expiry ) ) { - return 'infinity'; - } - - $expiry = strtotime( $expiry ); - - if ( $expiry < 0 || $expiry === false ) { - return false; - } - - return wfTimestamp( TS_MW, $expiry ); + return BlockUser::parseExpiryInput( $expiry ); } /** @@ -1162,56 +966,6 @@ class SpecialBlock extends FormSpecialPage { ->checkBlockPermissions(); } - /** - * Return a comma-delimited list of "flags" to be passed to the log - * reader for this block, to provide more information in the logs - * @param array $data From HTMLForm data - * @param int $type DatabaseBlock::TYPE_ constant (USER, RANGE, or IP) - * @return string - */ - protected static function blockLogFlags( array $data, $type ) { - $config = RequestContext::getMain()->getConfig(); - - $blockAllowsUTEdit = $config->get( 'BlockAllowsUTEdit' ); - - $flags = []; - - # when blocking a user the option 'anononly' is not available/has no effect - # -> do not write this into log - if ( !$data['HardBlock'] && $type != DatabaseBlock::TYPE_USER ) { - // For grepping: message block-log-flags-anononly - $flags[] = 'anononly'; - } - - if ( $data['CreateAccount'] ) { - // For grepping: message block-log-flags-nocreate - $flags[] = 'nocreate'; - } - - # Same as anononly, this is not displayed when blocking an IP address - if ( !$data['AutoBlock'] && $type == DatabaseBlock::TYPE_USER ) { - // For grepping: message block-log-flags-noautoblock - $flags[] = 'noautoblock'; - } - - if ( $data['DisableEmail'] ) { - // For grepping: message block-log-flags-noemail - $flags[] = 'noemail'; - } - - if ( $blockAllowsUTEdit && $data['DisableUTEdit'] ) { - // For grepping: message block-log-flags-nousertalk - $flags[] = 'nousertalk'; - } - - if ( $data['HideUser'] ) { - // For grepping: message block-log-flags-hiddenname - $flags[] = 'hiddenname'; - } - - return implode( ',', $flags ); - } - /** * Process the form on POST submission. * @param array $data diff --git a/resources/src/mediawiki.special.block.js b/resources/src/mediawiki.special.block.js index 4eb42e40f46..96898c82844 100644 --- a/resources/src/mediawiki.special.block.js +++ b/resources/src/mediawiki.special.block.js @@ -48,7 +48,7 @@ isIpRange = isIp && blocktarget.match( /\/\d+$/ ), isNonEmptyIp = isIp && !isEmpty, expiryValue = expiryWidget.getValue(), - // infinityValues are the values the SpecialBlock class accepts as infinity (sf. wfIsInfinity) + // infinityValues are the values the BlockUser class accepts as infinity (sf. wfIsInfinity) infinityValues = [ 'infinite', 'indefinite', 'infinity', 'never' ], isIndefinite = infinityValues.indexOf( expiryValue ) !== -1, editingRestrictionValue = editingRestrictionWidget.getValue(), diff --git a/tests/phpunit/includes/api/ApiBlockTest.php b/tests/phpunit/includes/api/ApiBlockTest.php index df2d40eda86..7911bafb335 100644 --- a/tests/phpunit/includes/api/ApiBlockTest.php +++ b/tests/phpunit/includes/api/ApiBlockTest.php @@ -182,7 +182,7 @@ class ApiBlockTest extends ApiTestCase { public function testBlockWithProhibitedHide() { $this->expectException( ApiUsageException::class ); $this->expectExceptionMessage( - "You don't have permission to hide user names from the block log." + "You are not allowed to execute the action you have requested." ); $this->doBlock( [ 'hidename' => '' ] ); diff --git a/tests/phpunit/includes/block/BlockUserTest.php b/tests/phpunit/includes/block/BlockUserTest.php new file mode 100644 index 00000000000..efdd392d881 --- /dev/null +++ b/tests/phpunit/includes/block/BlockUserTest.php @@ -0,0 +1,72 @@ +user = $this->getTestUser()->getUser(); + $this->performer = $this->getTestSysop()->getUser(); + + // Prepare factory + $this->blockUserFactory = MediaWikiServices::getInstance()->getBlockUserFactory(); + } + + /** + * @covers MediaWiki\Block\BlockUser::placeBlock + */ + public function testValidTarget() { + $status = $this->blockUserFactory->newBlockUser( + $this->user, + $this->performer, + 'infinity', + 'test block' + )->placeBlock(); + $this->assertTrue( $status->isOK() ); + $block = $this->user->getBlock(); + $this->assertSame( 'test block', $block->getReasonComment()->text ); + $this->assertInstanceOf( DatabaseBlock::class, $block ); + $this->assertFalse( $block->getHideName() ); + $this->assertFalse( $block->isCreateAccountBlocked() ); + $this->assertTrue( $block->isUsertalkEditAllowed() ); + $this->assertFalse( $block->isEmailBlocked() ); + $this->assertTrue( $block->isAutoblocking() ); + } + + /** + * @covers MediaWiki\BLock\BlockUser::placeBlock + */ + public function testHideUser() { + $status = $this->blockUserFactory->newBlockUser( + $this->user, + $this->getTestUser( [ 'sysop', 'suppress' ] )->getUser(), + 'infinity', + 'test hideuser', + [ + 'isHideUser' => true + ] + )->placeBlock(); + $this->assertTrue( $status->isOK() ); + $block = $this->user->getBlock(); + $this->assertInstanceOf( DatabaseBlock::class, $block ); + $this->assertSame( 'test hideuser', $block->getReasonComment()->text ); + $this->assertTrue( $block->getHideName() ); + } +} diff --git a/tests/phpunit/includes/specials/SpecialBlockTest.php b/tests/phpunit/includes/specials/SpecialBlockTest.php index f0a553213ed..c3fb36a5b61 100644 --- a/tests/phpunit/includes/specials/SpecialBlockTest.php +++ b/tests/phpunit/includes/specials/SpecialBlockTest.php @@ -134,6 +134,7 @@ class SpecialBlockTest extends SpecialPageTestBase { public function testProcessForm() { $badActor = $this->getTestUser()->getUser(); $context = RequestContext::getMain(); + $context->setUser( $this->getTestSysop()->getUser() ); $page = $this->newSpecialPage(); $reason = 'test'; @@ -169,6 +170,7 @@ class SpecialBlockTest extends SpecialPageTestBase { $badActor = $this->getTestUser()->getUser(); $sysop = $this->getTestSysop()->getUser(); $context = RequestContext::getMain(); + $context->setUser( $sysop ); // Create a block that will be updated. $block = new DatabaseBlock( [ @@ -215,6 +217,7 @@ class SpecialBlockTest extends SpecialPageTestBase { public function testProcessFormRestrictions() { $badActor = $this->getTestUser()->getUser(); $context = RequestContext::getMain(); + $context->setUser( $this->getTestSysop()->getUser() ); $pageSaturn = $this->getExistingTestPage( 'Saturn' ); $pageMars = $this->getExistingTestPage( 'Mars' ); @@ -265,6 +268,7 @@ class SpecialBlockTest extends SpecialPageTestBase { public function testProcessFormRestrictionsChange() { $badActor = $this->getTestUser()->getUser(); $context = RequestContext::getMain(); + $context->setUser( $this->getTestSysop()->getUser() ); $pageSaturn = $this->getExistingTestPage( 'Saturn' ); $pageMars = $this->getExistingTestPage( 'Mars' ); @@ -402,7 +406,7 @@ class SpecialBlockTest extends SpecialPageTestBase { ); if ( $expected === 'ipb-prevent-user-talk-edit' ) { - $this->assertSame( $expected, $result[0] ); + $this->assertSame( $expected, $result->getErrorsArray()[0][0] ); } else { $block = DatabaseBlock::newFromTarget( $target ); $this->assertSame( $expected, $block->isUsertalkEditAllowed() ); @@ -486,7 +490,13 @@ class SpecialBlockTest extends SpecialPageTestBase { 'Target' => $target, 'PreviousTarget' => $target, 'Expiry' => 'infinity', + 'CreateAccount' => '0', + 'DisableUTEdit' => '0', + 'DisableEmail' => '0', + 'HardBlock' => '0', + 'AutoBlock' => '0', 'Confirm' => '0', + 'Watch' => '0', ]; $context = new DerivativeContext( RequestContext::getMain() ); @@ -497,7 +507,11 @@ class SpecialBlockTest extends SpecialPageTestBase { $context ); - $this->assertEquals( $expected, $result[0] ); + if ( $result instanceof Status ) { + $result = $result->getErrorsArray(); + } + $error = is_array( $result[0] ) ? $result[0][0] : $result[0]; + $this->assertEquals( $expected, $error ); } public function provideProcessFormErrors() { @@ -602,6 +616,7 @@ class SpecialBlockTest extends SpecialPageTestBase { 'AutoBlock' => '0', 'HideUser' => '1', 'Confirm' => '1', + 'Watch' => '0', ]; $context = new DerivativeContext( RequestContext::getMain() ); @@ -612,6 +627,9 @@ class SpecialBlockTest extends SpecialPageTestBase { $context ); + if ( $result instanceof Status ) { + $result = $result->getErrorsArray(); + } $error = is_array( $result[0] ) ? $result[0][0] : $result[0]; $this->assertEquals( $expected, $error ); } @@ -624,27 +642,27 @@ class SpecialBlockTest extends SpecialPageTestBase { 'HideUser' => '0', 'Confirm' => '0', ], - [ 'hideuser' ], + [ 'block', 'hideuser' ], 'ipb_already_blocked', ], 'Reblock user with Reblock false' => [ [ 'Reblock' => '0' ], - [ 'hideuser' ], + [ 'block', 'hideuser' ], 'ipb_already_blocked', ], 'Reblock with confirm True but target has changed' => [ [ 'PreviousTarget' => '1.2.3.4' ], - [ 'hideuser' ], + [ 'block', 'hideuser' ], 'ipb_already_blocked', ], 'Reblock with same block' => [ [ 'HideUser' => '1' ], - [ 'hideuser' ], + [ 'block', 'hideuser' ], 'ipb_already_blocked', ], 'Reblock hidden user with wrong permissions' => [ [ 'HideUser' => '0' ], - [ 'hideuser' => false ], + [ 'block', 'hideuser' => false ], 'cant-see-hidden-user', ], ]; @@ -656,13 +674,19 @@ class SpecialBlockTest extends SpecialPageTestBase { */ public function testProcessFormErrorsHideUser( $data, $permissions, $expected ) { $performer = $this->getTestSysop()->getUser(); - $this->overrideUserPermissions( $performer, $permissions ); + $this->overrideUserPermissions( $performer, array_merge( $permissions, [ 'block' ] ) ); $defaultData = [ 'Target' => $this->getTestUser()->getUser(), 'HideUser' => '1', 'Expiry' => 'infinity', 'Confirm' => '1', + 'CreateAccount' => '0', + 'DisableUTEdit' => '0', + 'DisableEmail' => '0', + 'HardBlock' => '0', + 'AutoBlock' => '0', + 'Watch' => '0', ]; $context = new DerivativeContext( RequestContext::getMain() ); @@ -673,7 +697,11 @@ class SpecialBlockTest extends SpecialPageTestBase { $context ); - $this->assertEquals( $expected, $result[0] ); + if ( $result instanceof Status ) { + $result = $result->getErrorsArray(); + } + $error = is_array( $result[0] ) ? $result[0][0] : $result[0]; + $this->assertEquals( $expected, $error ); } public function provideProcessFormErrorsHideUser() { @@ -708,7 +736,7 @@ class SpecialBlockTest extends SpecialPageTestBase { $this->setMwGlobals( [ 'wgHideUserContribLimit' => 0 ] ); $performer = $this->getTestSysop()->getUser(); - $this->overrideUserPermissions( $performer, [ 'hideuser' ] ); + $this->overrideUserPermissions( $performer, [ 'block', 'hideuser' ] ); $userToBlock = $this->getTestUser()->getUser(); $pageSaturn = $this->getExistingTestPage( 'Saturn' ); @@ -726,14 +754,24 @@ class SpecialBlockTest extends SpecialPageTestBase { $result = $this->newSpecialPage()->processForm( [ 'Target' => $userToBlock, + 'CreateAccount' => '1', 'HideUser' => '1', 'Expiry' => 'infinity', 'Confirm' => '1', + 'DisableUTEdit' => '0', + 'DisableEmail' => '0', + 'HardBlock' => '0', + 'AutoBlock' => '0', + 'Watch' => '0', ], $context ); - $this->assertEquals( 'ipb_hide_invalid', $result[0][0] ); + if ( $result instanceof Status ) { + $result = $result->getErrorsArray(); + } + $error = is_array( $result[0] ) ? $result[0][0] : $result[0]; + $this->assertEquals( 'ipb_hide_invalid', $error ); } /**