Move CSRF token handling into MediaWiki\Session\Session
authorBrad Jorsch <bjorsch@wikimedia.org>
Tue, 22 Sep 2015 14:33:24 +0000 (10:33 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Wed, 27 Jan 2016 20:27:20 +0000 (15:27 -0500)
User keeps most of its token-related methods because anon edit tokens
are special. Login and createaccount tokens are completely moved.

Change-Id: I524218fab7e2d78fd24482ad364428e98dc48bdf

20 files changed:
RELEASE-NOTES-1.27
autoload.php
docs/hooks.txt
includes/api/ApiBase.php
includes/api/ApiCheckToken.php
includes/api/ApiCreateAccount.php
includes/api/ApiLogin.php
includes/api/ApiQueryTokens.php
includes/api/ApiTokens.php
includes/session/Session.php
includes/session/Token.php [new file with mode: 0644]
includes/specials/SpecialChangePassword.php
includes/specials/SpecialUserlogin.php
includes/user/LoggedOutEditToken.php [new file with mode: 0644]
includes/user/User.php
tests/phpunit/includes/api/ApiCreateAccountTest.php
tests/phpunit/includes/api/ApiLoginTest.php
tests/phpunit/includes/api/ApiTestCase.php
tests/phpunit/includes/session/SessionTest.php
tests/phpunit/includes/session/TokenTest.php [new file with mode: 0644]

index 50d40a6..2ce8046 100644 (file)
@@ -81,6 +81,8 @@ production.
    MediaWiki\Session\SessionProvider.
 ** The User cannot be loaded from session until after Setup.php completes.
    Attempts to do so will be ignored and the User will remain unloaded.
+** CSRF tokens may be fetched from the MediaWiki\Session\Session, which uses
+   the MediaWiki\Session\Token class.
 * MediaWiki will now auto-create users as necessary, removing the need for
   extensions to do so. An 'autocreateaccount' right is added to allow
   auto-creation when 'createaccount' is not granted to all users.
@@ -88,6 +90,10 @@ production.
 * Most cookie-handling methods in User are deprecated.
 * $wgAllowAsyncCopyUploads and $CopyUploadAsyncTimeout were removed. This was an
   experimental feature that has never worked.
+* Login and createaccount tokens now vary by timestamp.
+* LoginForm::getLoginToken() and LoginForm::getCreateaccountToken()
+  return a MediaWiki\Session\Token, and tokens must be checked using that
+  class's methods.
 
 === New features in 1.27 ===
 * $wgDataCenterId and $wgDataCenterRoles where added, which will serve as
index 2f23924..6de1d79 100644 (file)
@@ -721,6 +721,7 @@ $wgAutoloadLocalClasses = array(
        'LogFormatter' => __DIR__ . '/includes/logging/LogFormatter.php',
        'LogPage' => __DIR__ . '/includes/logging/LogPage.php',
        'LogPager' => __DIR__ . '/includes/logging/LogPager.php',
+       'LoggedOutEditToken' => __DIR__ . '/includes/user/LoggedOutEditToken.php',
        'LoggedUpdateMaintenance' => __DIR__ . '/maintenance/Maintenance.php',
        'LoginForm' => __DIR__ . '/includes/specials/SpecialUserlogin.php',
        'LonelyPagesPage' => __DIR__ . '/includes/specials/SpecialLonelypages.php',
@@ -797,6 +798,7 @@ $wgAutoloadLocalClasses = array(
        'MediaWiki\\Session\\SessionManagerInterface' => __DIR__ . '/includes/session/SessionManagerInterface.php',
        'MediaWiki\\Session\\SessionProvider' => __DIR__ . '/includes/session/SessionProvider.php',
        'MediaWiki\\Session\\SessionProviderInterface' => __DIR__ . '/includes/session/SessionProviderInterface.php',
+       'MediaWiki\\Session\\Token' => __DIR__ . '/includes/session/Token.php',
        'MediaWiki\\Session\\UserInfo' => __DIR__ . '/includes/session/UserInfo.php',
        'MediaWiki\\Site\\MediaWikiPageNameNormalizer' => __DIR__ . '/includes/site/MediaWikiPageNameNormalizer.php',
        'MediaWiki\\Tidy\\Html5Depurate' => __DIR__ . '/includes/tidy/Html5Depurate.php',
index 2b5e1e0..b516122 100644 (file)
@@ -513,7 +513,8 @@ sites statistics information.
 'ApiQueryTokensRegisterTypes': Use this hook to add additional token types to
 action=query&meta=tokens. Note that most modules will probably be able to use
 the 'csrf' token instead of creating their own token types.
-&$salts: array( type => salt to pass to User::getEditToken() )
+&$salts: array( type => salt to pass to User::getEditToken() or array of salt
+  and key to pass to Session::getToken() )
 
 'APIQueryUsersTokens': DEPRECATED! Use ApiQueryTokensRegisterTypes instead.
 Use this hook to add custom token to list=users. Every token has an action,
index 13d13a6..425a337 100644 (file)
@@ -1260,11 +1260,10 @@ abstract class ApiBase extends ContextSource {
                        );
                }
 
-               if ( $this->getUser()->matchEditToken(
-                       $token,
-                       $salts[$tokenType],
-                       $this->getRequest()
-               ) ) {
+               $tokenObj = ApiQueryTokens::getToken(
+                       $this->getUser(), $this->getRequest()->getSession(), $salts[$tokenType]
+               );
+               if ( $tokenObj->match( $token ) ) {
                        return true;
                }
 
index 28c6ece..dfcbaf8 100644 (file)
@@ -32,21 +32,22 @@ class ApiCheckToken extends ApiBase {
                $params = $this->extractRequestParams();
                $token = $params['token'];
                $maxage = $params['maxtokenage'];
-               $request = $this->getRequest();
                $salts = ApiQueryTokens::getTokenTypeSalts();
-               $salt = $salts[$params['type']];
 
                $res = array();
 
-               if ( $this->getUser()->matchEditToken( $token, $salt, $request, $maxage ) ) {
+               $tokenObj = ApiQueryTokens::getToken(
+                       $this->getUser(), $this->getRequest()->getSession(), $salts[$params['type']]
+               );
+               if ( $tokenObj->match( $token, $maxage ) ) {
                        $res['result'] = 'valid';
-               } elseif ( $maxage !== null && $this->getUser()->matchEditToken( $token, $salt, $request ) ) {
+               } elseif ( $maxage !== null && $tokenObj->match( $token ) ) {
                        $res['result'] = 'expired';
                } else {
                        $res['result'] = 'invalid';
                }
 
-               $ts = User::getEditTokenTimestamp( $token );
+               $ts = MediaWiki\Session\Token::getTimestamp( $token );
                if ( $ts !== null ) {
                        $mwts = new MWTimestamp();
                        $mwts->timestamp->setTimestamp( $ts );
index a044be2..d6baf34 100644 (file)
@@ -149,8 +149,11 @@ class ApiCreateAccount extends ApiBase {
                        // Token was incorrect, so add it to result, but don't throw an exception
                        // since not having the correct token is part of the normal
                        // flow of events.
-                       $result['token'] = LoginForm::getCreateaccountToken();
+                       $result['token'] = LoginForm::getCreateaccountToken()->toString();
                        $result['result'] = 'NeedToken';
+                       $this->setWarning( 'Fetching a token via action=createaccount is deprecated. ' .
+                               'Use action=query&meta=tokens&type=createaccount instead.' );
+                       $this->logFeatureUsage( 'action=createaccount&!token' );
                } elseif ( !$status->isOK() ) {
                        // There was an error. Die now.
                        $this->dieStatus( $status );
@@ -200,7 +203,11 @@ class ApiCreateAccount extends ApiBase {
                                ApiBase::PARAM_TYPE => 'password',
                        ),
                        'domain' => null,
-                       'token' => null,
+                       'token' => array(
+                               ApiBase::PARAM_TYPE => 'string',
+                               ApiBase::PARAM_REQUIRED => false, // for BC
+                               ApiBase::PARAM_HELP_MSG => array( 'api-help-param-token', 'createaccount' ),
+                       ),
                        'email' => array(
                                ApiBase::PARAM_TYPE => 'string',
                                ApiBase::PARAM_REQUIRED => $this->getConfig()->get( 'EmailConfirmToEdit' ),
index 860e3b2..03cd666 100644 (file)
@@ -84,12 +84,9 @@ class ApiLogin extends ApiBase {
 
                // Check login token
                $token = LoginForm::getLoginToken();
-               if ( !$token ) {
-                       LoginForm::setLoginToken();
+               if ( $token->wasNew() || !$params['token'] ) {
                        $authRes = LoginForm::NEED_TOKEN;
-               } elseif ( !$params['token'] ) {
-                       $authRes = LoginForm::NEED_TOKEN;
-               } elseif ( $token !== $params['token'] ) {
+               } elseif ( !$token->match( $params['token'] ) ) {
                        $authRes = LoginForm::WRONG_TOKEN;
                }
 
@@ -159,7 +156,10 @@ class ApiLogin extends ApiBase {
 
                        case LoginForm::NEED_TOKEN:
                                $result['result'] = 'NeedToken';
-                               $result['token'] = LoginForm::getLoginToken();
+                               $result['token'] = LoginForm::getLoginToken()->toString();
+                               $this->setWarning( 'Fetching a token via action=login is deprecated. ' .
+                                  'Use action=query&meta=tokens&type=login instead.' );
+                               $this->logFeatureUsage( 'action=login&!lgtoken' );
 
                                // @todo: See above about deprecation
                                $result['cookieprefix'] = $this->getConfig()->get( 'CookiePrefix' );
@@ -254,7 +254,11 @@ class ApiLogin extends ApiBase {
                                ApiBase::PARAM_TYPE => 'password',
                        ),
                        'domain' => null,
-                       'token' => null,
+                       'token' => array(
+                               ApiBase::PARAM_TYPE => 'string',
+                               ApiBase::PARAM_REQUIRED => false, // for BC
+                               ApiBase::PARAM_HELP_MSG => array( 'api-help-param-token', 'login' ),
+                       ),
                );
        }
 
index f887664..3f3464b 100644 (file)
@@ -44,16 +44,24 @@ class ApiQueryTokens extends ApiQueryBase {
                        return;
                }
 
+               $user = $this->getUser();
+               $session = $this->getRequest()->getSession();
                $salts = self::getTokenTypeSalts();
                foreach ( $params['type'] as $type ) {
-                       $salt = $salts[$type];
-                       $val = $this->getUser()->getEditToken( $salt, $this->getRequest() );
-                       $res[$type . 'token'] = $val;
+                       $res[$type . 'token'] = self::getToken( $user, $session, $salts[$type] )->toString();
                }
 
                $this->getResult()->addValue( 'query', $this->getModuleName(), $res );
        }
 
+       /**
+        * Get the salts for known token types
+        * @return (string|array)[] Returning a string will use that as the salt
+        *  for User::getEditTokenObject() to fetch the token, which will give a
+        *  LoggedOutEditToken (always "+\\") for anonymous users. Returning an
+        *  array will use it as parameters to MediaWiki\\Session\\Session::getToken(),
+        *  which will always return a full token even for anonymous users.
+        */
        public static function getTokenTypeSalts() {
                static $salts = null;
                if ( !$salts ) {
@@ -63,6 +71,8 @@ class ApiQueryTokens extends ApiQueryBase {
                                'patrol' => 'patrol',
                                'rollback' => 'rollback',
                                'userrights' => 'userrights',
+                               'login' => array( '', 'login' ),
+                               'createaccount' => array( '', 'createaccount' ),
                        );
                        Hooks::run( 'ApiQueryTokensRegisterTypes', array( &$salts ) );
                        ksort( $salts );
@@ -71,6 +81,27 @@ class ApiQueryTokens extends ApiQueryBase {
                return $salts;
        }
 
+       /**
+        * Get a token from a salt
+        * @param User $user
+        * @param MediaWiki\\Session\\Session $session
+        * @param string|array $salt A string will be used as the salt for
+        *  User::getEditTokenObject() to fetch the token, which will give a
+        *  LoggedOutEditToken (always "+\\") for anonymous users. An array will
+        *  be used as parameters to MediaWiki\\Session\\Session::getToken(), which
+        *  will always return a full token even for anonymous users. An array will
+        *  also persist the session.
+        * @return MediaWiki\\Session\\Token
+        */
+       public static function getToken( User $user, MediaWiki\Session\Session $session, $salt ) {
+               if ( is_array( $salt ) ) {
+                       $session->persist();
+                       return call_user_func_array( array( $session, 'getToken' ), $salt );
+               } else {
+                       return $user->getEditTokenObject( $salt, $session->getRequest() );
+               }
+       }
+
        public function getAllowedParams() {
                return array(
                        'type' => array(
@@ -90,6 +121,11 @@ class ApiQueryTokens extends ApiQueryBase {
                );
        }
 
+       public function isReadMode() {
+               // So login tokens can be fetched on private wikis
+               return false;
+       }
+
        public function getCacheMode( $params ) {
                return 'private';
        }
index f92526d..c10c938 100644 (file)
@@ -81,7 +81,7 @@ class ApiTokens extends ApiBase {
                foreach ( ApiQueryTokens::getTokenTypeSalts() as $name => $salt ) {
                        if ( !isset( $types[$name] ) ) {
                                $types[$name] = function () use ( $salt, $user, $request ) {
-                                       return $user->getEditToken( $salt, $request );
+                                       return ApiQueryTokens::getToken( $user, $request->getSession(), $salt )->toString();
                                };
                        }
                }
index 840baa7..4ad69ae 100644 (file)
@@ -314,6 +314,58 @@ final class Session implements \Countable, \Iterator {
                }
        }
 
+       /**
+        * Fetch a CSRF token from the session
+        *
+        * Note that this does not persist the session, which you'll probably want
+        * to do if you want the token to actually be useful.
+        *
+        * @param string|string[] $salt Token salt
+        * @param string $key Token key
+        * @return MediaWiki\\Session\\SessionToken
+        */
+       public function getToken( $salt = '', $key = 'default' ) {
+               $new = false;
+               $secrets = $this->get( 'wsTokenSecrets' );
+               if ( !is_array( $secrets ) ) {
+                       $secrets = array();
+               }
+               if ( isset( $secrets[$key] ) && is_string( $secrets[$key] ) ) {
+                       $secret = $secrets[$key];
+               } else {
+                       $secret = \MWCryptRand::generateHex( 32 );
+                       $secrets[$key] = $secret;
+                       $this->set( 'wsTokenSecrets', $secrets );
+                       $new = true;
+               }
+               if ( is_array( $salt ) ) {
+                       $salt = join( '|', $salt );
+               }
+               return new Token( $secret, (string)$salt, $new );
+       }
+
+       /**
+        * Remove a CSRF token from the session
+        *
+        * The next call to self::getToken() with $key will generate a new secret.
+        *
+        * @param string $key Token key
+        */
+       public function resetToken( $key = 'default' ) {
+               $secrets = $this->get( 'wsTokenSecrets' );
+               if ( is_array( $secrets ) && isset( $secrets[$key] ) ) {
+                       unset( $secrets[$key] );
+                       $this->set( 'wsTokenSecrets', $secrets );
+               }
+       }
+
+       /**
+        * Remove all CSRF tokens from the session
+        */
+       public function resetAllTokens() {
+               $this->remove( 'wsTokenSecrets' );
+       }
+
        /**
         * Delay automatic saving while multiple updates are being made
         *
diff --git a/includes/session/Token.php b/includes/session/Token.php
new file mode 100644 (file)
index 0000000..9b4a73c
--- /dev/null
@@ -0,0 +1,125 @@
+<?php
+/**
+ * MediaWiki session token
+ *
+ * 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
+ * @ingroup Session
+ */
+
+namespace MediaWiki\Session;
+
+/**
+ * Value object representing a CSRF token
+ *
+ * @ingroup Session
+ * @since 1.27
+ */
+class Token {
+       /** CSRF token suffix. Plus and terminal backslash are included to stop
+        * editing from certain broken proxies. */
+       const SUFFIX = '+\\';
+
+       private $secret = '';
+       private $salt = '';
+       private $new = false;
+
+       /**
+        * @param string $secret Token secret
+        * @param string $salt Token salt
+        * @param bool $new Whether the secret was newly-created
+        */
+       public function __construct( $secret, $salt, $new = false ) {
+               $this->secret = $secret;
+               $this->salt = $salt;
+               $this->new = $new;
+       }
+
+       /**
+        * Decode the timestamp from a token string
+        *
+        * Does not validate the token beyond the syntactic checks necessary to
+        * be able to extract the timestamp.
+        *
+        * @param string $token
+        * @param int|null
+        */
+       public static function getTimestamp( $token ) {
+               $suffixLen = strlen( self::SUFFIX );
+               $len = strlen( $token );
+               if ( $len <= 32 + $suffixLen ||
+                       substr( $token, -$suffixLen ) !== self::SUFFIX ||
+                       strspn( $token, '0123456789abcdef' ) + $suffixLen !== $len
+               ) {
+                       return null;
+               }
+
+               return hexdec( substr( $token, 32, -$suffixLen ) );
+       }
+
+       /**
+        * Get the string representation of the token at a timestamp
+        * @param int timestamp
+        * @return string
+        */
+       protected function toStringAtTimestamp( $timestamp ) {
+               return hash_hmac( 'md5', $timestamp . $this->salt, $this->secret, false ) .
+                       dechex( $timestamp ) .
+                       self::SUFFIX;
+       }
+
+       /**
+        * Get the string representation of the token
+        * @return string
+        */
+       public function toString() {
+               return $this->toStringAtTimestamp( wfTimestamp() );
+       }
+
+       public function __toString() {
+               return $this->toString();
+       }
+
+       /**
+        * Test if the token-string matches this token
+        * @param string $userToken
+        * @param int|null $maxAge Return false if $userToken is older than this many seconds
+        * @return bool
+        */
+       public function match( $userToken, $maxAge = null ) {
+               $timestamp = self::getTimestamp( $userToken );
+               if ( $timestamp === null ) {
+                       return false;
+               }
+               if ( $maxAge !== null && $timestamp < wfTimestamp() - $maxAge ) {
+                       // Expired token
+                       return false;
+               }
+
+               $sessionToken = $this->toStringAtTimestamp( $timestamp );
+               return hash_equals( $sessionToken, $userToken );
+       }
+
+       /**
+        * Indicate whether this token was just created
+        * @return bool
+        */
+       public function wasNew() {
+               return $this->new;
+       }
+
+}
index 8656798..3e5a936 100644 (file)
@@ -111,13 +111,10 @@ class SpecialChangePassword extends FormSpecialPage {
                );
 
                if ( !$this->getUser()->isLoggedIn() ) {
-                       if ( !LoginForm::getLoginToken() ) {
-                               LoginForm::setLoginToken();
-                       }
                        $fields['LoginOnChangeToken'] = array(
                                'type' => 'hidden',
                                'label' => 'Change Password Token',
-                               'default' => LoginForm::getLoginToken(),
+                               'default' => LoginForm::getLoginToken()->toString(),
                        );
                }
 
@@ -179,7 +176,7 @@ class SpecialChangePassword extends FormSpecialPage {
                }
 
                if ( !$this->getUser()->isLoggedIn()
-                       && $request->getVal( 'wpLoginOnChangeToken' ) !== LoginForm::getLoginToken()
+                       && !LoginForm::getLoginToken()->match( $request->getVal( 'wpLoginOnChangeToken' ) )
                ) {
                        // Potential CSRF (bug 62497)
                        return false;
@@ -218,8 +215,8 @@ class SpecialChangePassword extends FormSpecialPage {
                        $this->getOutput()->returnToMain();
                } else {
                        $request = $this->getRequest();
-                       LoginForm::setLoginToken();
-                       $token = LoginForm::getLoginToken();
+                       LoginForm::clearLoginToken();
+                       $token = LoginForm::getLoginToken()->toString();
                        $data = array(
                                'action' => 'submitlogin',
                                'wpName' => $this->mUserName,
index 9473dff..d9a5e4f 100644 (file)
@@ -531,9 +531,8 @@ class LoginForm extends SpecialPage {
                }
 
                # Request forgery checks.
-               if ( !self::getCreateaccountToken() ) {
-                       self::setCreateaccountToken();
-
+               $token = self::getCreateaccountToken();
+               if ( $token->wasNew() ) {
                        return Status::newFatal( 'nocookiesfornew' );
                }
 
@@ -543,7 +542,7 @@ class LoginForm extends SpecialPage {
                }
 
                # Validate the createaccount token
-               if ( $this->mToken !== self::getCreateaccountToken() ) {
+               if ( !$token->match( $this->mToken ) ) {
                        return Status::newFatal( 'sessionfailure' );
                }
 
@@ -737,9 +736,8 @@ class LoginForm extends SpecialPage {
                // but wrong-token attempts do.
 
                // If the user doesn't have a login token yet, set one.
-               if ( !self::getLoginToken() ) {
-                       self::setLoginToken();
-
+               $token = self::getLoginToken();
+               if ( $token->wasNew() ) {
                        return self::NEED_TOKEN;
                }
                // If the user didn't pass a login token, tell them we need one
@@ -753,7 +751,7 @@ class LoginForm extends SpecialPage {
                }
 
                // Validate the login token
-               if ( $this->mToken !== self::getLoginToken() ) {
+               if ( !$token->match( $this->mToken ) ) {
                        return self::WRONG_TOKEN;
                }
 
@@ -1492,15 +1490,9 @@ class LoginForm extends SpecialPage {
                $template->set( 'loggedinuser', $user->getName() );
 
                if ( $this->mType == 'signup' ) {
-                       if ( !self::getCreateaccountToken() ) {
-                               self::setCreateaccountToken();
-                       }
-                       $template->set( 'token', self::getCreateaccountToken() );
+                       $template->set( 'token', self::getCreateaccountToken()->toString() );
                } else {
-                       if ( !self::getLoginToken() ) {
-                               self::setLoginToken();
-                       }
-                       $template->set( 'token', self::getLoginToken() );
+                       $template->set( 'token', self::getLoginToken()->toString() );
                }
 
                # Prepare language selection links as needed
@@ -1576,22 +1568,25 @@ class LoginForm extends SpecialPage {
 
        /**
         * Get the login token from the current session
-        * @return mixed
+        * @since 1.27 returns a MediaWiki\\Session\\Token instead of a string
+        * @return MediaWiki\\Session\\Token
         */
        public static function getLoginToken() {
                global $wgRequest;
-
-               return $wgRequest->getSessionData( 'wsLoginToken' );
+               return $wgRequest->getSession()->getToken( '', 'login' );
        }
 
        /**
-        * Randomly generate a new login token and attach it to the current session
+        * Formerly randomly generated a login token that would be returned by
+        * $this->getLoginToken().
+        *
+        * Since 1.27, this is a no-op. The token is generated as necessary by
+        * $this->getLoginToken().
+        *
+        * @deprecated since 1.27
         */
        public static function setLoginToken() {
-               global $wgRequest;
-               // Generate a token directly instead of using $user->getEditToken()
-               // because the latter reuses wsEditToken in the session
-               $wgRequest->setSessionData( 'wsLoginToken', MWCryptRand::generateHex( 32 ) );
+               wfDeprecated( __METHOD__, '1.27' );
        }
 
        /**
@@ -1599,24 +1594,30 @@ class LoginForm extends SpecialPage {
         */
        public static function clearLoginToken() {
                global $wgRequest;
-               $wgRequest->setSessionData( 'wsLoginToken', null );
+               $wgRequest->getSession()->resetToken( 'login' );
        }
 
        /**
         * Get the createaccount token from the current session
-        * @return mixed
+        * @since 1.27 returns a MediaWiki\\Session\\Token instead of a string
+        * @return MediaWiki\\Session\\Token
         */
        public static function getCreateaccountToken() {
                global $wgRequest;
-               return $wgRequest->getSessionData( 'wsCreateaccountToken' );
+               return $wgRequest->getSession()->getToken( '', 'createaccount' );
        }
 
        /**
-        * Randomly generate a new createaccount token and attach it to the current session
+        * Formerly randomly generated a createaccount token that would be returned
+        * by $this->getCreateaccountToken().
+        *
+        * Since 1.27, this is a no-op. The token is generated as necessary by
+        * $this->getCreateaccountToken().
+        *
+        * @deprecated since 1.27
         */
        public static function setCreateaccountToken() {
-               global $wgRequest;
-               $wgRequest->setSessionData( 'wsCreateaccountToken', MWCryptRand::generateHex( 32 ) );
+               wfDeprecated( __METHOD__, '1.27' );
        }
 
        /**
@@ -1624,7 +1625,7 @@ class LoginForm extends SpecialPage {
         */
        public static function clearCreateaccountToken() {
                global $wgRequest;
-               $wgRequest->setSessionData( 'wsCreateaccountToken', null );
+               $wgRequest->getSession()->resetToken( 'createaccount' );
        }
 
        /**
diff --git a/includes/user/LoggedOutEditToken.php b/includes/user/LoggedOutEditToken.php
new file mode 100644 (file)
index 0000000..14548f4
--- /dev/null
@@ -0,0 +1,47 @@
+<?php
+/**
+ * MediaWiki edit token
+ *
+ * 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
+ * @ingroup Session
+ */
+
+use MediaWiki\Session\Token;
+
+/**
+ * Value object representing a logged-out user's edit token
+ *
+ * This exists so that code generically dealing with MediaWiki\\Session\\Token
+ * (i.e. the API) doesn't have to have so many special cases for anon edit
+ * tokens.
+ *
+ * @since 1.27
+ */
+class LoggedOutEditToken extends MediaWiki\Session\Token {
+       public function __construct() {
+               parent::__construct( '', '', false );
+       }
+
+       protected function toStringAtTimestamp( $timestamp ) {
+               return self::SUFFIX;
+       }
+
+       public function match( $userToken, $maxAge = null ) {
+               return $userToken === self::SUFFIX;
+       }
+}
index 2fadd6b..a268117 100644 (file)
@@ -24,9 +24,10 @@ use MediaWiki\Session\SessionManager;
 
 /**
  * String Some punctuation to prevent editing from broken text-mangling proxies.
+ * @deprecated since 1.27, use \\MediaWiki\\Session\\Token::SUFFIX
  * @ingroup Constants
  */
-define( 'EDIT_TOKEN_SUFFIX', '+\\' );
+define( 'EDIT_TOKEN_SUFFIX', MediaWiki\Session\Token::SUFFIX );
 
 /**
  * The User object encapsulates all of the user-specific settings (user_id,
@@ -47,6 +48,7 @@ class User implements IDBAccessObject {
        /**
         * Global constant made accessible as class constants so that autoloader
         * magic can be used.
+        * @deprecated since 1.27, use \\MediaWiki\\Session\\Token::SUFFIX
         */
        const EDIT_TOKEN_SUFFIX = EDIT_TOKEN_SUFFIX;
 
@@ -4066,30 +4068,25 @@ class User implements IDBAccessObject {
        }
 
        /**
-        * Internal implementation for self::getEditToken() and
-        * self::matchEditToken().
+        * Initialize (if necessary) and return a session 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.
         *
-        * @param string|array $salt
-        * @param WebRequest $request
-        * @param string|int $timestamp
-        * @return string
+        * @since 1.27
+        * @param string|array $salt Array of Strings Optional function-specific data for hashing
+        * @param WebRequest|null $request WebRequest object to use or null to use $wgRequest
+        * @return MediaWiki\\Session\\Token The new edit token
         */
-       private function getEditTokenAtTimestamp( $salt, $request, $timestamp ) {
+       public function getEditTokenObject( $salt = '', $request = null ) {
                if ( $this->isAnon() ) {
-                       return self::EDIT_TOKEN_SUFFIX;
-               } else {
-                       $token = $request->getSessionData( 'wsEditToken' );
-                       if ( $token === null ) {
-                               $token = MWCryptRand::generateHex( 32 );
-                               $request->setSessionData( 'wsEditToken', $token );
-                       }
-                       if ( is_array( $salt ) ) {
-                               $salt = implode( '|', $salt );
-                       }
-                       return hash_hmac( 'md5', $timestamp . $salt, $token, false ) .
-                               dechex( $timestamp ) .
-                               self::EDIT_TOKEN_SUFFIX;
+                       return new LoggedOutEditToken();
+               }
+
+               if ( !$request ) {
+                       $request = $this->getRequest();
                }
+               return $request->getSession()->getToken( $salt );
        }
 
        /**
@@ -4099,29 +4096,23 @@ class User implements IDBAccessObject {
         * submission.
         *
         * @since 1.19
-        *
         * @param string|array $salt Array of Strings Optional function-specific data for hashing
         * @param WebRequest|null $request WebRequest object to use or null to use $wgRequest
         * @return string The new edit token
         */
        public function getEditToken( $salt = '', $request = null ) {
-               return $this->getEditTokenAtTimestamp(
-                       $salt, $request ?: $this->getRequest(), wfTimestamp()
-               );
+               return $this->getEditTokenObject( $salt, $request )->toString();
        }
 
        /**
         * Get the embedded timestamp from a token.
+        * @deprecated since 1.27, use \\MediaWiki\\Session\\Token::getTimestamp instead.
         * @param string $val Input token
         * @return int|null
         */
        public static function getEditTokenTimestamp( $val ) {
-               $suffixLen = strlen( self::EDIT_TOKEN_SUFFIX );
-               if ( strlen( $val ) <= 32 + $suffixLen ) {
-                       return null;
-               }
-
-               return hexdec( substr( $val, 32, -$suffixLen ) );
+               wfDeprecated( __METHOD__, '1.27' );
+               return MediaWiki\Session\Token::getTimestamp( $val );
        }
 
        /**
@@ -4137,28 +4128,7 @@ class User implements IDBAccessObject {
         * @return bool Whether the token matches
         */
        public function matchEditToken( $val, $salt = '', $request = null, $maxage = null ) {
-               if ( $this->isAnon() ) {
-                       return $val === self::EDIT_TOKEN_SUFFIX;
-               }
-
-               $timestamp = self::getEditTokenTimestamp( $val );
-               if ( $timestamp === null ) {
-                       return false;
-               }
-               if ( $maxage !== null && $timestamp < wfTimestamp() - $maxage ) {
-                       // Expired token
-                       return false;
-               }
-
-               $sessionToken = $this->getEditTokenAtTimestamp(
-                       $salt, $request ?: $this->getRequest(), $timestamp
-               );
-
-               if ( !hash_equals( $sessionToken, $val ) ) {
-                       wfDebug( "User::matchEditToken: broken session data\n" );
-               }
-
-               return hash_equals( $sessionToken, $val );
+               return $this->getEditTokenObject( $salt, $request )->match( $val, $maxage );
        }
 
        /**
index 3945102..35905c5 100644 (file)
@@ -10,7 +10,6 @@
 class ApiCreateAccountTest extends ApiTestCase {
        protected function setUp() {
                parent::setUp();
-               LoginForm::setCreateaccountToken();
                $this->setMwGlobals( array( 'wgEnableEmail' => true ) );
        }
 
@@ -114,7 +113,7 @@ class ApiCreateAccountTest extends ApiTestCase {
        public function testNoName() {
                $this->doApiRequest( array(
                        'action' => 'createaccount',
-                       'token' => LoginForm::getCreateaccountToken(),
+                       'token' => LoginForm::getCreateaccountToken()->toString(),
                        'password' => 'password',
                ) );
        }
@@ -127,7 +126,7 @@ class ApiCreateAccountTest extends ApiTestCase {
                $this->doApiRequest( array(
                        'action' => 'createaccount',
                        'name' => 'testName',
-                       'token' => LoginForm::getCreateaccountToken(),
+                       'token' => LoginForm::getCreateaccountToken()->toString(),
                ) );
        }
 
@@ -139,7 +138,7 @@ class ApiCreateAccountTest extends ApiTestCase {
                $this->doApiRequest( array(
                        'action' => 'createaccount',
                        'name' => 'Apitestsysop',
-                       'token' => LoginForm::getCreateaccountToken(),
+                       'token' => LoginForm::getCreateaccountToken()->toString(),
                        'password' => 'password',
                        'email' => 'test@domain.test',
                ) );
@@ -153,7 +152,7 @@ class ApiCreateAccountTest extends ApiTestCase {
                $this->doApiRequest( array(
                        'action' => 'createaccount',
                        'name' => 'Test User',
-                       'token' => LoginForm::getCreateaccountToken(),
+                       'token' => LoginForm::getCreateaccountToken()->toString(),
                        'password' => 'password',
                        'email' => 'invalid',
                ) );
index 4085925..894feb7 100644 (file)
@@ -14,11 +14,11 @@ class ApiLoginTest extends ApiTestCase {
         */
        public function testApiLoginNoName() {
                $session = array(
-                       'wsLoginToken' => 'foobar'
+                       'wsTokenSecrets' => array( 'login' => 'foobar' ),
                );
                $data = $this->doApiRequest( array( 'action' => 'login',
                        'lgname' => '', 'lgpassword' => self::$users['sysop']->password,
-                       'lgtoken' => 'foobar',
+                       'lgtoken' => (string)( new MediaWiki\Session\Token( 'foobar', '' ) )
                ), $session );
                $this->assertEquals( 'NoName', $data[0]['login']['result'] );
        }
index 25ffcb7..52c9fec 100644 (file)
@@ -148,12 +148,12 @@ abstract class ApiTestCase extends MediaWikiLangTestCase {
                if ( isset( $session['wsToken'] ) && $session['wsToken'] ) {
                        // @todo Why does this directly mess with the session? Fix that.
                        // add edit token to fake session
-                       $session['wsEditToken'] = $session['wsToken'];
+                       $session['wsTokenSecrets']['default'] = $session['wsToken'];
                        // add token to request parameters
                        $timestamp = wfTimestamp();
                        $params['token'] = hash_hmac( 'md5', $timestamp, $session['wsToken'] ) .
                                dechex( $timestamp ) .
-                               User::EDIT_TOKEN_SUFFIX;
+                               MediaWiki\Session\Token::SUFFIX;
 
                        return $this->doApiRequest( $params, $session, false, $user );
                } else {
index efc92f7..858996d 100644 (file)
@@ -199,4 +199,51 @@ class SessionTest extends MediaWikiTestCase {
                $this->assertTrue( $backend->dirty );
        }
 
+       public function testTokens() {
+               $rc = new \ReflectionClass( 'MediaWiki\\Session\\Session' );
+               if ( !method_exists( $rc, 'newInstanceWithoutConstructor' ) ) {
+                       $this->markTestSkipped(
+                               'ReflectionClass::newInstanceWithoutConstructor isn\'t available'
+                       );
+               }
+
+               // Instead of actually constructing the Session, we use reflection to
+               // bypass the constructor and plug a mock SessionBackend into the
+               // private fields to avoid having to actually create a SessionBackend.
+               $backend = new DummySessionBackend;
+               $session = $rc->newInstanceWithoutConstructor();
+               $priv = \TestingAccessWrapper::newFromObject( $session );
+               $priv->backend = $backend;
+               $priv->index = 42;
+
+               $token = \TestingAccessWrapper::newFromObject( $session->getToken() );
+               $this->assertArrayHasKey( 'wsTokenSecrets', $backend->data );
+               $this->assertArrayHasKey( 'default', $backend->data['wsTokenSecrets'] );
+               $secret = $backend->data['wsTokenSecrets']['default'];
+               $this->assertSame( $secret, $token->secret );
+               $this->assertSame( '', $token->salt );
+               $this->assertTrue( $token->wasNew() );
+
+               $token = \TestingAccessWrapper::newFromObject( $session->getToken( 'foo' ) );
+               $this->assertSame( $secret, $token->secret );
+               $this->assertSame( 'foo', $token->salt );
+               $this->assertFalse( $token->wasNew() );
+
+               $backend->data['wsTokenSecrets']['secret'] = 'sekret';
+               $token = \TestingAccessWrapper::newFromObject(
+                       $session->getToken( array( 'bar', 'baz' ), 'secret' )
+               );
+               $this->assertSame( 'sekret', $token->secret );
+               $this->assertSame( 'bar|baz', $token->salt );
+               $this->assertFalse( $token->wasNew() );
+
+               $session->resetToken( 'secret' );
+               $this->assertArrayHasKey( 'wsTokenSecrets', $backend->data );
+               $this->assertArrayHasKey( 'default', $backend->data['wsTokenSecrets'] );
+               $this->assertArrayNotHasKey( 'secret', $backend->data['wsTokenSecrets'] );
+
+               $session->resetAllTokens();
+               $this->assertArrayNotHasKey( 'wsTokenSecrets', $backend->data );
+
+       }
 }
diff --git a/tests/phpunit/includes/session/TokenTest.php b/tests/phpunit/includes/session/TokenTest.php
new file mode 100644 (file)
index 0000000..113f409
--- /dev/null
@@ -0,0 +1,66 @@
+<?php
+
+namespace MediaWiki\Session;
+
+use MediaWikiTestCase;
+
+/**
+ * @group Session
+ * @covers MediaWiki\Session\Token
+ */
+class TokenTest extends MediaWikiTestCase {
+
+       public function testBasics() {
+               $token = $this->getMockBuilder( 'MediaWiki\\Session\\Token' )
+                       ->setMethods( array( 'toStringAtTimestamp' ) )
+                       ->setConstructorArgs( array( 'sekret', 'salty', true ) )
+                       ->getMock();
+               $token->expects( $this->any() )->method( 'toStringAtTimestamp' )
+                       ->will( $this->returnValue( 'faketoken+\\' ) );
+
+               $this->assertSame( 'faketoken+\\', $token->toString() );
+               $this->assertSame( 'faketoken+\\', (string)$token );
+               $this->assertTrue( $token->wasNew() );
+
+               $token = new Token( 'sekret', 'salty', false );
+               $this->assertFalse( $token->wasNew() );
+       }
+
+       public function testToStringAtTimestamp() {
+               $token = \TestingAccessWrapper::newFromObject( new Token( 'sekret', 'salty', false ) );
+
+               $this->assertSame(
+                       'd9ade0c7d4349e9df9094e61c33a5a0d5644fde2+\\',
+                       $token->toStringAtTimestamp( 1447362018 )
+               );
+               $this->assertSame(
+                       'ee2f7a2488dea9176c224cfb400d43be5644fdea+\\',
+                       $token->toStringAtTimestamp( 1447362026 )
+               );
+       }
+
+       public function testGetTimestamp() {
+               $this->assertSame(
+                       1447362018, Token::getTimestamp( 'd9ade0c7d4349e9df9094e61c33a5a0d5644fde2+\\' )
+               );
+               $this->assertSame(
+                       1447362026, Token::getTimestamp( 'ee2f7a2488dea9176c224cfb400d43be5644fdea+\\' )
+               );
+               $this->assertNull( Token::getTimestamp( 'ee2f7a2488dea9176c224cfb400d43be5644fdea-\\' ) );
+               $this->assertNull( Token::getTimestamp( 'ee2f7a2488dea9176c224cfb400d43be+\\' ) );
+
+               $this->assertNull( Token::getTimestamp( 'ee2f7a2488dea9x76c224cfb400d43be5644fdea+\\' ) );
+       }
+
+       public function testMatch() {
+               $token = \TestingAccessWrapper::newFromObject( new Token( 'sekret', 'salty', false ) );
+
+               $test = $token->toStringAtTimestamp( time() - 10 );
+               $this->assertTrue( $token->match( $test ) );
+               $this->assertTrue( $token->match( $test, 12 ) );
+               $this->assertFalse( $token->match( $test, 8 ) );
+
+               $this->assertFalse( $token->match( 'ee2f7a2488dea9176c224cfb400d43be5644fdea-\\' ) );
+       }
+
+}