Merge "Move CRSF token generation to CsrfTokenSet"

RELEASE-NOTES-1.37

@@ -410,9 +410,11 @@ because of Phabricator reports.
   BaseTemplateAfterPortlet hook, which were deprecated in 1.35,
   now emit deprecation warnings.
 * LocalFile::getHistory hook is deprecated.
-* User::matchEditTokenNoSuffix was deprecated without replacement.
-  It was introduce to be able to provide custom error message if the token was
-  submitted, but ending slashes were stripped by some ASCII mangling proxy.
+* User::getEditTokenObject, ::getEditToken, ::matchEditToken were deprecated.
+  Use CsrfTokenRepository, which is available via IContextSource, instead.
+  ::matchEditTokenNoSuffix was deprecated without replacement.
+  It was introduced to be able to provide custom error message if the token
+  was submitted, but ending slashes were stripped by some ASCII mangling proxy.
   Use matchToken instead, such proxies are much less common now and there's
   not much benefit in customising the error message.
 * ContentHandler::getForTitle(), deprecated since 1.35, now emits

includes/context/ContextSource.php

@@ -20,6 +20,7 @@
  */
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Permissions\Authority;
+use MediaWiki\Session\CsrfTokenSet;
 use Wikimedia\NonSerializable\NonSerializableTrait;
 
 /**
@@ -208,4 +209,14 @@ abstract class ContextSource implements IContextSource {
 	public function exportSession() {
 		return $this->getContext()->exportSession();
 	}
+
+	/**
+	 * Get a repository to obtain and match CSRF tokens.
+	 *
+	 * @return CsrfTokenSet
+	 * @since 1.37
+	 */
+	public function getCsrfTokenSet() : CsrfTokenSet {
+		return $this->getContext()->getCsrfTokenSet();
+	}
 }

includes/context/IContextSource.php

@@ -22,6 +22,7 @@
  */
 
 use MediaWiki\Permissions\Authority;
+use MediaWiki\Session\CsrfTokenSetProvider;
 
 /**
  * Interface for objects which can provide a MediaWiki context on request
@@ -54,7 +55,7 @@ use MediaWiki\Permissions\Authority;
  *
  * @unstable for implementation, extensions should subclass ContextSource instead.
  */
-interface IContextSource extends MessageLocalizer {
+interface IContextSource extends MessageLocalizer, CsrfTokenSetProvider {
 
 	/**
 	 * @return WebRequest

includes/context/RequestContext.php

@@ -25,6 +25,7 @@
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Permissions\Authority;
+use MediaWiki\Session\CsrfTokenSet;
 use Wikimedia\AtEase\AtEase;
 use Wikimedia\IPUtils;
 use Wikimedia\NonSerializable\NonSerializableTrait;
@@ -530,6 +531,10 @@ class RequestContext implements IContextSource, MutableContext {
 		];
 	}
 
+	public function getCsrfTokenSet() : CsrfTokenSet {
+		return new CsrfTokenSet( $this->getRequest() );
+	}
+
 	/**
 	 * Import an client IP address, HTTP headers, user ID, and session ID
 	 *

includes/session/CsrfTokenSet.php

@@ -0,0 +1,112 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Session;
+
+use LoggedOutEditToken;
+use WebRequest;
+
+/**
+ * Stores and matches CSRF tokens belonging to a given session user.
+ * @since 1.37
+ * @package MediaWiki\Session
+ */
+class CsrfTokenSet {
+
+	/**
+	 * @var string default name for the form field to place the token in.
+	 */
+	public const DEFAULT_FIELD_NAME = 'wpEditToken';
+
+	/**
+	 * @var WebRequest
+	 */
+	private $request;
+
+	/**
+	 * @param WebRequest $request
+	 */
+	public function __construct( WebRequest $request ) {
+		$this->request = $request;
+	}
+
+	/**
+	 * Initialize (if necessary) and return a current user CSRF token
+	 * value which can be used in edit forms to show that the user's
+	 * login credentials aren't being hijacked with a foreign form
+	 * submission.
+	 *
+	 * The $salt for 'edit' and 'csrf' tokens is the default (empty string).
+	 *
+	 * @param string|string[] $salt Optional function-specific data for hashing
+	 * @return Token
+	 * @since 1.37
+	 */
+	public function getToken( $salt = '' ) : Token {
+		$session = $this->request->getSession();
+		if ( !$session->getUser()->isRegistered() ) {
+			return new LoggedOutEditToken();
+		}
+		return $session->getToken( $salt );
+	}
+
+	/**
+	 * Check if a request contains a value named $valueName with the token value
+	 * stored in the session.
+	 *
+	 * @param string $fieldName
+	 * @param string|string[] $salt
+	 * @return bool
+	 * @since 1.37
+	 * @see self::matchCSRFToken
+	 */
+	public function matchTokenField(
+		string $fieldName = self::DEFAULT_FIELD_NAME,
+		$salt = ''
+	) : bool {
+		return $this->matchToken( $this->request->getVal( $fieldName ), $salt );
+	}
+
+	/**
+	 * Check if a value matches with the token value stored in the session.
+	 * A match should confirm that the form was submitted from the user's own
+	 * login session, not a form submission from a third-party site.
+	 *
+	 * @param string|null $value
+	 * @param string|string[] $salt
+	 * @return bool
+	 * @since 1.37
+	 */
+	public function matchToken(
+		?string $value,
+		$salt = ''
+	) : bool {
+		if ( !$value ) {
+			return false;
+		}
+		$session = $this->request->getSession();
+		// It's expensive to generate a new registered user token, so take a shortcut.
+		// Anon tokens are cheap and all the same, so we can afford to generate one just to match.
+		if ( $session->getUser()->isRegistered() && !$session->hasToken() ) {
+			return false;
+		}
+		return $this->getToken( $salt )->match( $value );
+	}
+}

includes/session/CsrfTokenSetProvider.php

@@ -0,0 +1,36 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Session;
+
+/**
+ * Provides an instance of CsrfTokenSet.
+ * @since 1.37
+ * @package MediaWiki\Session
+ */
+interface CsrfTokenSetProvider {
+
+	/**
+	 * Get a set of CSRF tokens to obtain and match specific tokens.
+	 *
+	 * @return CsrfTokenSet
+	 */
+	public function getCsrfTokenSet(): CsrfTokenSet;
+}

includes/session/Session.php

@@ -338,6 +338,21 @@ class Session implements \Countable, \Iterator, \ArrayAccess {
 		}
 	}
 
+	/**
+	 * Check if a CSRF token is set for the session
+	 *
+	 * @since 1.37
+	 * @param string $key Token key
+	 * @return bool
+	 */
+	public function hasToken( string $key = 'default' ): bool {
+		$secrets = $this->get( 'wsTokenSecrets' );
+		if ( !is_array( $secrets ) ) {
+			return false;
+		}
+		return isset( $secrets[$key] ) && is_string( $secrets[$key] );
+	}
+
 	/**
 	 * Fetch a CSRF token from the session
 	 *

includes/session/Token.php

@@ -102,11 +102,14 @@ class Token {
 
 	/**
 	 * Test if the token-string matches this token
-	 * @param string $userToken
+	 * @param string|null $userToken
 	 * @param int|null $maxAge Return false if $userToken is older than this many seconds
 	 * @return bool
 	 */
 	public function match( $userToken, $maxAge = null ) {
+		if ( !$userToken ) {
+			return false;
+		}
 		$timestamp = self::getTimestamp( $userToken );
 		if ( $timestamp === null ) {
 			return false;

includes/user/User.php

@@ -3634,6 +3634,7 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
 	 * submission.
 	 *
 	 * @since 1.27
+	 * @deprecated since 1.37. Use CsrfTokenSet::getToken instead
 	 * @param string|string[] $salt Optional function-specific data for hashing
 	 * @param WebRequest|null $request WebRequest object to use, or null to use the global request
 	 * @return MediaWiki\Session\Token The new edit token
@@ -3658,6 +3659,7 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
 	 * The $salt for 'edit' and 'csrf' tokens is the default (empty string).
 	 *
 	 * @since 1.19
+	 * @deprecated since 1.37. Use CsrfTokenSet::getToken instead
 	 * @param string|string[] $salt Optional function-specific data for hashing
 	 * @param WebRequest|null $request WebRequest object to use, or null to use the global request
 	 * @return string The new edit token
@@ -3672,6 +3674,7 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
 	 * user's own login session, not a form submission from a third-party
 	 * site.
 	 *
+	 * @deprecated since 1.37. Use CsrfTokenSet::matchToken instead
 	 * @param string $val Input value to compare
 	 * @param string|array $salt Optional function-specific data for hashing
 	 * @param WebRequest|null $request Object to use, or null to use the global request
@@ -3686,7 +3689,7 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
 	 * Check given value against the token value stored in the session,
 	 * ignoring the suffix.
 	 *
-	 * @deprecated since 1.37. No replacement is provided, use ::matchToken
+	 * @deprecated since 1.37. No replacement was provided.
 	 * @param string $val Input value to compare
 	 * @param string|array $salt Optional function-specific data for hashing
 	 * @param WebRequest|null $request Object to use, or null to use the global request

tests/phpunit/includes/session/CsrfTokenSetTest.php

@@ -0,0 +1,49 @@
+<?php
+
+namespace MediaWiki\Tests\Session;
+
+use MediaWiki\Session\CsrfTokenSet;
+use MediaWiki\Session\SessionManager;
+use MediaWikiIntegrationTestCase;
+use User;
+use WebRequest;
+
+/**
+ * @covers \MediaWiki\Session\CsrfTokenSet
+ * @package MediaWiki\Tests\Unit\Session
+ */
+class CsrfTokenSetTest extends MediaWikiIntegrationTestCase {
+
+	private function makeRequest( bool $userRegistered ): WebRequest {
+		$webRequest = new WebRequest();
+		$session1 = SessionManager::singleton()->getEmptySession( $webRequest );
+		$session1->setUser( $userRegistered ? $this->getTestUser()->getUser() : new User() );
+		return $webRequest;
+	}
+
+	public function testCSRFTokens_anon() {
+		$webRequest1 = $this->makeRequest( false );
+		$tokenRepo1 = new CsrfTokenSet( $webRequest1 );
+		$token = $tokenRepo1->getToken()->toString();
+		$webRequest2 = $this->makeRequest( false );
+		$tokenRepo2 = new CsrfTokenSet( $webRequest2 );
+		$this->assertTrue( $tokenRepo2->matchToken( $token ) );
+		$webRequest2->setVal( 'wpBlabla', $token );
+		$this->assertTrue( $tokenRepo2->matchTokenField( 'wpBlabla' ) );
+	}
+
+	public function testCSRFTokens_registered() {
+		$webRequest1 = $this->makeRequest( true );
+		$tokenRepo1 = new CsrfTokenSet( $webRequest1 );
+		$token = $tokenRepo1->getToken()->toString();
+		$this->assertTrue( $tokenRepo1->matchToken( $token ) );
+		$this->assertFalse( $tokenRepo1->matchTokenField( 'wpBlabla' ) );
+		$webRequest1->setVal( 'wpBlabla', $token );
+		$this->assertTrue( $tokenRepo1->matchTokenField( 'wpBlabla' ) );
+		$webRequest2 = $this->makeRequest( true );
+		$webRequest2->setVal( 'wpBlabla', $token );
+		$tokenRepo2 = new CsrfTokenSet( $webRequest2 );
+		$this->assertFalse( $tokenRepo2->matchTokenField( 'wpBlabla' ) );
+		$this->assertFalse( $tokenRepo2->matchToken( $token ) );
+	}
+}

tests/phpunit/includes/session/SessionTest.php

@@ -9,7 +9,7 @@ use Wikimedia\TestingAccessWrapper;
 
 /**
  * @group Session
- * @covers MediaWiki\Session\Session
+ * @covers \MediaWiki\Session\Session
  */
 class SessionTest extends MediaWikiIntegrationTestCase {
 

tests/phpunit/unit/includes/session/SessionUnitTest.php

@@ -222,7 +222,10 @@ class SessionUnitTest extends MediaWikiUnitTestCase {
 		$priv = TestingAccessWrapper::newFromObject( $session );
 		$backend = $priv->backend;
 
+		$this->assertFalse( $session->hasToken() );
 		$token = TestingAccessWrapper::newFromObject( $session->getToken() );
+		// Session::getToken auto-initializes the token.
+		$this->assertTrue( $session->hasToken() );
 		$this->assertArrayHasKey( 'wsTokenSecrets', $backend->data );
 		$this->assertArrayHasKey( 'default', $backend->data['wsTokenSecrets'] );
 		$secret = $backend->data['wsTokenSecrets']['default'];
@@ -235,10 +238,14 @@ class SessionUnitTest extends MediaWikiUnitTestCase {
 		$this->assertSame( 'foo', $token->salt );
 		$this->assertFalse( $token->wasNew() );
 
+		$this->assertFalse( $session->hasToken( 'secret' ) );
 		$backend->data['wsTokenSecrets']['secret'] = 'sekret';
 		$token = TestingAccessWrapper::newFromObject(
 			$session->getToken( [ 'bar', 'baz' ], 'secret' )
 		);
+		// Session::getToken auto-initializes the token.
+		$this->assertTrue( $session->hasToken() );
+		$this->assertTrue( $session->hasToken( 'secret' ) );
 		$this->assertSame( 'sekret', $token->secret );
 		$this->assertSame( 'bar|baz', $token->salt );
 		$this->assertFalse( $token->wasNew() );

tests/phpunit/unit/includes/session/TokenTest.php

@@ -7,7 +7,7 @@ use Wikimedia\TestingAccessWrapper;
 
 /**
  * @group Session
- * @covers MediaWiki\Session\Token
+ * @covers \MediaWiki\Session\Token
  */
 class TokenTest extends MediaWikiUnitTestCase {
 
@@ -56,6 +56,8 @@ class TokenTest extends MediaWikiUnitTestCase {
 	public function testMatch() {
 		$token = TestingAccessWrapper::newFromObject( new Token( 'sekret', 'salty', false ) );
 
+		$this->assertFalse( $token->match( null ) );
+
 		$test = $token->toStringAtTimestamp( time() - 10 );
 		$this->assertTrue( $token->match( $test ) );
 		$this->assertTrue( $token->match( $test, 12 ) );
@@ -63,5 +65,4 @@ class TokenTest extends MediaWikiUnitTestCase {
 
 		$this->assertFalse( $token->match( 'ee2f7a2488dea9176c224cfb400d43be5644fdea-\\' ) );
 	}
-
 }