API: Spread autoblocks from action=edit and action=move
authorBrad Jorsch <bjorsch@wikimedia.org>
Mon, 18 Feb 2019 19:46:05 +0000 (14:46 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Tue, 19 Feb 2019 22:34:48 +0000 (17:34 -0500)
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
includes/api/ApiBase.php
includes/api/ApiEditPage.php
includes/api/ApiMove.php
tests/phpunit/includes/api/ApiEditPageTest.php
tests/phpunit/includes/api/ApiMoveTest.php

index cca2c5d..3141561 100644 (file)
@@ -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
index 1d209fd..dfaff8b 100644 (file)
@@ -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 ) );
                }
        }
index 5e5efa5..8131ea5 100644 (file)
@@ -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'];
index f6b6b35..cc4490e 100644 (file)
@@ -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 );
                }
 
index de0af0b..1706ad1 100644 (file)
@@ -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();
index 1e66a7d..b9c49b1 100644 (file)
@@ -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() {