From 01a3b2b0bf6519922c4e13f5b68cf7cfb3547a21 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 1 Dec 2016 11:51:03 -0500 Subject: [PATCH] Add the concept of "system blocks" Blocks made for configured proxies, dnsbls, or the configured range soft-blocks being added in I6c11a6b9 aren't real blocks stored in the database. Let's actually flag these blocks as such and use a more appropriate message when displaying them to the user. Change-Id: I697e3eec2520792e98c193200c2b1c28c35bf382 --- RELEASE-NOTES-1.29 | 2 ++ includes/Block.php | 35 +++++++++++++++++-- includes/EditPage.php | 6 +++- includes/api/ApiBase.php | 7 ++++ includes/api/ApiMessage.php | 1 + includes/api/ApiQueryUserInfo.php | 4 +++ includes/api/i18n/en.json | 1 + includes/api/i18n/qqq.json | 1 + includes/user/User.php | 26 ++++++++------ languages/i18n/en.json | 1 + languages/i18n/qqq.json | 7 ++-- tests/phpunit/includes/BlockTest.php | 29 +++++++++++++++ .../phpunit/includes/TitlePermissionTest.php | 16 +++++++++ tests/phpunit/includes/api/ApiBaseTest.php | 4 +++ 14 files changed, 124 insertions(+), 16 deletions(-) diff --git a/RELEASE-NOTES-1.29 b/RELEASE-NOTES-1.29 index 4271d0de06..cb40e9a152 100644 --- a/RELEASE-NOTES-1.29 +++ b/RELEASE-NOTES-1.29 @@ -31,6 +31,8 @@ production. they move to a new IP address. This is disabled by default. * Added ILocalizedException interface to standardize the use of localized exceptions, largely so the API can handle them more sensibly. +* Blocks created automatically by MediaWiki, such as for configured proxies or + dnsbls, are now indicated as such and use a new i18n message when displayed. === External library changes in 1.29 === diff --git a/includes/Block.php b/includes/Block.php index 8663d0328c..792bcd9320 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -74,6 +74,9 @@ class Block { /** @var bool */ protected $isAutoblocking; + /** @var string|null */ + protected $systemBlockType; + # TYPE constants const TYPE_USER = 1; const TYPE_IP = 2; @@ -99,6 +102,10 @@ class Block { * blockEmail bool Disallow sending emails * allowUsertalk bool Allow the target to edit its own talk page * byText string Username of the blocker (for foreign users) + * systemBlock string Indicate that this block is automatically + * created by MediaWiki rather than being stored + * in the database. Value is a string to return + * from self::getSystemBlockType(). * * @since 1.26 accepts $options array instead of individual parameters; order * of parameters above reflects the original order @@ -119,6 +126,7 @@ class Block { 'blockEmail' => false, 'allowUsertalk' => false, 'byText' => '', + 'systemBlock' => null, ]; if ( func_num_args() > 1 || !is_array( $options ) ) { @@ -162,6 +170,7 @@ class Block { $this->prevents( 'createaccount', (bool)$options['createAccount'] ); $this->mFromMaster = false; + $this->systemBlockType = $options['systemBlock']; } /** @@ -461,6 +470,11 @@ class Block { */ public function insert( $dbw = null ) { global $wgBlockDisablesLogin; + + if ( $this->getSystemBlockType() !== null ) { + throw new MWException( 'Cannot insert a system block into the database' ); + } + wfDebug( "Block::insert; timestamp {$this->mTimestamp}\n" ); if ( $dbw === null ) { @@ -741,6 +755,11 @@ class Block { return false; } + # Don't autoblock for system blocks + if ( $this->getSystemBlockType() !== null ) { + throw new MWException( 'Cannot autoblock from a system block' ); + } + # Check for presence on the autoblock whitelist. if ( self::isWhitelistedFromAutoblocks( $autoblockIP ) ) { return false; @@ -934,6 +953,14 @@ class Block { return $this->mId; } + /** + * Get the system block type, if any + * @return string|null + */ + public function getSystemBlockType() { + return $this->systemBlockType; + } + /** * Get/set a flag determining whether the master is used for reads * @@ -1469,14 +1496,18 @@ class Block { * This could be a username, an IP range, or a single IP. */ $intended = $this->getTarget(); + $systemBlockType = $this->getSystemBlockType(); + $lang = $context->getLanguage(); return [ - $this->mAuto ? 'autoblockedtext' : 'blockedtext', + $systemBlockType !== null + ? 'systemblockedtext' + : ( $this->mAuto ? 'autoblockedtext' : 'blockedtext' ), $link, $reason, $context->getRequest()->getIP(), $this->getByName(), - $this->getId(), + $systemBlockType !== null ? $systemBlockType : $this->getId(), $lang->formatExpiry( $this->mExpiry ), (string)$intended, $lang->userTimeAndDate( $this->mTimestamp, $context->getUser() ), diff --git a/includes/EditPage.php b/includes/EditPage.php index 7a0a11f7d1..4f1e47d63b 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -661,7 +661,11 @@ class EditPage { $remove = []; foreach ( $permErrors as $error ) { if ( ( $this->preview || $this->diff ) - && ( $error[0] == 'blockedtext' || $error[0] == 'autoblockedtext' ) + && ( + $error[0] == 'blockedtext' || + $error[0] == 'autoblockedtext' || + $error[0] == 'systemblockedtext' + ) ) { $remove[] = $error; } diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 6fad630cb1..c1ad9479e2 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -1627,6 +1627,12 @@ abstract class ApiBase extends ContextSource { 'autoblocked', [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ] ) ); + } elseif ( is_array( $error ) && $error[0] === 'systemblockedtext' && $user->getBlock() ) { + $status->fatal( ApiMessage::create( + 'apierror-systemblocked', + 'blocked', + [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ] + ) ); } else { call_user_func_array( [ $status, 'fatal' ], (array)$error ); } @@ -2464,6 +2470,7 @@ abstract class ApiBase extends ContextSource { 'confirmedittext' => 'confirmedittext', 'blockedtext' => 'apierror-blocked', 'autoblockedtext' => 'apierror-autoblocked', + 'systemblockedtext' => 'apierror-systemblocked', 'actionthrottledtext' => 'apierror-ratelimited', 'alreadyrolled' => 'alreadyrolled', 'cantrollback' => 'cantrollback', diff --git a/includes/api/ApiMessage.php b/includes/api/ApiMessage.php index 9d69a771d4..9e42d5fd73 100644 --- a/includes/api/ApiMessage.php +++ b/includes/api/ApiMessage.php @@ -126,6 +126,7 @@ trait ApiMessageTrait { 'rcpatroldisabled' => 'patroldisabled', 'readonlytext' => 'readonly', 'sessionfailure' => 'badtoken', + 'systemblockedtext' => 'blocked', 'titleprotected' => 'protectedtitle', 'undo-failure' => 'undofailure', 'userrights-nodatabase' => 'nosuchdatabase', diff --git a/includes/api/ApiQueryUserInfo.php b/includes/api/ApiQueryUserInfo.php index 3b604786ab..60e122cdd1 100644 --- a/includes/api/ApiQueryUserInfo.php +++ b/includes/api/ApiQueryUserInfo.php @@ -64,6 +64,7 @@ class ApiQueryUserInfo extends ApiQueryBase { * - blockreason - reason provided for the block * - blockedtimestamp - timestamp for when the block was placed/modified * - blockexpiry - expiry time of the block + * - systemblocktype - system block type, if any */ public static function getBlockInfo( Block $block ) { global $wgContLang; @@ -76,6 +77,9 @@ class ApiQueryUserInfo extends ApiQueryBase { $vals['blockexpiry'] = $wgContLang->formatExpiry( $block->getExpiry(), TS_ISO_8601, 'infinite' ); + if ( $block->getSystemBlockType() !== null ) { + $vals['systemblocktype'] = $block->getSystemBlockType(); + } return $vals; } diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json index c0b5dbe2c6..d74889447d 100644 --- a/includes/api/i18n/en.json +++ b/includes/api/i18n/en.json @@ -1707,6 +1707,7 @@ "apierror-stashpathinvalid": "File key of improper format or otherwise invalid: $1.", "apierror-stashwrongowner": "Wrong owner: $1", "apierror-stashzerolength": "File is of zero length, and could not be stored in the stash: $1.", + "apierror-systemblocked": "You have been blocked automatically by MediaWiki.", "apierror-templateexpansion-notwikitext": "Template expansion is only supported for wikitext content. $1 uses content model $2.", "apierror-toofewexpiries": "$1 expiry {{PLURAL:$1|timestamp was|timestamps were}} provided where $2 {{PLURAL:$2|was|were}} needed.", "apierror-unknownaction": "The action specified, $1, is not recognized.", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index 3f1b7a7231..eb479ca70f 100644 --- a/includes/api/i18n/qqq.json +++ b/includes/api/i18n/qqq.json @@ -1601,6 +1601,7 @@ "apierror-stashpathinvalid": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text. Currently this is probably English, hopefully we'll fix that in the future.", "apierror-stashwrongowner": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text, which should already end with punctuation. Currently this is probably English, hopefully we'll fix that in the future.", "apierror-stashzerolength": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text. Currently this is probably English, hopefully we'll fix that in the future.", + "apierror-systemblocked": "{{doc-apierror}}", "apierror-templateexpansion-notwikitext": "{{doc-apierror}}\n\nParameters:\n* $1 - Page title.\n* $2 - Content model.", "apierror-toofewexpiries": "{{doc-apierror}}\n\nParameters:\n* $1 - Number provided.\n* $2 - Number needed.", "apierror-unknownaction": "{{doc-apierror}}\n\nParameters:\n* $1 - Action provided.", diff --git a/includes/user/User.php b/includes/user/User.php index f1742b321d..4e8a5ffcd9 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -1643,16 +1643,20 @@ class User implements IDBAccessObject { if ( !$block instanceof Block && $ip !== null && !in_array( $ip, $wgProxyWhitelist ) ) { // Local list if ( self::isLocallyBlockedProxy( $ip ) ) { - $block = new Block; - $block->setBlocker( wfMessage( 'proxyblocker' )->text() ); - $block->mReason = wfMessage( 'proxyblockreason' )->text(); - $block->setTarget( $ip ); + $block = new Block( [ + 'byText' => wfMessage( 'proxyblocker' )->text(), + 'reason' => wfMessage( 'proxyblockreason' )->text(), + 'address' => $ip, + 'systemBlock' => 'proxy', + ] ); $this->blockTrigger = 'proxy-block'; } elseif ( $this->isAnon() && $this->isDnsBlacklisted( $ip ) ) { - $block = new Block; - $block->setBlocker( wfMessage( 'sorbs' )->text() ); - $block->mReason = wfMessage( 'sorbsreason' )->text(); - $block->setTarget( $ip ); + $block = new Block( [ + 'byText' => wfMessage( 'sorbs' )->text(), + 'reason' => wfMessage( 'sorbsreason' )->text(), + 'address' => $ip, + 'systemBlock' => 'dnsbl', + ] ); $this->blockTrigger = 'openproxy-block'; } } @@ -2067,8 +2071,10 @@ class User implements IDBAccessObject { if ( $blocked && $block === null ) { // back-compat: UserIsBlockedGlobally didn't have $block param first - $block = new Block; - $block->setTarget( $ip ); + $block = new Block( [ + 'address' => $ip, + 'systemBlock' => 'global-block' + ] ); } $this->mGlobalBlock = $blocked ? $block : false; diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 882357f8c0..6a8bbd8ec6 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -677,6 +677,7 @@ "blockedtitle": "User is blocked", "blockedtext": "Your username or IP address has been blocked.\n\nThe block was made by $1.\nThe reason given is $2.\n\n* Start of block: $8\n* Expiration of block: $6\n* Intended blockee: $7\n\nYou can contact $1 or another [[{{MediaWiki:Grouppage-sysop}}|administrator]] to discuss the block.\nYou cannot use the \"email this user\" feature unless a valid email address is specified in your [[Special:Preferences|account preferences]] and you have not been blocked from using it.\nYour current IP address is $3, and the block ID is #$5.\nPlease include all above details in any queries you make.", "autoblockedtext": "Your IP address has been automatically blocked because it was used by another user, who was blocked by $1.\nThe reason given is:\n\n:$2\n\n* Start of block: $8\n* Expiration of block: $6\n* Intended blockee: $7\n\nYou may contact $1 or one of the other [[{{MediaWiki:Grouppage-sysop}}|administrators]] to discuss the block.\n\nNote that you may not use the \"email this user\" feature unless you have a valid email address registered in your [[Special:Preferences|user preferences]] and you have not been blocked from using it.\n\nYour current IP address is $3, and the block ID is #$5.\nPlease include all above details in any queries you make.", + "systemblockedtext": "Your username or IP address has been automatically blocked by MediaWiki.\nThe reason given is:\n\n:$2\n\n* Start of block: $8\n* Expiration of block: $6\n* Intended blockee: $7\n\nYour current IP address is $3.\nPlease include all above details in any queries you make.", "blockednoreason": "no reason given", "whitelistedittext": "Please $1 to edit pages.", "confirmedittext": "You must confirm your email address before editing pages.\nPlease set and validate your email address through your [[Special:Preferences|user preferences]].", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index d292c7264f..9e209f2c39 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -858,9 +858,10 @@ "summary-preview": "Preview of the edit summary, shown under the edit summary itself.\nShould match: {{msg-mw|summary}}.", "subject-preview": "Should match {{msg-mw|subject}}", "previewerrortext": "When a user has the editing preference LivePreview enabled, clicked the Preview or Show Changes button in the edit page and the action did not succeed.", - "blockedtitle": "Used as title displayed for blocked users. The corresponding message body is one of the following messages:\n* {{msg-mw|Blockedtext|notext=1}}\n* {{msg-mw|Autoblockedtext|notext=1}}", - "blockedtext": "Text displayed to blocked users.\n\n\"email this user\" should be consistent with {{msg-mw|Emailuser}}.\n\nParameters:\n* $1 - the blocking sysop (with a link to his/her userpage)\n* $2 - the reason for the block\n* $3 - the current IP address of the blocked user\n* $4 - (Unused) the blocking sysop's username (plain text, without the link)\n* $5 - the unique numeric identifier of the applied autoblock\n* $6 - the expiry of the block\n* $7 - the intended target of the block (what the blocking user specified in the blocking form)\n* $8 - the timestamp when the block started\nSee also:\n* {{msg-mw|Grouppage-sysop}}\n* {{msg-mw|Autoblockedtext}}", - "autoblockedtext": "Text displayed to automatically blocked users.\n\n\"email this user\" should be consistent with {{msg-mw|Emailuser}}.\n\nParameters:\n* $1 - the blocking sysop (with a link to his/her userpage)\n* $2 - the reason for the block (in case of autoblocks: {{msg-mw|autoblocker}})\n* $3 - the current IP address of the blocked user\n* $4 - (Unused) the blocking sysop's username (plain text, without the link). Use it for GENDER.\n* $5 - the unique numeric identifier of the applied autoblock\n* $6 - the expiry of the block\n* $7 - the intended target of the block (what the blocking user specified in the blocking form)\n* $8 - the timestamp when the block started\nSee also:\n* {{msg-mw|Grouppage-sysop}}\n* {{msg-mw|Blockedtext}}", + "blockedtitle": "Used as title displayed for blocked users. The corresponding message body is one of the following messages:\n* {{msg-mw|Blockedtext|notext=1}}\n* {{msg-mw|Autoblockedtext|notext=1}}\n* {{msg-mw|Systemblockedtext}}", + "blockedtext": "Text displayed to blocked users.\n\n\"email this user\" should be consistent with {{msg-mw|Emailuser}}.\n\nParameters:\n* $1 - the blocking sysop (with a link to his/her userpage)\n* $2 - the reason for the block\n* $3 - the current IP address of the blocked user\n* $4 - (Unused) the blocking sysop's username (plain text, without the link)\n* $5 - the unique numeric identifier of the applied autoblock\n* $6 - the expiry of the block\n* $7 - the intended target of the block (what the blocking user specified in the blocking form)\n* $8 - the timestamp when the block started\nSee also:\n* {{msg-mw|Grouppage-sysop}}\n* {{msg-mw|Autoblockedtext}}\n* {{msg-mw|Systemblockedtext}}", + "autoblockedtext": "Text displayed to automatically blocked users.\n\n\"email this user\" should be consistent with {{msg-mw|Emailuser}}.\n\nParameters:\n* $1 - the blocking sysop (with a link to his/her userpage)\n* $2 - the reason for the block (in case of autoblocks: {{msg-mw|autoblocker}})\n* $3 - the current IP address of the blocked user\n* $4 - (Unused) the blocking sysop's username (plain text, without the link). Use it for GENDER.\n* $5 - the unique numeric identifier of the applied autoblock\n* $6 - the expiry of the block\n* $7 - the intended target of the block (what the blocking user specified in the blocking form)\n* $8 - the timestamp when the block started\nSee also:\n* {{msg-mw|Grouppage-sysop}}\n* {{msg-mw|Blockedtext}}\n* {{msg-mw|Systemblockedtext}}", + "systemblockedtext": "Text displayed to requests blocked by MediaWiki configuration.\n\n\"email this user\" should be consistent with {{msg-mw|Emailuser}}.\n\nParameters:\n* $1 - (Unused) A dummy user attributed as the blocker, possibly as a link to a user page.\n* $2 - the reason for the block\n* $3 - the current IP address of the blocked user\n* $4 - (Unused) the dummy blocking user's username (plain text, without the link).\n* $5 - A short string indicating the type of system block.\n* $6 - the expiry of the block\n* $7 - the intended target of the block\n* $8 - the timestamp when the block started\nSee also:\n* {{msg-mw|Grouppage-sysop}}\n* {{msg-mw|Blockedtext}}\n* {{msg-mw|Autoblockedtext}}", "blockednoreason": "Substituted with $2 in the following message if the reason is not given:\n* {{msg-mw|cantcreateaccount-text}}.\n{{Identical|No reason given}}", "whitelistedittext": "Used as error message. Parameters:\n* $1 - a link to [[Special:UserLogin]] with {{msg-mw|loginreqlink}} as link description\nSee also:\n* {{msg-mw|Nocreatetext}}\n* {{msg-mw|Uploadnologintext}}\n* {{msg-mw|Loginreqpagetext}}", "confirmedittext": "Used as error message.", diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php index 9f386599eb..12b277e94f 100644 --- a/tests/phpunit/includes/BlockTest.php +++ b/tests/phpunit/includes/BlockTest.php @@ -409,4 +409,33 @@ class BlockTest extends MediaWikiLangTestCase { "Account creation should not be blocked by default" ); } + + public function testSystemBlocks() { + $blockOptions = [ + 'address' => 'UTBlockee', + 'reason' => 'test system block', + 'timestamp' => wfTimestampNow(), + 'expiry' => $this->db->getInfinity(), + 'byText' => 'MetaWikiUser', + 'systemBlock' => 'test', + 'enableAutoblock' => true, + ]; + $block = new Block( $blockOptions ); + + $this->assertSame( 'test', $block->getSystemBlockType() ); + + try { + $block->insert(); + $this->fail( 'Expected exception not thrown' ); + } catch ( MWException $ex ) { + $this->assertSame( 'Cannot insert a system block into the database', $ex->getMessage() ); + } + + try { + $block->doAutoblock( '192.0.2.2' ); + $this->fail( 'Expected exception not thrown' ); + } catch ( MWException $ex ) { + $this->assertSame( 'Cannot autoblock from a system block', $ex->getMessage() ); + } + } } diff --git a/tests/phpunit/includes/TitlePermissionTest.php b/tests/phpunit/includes/TitlePermissionTest.php index 5ecdf56134..9121178a46 100644 --- a/tests/phpunit/includes/TitlePermissionTest.php +++ b/tests/phpunit/includes/TitlePermissionTest.php @@ -787,5 +787,21 @@ class TitlePermissionTest extends MediaWikiLangTestCase { # $action != 'read' && $action != 'createaccount' && $user->isBlockedFrom( $this ) # $user->blockedFor() == '' # $user->mBlock->mExpiry == 'infinity' + + $this->user->mBlockedby = $this->user->getName(); + $this->user->mBlock = new Block( [ + 'address' => '127.0.8.1', + 'by' => $this->user->getId(), + 'reason' => 'no reason given', + 'timestamp' => $now, + 'auto' => false, + 'expiry' => 10, + 'systemBlock' => 'test', + ] ); + $this->assertEquals( [ [ 'systemblockedtext', + '[[User:Useruser|Useruser]]', 'no reason given', '127.0.0.1', + 'Useruser', 'test', '23:00, 31 December 1969', '127.0.8.1', + $wgLang->timeanddate( wfTimestamp( TS_MW, $now ), true ) ] ], + $this->title->getUserPermissionsErrors( 'move-target', $this->user ) ); } } diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php index 96f3e44f0f..7327e85471 100644 --- a/tests/phpunit/includes/api/ApiBaseTest.php +++ b/tests/phpunit/includes/api/ApiBaseTest.php @@ -135,11 +135,13 @@ class ApiBaseTest extends ApiTestCase { $expect = Status::newGood(); $expect->fatal( 'blockedtext' ); $expect->fatal( 'autoblockedtext' ); + $expect->fatal( 'systemblockedtext' ); $expect->fatal( 'mainpage' ); $expect->fatal( 'parentheses', 'foobar' ); $this->assertEquals( $expect, $mock->errorArrayToStatus( [ [ 'blockedtext' ], [ 'autoblockedtext' ], + [ 'systemblockedtext' ], 'mainpage', [ 'parentheses', 'foobar' ], ] ) ); @@ -158,11 +160,13 @@ class ApiBaseTest extends ApiTestCase { $expect = Status::newGood(); $expect->fatal( ApiMessage::create( 'apierror-blocked', 'blocked', $blockinfo ) ); $expect->fatal( ApiMessage::create( 'apierror-autoblocked', 'autoblocked', $blockinfo ) ); + $expect->fatal( ApiMessage::create( 'apierror-systemblocked', 'blocked', $blockinfo ) ); $expect->fatal( 'mainpage' ); $expect->fatal( 'parentheses', 'foobar' ); $this->assertEquals( $expect, $mock->errorArrayToStatus( [ [ 'blockedtext' ], [ 'autoblockedtext' ], + [ 'systemblockedtext' ], 'mainpage', [ 'parentheses', 'foobar' ], ], $user ) ); -- 2.20.1