Remove the localStorage replication of the block cookie

The block cookie was being replicated to localStorage in an attempt
to make it harder for users to get around the block by deleting the
cookie (and changing IP addresses).

This whole setup was hard to test, had a few bugs (e.g. the localStorage
value would never expire), and given that it is a minor improvement
over just a plain cookie, it is now being removed. The cookie is only
intended to stop casual block-evaders (other users will get around it
by deleting the cookie or using incognito mode) and so it is not felt
worth having the extra complexity that will only guard against people
who know to remove cookies, not use incognito mode, and yet don't know
to remove localStorage.

Bug: T152952
Change-Id: Ifb06dc2390f4d648d7fcb39e30267de5eddc6941

diff --git a/includes/Block.php b/includes/Block.php
index 1f4041b..cf6642a 100644 (file)
--- a/includes/Block.php
+++ b/includes/Block.php
@@ -1450,13 +1450,9 @@ class Block {
         * Set the 'BlockID' cookie to this block's ID and expiry time. The cookie's expiry will be
         * the same as the block's, to a maximum of 24 hours.
         *
-        * An empty value can also be set, in order to retain the cookie but remove the block ID
-        * (e.g. as used in User::getBlockedStatus).
-        *
         * @param WebResponse $response The response on which to set the cookie.
-        * @param boolean $setEmpty Whether to set the cookie's value to the empty string.
         */
-       public function setCookie( WebResponse $response, $setEmpty = false ) {
+       public function setCookie( WebResponse $response ) {
                // Calculate the default expiry time.
                $maxExpiryTime = wfTimestamp( TS_MW, wfTimestamp() + ( 24 * 60 * 60 ) );
 
@@ -1467,12 +1463,21 @@ class Block {
                }
 
                // Set the cookie. Reformat the MediaWiki datetime as a Unix timestamp for the cookie.
-               $cookieValue = $setEmpty ? '' : $this->getCookieValue();
                $expiryValue = DateTime::createFromFormat( 'YmdHis', $expiryTime )->format( 'U' );
                $cookieOptions = [ 'httpOnly' => false ];
+               $cookieValue = $this->getCookieValue();
                $response->setCookie( 'BlockID', $cookieValue, $expiryValue, $cookieOptions );
        }
 
+       /**
+        * Unset the 'BlockID' cookie.
+        *
+        * @param WebResponse $response The response on which to unset the cookie.
+        */
+       public static function clearCookie( WebResponse $response ) {
+               $response->clearCookie( 'BlockID', [ 'httpOnly' => false ] );
+       }
+
        /**
         * Get the BlockID cookie's value for this block. This is usually the block ID concatenated
         * with an HMAC in order to avoid spoofing (T152951), but if wgSecretKey is not set will just

diff --git a/includes/EditPage.php b/includes/EditPage.php
index c22125a..7a58326 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -2318,12 +2318,9 @@ class EditPage {
        }
 
        public function setHeaders() {
-               global $wgOut, $wgUser, $wgAjaxEditStash, $wgCookieSetOnAutoblock;
+               global $wgOut, $wgUser, $wgAjaxEditStash;
 
                $wgOut->addModules( 'mediawiki.action.edit' );
-               if ( $wgCookieSetOnAutoblock === true ) {
-                       $wgOut->addModules( 'mediawiki.user.blockcookie' );
-               }
                $wgOut->addModuleStyles( 'mediawiki.action.edit.styles' );
 
                if ( $wgUser->getOption( 'showtoolbar' ) ) {

diff --git a/includes/user/User.php b/includes/user/User.php
index 0acdb55..b3c6f96 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -1745,11 +1745,12 @@ class User implements IDBAccessObject {
                                        $this->blockTrigger = 'cookie-block';
                                        return $tmpBlock;
                                } else {
-                                       // If the block is not valid, clear the block cookie (but don't delete it,
-                                       // because it needs to be cleared from LocalStorage as well and an empty string
-                                       // value is checked for in the mediawiki.user.blockcookie module).
-                                       $tmpBlock->setCookie( $this->getRequest()->response(), true );
+                                       // If the block is not valid, remove the cookie.
+                                       Block::clearCookie( $this->getRequest()->response() );
                                }
+                       } else {
+                               // If the block doesn't exist, remove the cookie.
+                               Block::clearCookie( $this->getRequest()->response() );
                        }
                }
                return false;

diff --git a/resources/Resources.php b/resources/Resources.php
index 6d08c44..11925b0 100644 (file)
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -1352,11 +1352,6 @@ return [
                'dependencies' => 'mediawiki.util',
                'targets' => [ 'desktop', 'mobile' ],
        ],
-       'mediawiki.user.blockcookie' => [
-               'scripts' => 'resources/src/mediawiki/mediawiki.user.blockcookie.js',
-               'dependencies' => [ 'mediawiki.cookie', 'mediawiki.storage' ],
-               'targets' => [ 'desktop', 'mobile' ],
-       ],
        'mediawiki.user' => [
                'scripts' => 'resources/src/mediawiki/mediawiki.user.js',
                'dependencies' => [

diff --git a/resources/src/mediawiki/mediawiki.user.blockcookie.js b/resources/src/mediawiki/mediawiki.user.blockcookie.js
deleted file mode 100644 (file)
index ffff039..0000000
--- a/resources/src/mediawiki/mediawiki.user.blockcookie.js
+++ /dev/null
@@ -1,23 +0,0 @@
-( function ( mw ) {
-
-       // If a user has been autoblocked, a cookie is set.
-       // Its value is replicated here in localStorage to guard against cookie-removal.
-       // This module will only be loaded when $wgCookieSetOnAutoblock is true.
-       // Ref: https://phabricator.wikimedia.org/T5233
-
-       if ( !mw.cookie.get( 'BlockID' ) && mw.storage.get( 'blockID' ) ) {
-               // The block ID exists in storage, but not in the cookie.
-               mw.cookie.set( 'BlockID', mw.storage.get( 'blockID' ) );
-
-       } else if ( parseInt( mw.cookie.get( 'BlockID' ), 10 ) > 0 && !mw.storage.get( 'blockID' ) ) {
-               // The block ID exists in the cookie, but not in storage.
-               // (When a block expires the cookie remains but its value is '', hence the integer check above.)
-               mw.storage.set( 'blockID', mw.cookie.get( 'BlockID' ) );
-
-       } else if ( mw.cookie.get( 'BlockID' ) === '' && mw.storage.get( 'blockID' ) ) {
-               // If only the empty string is in the cookie, remove the storage value. The block is no longer valid.
-               mw.storage.remove( 'blockID' );
-
-       }
-
-}( mediaWiki ) );