Merge "Allow for time-limited tokens"

diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index 48d7b05..322e664 100644 (file)
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -12,6 +12,10 @@ production.
 * $wgPageShowWatchingUsers was removed.
 * $wgLocalVirtualHosts has been added to replace $wgConf->localVHosts.
 * $wgAntiLockFlags was removed.
+* Edit tokens returned from User::getEditToken may change on every call. Token
+  validity must be checked by passing the user-supplied token to
+  User::matchEditToken rather than by testing for equality with a
+  newly-generated token.
 
 === New features in 1.25 ===
 * (bug 58139) ResourceLoaderFileModule now supports language fallback
@@ -33,6 +37,8 @@ production.
   - Usage instructions: https://www.mediawiki.org/wiki/Help:Page_status_indicators
   - Adjusting custom skins to support indicators:
     https://www.mediawiki.org/wiki/Manual:Skinning#Page_status_indicators
+* Edit tokens may now be time-limited: passing a maximum age to
+  User::matchEditToken will reject any older tokens.
 
 === Bug fixes in 1.25 ===
 * (bug 71003) No additional code will be generated to try to load CSS-embedded

diff --git a/includes/User.php b/includes/User.php
index 2fa6849..90d33fb 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -3877,22 +3877,15 @@ class User implements IDBAccessObject {
        }
 
        /**
-        * 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.
-        *
-        * @since 1.19
+        * Internal implementation for self::getEditToken() and
+        * self::matchEditToken().
         *
-        * @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
+        * @param string|array $salt
+        * @param WebRequest $request
+        * @param string|int $timestamp
+        * @return string
         */
-       public function getEditToken( $salt = '', $request = null ) {
-               if ( $request == null ) {
-                       $request = $this->getRequest();
-               }
-
+       private function getEditTokenAtTimestamp( $salt, $request, $timestamp ) {
                if ( $this->isAnon() ) {
                        return self::EDIT_TOKEN_SUFFIX;
                } else {
@@ -3904,10 +3897,30 @@ class User implements IDBAccessObject {
                        if ( is_array( $salt ) ) {
                                $salt = implode( '|', $salt );
                        }
-                       return md5( $token . $salt ) . self::EDIT_TOKEN_SUFFIX;
+                       return hash_hmac( 'md5', $timestamp . $salt, $token, false ) .
+                               dechex( $timestamp ) .
+                               self::EDIT_TOKEN_SUFFIX;
                }
        }
 
+       /**
+        * 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.
+        *
+        * @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()
+               );
+       }
+
        /**
         * Generate a looking random token for various uses.
         *
@@ -3928,15 +3941,34 @@ class User implements IDBAccessObject {
         * @param string $val Input value to compare
         * @param string $salt Optional function-specific data for hashing
         * @param WebRequest|null $request Object to use or null to use $wgRequest
+        * @param int $maxage Fail tokens older than this, in seconds
         * @return bool Whether the token matches
         */
-       public function matchEditToken( $val, $salt = '', $request = null ) {
-               $sessionToken = $this->getEditToken( $salt, $request );
+       public function matchEditToken( $val, $salt = '', $request = null, $maxage = null ) {
+               if ( $this->isAnon() ) {
+                       return $val === self::EDIT_TOKEN_SUFFIX;
+               }
+
+               $suffixLen = strlen( self::EDIT_TOKEN_SUFFIX );
+               if ( strlen( $val ) <= 32 + $suffixLen ) {
+                       return false;
+               }
+
+               $timestamp = hexdec( substr( $val, 32, -$suffixLen ) );
+               if ( $maxage !== null && $timestamp < wfTimestamp() - $maxage ) {
+                       // Expired token
+                       return false;
+               }
+
+               $sessionToken = $this->getEditTokenAtTimestamp(
+                       $salt, $request ?: $this->getRequest(), $timestamp
+               );
+
                if ( $val != $sessionToken ) {
                        wfDebug( "User::matchEditToken: broken session data\n" );
                }
 
-               return $val == $sessionToken;
+               return hash_equals( $sessionToken, $val );
        }
 
        /**
@@ -3946,11 +3978,12 @@ class User implements IDBAccessObject {
         * @param string $val Input value to compare
         * @param string $salt Optional function-specific data for hashing
         * @param WebRequest|null $request Object to use or null to use $wgRequest
+        * @param int $maxage Fail tokens older than this, in seconds
         * @return bool Whether the token matches
         */
-       public function matchEditTokenNoSuffix( $val, $salt = '', $request = null ) {
-               $sessionToken = $this->getEditToken( $salt, $request );
-               return substr( $sessionToken, 0, 32 ) == substr( $val, 0, 32 );
+       public function matchEditTokenNoSuffix( $val, $salt = '', $request = null, $maxage = null ) {
+               $val = substr( $val, 0, strspn( $val, '0123456789abcdef' ) ) . self::EDIT_TOKEN_SUFFIX;
+               return $this->matchEditToken( $val, $salt, $request, $maxage );
        }
 
        /**

diff --git a/tests/phpunit/includes/api/ApiTestCase.php b/tests/phpunit/includes/api/ApiTestCase.php
index 811768d..9a552fa 100644 (file)
--- a/tests/phpunit/includes/api/ApiTestCase.php
+++ b/tests/phpunit/includes/api/ApiTestCase.php
@@ -139,10 +139,14 @@ 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'];
                        // add token to request parameters
-                       $params['token'] = md5( $session['wsToken'] ) . User::EDIT_TOKEN_SUFFIX;
+                       $timestamp = wfTimestamp();
+                       $params['token'] = hash_hmac( 'md5', $timestamp, $session['wsToken'] ) .
+                               dechex( $timestamp ) .
+                               User::EDIT_TOKEN_SUFFIX;
 
                        return $this->doApiRequest( $params, $session, false, $user );
                } else {