Send a cookie with IP/IP-Range blocks when blocking logged-out users

A cookie will be set when ip users try to edit and their IP has been
blocked or if they try to create an account and the block prevents
account creation

This feature is disabled by default and can be enabled by
setting the new $wgCookieSetOnIpBlock config variable to true.

Note: this is meant to discourage vandals that try to avoid blocks by
switching their ip address while editing anonymously.

Bug: T152462
Change-Id: I0b78a5e174bcd882edea39e868a08f9a347f5aba

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index f1efd4f..75f605a 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -21,6 +21,9 @@ production.
   adds a defense-in-depth feature to stop an attacker who has found a bug in
   the parser allowing them to insert malicious attributes. Disabled by default,
   you can configure this via $wgCSPHeader and $wgCSPReportOnlyHeader.
+* New configuration variable has been added: $wgCookieSetOnIpBlock.
+  This determines whether to set a cookie when an IP user is blocked. Doing so means
+  that a blocked user, even after moving to a new IP address, will still be blocked.
 
 === New features in 1.32 ===
 * (T112474) Generalized the ResourceLoader mechanism for overriding modules
@@ -28,6 +31,8 @@ production.
 * Added 'ApiParseMakeOutputPage' hook.
 * (T174313) Added checkbox on Special:ListUsers to display only users in
   temporary user groups.
+* (T152462) A cookie can now be set when an IP user is blocked to track that user if
+  they move to a new IP address. This is disabled by default.
 
 === External library changes in 1.32 ===
 * …

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 87ca016..e529d2d 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -6043,6 +6043,15 @@ $wgSessionName = false;
  */
 $wgCookieSetOnAutoblock = false;
 
+/**
+ * Whether to set a cookie when a logged-out user is blocked. Doing so means that a blocked user,
+ * even after moving to a new IP address, will still be blocked. This cookie will contain an
+ * authentication code if $wgSecretKey is set, or otherwise will just be the block ID (in which
+ * case there is a possibility of an attacker discovering the names of revdeleted users, so it
+ * is best to use this in conjunction with $wgSecretKey being set).
+ */
+$wgCookieSetOnIpBlock = false;
+
 /** @} */ # end of cookie settings }
 
 /************************************************************************//**

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 67ce1f3..0026ba8 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -587,6 +587,10 @@ class EditPage {
                $permErrors = $this->getEditPermissionErrors( $this->save ? 'secure' : 'full' );
                if ( $permErrors ) {
                        wfDebug( __METHOD__ . ": User can't edit\n" );
+
+                       // track block with a cookie if it doesn't exists already
+                       $this->context->getUser()->trackBlockWithCookie();
+
                        // Auto-block user's IP if the account was "hard" blocked
                        if ( !wfReadOnly() ) {
                                DeferredUpdates::addCallableUpdate( function () {

diff --git a/includes/specials/SpecialCreateAccount.php b/includes/specials/SpecialCreateAccount.php
index 73beafc..2f87c47 100644 (file)
--- a/includes/specials/SpecialCreateAccount.php
+++ b/includes/specials/SpecialCreateAccount.php
@@ -63,6 +63,10 @@ class SpecialCreateAccount extends LoginSignupSpecialPage {
                $user = $this->getUser();
                $status = AuthManager::singleton()->checkAccountCreatePermissions( $user );
                if ( !$status->isGood() ) {
+                       // track block with a cookie if it doesn't exists already
+                       if ( $user->isBlockedFromCreateAccount() ) {
+                               $user->trackBlockWithCookie();
+                       }
                        throw new ErrorPageError( 'createacct-error', $status->getMessage() );
                }
        }

diff --git a/includes/user/User.php b/includes/user/User.php
index 7189bea..b5fa97f 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -1345,32 +1345,54 @@ class User implements IDBAccessObject, UserIdentity {
                $user = $session->getUser();
                if ( $user->isLoggedIn() ) {
                        $this->loadFromUserObject( $user );
-
-                       // If this user is autoblocked, set a cookie to track the Block. This has to be done on
-                       // every session load, because an autoblocked editor might not edit again from the same
-                       // IP address after being blocked.
-                       $config = RequestContext::getMain()->getConfig();
-                       if ( $config->get( 'CookieSetOnAutoblock' ) === true ) {
-                               $block = $this->getBlock();
-                               $shouldSetCookie = $this->getRequest()->getCookie( 'BlockID' ) === null
-                                       && $block
-                                       && $block->getType() === Block::TYPE_USER
-                                       && $block->isAutoblocking();
-                               if ( $shouldSetCookie ) {
-                                       wfDebug( __METHOD__ . ': User is autoblocked, setting cookie to track' );
-                                       $block->setCookie( $this->getRequest()->response() );
-                               }
+                       if ( $user->isBlocked() ) {
+                               // If this user is autoblocked, set a cookie to track the Block. This has to be done on
+                               // every session load, because an autoblocked editor might not edit again from the same
+                               // IP address after being blocked.
+                               $this->trackBlockWithCookie();
                        }
 
                        // Other code expects these to be set in the session, so set them.
                        $session->set( 'wsUserID', $this->getId() );
                        $session->set( 'wsUserName', $this->getName() );
                        $session->set( 'wsToken', $this->getToken() );
+
                        return true;
                }
+
                return false;
        }
 
+       /**
+        * Set the 'BlockID' cookie depending on block type and user authentication status.
+        */
+       public function trackBlockWithCookie() {
+               $block = $this->getBlock();
+               if ( $block && $this->getRequest()->getCookie( 'BlockID' ) === null ) {
+                       $config = RequestContext::getMain()->getConfig();
+                       $shouldSetCookie = false;
+
+                       if ( $this->isAnon() && $config->get( 'CookieSetOnIpBlock' ) ) {
+                               // If user is logged-out, set a cookie to track the Block
+                               $shouldSetCookie = in_array( $block->getType(), [
+                                       Block::TYPE_IP, Block::TYPE_RANGE
+                               ] );
+                               if ( $shouldSetCookie ) {
+                                       $block->setCookie( $this->getRequest()->response() );
+
+                                       // temporary measure the use of cookies on ip blocks
+                                       $stats = MediaWikiServices::getInstance()->getStatsdDataFactory();
+                                       $stats->increment( 'block.ipblock.setCookie.success' );
+                               }
+                       } elseif ( $this->isLoggedIn() && $config->get( 'CookieSetOnAutoblock' ) ) {
+                               $shouldSetCookie = $block->getType() === Block::TYPE_USER && $block->isAutoblocking();
+                               if ( $shouldSetCookie ) {
+                                       $block->setCookie( $this->getRequest()->response() );
+                               }
+                       }
+               }
+       }
+
        /**
         * Load user and user_group data from the database.
         * $this->mId must be set, this is how the user is identified.
@@ -1896,12 +1918,24 @@ class User implements IDBAccessObject, UserIdentity {
                        // An ID was found in the cookie.
                        $tmpBlock = Block::newFromID( $blockCookieId );
                        if ( $tmpBlock instanceof Block ) {
-                               // Check the validity of the block.
-                               $blockIsValid = $tmpBlock->getType() == Block::TYPE_USER
-                                       && !$tmpBlock->isExpired()
-                                       && $tmpBlock->isAutoblocking();
                                $config = RequestContext::getMain()->getConfig();
-                               $useBlockCookie = ( $config->get( 'CookieSetOnAutoblock' ) === true );
+
+                               switch ( $tmpBlock->getType() ) {
+                                       case Block::TYPE_USER:
+                                               $blockIsValid = !$tmpBlock->isExpired() && $tmpBlock->isAutoblocking();
+                                               $useBlockCookie = ( $config->get( 'CookieSetOnAutoblock' ) === true );
+                                               break;
+                                       case Block::TYPE_IP:
+                                       case Block::TYPE_RANGE:
+                                               // If block is type IP or IP range, load only if user is not logged in (T152462)
+                                               $blockIsValid = !$tmpBlock->isExpired() && !$this->isLoggedIn();
+                                               $useBlockCookie = ( $config->get( 'CookieSetOnIpBlock' ) === true );
+                                               break;
+                                       default:
+                                               $blockIsValid = false;
+                                               $useBlockCookie = false;
+                               }
+
                                if ( $blockIsValid && $useBlockCookie ) {
                                        // Use the block.
                                        return $tmpBlock;

diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index e819d35..3eb6abd 100644 (file)
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -1199,4 +1199,113 @@ class UserTest extends MediaWikiTestCase {
                $this->assertFalse( $user->isBlockedFrom( $ut ) );
        }
 
+       /**
+        * Block cookie should be set for IP Blocks if
+        * wgCookieSetOnIpBlock is set to true
+        */
+       public function testIpBlockCookieSet() {
+               $this->setMwGlobals( [
+                       'wgCookieSetOnIpBlock' => true,
+                       'wgCookiePrefix' => 'wiki',
+                       'wgSecretKey' => MWCryptRand::generateHex( 64, true ),
+               ] );
+
+               // setup block
+               $block = new Block( [
+                       'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 5 * 60 * 60 ) ),
+               ] );
+               $block->setTarget( '1.2.3.4' );
+               $block->setBlocker( $this->getTestSysop()->getUser() );
+               $block->insert();
+
+               // setup request
+               $request = new FauxRequest();
+               $request->setIP( '1.2.3.4' );
+
+               // get user
+               $user = User::newFromSession( $request );
+               $user->trackBlockWithCookie();
+
+               // test cookie was set
+               $cookies = $request->response()->getCookies();
+               $this->assertArrayHasKey( 'wikiBlockID', $cookies );
+
+               // clean up
+               $block->delete();
+       }
+
+       /**
+        * Block cookie should NOT be set when wgCookieSetOnIpBlock
+        * is disabled
+        */
+       public function testIpBlockCookieNotSet() {
+               $this->setMwGlobals( [
+                       'wgCookieSetOnIpBlock' => false,
+                       'wgCookiePrefix' => 'wiki',
+                       'wgSecretKey' => MWCryptRand::generateHex( 64, true ),
+               ] );
+
+               // setup block
+               $block = new Block( [
+                       'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 5 * 60 * 60 ) ),
+               ] );
+               $block->setTarget( '1.2.3.4' );
+               $block->setBlocker( $this->getTestSysop()->getUser() );
+               $block->insert();
+
+               // setup request
+               $request = new FauxRequest();
+               $request->setIP( '1.2.3.4' );
+
+               // get user
+               $user = User::newFromSession( $request );
+               $user->trackBlockWithCookie();
+
+               // test cookie was not set
+               $cookies = $request->response()->getCookies();
+               $this->assertArrayNotHasKey( 'wikiBlockID', $cookies );
+
+               // clean up
+               $block->delete();
+       }
+
+       /**
+        * When an ip user is blocked and then they log in, cookie block
+        * should be invalid and the cookie removed.
+        */
+       public function testIpBlockCookieIgnoredWhenUserLoggedIn() {
+               $this->setMwGlobals( [
+                       'wgAutoblockExpiry' => 8000,
+                       'wgCookieSetOnIpBlock' => true,
+                       'wgCookiePrefix' => 'wiki',
+                       'wgSecretKey' => MWCryptRand::generateHex( 64, true ),
+               ] );
+
+               // setup block
+               $block = new Block( [
+                       'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 40 * 60 * 60 ) ),
+               ] );
+               $block->setTarget( '1.2.3.4' );
+               $block->setBlocker( $this->getTestSysop()->getUser() );
+               $block->insert();
+
+               // setup request
+               $request = new FauxRequest();
+               $request->setIP( '1.2.3.4' );
+               $request->getSession()->setUser( $this->getTestUser()->getUser() );
+               $request->setCookie( 'BlockID', $block->getCookieValue() );
+
+               // setup user
+               $user = User::newFromSession( $request );
+
+               // logged in users should be inmune to cookie block of type ip/range
+               $this->assertFalse( $user->isBlocked() );
+
+               // cookie is being cleared
+               $cookies = $request->response()->getCookies();
+               $this->assertEquals( '', $cookies['wikiBlockID']['value'] );
+
+               // clean up
+               $block->delete();
+       }
 }