commit c88a07385288e5a3080faa6ba495519d3cb491f8
author Alex Fandrianto <alexfandrianto@google.com> Tue May 12 11:18:37 2015 -0700
committer Alex Fandrianto <alexfandrianto@google.com> Tue May 12 11:19:50 2015 -0700
tree f6565fba053c3dbee457ed8183cd435fb61c2988
parent 3875f46123533b22f0d54354c7e0d4f2ba1c3ad7

js/core: Extension Caveat tab race fix

Fix race with the account name.
By "locking" early (not in a callback) and setting the account name
immediately, we ensure that any later getCaveats calls will reuse the
same outstanding request tab.

Removed unnecessary the authState flag and corresponding check.
Was this needed? It's a random value, so any (racing) tabs
won't know it's actual value. We also have requestId if we need things
to be unique.

Removed unnecessary _lastRequestId and requestId. The former was an
internal tracking variable that was unused. The latter was sent in
the url, but was also unused.

(Tested on HelloPeer using the build-dev extension.)

Change-Id: I2e9fd7dac3191c3e3c91b5e49aaf6d5cf24eaa42

extension/src/background/addcaveats.js

diff --git a/extension/src/background/addcaveats.js b/extension/src/background/addcaveats.js
index 1852579..7059519 100644
--- a/extension/src/background/addcaveats.js
+++ b/extension/src/background/addcaveats.js
@@ -56,10 +56,8 @@
   var backgroundPort = chrome.runtime.connect();
   backgroundPort.postMessage({
     type: 'assocAccount:finish',
-    requestId: parseInt(params.requestId),
     origin: params.origin,
     caveats: caveats,
-    authState: params.authState,
     cancel: cancel
   });
   backgroundPort.disconnect();

extension/src/background/auth-handler.js

diff --git a/extension/src/background/auth-handler.js b/extension/src/background/auth-handler.js
index 601f280..90b3815 100644
--- a/extension/src/background/auth-handler.js
+++ b/extension/src/background/auth-handler.js
@@ -7,7 +7,6 @@
  */
 
 var getOrigin = require('./util').getOrigin;
-var random = require('../../../src/lib/random');
 
 module.exports = AuthHandler;
 
@@ -18,9 +17,6 @@
 
   this._channel = channel;
 
-  // Auth request id, incremented on each auth request.
-  this._lastRequestId = 0;
-
   // Map from origins to the Vanadium app ports of tabs with active requests.
   // This keeps track of existing auth tabs for an origin so new ones are not
   // started.
@@ -126,6 +122,9 @@
 
 // Pop up a new tab asking the user to chose their caveats.
 AuthHandler.prototype.getCaveats = function(account, origin, appPort) {
+  // Store the account name on the appPort.
+  appPort.account = account;
+
   var outstandingAuthRequests = this._outstandingAuthRequests;
   var caveatTabOrigins = this._caveatTabOrigins;
   if (origin in this._outstandingAuthRequests) {
@@ -142,13 +141,7 @@
 
     return;
   }
-
-  this._lastRequestId++;
-  var requestId = this._lastRequestId;
-
-  // Store the account name, random salt, and timestamp on the port.
-  appPort.account = account;
-  appPort.authState = random.hex();
+  outstandingAuthRequests[origin] = [appPort];
 
   // Get  currently active tab in the window.
   var windowId = appPort.sender.tab.windowId;
@@ -161,11 +154,9 @@
     }
 
     chrome.tabs.create({
-      url: chrome.extension.getURL('html/addcaveats.html') + '?requestId=' +
-        requestId + '&origin=' + encodeURIComponent(origin) +
-        '&authState=' + appPort.authState
+      url: chrome.extension.getURL('html/addcaveats.html') + '?origin=' +
+        encodeURIComponent(origin)
     }, function(tab) {
-      outstandingAuthRequests[origin] = [appPort];
       caveatTabOrigins[tab.id] = origin;
     });
   });
@@ -291,11 +282,6 @@
           new Error('Invalid origin.'));
     }
 
-    if (msg.authState !== appPort.authState) {
-      return sendErrorToContentScript('auth', appPort,
-          new Error('Port not authorized.'));
-    }
-
     if (!appPort.account) {
       return sendErrorToContentScript('auth', appPort,
           new Error('No port.account.'));