From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Mon, 1 Feb 2016 21:59:27 +0000 (-0500)
Subject: Introduce User::INVALID_TOKEN
X-Git-Tag: 1.31.0-rc.0~8063^2
X-Git-Url: http://git.cyclocoop.org//%27%40script%40/%27?a=commitdiff_plain;h=30a9eae82177368b6246acc65b5331e81d4605ae;p=lhc%2Fweb%2Fwiklou.git

Introduce User::INVALID_TOKEN

To avoid having to have SessionManager try to reset sessions on every
request, we set the user_token to a special value. When that value is
present, User::getToken() returns a different value every time (so
existing checks will fail) and User::setToken() refuses to alter it.

Bug: T124414
Change-Id: Ie4c84ce993e40a081288cf5a543f8ba99f98806a
---

diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php
index 4b38a5c318..c1bbc8fa52 100644
--- a/includes/session/SessionManager.php
+++ b/includes/session/SessionManager.php
@@ -536,13 +536,6 @@ final class SessionManager implements SessionManagerInterface {
 	public function preventSessionsForUser( $username ) {
 		$this->preventUsers[$username] = true;
 
-		// Reset the user's token to kill existing sessions
-		$user = User::newFromName( $username );
-		if ( $user && $user->getToken( false ) ) {
-			$user->setToken();
-			$user->saveSettings();
-		}
-
 		// Instruct the session providers to kill any other sessions too.
 		foreach ( $this->getProviders() as $provider ) {
 			$provider->preventSessionsForUser( $username );
diff --git a/includes/user/User.php b/includes/user/User.php
index 0fa7d59f7b..be528b68e8 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -45,6 +45,11 @@ class User implements IDBAccessObject {
 	 */
 	const TOKEN_LENGTH = 32;
 
+	/**
+	 * @const string An invalid value for user_token
+	 */
+	const INVALID_TOKEN = '*** INVALID ***';
+
 	/**
 	 * Global constant made accessible as class constants so that autoloader
 	 * magic can be used.
@@ -641,7 +646,8 @@ class User implements IDBAccessObject {
 		$user = self::newFromRow( $row );
 
 		// A user is considered to exist as a non-system user if it has a
-		// password set, or a temporary password set, or an email set.
+		// password set, or a temporary password set, or an email set, or a
+		// non-invalid token.
 		$passwordFactory = new PasswordFactory();
 		$passwordFactory->init( RequestContext::getMain()->getConfig() );
 		try {
@@ -657,7 +663,7 @@ class User implements IDBAccessObject {
 			$newpassword = PasswordFactory::newInvalidPassword();
 		}
 		if ( !$password instanceof InvalidPassword || !$newpassword instanceof InvalidPassword
-			|| $user->mEmail
+			|| $user->mEmail || $user->mToken !== self::INVALID_TOKEN
 		) {
 			// User exists. Steal it?
 			if ( !$options['steal'] ) {
@@ -677,11 +683,11 @@ class User implements IDBAccessObject {
 				__METHOD__
 			);
 			$user->invalidateEmail();
+			$user->mToken = self::INVALID_TOKEN;
 			$user->saveSettings();
+			SessionManager::singleton()->preventSessionsForUser( $user->getName() );
 		}
 
-		SessionManager::singleton()->preventSessionsForUser( $user->getName() );
-
 		return $user;
 	}
 
@@ -2439,13 +2445,18 @@ class User implements IDBAccessObject {
 			$this->setToken();
 		}
 
-		// If the user doesn't have a token, return null to indicate that.
-		// Otherwise, hmac the version with the secret if we have a version.
 		if ( !$this->mToken ) {
+			// The user doesn't have a token, return null to indicate that.
 			return null;
+		} elseif ( $this->mToken === self::INVALID_TOKEN ) {
+			// We return a random value here so existing token checks are very
+			// likely to fail.
+			return MWCryptRand::generateHex( self::TOKEN_LENGTH );
 		} elseif ( $wgAuthenticationTokenVersion === null ) {
+			// $wgAuthenticationTokenVersion not in use, so return the raw secret
 			return $this->mToken;
 		} else {
+			// $wgAuthenticationTokenVersion in use, so hmac it.
 			$ret = MWCryptHash::hmac( $wgAuthenticationTokenVersion, $this->mToken, false );
 
 			// The raw hash can be overly long. Shorten it up.
@@ -2466,7 +2477,10 @@ class User implements IDBAccessObject {
 	 */
 	public function setToken( $token = false ) {
 		$this->load();
-		if ( !$token ) {
+		if ( $this->mToken === self::INVALID_TOKEN ) {
+			\MediaWiki\Logger\LoggerFactory::getInstance( 'session' )
+				->debug( __METHOD__ . ": Ignoring attempt to set token for system user \"$this\"" );
+		} elseif ( !$token ) {
 			$this->mToken = MWCryptRand::generateHex( self::TOKEN_LENGTH );
 		} else {
 			$this->mToken = $token;
diff --git a/tests/phpunit/includes/session/SessionManagerTest.php b/tests/phpunit/includes/session/SessionManagerTest.php
index 4fde3410f9..7277a1cd10 100644
--- a/tests/phpunit/includes/session/SessionManagerTest.php
+++ b/tests/phpunit/includes/session/SessionManagerTest.php
@@ -1067,12 +1067,8 @@ class SessionManagerTest extends MediaWikiTestCase {
 			$this->objectCacheDef( $provider1 ),
 		) );
 
-		$user = User::newFromName( 'UTSysop' );
-		$token = $user->getToken( true );
-
 		$this->assertFalse( $manager->isUserSessionPrevented( 'UTSysop' ) );
 		$manager->preventSessionsForUser( 'UTSysop' );
-		$this->assertNotEquals( $token, User::newFromName( 'UTSysop' )->getToken() );
 		$this->assertTrue( $manager->isUserSessionPrevented( 'UTSysop' ) );
 	}