Bug 978896 - FxA: watch() should get silent assertion if possible. r=ferjm
authorJed Parsons <jedp@mozilla.com>
Tue, 11 Mar 2014 17:49:26 -0700
changeset 191889 52892649259ed84fef1955c352150959638124eb
parent 191888 11685d14b3b4655794c11864d6b6a1dfe153970c
child 191890 af491832ff954900098bac1053d9d59359c984bd
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersferjm
bugs978896
milestone30.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 978896 - FxA: watch() should get silent assertion if possible. r=ferjm
dom/identity/DOMIdentity.jsm
dom/identity/nsDOMIdentity.js
dom/identity/tests/mochitest/file_declareAudience.html
dom/identity/tests/mochitest/test_declareAudience.html
dom/identity/tests/mochitest/test_syntheticEvents.html
services/fxaccounts/FxAccountsManager.jsm
toolkit/identity/FirefoxAccounts.jsm
toolkit/identity/tests/unit/head_identity.js
toolkit/identity/tests/unit/test_firefox_accounts.js
--- a/dom/identity/DOMIdentity.jsm
+++ b/dom/identity/DOMIdentity.jsm
@@ -330,16 +330,17 @@ this.DOMIdentity = {
 
   _watch: function DOMIdentity__watch(message, targetMM) {
     log("DOMIdentity__watch: " + message.id);
     let context = this.newContext(message, targetMM);
     this.getService(message).RP.watch(context);
   },
 
   _unwatch: function DOMIdentity_unwatch(message, targetMM) {
+    log("DOMIDentity__unwatch: " + message.id);
     this.getService(message).RP.unwatch(message.id, targetMM);
   },
 
   _request: function DOMIdentity__request(message) {
     this.getService(message).RP.request(message.id, message);
   },
 
   _logout: function DOMIdentity__logout(message) {
--- a/dom/identity/nsDOMIdentity.js
+++ b/dom/identity/nsDOMIdentity.js
@@ -665,17 +665,17 @@ nsDOMIdentity.prototype = {
     message.audience = _audience;
 
     this._log("DOMIdentityMessage: " + JSON.stringify(message));
 
     return message;
   },
 
   uninit: function DOMIdentity_uninit() {
-    this._log("nsDOMIdentity uninit()");
+    this._log("nsDOMIdentity uninit() " + this._id);
     this._identityInternal._mm.sendAsyncMessage(
       "Identity:RP:Unwatch",
       { id: this._id }
     );
   }
 
 };
 
--- a/dom/identity/tests/mochitest/file_declareAudience.html
+++ b/dom/identity/tests/mochitest/file_declareAudience.html
@@ -36,16 +36,20 @@
 
   onmessage = function(event) {
     navigator.mozId.watch({
       wantIssuer: "firefox-accounts",
       audience: event.data.audience,
       onready: onready,
       onlogin: onlogin,
       onerror: onerror,
-      onlogout: function() {},
+
+      // onlogout will actually be called every time watch() is invoked,
+      // because fxa will find no signed-in user and so trigger logout.
+      // For this test, though, we don't care and just ignore logout.
+      onlogout: function () {},
     });
   };
 
 </script>
 </div>
 </body>
 </html>
--- a/dom/identity/tests/mochitest/test_declareAudience.html
+++ b/dom/identity/tests/mochitest/test_declareAudience.html
@@ -29,17 +29,23 @@ Components.utils.import("resource://gre/
 is("appStatus" in document.nodePrincipal, true,
    "appStatus should be present in nsIPrincipal, if not the rest of this test will fail");
 
 // Mock the Firefox Accounts manager to generate a keypair and provide a fake
 // cert for the caller on each getAssertion request.
 function MockFXAManager() {}
 
 MockFXAManager.prototype = {
-  getAssertion: function(audience) {
+  getAssertion: function(audience, options) {
+    // Always reject a request for a silent assertion, simulating the
+    // scenario in which there is no signed-in user to begin with.
+    if (options.silent) {
+      return Promise.resolve(null);
+    }
+
     let deferred = Promise.defer();
     jwcrypto.generateKeyPair("DS160", (err, kp) => {
       if (err) {
         return deferred.reject(err);
       }
       jwcrypto.generateAssertion("fake-cert", kp, audience, (err, assertion) => {
         if (err) {
           return deferred.reject(err);
@@ -132,17 +138,17 @@ let testRunner = runTest();
 // have more than one message from the onerror handler in the client.  So we keep
 // track of received errors; once they reach the expected count, we are done.
 function receiveMessage(event) {
   let result = JSON.parse(event.data);
   let app = apps[appIndex];
   let expected = app.expected;
 
   is(result.success, expected.success,
-     "Assertion request " + (expected.success ? "succeeds" : "fails"));
+     "Assertion request succeeds");
 
   if (expected.success) {
     // Confirm that the assertion audience and origin are as expected
     let components = extractAssertionComponents(result.backedAssertion);
     is(components.payload.aud, app.wantAudience || app.origin,
        "Got desired assertion audience");
 
   } else {
@@ -175,17 +181,16 @@ function receiveMessage(event) {
   }
 }
 
 window.addEventListener("message", receiveMessage, false, true);
 
 function runTest() {
   for (let app of apps) {
     dump("** Testing " + app.title + "\n");
-
     // Set up state for message handler
     expectedErrors = 0;
     receivedErrors = [];
     if (!app.expected.success) {
       expectedErrors += 1;
     }
     if (app.expected.underprivileged) {
       expectedErrors += 1;
--- a/dom/identity/tests/mochitest/test_syntheticEvents.html
+++ b/dom/identity/tests/mochitest/test_syntheticEvents.html
@@ -26,17 +26,20 @@ Components.utils.import("resource://gre/
 Components.utils.import("resource://gre/modules/identity/jwcrypto.jsm");
 Components.utils.import("resource://gre/modules/identity/FirefoxAccounts.jsm");
 
 // Mock the Firefox Accounts manager to give a dummy assertion, just to confirm
 // that we're making the trip through the dom/identity and toolkit/identity
 // plumbing.
 function MockFXAManager() {}
 MockFXAManager.prototype = {
-  getAssertion: function() {
+  getAssertion: function(audience, options) {
+    if (options.silent) {
+      return Promise.resolve(null);
+    }
     return Promise.resolve("here~you.go.dude");
   }
 };
 
 let originalManager = FirefoxAccounts.fxAccountsManager;
 FirefoxAccounts.fxAccountsManager = new MockFXAManager();
 
 // Mock IdentityService (Persona) so we can test request() while not handling
--- a/services/fxaccounts/FxAccountsManager.jsm
+++ b/services/fxaccounts/FxAccountsManager.jsm
@@ -351,18 +351,28 @@ this.FxAccountsManager = {
         }
         log.debug(JSON.stringify(this._user));
         return Promise.resolve(this._user);
       },
       reason => { return this._serverError(reason); }
     );
   },
 
+  /*
+   * Try to get an assertion for the given audience.
+   *
+   * aOptions can include:
+   *
+   *   refreshAuthentication  - (bool) Force re-auth.
+   *
+   *   silent                 - (bool) Prevent any UI interaction.
+   *                            I.e., try to get an automatic assertion.
+   *
+   */
   getAssertion: function(aAudience, aOptions) {
-    log.debug("getAssertion " + aAudience + JSON.stringify(aOptions));
     if (!aAudience) {
       return this._error(ERROR_INVALID_AUDIENCE);
     }
 
     if (Services.io.offline) {
       return this._error(ERROR_OFFLINE);
     }
 
@@ -385,27 +395,35 @@ this.FxAccountsManager = {
             }
 
             if ((Date.now() / 1000) - this._activeSession.authAt > gracePeriod) {
               // Grace period expired, so we sign out and request the user to
               // authenticate herself again. If the authentication succeeds, we
               // will return the assertion. Otherwise, we will return an error.
               return this._signOut().then(
                 () => {
+                  if (aOptions.silent) {
+                    return Promise.resolve(null);
+                  }
                   return this._uiRequest(UI_REQUEST_REFRESH_AUTH,
                                          aAudience, user.accountId);
                 }
               );
             }
           }
 
           return this._getAssertion(aAudience);
         }
 
         log.debug("No signed in user");
+
+        if (aOptions.silent) {
+          return Promise.resolve(null);
+        }
+
         // If there is no currently signed in user, we trigger the signIn UI
         // flow.
         return this._uiRequest(UI_REQUEST_SIGN_IN_FLOW, aAudience);
       }
     );
   }
 
 };
--- a/toolkit/identity/FirefoxAccounts.jsm
+++ b/toolkit/identity/FirefoxAccounts.jsm
@@ -82,30 +82,48 @@ FxAccountsService.prototype = {
    *                  - doLogin()
    *                  - doLogout()
    *                  - doError()
    *                  - doCancel()
    *
    */
   watch: function watch(aRpCaller) {
     this._rpFlows.set(aRpCaller.id, aRpCaller);
+    log.debug("watch: " + aRpCaller.id);
     log.debug("Current rp flows: " + this._rpFlows.size);
 
-    // Nothing to do but call ready()
+    // Log the user in, if possible, and then call ready().
     let runnable = {
       run: () => {
-        this.doReady(aRpCaller.id);
+        this.fxAccountsManager.getAssertion(aRpCaller.audience, {silent:true}).then(
+          data => {
+            if (data) {
+              this.doLogin(aRpCaller.id, data);
+            } else {
+              this.doLogout(aRpCaller.id);
+            }
+            this.doReady(aRpCaller.id);
+          },
+          error => {
+            log.error("get silent assertion failed: " + JSON.stringify(error));
+            this.doError(aRpCaller.id, error.toString());
+          }
+        );
       }
     };
     Services.tm.currentThread.dispatch(runnable,
                                        Ci.nsIThread.DISPATCH_NORMAL);
   },
 
-  unwatch: function(aRpCaller, aTargetMM) {
-    // nothing to do
+  /**
+   * Delete the flow when the screen is unloaded
+   */
+  unwatch: function(aRpCallerId, aTargetMM) {
+    log.debug("unwatching: " + aRpCallerId);
+    this._rpFlows.delete(aRpCallerId);
   },
 
   /**
    * Initiate a login with user interaction as a result of a call to
    * navigator.id.request().
    *
    * @param aRPId
    *        (integer)  the id of the doc object obtained in .watch()
@@ -155,17 +173,19 @@ FxAccountsService.prototype = {
     if (!this._rpFlows.has(aRpCallerId)) {
       log.error("logout() called before watch()");
       return;
     }
 
     // Call logout() on the next tick
     let runnable = {
       run: () => {
-        this.doLogout(aRpCallerId);
+        this.fxAccountsManager.signOut().then(() => {
+          this.doLogout(aRpCallerId);
+        });
       }
     };
     Services.tm.currentThread.dispatch(runnable,
                                        Ci.nsIThread.DISPATCH_NORMAL);
   },
 
   childProcessShutdown: function childProcessShutdown(messageManager) {
     for (let [key,] of this._rpFlows) {
--- a/toolkit/identity/tests/unit/head_identity.js
+++ b/toolkit/identity/tests/unit/head_identity.js
@@ -4,17 +4,17 @@
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 const Cr = Components.results;
 
 Cu.import("resource://testing-common/httpd.js");
 
 // XXX until bug 937114 is fixed
-Cu.importGlobalProperties(['atob']);
+Cu.importGlobalProperties(["atob"]);
 
 // The following boilerplate makes sure that XPCom calls
 // that use the profile directory work.
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "jwcrypto",
@@ -38,17 +38,17 @@ XPCOMUtils.defineLazyServiceGetter(this,
                                    "nsIUUIDGenerator");
 
 const TEST_MESSAGE_MANAGER = "Mr McFeeley";
 const TEST_URL = "https://myfavoritebacon.com";
 const TEST_URL2 = "https://myfavoritebaconinacan.com";
 const TEST_USER = "user@mozilla.com";
 const TEST_PRIVKEY = "fake-privkey";
 const TEST_CERT = "fake-cert";
-const TEST_ASSERTION = "face-assertion";
+const TEST_ASSERTION = "fake-assertion";
 const TEST_IDPPARAMS = {
   domain: "myfavoriteflan.com",
   authentication: "/foo/authenticate.html",
   provisioning: "/foo/provision.html"
 };
 
 // The following are utility functions for Identity testing
 
@@ -67,18 +67,18 @@ function partial(fn) {
   };
 }
 
 function uuid() {
   return uuidGenerator.generateUUID().toString();
 }
 
 function base64UrlDecode(s) {
-  s = s.replace(/-/g, '+');
-  s = s.replace(/_/g, '/');
+  s = s.replace(/-/g, "+");
+  s = s.replace(/_/g, "/");
 
   // Replace padding if it was stripped by the sender.
   // See http://tools.ietf.org/html/rfc4648#section-4
   switch (s.length % 4) {
     case 0:
       break; // No pad chars in this case
     case 2:
       s += "==";
@@ -96,25 +96,25 @@ function base64UrlDecode(s) {
 
 // create a mock "doc" object, which the Identity Service
 // uses as a pointer back into the doc object
 function mock_doc(aIdentity, aOrigin, aDoFunc) {
   let mockedDoc = {};
   mockedDoc.id = uuid();
   mockedDoc.loggedInUser = aIdentity;
   mockedDoc.origin = aOrigin;
-  mockedDoc['do'] = aDoFunc;
+  mockedDoc["do"] = aDoFunc;
   mockedDoc._mm = TEST_MESSAGE_MANAGER;
-  mockedDoc.doReady = partial(aDoFunc, 'ready');
-  mockedDoc.doLogin = partial(aDoFunc, 'login');
-  mockedDoc.doLogout = partial(aDoFunc, 'logout');
-  mockedDoc.doError = partial(aDoFunc, 'error');
-  mockedDoc.doCancel = partial(aDoFunc, 'cancel');
-  mockedDoc.doCoffee = partial(aDoFunc, 'coffee');
-  mockedDoc.childProcessShutdown = partial(aDoFunc, 'child-process-shutdown');
+  mockedDoc.doReady = partial(aDoFunc, "ready");
+  mockedDoc.doLogin = partial(aDoFunc, "login");
+  mockedDoc.doLogout = partial(aDoFunc, "logout");
+  mockedDoc.doError = partial(aDoFunc, "error");
+  mockedDoc.doCancel = partial(aDoFunc, "cancel");
+  mockedDoc.doCoffee = partial(aDoFunc, "coffee");
+  mockedDoc.childProcessShutdown = partial(aDoFunc, "child-process-shutdown");
 
   mockedDoc.RP = mockedDoc;
 
   return mockedDoc;
 }
 
 function mock_fxa_rp(aIdentity, aOrigin, aDoFunc) {
   let mockedDoc = {};
@@ -122,19 +122,19 @@ function mock_fxa_rp(aIdentity, aOrigin,
   mockedDoc.emailHint = aIdentity;
   mockedDoc.origin = aOrigin;
   mockedDoc.wantIssuer = "firefox-accounts";
   mockedDoc._mm = TEST_MESSAGE_MANAGER;
 
   mockedDoc.doReady = partial(aDoFunc, "ready");
   mockedDoc.doLogin = partial(aDoFunc, "login");
   mockedDoc.doLogout = partial(aDoFunc, "logout");
-  mockedDoc.doError = partial(aDoFunc, 'error');
-  mockedDoc.doCancel = partial(aDoFunc, 'cancel');
-  mockedDoc.childProcessShutdown = partial(aDoFunc, 'child-process-shutdown');
+  mockedDoc.doError = partial(aDoFunc, "error");
+  mockedDoc.doCancel = partial(aDoFunc, "cancel");
+  mockedDoc.childProcessShutdown = partial(aDoFunc, "child-process-shutdown");
 
   mockedDoc.RP = mockedDoc;
 
   return mockedDoc;
 }
 
 // mimicking callback funtionality for ease of testing
 // this observer auto-removes itself after the observe function
--- a/toolkit/identity/tests/unit/test_firefox_accounts.js
+++ b/toolkit/identity/tests/unit/test_firefox_accounts.js
@@ -9,23 +9,29 @@ Cu.import("resource://gre/modules/DOMIde
 XPCOMUtils.defineLazyModuleGetter(this, "FirefoxAccounts",
                                   "resource://gre/modules/identity/FirefoxAccounts.jsm");
 
 // Make the profile dir available; this is necessary so that
 // services/fxaccounts/FxAccounts.jsm can read and write its signed-in user
 // data.
 do_get_profile();
 
-function MockFXAManager() {}
+function MockFXAManager() {
+  this.signedIn = true;
+}
 MockFXAManager.prototype = {
   getAssertion: function(audience) {
-    let deferred = Promise.defer();
-    deferred.resolve(TEST_ASSERTION);
-    return deferred.promise;
-  }
+    let result = this.signedIn ? TEST_ASSERTION : null;
+    return Promise.resolve(result);
+  },
+
+  signOut: function() {
+    this.signedIn = false;
+    return Promise.resolve(null);
+  },
 }
 
 let originalManager = FirefoxAccounts.fxAccountsManager;
 FirefoxAccounts.fxAccountsManager = new MockFXAManager();
 do_register_cleanup(() => {
   log("restoring fxaccountsmanager");
   FirefoxAccounts.fxAccountsManager = originalManager;
 });
@@ -40,76 +46,122 @@ function test_mock() {
 
   FirefoxAccounts.fxAccountsManager.getAssertion().then(assertion => {
     do_check_eq(assertion, TEST_ASSERTION);
     do_test_finished();
     run_next_test();
   });
 }
 
-function test_watch() {
+function test_watch_signed_in() {
   do_test_pending();
 
+  let received = [];
+
+  let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, data) {
+    received.push([method, data]);
+
+    if (method == "ready") {
+      // confirm that we were signed in and then ready was called
+      do_check_eq(received.length, 2);
+      do_check_eq(received[0][0], "login");
+      do_check_eq(received[0][1], TEST_ASSERTION);
+      do_check_eq(received[1][0], "ready");
+      do_test_finished();
+      run_next_test();
+    }
+  });
+
+  FirefoxAccounts.RP.watch(mockedRP);
+}
+
+function test_watch_signed_out() {
+  do_test_pending();
+
+  let received = [];
+  let signedInState = FirefoxAccounts.fxAccountsManager.signedIn;
+  FirefoxAccounts.fxAccountsManager.signedIn = false;
+
   let mockedRP = mock_fxa_rp(null, TEST_URL, function(method) {
-    do_check_eq(method, "ready");
-    do_test_finished();
-    run_next_test();
+    received.push(method);
+
+    if (method == "ready") {
+      // confirm that we were signed out and then ready was called
+      do_check_eq(received.length, 2);
+      do_check_eq(received[0], "logout");
+      do_check_eq(received[1], "ready");
+
+      // restore initial state
+      FirefoxAccounts.fxAccountsManager.signedIn = signedInState;
+      do_test_finished();
+      run_next_test();
+    }
   });
 
   FirefoxAccounts.RP.watch(mockedRP);
 }
 
 function test_request() {
   do_test_pending();
 
   let received = [];
 
-  let mockedRP = mock_fxa_rp(null, TEST_URL, function(method) {
-    // We will received "ready" as a result of watch(), then "login"
-    // as a result of request()
-    received.push(method);
+  // initially signed out
+  let signedInState = FirefoxAccounts.fxAccountsManager.signedIn;
+  FirefoxAccounts.fxAccountsManager.signedIn = false;
+
+  let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, data) {
+    received.push([method, data]);
+
+    // On watch(), we are signed out.  Then we call request().
+    if (received.length === 2) {
+      do_check_eq(received[0][0], "logout");
+      do_check_eq(received[1][0], "ready");
 
-    if (received.length == 2) {
-      do_check_eq(received[0], "ready");
-      do_check_eq(received[1], "login");
+      // Pretend request() showed ux and the user signed in
+      FirefoxAccounts.fxAccountsManager.signedIn = true;
+      FirefoxAccounts.RP.request(mockedRP.id);
+    }
+
+    if (received.length === 3) {
+      do_check_eq(received[2][0], "login");
+      do_check_eq(received[2][1], TEST_ASSERTION);
+
+      // restore initial state
+      FirefoxAccounts.fxAccountsManager.signedIn = signedInState;
       do_test_finished();
       run_next_test();
     }
-
-    // Second, call request()
-    if (method == "ready") {
-      FirefoxAccounts.RP.request(mockedRP.id);
-    }
   });
 
   // First, call watch()
   FirefoxAccounts.RP.watch(mockedRP);
 }
 
 function test_logout() {
   do_test_pending();
 
   let received = [];
 
   let mockedRP = mock_fxa_rp(null, TEST_URL, function(method) {
-    // We will receive "ready" as a result of watch(), and "logout"
-    // as a result of logout()
     received.push(method);
 
-    if (received.length == 2) {
-      do_check_eq(received[0], "ready");
-      do_check_eq(received[1], "logout");
+    // At first, watch() signs us in automatically.  Then we sign out.
+    if (received.length === 2) {
+      do_check_eq(received[0], "login");
+      do_check_eq(received[1], "ready");
+
+      FirefoxAccounts.RP.logout(mockedRP.id);
+    }
+
+    if (received.length === 3) {
+      do_check_eq(received[2], "logout");
       do_test_finished();
       run_next_test();
     }
-
-    if (method == "ready") {
-      // Second, call logout()
-      FirefoxAccounts.RP.logout(mockedRP.id);
-    }
   });
 
   // First, call watch()
   FirefoxAccounts.RP.watch(mockedRP);
 }
 
 function test_error() {
   do_test_pending();
@@ -117,41 +169,31 @@ function test_error() {
   let received = [];
 
   // Mock the fxAccountsManager so that getAssertion rejects its promise and
   // triggers our onerror handler.  (This is the method that's used internally
   // by FirefoxAccounts.RP.request().)
   let originalManager = FirefoxAccounts.fxAccountsManager;
   FirefoxAccounts.RP.fxAccountsManager = {
     getAssertion: function(audience) {
-      return Promise.reject("barf!");
+      return Promise.reject(new Error("barf!"));
     }
   };
 
   let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, message) {
-    // We will receive "ready" as a result of watch(), and "logout"
-    // as a result of logout()
-    received.push([method, message]);
-
-    if (received.length == 2) {
-      do_check_eq(received[0][0], "ready");
-
-      do_check_eq(received[1][0], "error");
-      do_check_eq(received[1][1], "barf!");
+    // We will immediately receive an error, due to watch()'s attempt
+    // to getAssertion().
+    do_check_eq(method, "error");
+    do_check_true(/barf/.test(message));
 
-      // Put things back the way they were
-      FirefoxAccounts.fxAccountsManager = originalManager;
+    // Put things back the way they were
+    FirefoxAccounts.fxAccountsManager = originalManager;
 
-      do_test_finished();
-      run_next_test();
-    }
-
-    if (method == "ready") {
-      FirefoxAccounts.RP.request(mockedRP.id);
-    }
+    do_test_finished();
+    run_next_test();
   });
 
   // First, call watch()
   FirefoxAccounts.RP.watch(mockedRP);
 }
 
 function test_child_process_shutdown() {
   do_test_pending();
@@ -192,17 +234,18 @@ function test_child_process_shutdown() {
 
   // fake a dom window context
   DOMIdentity.newContext(mockedRP, mockedRP._mm);
 }
 
 let TESTS = [
   test_overall,
   test_mock,
-  test_watch,
+  test_watch_signed_in,
+  test_watch_signed_out,
   test_request,
   test_logout,
   test_error,
   test_child_process_shutdown,
 ];
 
 TESTS.forEach(add_test);