From 02cb7aefef02d5d416ebdb53f9c4767508a99141 Mon Sep 17 00:00:00 2001 From: Thalia Date: Sat, 9 Feb 2019 12:17:54 +0000 Subject: [PATCH] Separate out different functionalities of Block::prevents Block::prevents plays several different roles: * acts as get/setter for Boolean properties that correspond to ipb_create_account, ipb_block_email and ipb_allow_usertalk * calculates whether a block blocks a given right, based on Block properties, global configs, white/blacklists and anonymous user rights * decides whether a block prevents editing of the target's own user talk page (listed separately because 'editownusertalk' is not a right) This patch: * renames mDisableUsertalk to allowEditUsertalk (and reverses the value), to match the field ipb_allow_usertalk and make this logic easier to follow * renames mCreateAccount to blockCreateAccount, to make it clear that the flag blocks account creation when true, and make this logic easier to follow * decouples the block that is stored in the database (which now reflects the form that the admin submitted) and the behaviour of the block on enforcement (since the properties set by the admin can be overridden by global configs) - so if the global configs change, the block behaviour could too * creates get/setters for blockCreateAccount, mBlockEmail and allowEditUsertalk properties * creates appliesToRight, exclusively for checking whether the block blocks a given right, taking into account the block properties, global configs and anonymous user rights * creates appliesToUsertalk, for checking whether the block blocks a user from editing their own talk page. The block is unaware of the user trying to make the edit, and this user is not always the same as the block target, e.g. if the block target is an IP range. Therefore the user's talk page is passed in to this method. appliesToUsertalk can be called from anywhere where the user is known * uses the get/setters wherever Block::prevents was being used as such * uses appliesToRight whenever Block::prevents was being used to determine if the block blocks a given right * uses appliesToUsertalk in User::isBlockedFrom Bug: T211578 Bug: T214508 Change-Id: I0e131696419211319082cb454f4f05297e55d22e --- RELEASE-NOTES-1.33 | 4 + includes/Block.php | 231 +++++++++++++++--- includes/Title.php | 2 +- includes/specials/SpecialBlock.php | 20 +- includes/user/PasswordReset.php | 2 +- includes/user/User.php | 34 +-- maintenance/cleanupBlocks.php | 11 +- tests/phpunit/includes/BlockTest.php | 14 +- .../includes/specials/SpecialBlockTest.php | 4 +- 9 files changed, 239 insertions(+), 83 deletions(-) diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index 9a974eeddd..a78f1658bb 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -315,6 +315,10 @@ because of Phabricator reports. * Passing a User object or null as the third parameter to ApiBase::checkTitleUserPermissions() has been deprecated. Pass an array [ 'user' => $user ] instead. +* (T211578) Block::prevents is deprecated. Use Block::isEmailBlocked, + Block::isCreateAccountBlocked and Block::isUsertalkEditAllowed to get and set + block properties; use Block::appliesToRight and Block::appliesToUsertalk to + check block behaviour. === Other changes in 1.33 === * (T201747) Html::openElement() warns if given an element name with a space diff --git a/includes/Block.php b/includes/Block.php index 573ce3dc4c..11dbcd1964 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -57,10 +57,10 @@ class Block { private $mBlockEmail; /** @var bool */ - private $mDisableUsertalk; + private $allowUsertalk; /** @var bool */ - private $mCreateAccount; + private $blockCreateAccount; /** @var User|string */ private $target; @@ -180,11 +180,9 @@ class Block { $this->isHardblock( !$options['anonOnly'] ); $this->isAutoblocking( (bool)$options['enableAutoblock'] ); $this->isSitewide( (bool)$options['sitewide'] ); - - # Prevention measures - $this->prevents( 'sendemail', (bool)$options['blockEmail'] ); - $this->prevents( 'editownusertalk', !$options['allowUsertalk'] ); - $this->prevents( 'createaccount', (bool)$options['createAccount'] ); + $this->isEmailBlocked( (bool)$options['blockEmail'] ); + $this->isCreateAccountBlocked( (bool)$options['createAccount'] ); + $this->isUsertalkEditAllowed( (bool)$options['allowUsertalk'] ); $this->mFromMaster = false; $this->systemBlockType = $options['systemBlock']; @@ -302,12 +300,12 @@ class Block { && $this->type == $block->type && $this->mAuto == $block->mAuto && $this->isHardblock() == $block->isHardblock() - && $this->prevents( 'createaccount' ) == $block->prevents( 'createaccount' ) + && $this->isCreateAccountBlocked() == $block->isCreateAccountBlocked() && $this->mExpiry == $block->mExpiry && $this->isAutoblocking() == $block->isAutoblocking() && $this->mHideName == $block->mHideName - && $this->prevents( 'sendemail' ) == $block->prevents( 'sendemail' ) - && $this->prevents( 'editownusertalk' ) == $block->prevents( 'editownusertalk' ) + && $this->isEmailBlocked() == $block->isEmailBlocked() + && $this->isUsertalkEditAllowed() == $block->isUsertalkEditAllowed() && $this->mReason == $block->mReason && $this->isSitewide() == $block->isSitewide() // Block::getRestrictions() may perform a database query, so keep it at @@ -411,13 +409,12 @@ class Block { if ( $score < $bestBlockScore ) { $bestBlockScore = $score; $bestRow = $row; - $bestBlockPreventsEdit = $block->prevents( 'edit' ); + $bestBlockPreventsEdit = $block->appliesToRight( 'edit' ); } } if ( $bestRow !== null ) { $this->initFromRow( $bestRow ); - $this->prevents( 'edit', $bestBlockPreventsEdit ); return true; } else { return false; @@ -500,9 +497,9 @@ class Block { $this->isAutoblocking( $row->ipb_enable_autoblock ); $this->isSitewide( (bool)$row->ipb_sitewide ); - $this->prevents( 'createaccount', $row->ipb_create_account ); - $this->prevents( 'sendemail', $row->ipb_block_email ); - $this->prevents( 'editownusertalk', !$row->ipb_allow_usertalk ); + $this->isCreateAccountBlocked( $row->ipb_create_account ); + $this->isEmailBlocked( $row->ipb_block_email ); + $this->isUsertalkEditAllowed( $row->ipb_allow_usertalk ); } /** @@ -705,14 +702,14 @@ class Block { 'ipb_timestamp' => $dbw->timestamp( $this->mTimestamp ), 'ipb_auto' => $this->mAuto, 'ipb_anon_only' => !$this->isHardblock(), - 'ipb_create_account' => $this->prevents( 'createaccount' ), + 'ipb_create_account' => $this->isCreateAccountBlocked(), 'ipb_enable_autoblock' => $this->isAutoblocking(), 'ipb_expiry' => $expiry, 'ipb_range_start' => $this->getRangeStart(), 'ipb_range_end' => $this->getRangeEnd(), 'ipb_deleted' => intval( $this->mHideName ), // typecast required for SQLite - 'ipb_block_email' => $this->prevents( 'sendemail' ), - 'ipb_allow_usertalk' => !$this->prevents( 'editownusertalk' ), + 'ipb_block_email' => $this->isEmailBlocked(), + 'ipb_allow_usertalk' => $this->isUsertalkEditAllowed(), 'ipb_parent_block_id' => $this->mParentBlockId, 'ipb_sitewide' => $this->isSitewide(), ] + CommentStore::getStore()->insert( $dbw, 'ipb_reason', $this->mReason ) @@ -727,9 +724,9 @@ class Block { */ protected function getAutoblockUpdateArray( IDatabase $dbw ) { return [ - 'ipb_create_account' => $this->prevents( 'createaccount' ), + 'ipb_create_account' => $this->isCreateAccountBlocked(), 'ipb_deleted' => (int)$this->mHideName, // typecast required for SQLite - 'ipb_allow_usertalk' => !$this->prevents( 'editownusertalk' ), + 'ipb_allow_usertalk' => $this->isUsertalkEditAllowed(), 'ipb_sitewide' => $this->isSitewide(), ] + CommentStore::getStore()->insert( $dbw, 'ipb_reason', $this->mReason ) + ActorMigration::newMigration()->getInsertValues( $dbw, 'ipb_by', $this->getBlocker() ); @@ -914,10 +911,10 @@ class Block { $timestamp = wfTimestampNow(); $autoblock->mTimestamp = $timestamp; $autoblock->mAuto = 1; - $autoblock->prevents( 'createaccount', $this->prevents( 'createaccount' ) ); + $autoblock->isCreateAccountBlocked( $this->isCreateAccountBlocked() ); # Continue suppressing the name if needed $autoblock->mHideName = $this->mHideName; - $autoblock->prevents( 'editownusertalk', $this->prevents( 'editownusertalk' ) ); + $autoblock->isUsertalkEditAllowed( $this->isUsertalkEditAllowed() ); $autoblock->mParentBlockId = $this->mId; $autoblock->isSitewide( $this->isSitewide() ); $autoblock->setRestrictions( $this->getRestrictions() ); @@ -1146,9 +1143,97 @@ class Block { return wfSetVar( $this->isSitewide, $x ); } + /** + * Get or set the flag indicating whether this block blocks the target from + * creating an account. (Note that the flag may be overridden depending on + * global configs.) + * + * @since 1.33 + * @param null|bool $x Value to set (if null, just get the property value) + * @return bool Value of the property + */ + public function isCreateAccountBlocked( $x = null ) { + return wfSetVar( $this->blockCreateAccount, $x ); + } + + /** + * Get or set the flag indicating whether this block blocks the target from + * sending emails. (Note that the flag may be overridden depending on + * global configs.) + * + * @since 1.33 + * @param null|bool $x Value to set (if null, just get the property value) + * @return bool Value of the property + */ + public function isEmailBlocked( $x = null ) { + return wfSetVar( $this->mBlockEmail, $x ); + } + + /** + * Get or set the flag indicating whether this block blocks the target from + * editing their own user talk page. (Note that the flag may be overridden + * depending on global configs.) + * + * @since 1.33 + * @param null|bool $x Value to set (if null, just get the property value) + * @return bool Value of the property + */ + public function isUsertalkEditAllowed( $x = null ) { + return wfSetVar( $this->allowUsertalk, $x ); + } + + /** + * Determine whether the Block prevents a given right. A right + * may be blacklisted or whitelisted, or determined from a + * property on the Block object. For certain rights, the property + * may be overridden according to global configs. + * + * @since 1.33 + * @param string $right Right to check + * @return bool|null null if unrecognized right or unset property + */ + public function appliesToRight( $right ) { + $config = RequestContext::getMain()->getConfig(); + $blockDisablesLogin = $config->get( 'BlockDisablesLogin' ); + + $res = null; + switch ( $right ) { + case 'edit': + // TODO: fix this case to return proper value + $res = true; + break; + case 'createaccount': + $res = $this->isCreateAccountBlocked(); + break; + case 'sendemail': + $res = $this->isEmailBlocked(); + break; + case 'upload': + // Until T6995 is completed + $res = $this->isSitewide(); + break; + case 'read': + $res = false; + break; + case 'purge': + $res = false; + break; + } + if ( !$res && $blockDisablesLogin ) { + // If a block would disable login, then it should + // prevent any right that all users cannot do + $anon = new User; + $res = $anon->isAllowed( $right ) ? $res : true; + } + + return $res; + } + /** * Get/set whether the Block prevents a given action * + * @deprecated since 1.33, use appliesToRight to determine block + * behaviour, and specific methods to get/set properties * @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. @@ -1165,7 +1250,7 @@ class Block { $res = true; break; case 'createaccount': - $res = wfSetVar( $this->mCreateAccount, $x ); + $res = wfSetVar( $this->blockCreateAccount, $x ); break; case 'sendemail': $res = wfSetVar( $this->mBlockEmail, $x ); @@ -1179,7 +1264,8 @@ class Block { // since partially blocked users are always allowed to edit // their own talk page unless a restriction exists on the // page or User_talk: namespace - $res = wfSetVar( $this->mDisableUsertalk, $x ); + wfSetVar( $this->allowUsertalk, $x === null ? null : !$x ); + $res = !$this->isUserTalkEditAllowed(); // edit own user talk can be disabled by config if ( !$blockAllowsUTEdit ) { @@ -1406,8 +1492,8 @@ class Block { // Sort hard blocks before soft ones and secondarily sort blocks // that disable account creation before those that don't. usort( $blocks, function ( Block $a, Block $b ) { - $aWeight = (int)$a->isHardblock() . (int)$a->prevents( 'createaccount' ); - $bWeight = (int)$b->isHardblock() . (int)$b->prevents( 'createaccount' ); + $aWeight = (int)$a->isHardblock() . (int)$a->appliesToRight( 'createaccount' ); + $bWeight = (int)$b->isHardblock() . (int)$b->appliesToRight( 'createaccount' ); return strcmp( $bWeight, $aWeight ); // highest weight first } ); @@ -1431,7 +1517,7 @@ class Block { // is why the order of the blocks matters if ( !$block->isHardblock() && $blocksListExact['hard'] ) { break; - } elseif ( !$block->prevents( 'createaccount' ) && $blocksListExact['disable_create'] ) { + } elseif ( !$block->appliesToRight( 'createaccount' ) && $blocksListExact['disable_create'] ) { break; } @@ -1440,7 +1526,7 @@ class Block { if ( (string)$block->getTarget() === $checkip ) { if ( $block->isHardblock() ) { $blocksListExact['hard'] = $blocksListExact['hard'] ?: $block; - } elseif ( $block->prevents( 'createaccount' ) ) { + } elseif ( $block->appliesToRight( 'createaccount' ) ) { $blocksListExact['disable_create'] = $blocksListExact['disable_create'] ?: $block; } elseif ( $block->mAuto ) { $blocksListExact['auto'] = $blocksListExact['auto'] ?: $block; @@ -1455,7 +1541,7 @@ class Block { ) { if ( $block->isHardblock() ) { $blocksListRange['hard'] = $blocksListRange['hard'] ?: $block; - } elseif ( $block->prevents( 'createaccount' ) ) { + } elseif ( $block->appliesToRight( 'createaccount' ) ) { $blocksListRange['disable_create'] = $blocksListRange['disable_create'] ?: $block; } elseif ( $block->mAuto ) { $blocksListRange['auto'] = $blocksListRange['auto'] ?: $block; @@ -1813,10 +1899,93 @@ class Block { return $this; } + /** + * Determine whether the block allows the user to edit their own + * user talk page. This is done separately from Block::appliesToRight + * because there is no right for editing one's own user talk page + * and because the user's talk page needs to be passed into the + * Block object, which is unaware of the user. + * + * The ipb_allow_usertalk flag (which corresponds to the property + * allowUsertalk) is used on sitewide blocks and partial blocks + * that contain a namespace restriction on the user talk namespace, + * but do not contain a page restriction on the user's talk page. + * For all other (i.e. most) partial blocks, the flag is ignored, + * and the user can always edit their user talk page unless there + * is a page restriction on their user talk page, in which case + * they can never edit it. (Ideally the flag would be stored as + * null in these cases, but the database field isn't nullable.) + * + * @since 1.33 + * @param Title|null $usertalk The user's user talk page. If null, + * and if the target is a User, the target's userpage is used + * @return bool The user can edit their talk page + */ + public function appliesToUsertalk( Title $usertalk = null ) { + $target = $this->target; + $targetIsUser = $target instanceof User; + $targetName = $targetIsUser ? $target->getName() : $target; + + if ( !$usertalk ) { + if ( $targetIsUser ) { + $usertalk = $this->target->getTalkPage(); + } else { + throw new InvalidArgumentException( + '$usertalk must be provided if block target is not a user/IP' + ); + } + } + + if ( $usertalk->getNamespace() !== NS_USER_TALK ) { + throw new InvalidArgumentException( + '$usertalk must be a user talk page' + ); + } + + switch ( $this->type ) { + case self::TYPE_USER: + case self::TYPE_IP: + if ( $usertalk->getText() !== $targetName ) { + throw new InvalidArgumentException( + '$usertalk must be a talk page for the block target' + ); + } + break; + case self::TYPE_RANGE: + if ( !IP::isInRange( $usertalk->getText(), $target ) ) { + throw new InvalidArgumentException( + '$usertalk must be a talk page for an IP within the block target range' + ); + } + break; + default: + throw new LogicException( + 'Cannot determine validity of $usertalk for this type of block' + ); + } + + if ( !$this->isSitewide() ) { + if ( $this->appliesToPage( $usertalk->getArticleID() ) ) { + return true; + } + if ( !$this->appliesToNamespace( NS_USER_TALK ) ) { + return false; + } + } + + // This is a type of block which uses the ipb_allow_usertalk + // flag. The flag can still be overridden by global configs. + $config = RequestContext::getMain()->getConfig(); + if ( !$config->get( 'BlockAllowsUTEdit' ) ) { + return true; + } + return !$this->isUsertalkEditAllowed(); + } + /** * Checks if a block applies to a particular title * - * This check does not consider whether `$this->prevents( 'editownusertalk' )` + * This check does not consider whether `$this->isUsertalkEditAllowed` * returns false, as the identity of the user making the hypothetical edit * isn't known here (particularly in the case of IP hardblocks, range * blocks, and auto-blocks). @@ -1865,7 +2034,7 @@ class Block { /** * Checks if a block applies to a particular page * - * This check does not consider whether `$this->prevents( 'editownusertalk' )` + * This check does not consider whether `$this->isUsertalkEditAllowed` * returns false, as the identity of the user making the hypothetical edit * isn't known here (particularly in the case of IP hardblocks, range * blocks, and auto-blocks). diff --git a/includes/Title.php b/includes/Title.php index 849707e38b..6dd21c463f 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -2621,7 +2621,7 @@ class Title implements LinkTarget, IDBAccessObject { $block = $user->getBlock( $useReplica ); // The block may explicitly allow an action (like "read" or "upload"). - if ( $block && $block->prevents( $action ) === false ) { + if ( $block && $block->appliesToRight( $action ) === false ) { return $errors; } diff --git a/includes/specials/SpecialBlock.php b/includes/specials/SpecialBlock.php index 0b37910b8b..a00c42cd40 100644 --- a/includes/specials/SpecialBlock.php +++ b/includes/specials/SpecialBlock.php @@ -335,11 +335,11 @@ class SpecialBlock extends FormSpecialPage { || $block->getTarget() == $this->target ) # or if it is, the range is what we're about to block ) { $fields['HardBlock']['default'] = $block->isHardblock(); - $fields['CreateAccount']['default'] = $block->prevents( 'createaccount' ); + $fields['CreateAccount']['default'] = $block->isCreateAccountBlocked(); $fields['AutoBlock']['default'] = $block->isAutoblocking(); if ( isset( $fields['DisableEmail'] ) ) { - $fields['DisableEmail']['default'] = $block->prevents( 'sendemail' ); + $fields['DisableEmail']['default'] = $block->isEmailBlocked(); } if ( isset( $fields['HideUser'] ) ) { @@ -347,7 +347,7 @@ class SpecialBlock extends FormSpecialPage { } if ( isset( $fields['DisableUTEdit'] ) ) { - $fields['DisableUTEdit']['default'] = $block->prevents( 'editownusertalk' ); + $fields['DisableUTEdit']['default'] = !$block->isUsertalkEditAllowed(); } // If the username was hidden (ipb_deleted == 1), don't show the reason @@ -849,9 +849,9 @@ class SpecialBlock extends FormSpecialPage { $block->setBlocker( $performer ); $block->mReason = $data['Reason'][0]; $block->mExpiry = $expiryTime; - $block->prevents( 'createaccount', $data['CreateAccount'] ); - $block->prevents( 'editownusertalk', ( !$wgBlockAllowsUTEdit || $data['DisableUTEdit'] ) ); - $block->prevents( 'sendemail', $data['DisableEmail'] ); + $block->isCreateAccountBlocked( $data['CreateAccount'] ); + $block->isUsertalkEditAllowed( !$wgBlockAllowsUTEdit || !$data['DisableUTEdit'] ); + $block->isEmailBlocked( $data['DisableEmail'] ); $block->isHardblock( $data['HardBlock'] ); $block->isAutoblocking( $data['AutoBlock'] ); $block->mHideName = $data['HideUser']; @@ -919,12 +919,12 @@ class SpecialBlock extends FormSpecialPage { $priorBlock = clone $currentBlock; $currentBlock->isHardblock( $block->isHardblock() ); - $currentBlock->prevents( 'createaccount', $block->prevents( 'createaccount' ) ); + $currentBlock->isCreateAccountBlocked( $block->isCreateAccountBlocked() ); $currentBlock->mExpiry = $block->mExpiry; $currentBlock->isAutoblocking( $block->isAutoblocking() ); $currentBlock->mHideName = $block->mHideName; - $currentBlock->prevents( 'sendemail', $block->prevents( 'sendemail' ) ); - $currentBlock->prevents( 'editownusertalk', $block->prevents( 'editownusertalk' ) ); + $currentBlock->isEmailBlocked( $block->isEmailBlocked() ); + $currentBlock->isUsertalkEditAllowed( $block->isUsertalkEditAllowed() ); $currentBlock->mReason = $block->mReason; if ( $enablePartialBlocks ) { @@ -976,7 +976,7 @@ class SpecialBlock extends FormSpecialPage { } # Block constructor sanitizes certain block options on insert - $data['BlockEmail'] = $block->prevents( 'sendemail' ); + $data['BlockEmail'] = $block->isEmailBlocked(); $data['AutoBlock'] = $block->isAutoblocking(); # Prepare log parameters diff --git a/includes/user/PasswordReset.php b/includes/user/PasswordReset.php index f65bae5aa7..ef104cce5c 100644 --- a/includes/user/PasswordReset.php +++ b/includes/user/PasswordReset.php @@ -270,7 +270,7 @@ class PasswordReset implements LoggerAwareInterface { // Normal block. Maybe it was meant for someone else and the user just needs to log in; // or maybe it was issued specifically to prevent some IP from messing with password // reset? Go out on a limb and use the registration allowed flag to decide. - return $block->prevents( 'createaccount' ); + return $block->isCreateAccountBlocked(); } elseif ( $type === 'proxy' ) { // we disallow actions through proxy even if the user is logged in // so it makes sense to disallow password resets as well diff --git a/includes/user/User.php b/includes/user/User.php index f4e2e48a79..8173f5db8c 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -1927,7 +1927,7 @@ class User implements IDBAccessObject, UserIdentity { $this->mBlockedby = $block->getByName(); $this->mBlockreason = $block->mReason; $this->mHideName = $block->mHideName; - $this->mAllowUsertalk = !$block->prevents( 'editownusertalk' ); + $this->mAllowUsertalk = $block->isUsertalkEditAllowed(); } else { $this->mBlock = null; $this->mBlockedby = ''; @@ -2275,7 +2275,8 @@ class User implements IDBAccessObject, UserIdentity { * @return bool True if blocked, false otherwise */ public function isBlocked( $bFromReplica = true ) { - return $this->getBlock( $bFromReplica ) instanceof Block && $this->getBlock()->prevents( 'edit' ); + return $this->getBlock( $bFromReplica ) instanceof Block && + $this->getBlock()->appliesToRight( 'edit' ); } /** @@ -2305,26 +2306,7 @@ class User implements IDBAccessObject, UserIdentity { // Special handling for a user's own talk page. The block is not aware // of the user, so this must be done here. if ( $title->equals( $this->getTalkPage() ) ) { - if ( $block->isSitewide() ) { - // If the block is sitewide, whatever is set is what is honored. - // This must be checked here, because Block::appliesToPage will - // return true for a sitewide block. - $blocked = $block->prevents( 'editownusertalk' ); - } else { - // The page restrictions always take precedence over the namespace - // restrictions. If the user is explicity blocked from their own - // talk page, nothing can change that. - $blocked = $block->appliesToPage( $title->getArticleID() ); - - // If the block applies to the user talk namespace, then whatever is - // set is what is honored. - if ( !$blocked && $block->appliesToNamespace( NS_USER_TALK ) ) { - $blocked = $block->prevents( 'editownusertalk' ); - } - - // If another type of restriction is added, it should be checked - // here. - } + $blocked = $block->appliesToUsertalk( $title ); } else { $blocked = $block->appliesToTitle( $title ); } @@ -4536,7 +4518,7 @@ class User implements IDBAccessObject, UserIdentity { */ public function isBlockedFromCreateAccount() { $this->getBlockedStatus(); - if ( $this->mBlock && $this->mBlock->prevents( 'createaccount' ) ) { + if ( $this->mBlock && $this->mBlock->appliesToRight( 'createaccount' ) ) { return $this->mBlock; } @@ -4547,7 +4529,7 @@ class User implements IDBAccessObject, UserIdentity { $this->mBlockedFromCreateAccount = Block::newFromTarget( null, $this->getRequest()->getIP() ); } return $this->mBlockedFromCreateAccount instanceof Block - && $this->mBlockedFromCreateAccount->prevents( 'createaccount' ) + && $this->mBlockedFromCreateAccount->appliesToRight( 'createaccount' ) ? $this->mBlockedFromCreateAccount : false; } @@ -4558,7 +4540,7 @@ class User implements IDBAccessObject, UserIdentity { */ public function isBlockedFromEmailuser() { $this->getBlockedStatus(); - return $this->mBlock && $this->mBlock->prevents( 'sendemail' ); + return $this->mBlock && $this->mBlock->appliesToRight( 'sendemail' ); } /** @@ -4569,7 +4551,7 @@ class User implements IDBAccessObject, UserIdentity { */ public function isBlockedFromUpload() { $this->getBlockedStatus(); - return $this->mBlock && $this->mBlock->prevents( 'upload' ); + return $this->mBlock && $this->mBlock->appliesToRight( 'upload' ); } /** diff --git a/maintenance/cleanupBlocks.php b/maintenance/cleanupBlocks.php index cbf008415e..15bb4f3ede 100644 --- a/maintenance/cleanupBlocks.php +++ b/maintenance/cleanupBlocks.php @@ -92,11 +92,12 @@ class CleanupBlocks extends Maintenance { $keep = $block->getExpiry() > $bestBlock->getExpiry(); } if ( $keep === null ) { - foreach ( [ 'createaccount', 'sendemail', 'editownusertalk' ] as $action ) { - if ( $block->prevents( $action ) xor $bestBlock->prevents( $action ) ) { - $keep = $block->prevents( $action ); - break; - } + if ( $block->isCreateAccountBlocked() xor $bestBlock->isCreateAccountBlocked() ) { + $keep = $block->isCreateAccountBlocked(); + } elseif ( $block->isEmailBlocked() xor $bestBlock->isEmailBlocked() ) { + $keep = $block->isEmailBlocked(); + } elseif ( $block->isUsertalkEditAllowed() xor $bestBlock->isUsertalkEditAllowed() ) { + $keep = $block->isUsertalkEditAllowed(); } } diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php index f20481bde7..1f11ef2665 100644 --- a/tests/phpunit/includes/BlockTest.php +++ b/tests/phpunit/includes/BlockTest.php @@ -131,7 +131,7 @@ class BlockTest extends MediaWikiLangTestCase { } /** - * @covers Block::prevents + * @covers Block::appliesToRight */ public function testBlockedUserCanNotCreateAccount() { $username = 'BlockedUserToCreateAccountWith'; @@ -174,8 +174,8 @@ class BlockTest extends MediaWikiLangTestCase { // Reload block from DB $userBlock = Block::newFromTarget( $username ); $this->assertTrue( - (bool)$block->prevents( 'createaccount' ), - "Block object in DB should prevents 'createaccount'" + (bool)$block->appliesToRight( 'createaccount' ), + "Block object in DB should block right 'createaccount'" ); $this->assertInstanceOf( @@ -304,7 +304,7 @@ class BlockTest extends MediaWikiLangTestCase { $block->setBlocker( $blocker ); $block->mReason = $insBlock['desc']; $block->mExpiry = 'infinity'; - $block->prevents( 'createaccount', $insBlock['ACDisable'] ); + $block->isCreateAccountBlocked( $insBlock['ACDisable'] ); $block->isHardblock( $insBlock['isHardblock'] ); $block->isAutoblocking( $insBlock['isAutoBlocking'] ); $block->insert(); @@ -422,7 +422,7 @@ class BlockTest extends MediaWikiLangTestCase { # Check default parameter $this->assertFalse( - (bool)$block->prevents( 'createaccount' ), + (bool)$block->appliesToRight( 'createaccount' ), "Account creation should not be blocked by default" ); } @@ -761,14 +761,14 @@ class BlockTest extends MediaWikiLangTestCase { } /** - * @covers Block::prevents + * @covers Block::appliesToRight */ public function testBlockAllowsPurge() { $this->setMwGlobals( [ 'wgBlockDisablesLogin' => false, ] ); $block = new Block(); - $this->assertFalse( $block->prevents( 'purge' ) ); + $this->assertFalse( $block->appliesToRight( 'purge' ) ); } } diff --git a/tests/phpunit/includes/specials/SpecialBlockTest.php b/tests/phpunit/includes/specials/SpecialBlockTest.php index 0b3adc031f..8c8e2704e6 100644 --- a/tests/phpunit/includes/specials/SpecialBlockTest.php +++ b/tests/phpunit/includes/specials/SpecialBlockTest.php @@ -88,9 +88,9 @@ class SpecialBlockTest extends SpecialPageTestBase { $this->assertSame( (string)$block->getTarget(), $fields['Target']['default'] ); $this->assertSame( $block->isHardblock(), $fields['HardBlock']['default'] ); - $this->assertSame( $block->prevents( 'createaccount' ), $fields['CreateAccount']['default'] ); + $this->assertSame( $block->isCreateAccountBlocked(), $fields['CreateAccount']['default'] ); $this->assertSame( $block->isAutoblocking(), $fields['AutoBlock']['default'] ); - $this->assertSame( $block->prevents( 'editownusertalk' ), $fields['DisableUTEdit']['default'] ); + $this->assertSame( !$block->isUsertalkEditAllowed(), $fields['DisableUTEdit']['default'] ); $this->assertSame( $block->mReason, $fields['Reason']['default'] ); $this->assertSame( 'infinite', $fields['Expiry']['default'] ); } -- 2.20.1