From c073e531cf7a7fe055f002148e97159006b75c22 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Mon, 18 Feb 2019 14:46:05 -0500 Subject: [PATCH] API: Spread autoblocks from action=edit and action=move The code in EditPage and SpecialMovepage does this primarily in web UI code paths that aren't called by the API. EditPage also has a check in the internal code path used by the API, but ApiEditPage runs its own permissions check first and won't reach that code path. Bug: T216245 Change-Id: I6263c8b60a24f3195dba583463f1ce4b004f82f5 --- RELEASE-NOTES-1.33 | 6 ++++ includes/api/ApiBase.php | 18 +++++++--- includes/api/ApiEditPage.php | 3 +- includes/api/ApiMove.php | 1 + .../phpunit/includes/api/ApiEditPageTest.php | 8 +++-- tests/phpunit/includes/api/ApiMoveTest.php | 33 +++++++++++++++++++ 6 files changed, 62 insertions(+), 7 deletions(-) diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index cca2c5dfc3..3141561dfa 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -97,6 +97,7 @@ production. * action=setnotificationtimestamp will now update the watchlist asynchronously if entirewatchlist is set, so updates may not be visible immediately * Block info will be added to "blocked" errors from more modules. +* (T216245) Autoblocks will now be spread by action=edit and action=move. === Action API internal changes in 1.33 === * A number of deprecated methods for API documentation, intended for overriding @@ -113,6 +114,8 @@ production. hyphen. Methods such as ApiBase::dieWithError() and ApiMessageTrait::setApiCode() will throw an InvalidArgumentException if passed a bad code. +* ApiBase::checkTitleUserPermissions() now takes an options array as its third + parameter. Passing a User object or null is deprecated. === Languages updated in 1.33 === MediaWiki supports over 350 languages. Many localisations are updated regularly. @@ -299,6 +302,9 @@ because of Phabricator reports. Use require( 'mediawiki.language.specialCharacters' ) instead. * ChangeTags::purgeTagUsageCache() has been deprecated, and is expected to be removed in a future release. +* Passing a User object or null as the third parameter to + ApiBase::checkTitleUserPermissions() has been deprecated. Pass an array + [ 'user' => $user ] instead. === Other changes in 1.33 === * (T201747) Html::openElement() warns if given an element name with a space diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 1d209fdb09..dfaff8b76e 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -2112,15 +2112,21 @@ abstract class ApiBase extends ContextSource { /** * Helper function for permission-denied errors * @since 1.29 + * @since 1.33 Changed the third parameter from $user to $options. * @param Title $title * @param string|string[] $actions - * @param User|null $user + * @param array $options Additional options + * - user: (User) User to use rather than $this->getUser() + * - autoblock: (bool, default false) Whether to spread autoblocks + * For compatibility, passing a User object is treated as the value for the 'user' option. * @throws ApiUsageException if the user doesn't have all of the rights. */ - public function checkTitleUserPermissions( Title $title, $actions, $user = null ) { - if ( !$user ) { - $user = $this->getUser(); + public function checkTitleUserPermissions( Title $title, $actions, $options = [] ) { + if ( !is_array( $options ) ) { + wfDeprecated( '$user as the third parameter to ' . __METHOD__, '1.33' ); + $options = [ 'user' => $options ]; } + $user = $options['user'] ?? $this->getUser(); $errors = []; foreach ( (array)$actions as $action ) { @@ -2133,6 +2139,10 @@ abstract class ApiBase extends ContextSource { $this->trackBlockNotices( $errors ); } + if ( !empty( $options['autoblock'] ) ) { + $user->spreadAnyEditBlock(); + } + $this->dieStatus( $this->errorArrayToStatus( $errors, $user ) ); } } diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php index 5e5efa5b6a..8131ea550e 100644 --- a/includes/api/ApiEditPage.php +++ b/includes/api/ApiEditPage.php @@ -116,7 +116,8 @@ class ApiEditPage extends ApiBase { // Now let's check whether we're even allowed to do this $this->checkTitleUserPermissions( $titleObj, - $titleObj->exists() ? 'edit' : [ 'edit', 'create' ] + $titleObj->exists() ? 'edit' : [ 'edit', 'create' ], + [ 'autoblock' => true ] ); $toMD5 = $params['text']; diff --git a/includes/api/ApiMove.php b/includes/api/ApiMove.php index f6b6b35df2..cc4490e63f 100644 --- a/includes/api/ApiMove.php +++ b/includes/api/ApiMove.php @@ -86,6 +86,7 @@ class ApiMove extends ApiBase { $status = $this->movePage( $fromTitle, $toTitle, $params['reason'], !$params['noredirect'], $params['tags'] ?: [] ); if ( !$status->isOK() ) { + $user->spreadAnyEditBlock(); $this->dieStatus( $status ); } diff --git a/tests/phpunit/includes/api/ApiEditPageTest.php b/tests/phpunit/includes/api/ApiEditPageTest.php index de0af0b858..1706ad1002 100644 --- a/tests/phpunit/includes/api/ApiEditPageTest.php +++ b/tests/phpunit/includes/api/ApiEditPageTest.php @@ -1474,8 +1474,7 @@ class ApiEditPageTest extends ApiTestCase { public function testEditWhileBlocked() { $name = 'Help:' . ucfirst( __FUNCTION__ ); - $this->setExpectedException( ApiUsageException::class, - 'You have been blocked from editing.' ); + $this->assertNull( Block::newFromTarget( '127.0.0.1' ), 'Sanity check' ); $block = new Block( [ 'address' => self::$users['sysop']->getUser()->getName(), @@ -1483,6 +1482,7 @@ class ApiEditPageTest extends ApiTestCase { 'reason' => 'Capriciousness', 'timestamp' => '19370101000000', 'expiry' => 'infinity', + 'enableAutoblock' => true, ] ); $block->insert(); @@ -1492,6 +1492,10 @@ class ApiEditPageTest extends ApiTestCase { 'title' => $name, 'text' => 'Some text', ] ); + $this->fail( 'Expected exception not thrown' ); + } catch ( ApiUsageException $ex ) { + $this->assertSame( 'You have been blocked from editing.', $ex->getMessage() ); + $this->assertNotNull( Block::newFromTarget( '127.0.0.1' ), 'Autoblock spread' ); } finally { $block->delete(); self::$users['sysop']->getUser()->clearInstanceCache(); diff --git a/tests/phpunit/includes/api/ApiMoveTest.php b/tests/phpunit/includes/api/ApiMoveTest.php index 1e66a7d276..b9c49b11b4 100644 --- a/tests/phpunit/includes/api/ApiMoveTest.php +++ b/tests/phpunit/includes/api/ApiMoveTest.php @@ -130,6 +130,39 @@ class ApiMoveTest extends ApiTestCase { } } + public function testMoveWhileBlocked() { + $this->assertNull( Block::newFromTarget( '127.0.0.1' ), 'Sanity check' ); + + $block = new Block( [ + 'address' => self::$users['sysop']->getUser()->getName(), + 'by' => self::$users['sysop']->getUser()->getId(), + 'reason' => 'Capriciousness', + 'timestamp' => '19370101000000', + 'expiry' => 'infinity', + 'enableAutoblock' => true, + ] ); + $block->insert(); + + $name = ucfirst( __FUNCTION__ ); + $id = $this->createPage( $name ); + + try { + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + ] ); + $this->fail( 'Expected exception not thrown' ); + } catch ( ApiUsageException $ex ) { + $this->assertSame( 'You have been blocked from editing.', $ex->getMessage() ); + $this->assertNotNull( Block::newFromTarget( '127.0.0.1' ), 'Autoblock spread' ); + } finally { + $block->delete(); + self::$users['sysop']->getUser()->clearInstanceCache(); + $this->assertSame( $id, Title::newFromText( $name )->getArticleID() ); + } + } + // @todo File moving public function testPingLimiter() { -- 2.20.1