From c3ee8fa1f8ed66665a15bca83877c6a4d0fe2456 Mon Sep 17 00:00:00 2001 From: David Barratt Date: Tue, 8 Jan 2019 16:14:48 -0500 Subject: [PATCH] Ensure action object matches passed in right in Title::checkUserBlock() There is a possibility that the retrieved action (by name) will not match the passed in right, we should ensure that the retrieved action matches. Also adds 'purge' to the whitelist of actions that are not prevented by a block. Bug: T213220 Change-Id: I5671dcd6f004b1663c8a5c5aec200abbf742ec1d --- includes/Block.php | 3 +++ includes/Title.php | 13 ++++++++----- tests/phpunit/includes/BlockTest.php | 8 ++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/includes/Block.php b/includes/Block.php index db5fec3d36..85fa3415ce 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -1186,6 +1186,9 @@ class Block { case 'read': $res = false; break; + case 'purge': + $res = false; + break; } if ( !$res && $blockDisablesLogin ) { // If a block would disable login, then it should diff --git a/includes/Title.php b/includes/Title.php index f5904e2a05..9112855497 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -2703,10 +2703,11 @@ class Title implements LinkTarget, IDBAccessObject { } // Determine if the user is blocked from this action on this page. - // What gets passed into this method is a user right, not an action nmae. + // What gets passed into this method is a user right, not an action name. // There is no way to instantiate an action by restriction. However, this // will get the action where the restriction is the same. This may result // in actions being blocked that shouldn't be. + $actionObj = null; if ( Action::exists( $action ) ) { // Clone the title to prevent mutations to this object which is done // by Title::loadFromRow() in WikiPage::loadFromRow(). @@ -2714,14 +2715,16 @@ class Title implements LinkTarget, IDBAccessObject { // Creating an action will perform several database queries to ensure that // the action has not been overridden by the content type. // @todo FIXME: Pass the relevant context into this function. - $action = Action::factory( $action, $page, RequestContext::getMain() ); - } else { - $action = null; + $actionObj = Action::factory( $action, $page, RequestContext::getMain() ); + // Ensure that the retrieved action matches the restriction. + if ( $actionObj && $actionObj->getRestriction() !== $action ) { + $actionObj = null; + } } // If no action object is returned, assume that the action requires unblock // which is the default. - if ( !$action || $action->requiresUnblock() ) { + if ( !$actionObj || $actionObj->requiresUnblock() ) { if ( $user->isBlockedFrom( $this, $useReplica ) ) { // @todo FIXME: Pass the relevant context into this function. $errors[] = $block diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php index e4dce12801..a1510809ac 100644 --- a/tests/phpunit/includes/BlockTest.php +++ b/tests/phpunit/includes/BlockTest.php @@ -745,4 +745,12 @@ class BlockTest extends MediaWikiLangTestCase { $block->delete(); } + /** + * @covers Block::prevents + */ + public function testBlockAllowsPurge() { + $block = new Block(); + $this->assertFalse( $block->prevents( 'purge' ) ); + } + } -- 2.20.1