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

diff --git a/includes/session/SessionBackend.php b/includes/session/SessionBackend.php
index 67cbc6d..1c743dd 100644 (file)
--- 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 d84a2d6..06a765c 100644 (file)
--- 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 e844bb6..c01b9ec 100644 (file)
--- 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 121bb72..c38edd6 100644 (file)
--- 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 );