From 0bb2e95974ace91bafa6c575704ffba0d528b65f Mon Sep 17 00:00:00 2001 From: Thalia Date: Wed, 3 Jul 2019 16:40:44 +0100 Subject: [PATCH] Report more information about composite blocks in block error messages For any messages that use CompositeBlock::getPermissionsError, include details of the original blocks from which the composite block is made. If there are any database blocks, give their IDs and also explain that there may also be blocks due to IP blacklisting. If there are no database blocks, then explain that the IP must be blacklisted in multiple places. Bug: T212326 Change-Id: Id6ad0019f8add4d5e000da5e872338e87cca485e --- includes/block/CompositeBlock.php | 28 +++++++++ includes/block/SystemBlock.php | 3 +- languages/i18n/en.json | 4 +- languages/i18n/qqq.json | 4 +- .../includes/block/CompositeBlockTest.php | 60 +++++++++++++++++++ 5 files changed, 96 insertions(+), 3 deletions(-) diff --git a/includes/block/CompositeBlock.php b/includes/block/CompositeBlock.php index e0f69dda8a..45a630149c 100644 --- a/includes/block/CompositeBlock.php +++ b/includes/block/CompositeBlock.php @@ -120,12 +120,40 @@ class CompositeBlock extends AbstractBlock { return $maxExpiry; } + /** + * Get the IDs for the original blocks, ignoring any that are null + * + * @return int[] + */ + protected function getIds() { + $ids = []; + foreach ( $this->originalBlocks as $block ) { + $id = $block->getId(); + if ( $id !== null ) { + $ids[] = $id; + } + } + return $ids; + } + /** * @inheritDoc */ public function getPermissionsError( IContextSource $context ) { $params = $this->getBlockErrorParams( $context ); + $ids = implode( ', ', array_map( function ( $id ) { + return '#' . $id; + }, $this->getIds() ) ); + if ( $ids === '' ) { + $idsMsg = $context->msg( 'blockedtext-composite-no-ids' )->plain(); + } else { + $idsMsg = $context->msg( 'blockedtext-composite-ids', [ $ids ] )->plain(); + } + + // TODO: Clean up error messages params so we don't have to do this (T227174) + $params[ 4 ] = $idsMsg; + $msg = 'blockedtext-composite'; array_unshift( $params, $msg ); diff --git a/includes/block/SystemBlock.php b/includes/block/SystemBlock.php index 029e0b8741..0cbf12508b 100644 --- a/includes/block/SystemBlock.php +++ b/includes/block/SystemBlock.php @@ -76,7 +76,8 @@ class SystemBlock extends AbstractBlock { */ public function getPermissionsError( IContextSource $context ) { $params = $this->getBlockErrorParams( $context ); - // TODO: Clean up error messages params so we don't have to do this + + // TODO: Clean up error messages params so we don't have to do this (T227174) $params[ 4 ] = $this->getSystemBlockType(); $msg = 'systemblockedtext'; diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 0da5c5fc45..06c9e8b875 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -678,7 +678,9 @@ "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 \"{{int:emailuser}}\" 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", - "blockedtext-composite": "Your username or IP address has been blocked.\n\nThe reason given is:\n\n:$2.\n\n* Start of block: $8\n* Expiration of longest block: $6\n\nYour current IP address is $3.\nPlease include all above details in any queries you make.", + "blockedtext-composite": "Your username or IP address has been blocked.\n\nThe reason given is:\n\n:$2.\n\n* Start of block: $8\n* Expiration of longest block: $6\n\n* $5\n\nYour current IP address is $3.\nPlease include all above details in any queries you make.", + "blockedtext-composite-ids": "Relevant block IDs: $1 (your IP address may also be blacklisted)", + "blockedtext-composite-no-ids": "Your IP address appears in multiple blacklists", "blockedtext-composite-reason": "There are multiple blocks against your account and/or IP address", "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 6ba308b1ce..b0db8d1a32 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -887,7 +887,9 @@ "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|notext=1}}\n* {{msg-mw|Systemblockedtext|notext=1}}", "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|notext=1}}\n* {{msg-mw|Autoblockedtext|notext=1}}", "blockednoreason": "Substituted with $2 in the following message if the reason is not given:\n* {{msg-mw|cantcreateaccount-text}}.\n{{Identical|No reason given}}", - "blockedtext-composite": "Text displayed to requests blocked by more than one block.\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 - (Unused) placeholder for the block ID.\n* $6 - the expiry of the block with the longest duration\n* $7 - (Unused) the intended target of the block\n* $8 - the timestamp when the block started\nSee also:\n* {{msg-mw|Systemblockedtext|notext=1}}", + "blockedtext-composite": "Text displayed to requests blocked by more than one block.\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 - details of the individual blocks that this block is made from\n* $6 - the expiry of the block with the longest duration\n* $7 - (Unused) the intended target of the block\n* $8 - the timestamp when the block started\nSee also:\n* {{msg-mw|Systemblockedtext|notext=1}}", + "blockedtext-composite-ids": "Text displayed when a user is blocked by multiple blocks, if at least one block comes from the database.\n\nParameters:\n* $1 - IDs of the blocks from the database", + "blockedtext-composite-no-ids": "Text displayed when a user is blocked by multiple blocks, if all the blocks are due to the IP being blacklisted.", "blockedtext-composite-reason": "Reason given to blocked users who are affected by more than one block.\n\nSee also:\n* {{msg-mw|blockedtext-composite}}", "whitelistedittext": "Used as error message. Parameters:\n* $1 - a link to [[Special:UserLogin]] with {{msg-mw|loginreqlink}} as link description\n* $2 - an URL to the same\n\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/block/CompositeBlockTest.php b/tests/phpunit/includes/block/CompositeBlockTest.php index 5cd86b80b4..fe8cee76a7 100644 --- a/tests/phpunit/includes/block/CompositeBlockTest.php +++ b/tests/phpunit/includes/block/CompositeBlockTest.php @@ -243,6 +243,66 @@ class CompositeBlockTest extends MediaWikiLangTestCase { ]; } + /** + * @covers ::getPermissionsError + * @dataProvider provideGetPermissionsError + */ + public function testGetPermissionsError( $ids, $expectedIdsMsg ) { + // Some block options + $timestamp = time(); + $target = '1.2.3.4'; + $byText = 'MediaWiki default'; + $formattedByText = "\u{202A}{$byText}\u{202C}"; + $reason = ''; + $expiry = 'infinite'; + + $block = $this->getMockBuilder( CompositeBlock::class ) + ->setMethods( [ 'getIds', 'getBlockErrorParams' ] ) + ->getMock(); + $block->method( 'getIds' ) + ->willReturn( $ids ); + $block->method( 'getBlockErrorParams' ) + ->willReturn( [ + $formattedByText, + $reason, + $target, + $formattedByText, + null, + $timestamp, + $target, + $expiry, + ] ); + + $this->assertSame( [ + 'blockedtext-composite', + $formattedByText, + $reason, + $target, + $formattedByText, + $expectedIdsMsg, + $timestamp, + $target, + $expiry, + ], $block->getPermissionsError( RequestContext::getMain() ) ); + } + + public static function provideGetPermissionsError() { + return [ + 'All original blocks are system blocks' => [ + [], + 'Your IP address appears in multiple blacklists', + ], + 'One original block is a database block' => [ + [ 100 ], + 'Relevant block IDs: #100 (your IP address may also be blacklisted)', + ], + 'Several original blocks are database blocks' => [ + [ 100, 101, 102 ], + 'Relevant block IDs: #100, #101, #102 (your IP address may also be blacklisted)', + ], + ]; + } + /** * Get an instance of BlockRestrictionStore * -- 2.20.1