From 305bc75b27903237a9683ec1f329bcbec0ecd266 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Thu, 28 Jan 2016 13:27:01 -0500
Subject: [PATCH] SessionManager: Don't generate user tokens when checking the
 tokens

Looking at the pre-SessionManager token checking, it's apparently valid
to log in despite user_token being empty. The stored token just gets
compared against the empty string that got returned previously.

This also cleans up some checks that assumed $user->getToken() didn't
automatically create the token if one wasn't already set.

Bug: T125114
Change-Id: Ia3d2382e96e2a0146f33fb7193a2e00ea72e51a0
---
 includes/session/SessionBackend.php             |  2 +-
 includes/session/SessionManager.php             |  2 +-
 includes/session/UserInfo.php                   |  4 ++--
 tests/phpunit/includes/session/UserInfoTest.php | 10 +++++-----
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/includes/session/SessionBackend.php b/includes/session/SessionBackend.php
index 67cbc6d388..1c743dde96 100644
--- a/includes/session/SessionBackend.php
+++ b/includes/session/SessionBackend.php
@@ -544,7 +544,7 @@ final class SessionBackend {
 		// Ensure the user has a token
 		// @codeCoverageIgnoreStart
 		$anon = $this->user->isAnon();
-		if ( !$anon && !$this->user->getToken() ) {
+		if ( !$anon && !$this->user->getToken( false ) ) {
 			$this->logger->debug(
 				"SessionBackend $this->id creating token for user {$this->user} on save"
 			);
diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php
index d84a2d6ea0..06a765ca3a 100644
--- a/includes/session/SessionManager.php
+++ b/includes/session/SessionManager.php
@@ -520,7 +520,7 @@ final class SessionManager implements SessionManagerInterface {
 
 		// Reset the user's token to kill existing sessions
 		$user = User::newFromName( $username );
-		if ( $user && $user->getToken() ) {
+		if ( $user && $user->getToken( false ) ) {
 			$user->setToken( true );
 			$user->saveSettings();
 		}
diff --git a/includes/session/UserInfo.php b/includes/session/UserInfo.php
index e844bb6c11..c01b9eca9d 100644
--- a/includes/session/UserInfo.php
+++ b/includes/session/UserInfo.php
@@ -152,10 +152,10 @@ final class UserInfo {
 
 	/**
 	 * Return the user token
-	 * @return string|null
+	 * @return string
 	 */
 	public function getToken() {
-		return $this->user === null || $this->user->getId() === 0 ? null : $this->user->getToken( true );
+		return $this->user === null || $this->user->getId() === 0 ? '' : $this->user->getToken( false );
 	}
 
 	/**
diff --git a/tests/phpunit/includes/session/UserInfoTest.php b/tests/phpunit/includes/session/UserInfoTest.php
index 121bb72a77..c38edd694a 100644
--- a/tests/phpunit/includes/session/UserInfoTest.php
+++ b/tests/phpunit/includes/session/UserInfoTest.php
@@ -19,7 +19,7 @@ class UserInfoTest extends MediaWikiTestCase {
 		$this->assertTrue( $userinfo->isVerified() );
 		$this->assertSame( 0, $userinfo->getId() );
 		$this->assertSame( null, $userinfo->getName() );
-		$this->assertSame( null, $userinfo->getToken() );
+		$this->assertSame( '', $userinfo->getToken() );
 		$this->assertNotNull( $userinfo->getUser() );
 		$this->assertSame( $userinfo, $userinfo->verified() );
 		$this->assertSame( '<anon>', (string)$userinfo );
@@ -102,7 +102,7 @@ class UserInfoTest extends MediaWikiTestCase {
 		$this->assertFalse( $userinfo->isVerified() );
 		$this->assertSame( $user->getId(), $userinfo->getId() );
 		$this->assertSame( $user->getName(), $userinfo->getName() );
-		$this->assertSame( null, $userinfo->getToken() );
+		$this->assertSame( '', $userinfo->getToken() );
 		$this->assertInstanceOf( 'User', $userinfo->getUser() );
 		$userinfo2 = $userinfo->verified();
 		$this->assertNotSame( $userinfo2, $userinfo );
@@ -112,7 +112,7 @@ class UserInfoTest extends MediaWikiTestCase {
 		$this->assertTrue( $userinfo2->isVerified() );
 		$this->assertSame( $user->getId(), $userinfo2->getId() );
 		$this->assertSame( $user->getName(), $userinfo2->getName() );
-		$this->assertSame( null, $userinfo2->getToken() );
+		$this->assertSame( '', $userinfo2->getToken() );
 		$this->assertInstanceOf( 'User', $userinfo2->getUser() );
 		$this->assertSame( $userinfo2, $userinfo2->verified() );
 		$this->assertSame( "<+:{$user->getId()}:{$user->getName()}>", (string)$userinfo2 );
@@ -157,7 +157,7 @@ class UserInfoTest extends MediaWikiTestCase {
 		$this->assertFalse( $userinfo->isVerified() );
 		$this->assertSame( $user->getId(), $userinfo->getId() );
 		$this->assertSame( $user->getName(), $userinfo->getName() );
-		$this->assertSame( null, $userinfo->getToken() );
+		$this->assertSame( '', $userinfo->getToken() );
 		$this->assertSame( $user, $userinfo->getUser() );
 		$userinfo2 = $userinfo->verified();
 		$this->assertNotSame( $userinfo2, $userinfo );
@@ -167,7 +167,7 @@ class UserInfoTest extends MediaWikiTestCase {
 		$this->assertTrue( $userinfo2->isVerified() );
 		$this->assertSame( $user->getId(), $userinfo2->getId() );
 		$this->assertSame( $user->getName(), $userinfo2->getName() );
-		$this->assertSame( null, $userinfo2->getToken() );
+		$this->assertSame( '', $userinfo2->getToken() );
 		$this->assertSame( $user, $userinfo2->getUser() );
 		$this->assertSame( $userinfo2, $userinfo2->verified() );
 		$this->assertSame( "<+:{$user->getId()}:{$user->getName()}>", (string)$userinfo2 );
-- 
2.20.1