From: Nuria Ruiz <nuria@wikimedia.org>
Date: Sat, 31 Jan 2015 03:50:09 +0000 (-0800)
Subject: Using cryptoAPI if available in generateRandomSessionId
X-Git-Tag: 1.31.0-rc.0~12413^2
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/journal.php?a=commitdiff_plain;h=4860ea3ca6fb922608ba37b37acccd5987e7513f;p=lhc%2Fweb%2Fwiklou.git

Using cryptoAPI if available in generateRandomSessionId

BREAKING CHANGE:
The alphabet of the prior string returned was
A-Za-z0-9 and now it is 0-9A-F

Bug: T78449

Change-Id: I71b5ccc5887b842485971917809d1eb01bc56a90
---

diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index 3f24db013a..070e60a282 100644
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -344,6 +344,8 @@ changes to languages because of Bugzilla reports.
     Instead, do this:
       $form = HTMLForm::factory( 'vform', … );
 * Deprecated Revision methods getRawUser(), getRawUserText() and getRawComment().
+* BREAKING CHANGE: mediawiki.user.generateRandomSessionId:
+  The alphabet of the prior string returned was A-Za-z0-9 and now it is 0-9A-F
 
 == Compatibility ==
 
diff --git a/resources/src/mediawiki/mediawiki.user.js b/resources/src/mediawiki/mediawiki.user.js
index 5e0cfcc238..c3ec3f3778 100644
--- a/resources/src/mediawiki/mediawiki.user.js
+++ b/resources/src/mediawiki/mediawiki.user.js
@@ -3,13 +3,20 @@
  * @singleton
  */
 ( function ( mw, $ ) {
-	var user,
+	var user, i,
 		deferreds = {},
+		byteToHex = [],
 		// Extend the skeleton mw.user from mediawiki.js
 		// This is kind of ugly but we're stuck with this for b/c reasons
 		options = mw.user.options || new mw.Map(),
 		tokens = mw.user.tokens || new mw.Map();
 
+	// Maps for number -> hex string conversion (with padding)
+	// idea from: https://github.com/broofa/node-uuid/blob/master/uuid.js
+	for ( i = 0; i < 256; i++ ) {
+		byteToHex[i] = (i + 0x100).toString(16).substr(1);
+	}
+
 	/**
 	 * Get the current user's groups or rights
 	 *
@@ -49,22 +56,52 @@
 		tokens: tokens,
 
 		/**
-		 * Generate a random user session ID (32 alpha-numeric characters)
-		 *
+		 * Generate a random user session ID
 		 * This information would potentially be stored in a cookie to identify a user during a
-		 * session or series of sessions. Its uniqueness should not be depended on.
+		 * session or series of sessions. Its uniqueness should
+		 * not be depended on unless the browser supports the crypto API.
+		 *
+		 * Known problems with Math.random():
+		 * Using the Math.random function we have seen sets
+		 * with 1% of non uniques among 200.000 values with Safari providing most of these.
+		 * Given the prevalence of Safari in mobile the percentage of duplicates in
+		 * mobile usages of this code is probably higher.
+		 *
+		 * Rationale:
+		 * We need about 64 bits to make sure that probability of collision
+		 * on 500 million (5*10^8) is <= 1%
+		 * See: https://en.wikipedia.org/wiki/Birthday_problem#Probability_table
 		 *
-		 * @return {string} Random set of 32 alpha-numeric characters
+		 * @return {string} 64 bit integer in hex format, padded
 		 */
 		generateRandomSessionId: function () {
-			var i, r,
-				id = '',
-				seed = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
-			for ( i = 0; i < 32; i++ ) {
-				r = Math.floor( Math.random() * seed.length );
-				id += seed.charAt( r );
+			/*jshint bitwise:false */
+			var rnds, i, r, cryptoObj, hexRnds = new Array( 8 );
+			cryptoObj = window.crypto || window.msCrypto; // for IE 11
+
+			if ( cryptoObj ) {
+				// We fill an array with 8 random values, each of which is 8 bits.
+				// note that rnds is an array-like object not a true array
+				rnds = new Uint8Array( 8 );
+				cryptoObj.getRandomValues( rnds );
+			} else {
+				rnds = new Array( 8 );
+				// From: https://github.com/broofa/node-uuid/blob/master/uuid.js
+				for ( i = 0, r; i < 8; i++ ) {
+					if ( ( i & 0x03 ) === 0 ) {
+						r = Math.random() * 0x100000000;
+					}
+					rnds[i] = r >>> ( ( i & 0x03 ) << 3 ) & 0xff;
+				}
 			}
-			return id;
+			// convert to hex using byteToHex that already contains padding
+			for ( i = 0; i < rnds.length; i++ ) {
+				hexRnds[i] = byteToHex[rnds[i]];
+			}
+
+			// concatenation of two random integers with entrophy n and m
+			// returns a string with entrophy n+m if those strings are independent
+			return hexRnds.join( '' );
 		},
 
 		/**
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
index 91321a2fc9..fe22b7ab39 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
@@ -51,4 +51,22 @@
 
 		this.server.respond();
 	} );
+
+	QUnit.test( 'session numbers', 4, function ( assert ) {
+		/*global $:false */
+		var sessionId = mw.user.generateRandomSessionId(),
+		cryptoObj = window.crypto;
+
+		assert.equal( typeof sessionId, 'string', 'generateRandomSessionId should return a string' );
+		assert.equal( $.trim(sessionId), sessionId, 'generateRandomSessionId should not contain whitespace' );
+		// pretend crypto API is not there and do same test, make sure code runs
+		// through  Math.random loop
+		window.crypto = undefined;
+		sessionId =  mw.user.generateRandomSessionId();
+		assert.equal( typeof sessionId, 'string', 'generateRandomSessionId should return a string' );
+		assert.equal( sessionId.trim(), sessionId, 'generateRandomSessionId should not be empty' );
+		//restoring crypto object
+		window.crypto = cryptoObj;
+
+	} );
 }( mediaWiki ) );