jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/332737 )
Change subject: Remove the localStorage replication of the block cookie ...................................................................... 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 --- M includes/Block.php M includes/EditPage.php M includes/user/User.php M resources/Resources.php D resources/src/mediawiki/mediawiki.user.blockcookie.js 5 files changed, 17 insertions(+), 42 deletions(-) Approvals: Krinkle: Looks good to me, but someone else must approve jenkins-bot: Verified Kaldari: Looks good to me, approved diff --git a/includes/Block.php b/includes/Block.php index 1f4041b..cf6642a 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -1450,13 +1450,9 @@ * 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,13 +1463,22 @@ } // 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 * be the block ID. diff --git a/includes/EditPage.php b/includes/EditPage.php index c22125a..7a58326 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -2318,12 +2318,9 @@ } 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 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -1745,11 +1745,12 @@ $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 --- a/resources/Resources.php +++ b/resources/Resources.php @@ -1352,11 +1352,6 @@ '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 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 ) ); -- To view, visit https://gerrit.wikimedia.org/r/332737 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ifb06dc2390f4d648d7fcb39e30267de5eddc6941 Gerrit-PatchSet: 17 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Samwilson <s...@samwilson.id.au> Gerrit-Reviewer: Jack Phoenix <j...@countervandalism.net> Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com> Gerrit-Reviewer: Kaldari <rkald...@wikimedia.org> Gerrit-Reviewer: Krinkle <krinklem...@gmail.com> Gerrit-Reviewer: MusikAnimal <musikani...@wikimedia.org> Gerrit-Reviewer: Niharika29 <nko...@wikimedia.org> Gerrit-Reviewer: Samwilson <s...@samwilson.id.au> Gerrit-Reviewer: Tpt <thoma...@hotmail.fr> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits