Bug 1045738 - Allow FxA to sign an assertion while offline if its certificate is viable. r=ferjm, a=bajaj
authorSam Penrose <spenrose@mozilla.com>
Tue, 07 Oct 2014 16:09:54 -0700
changeset 220604 cb72824df1d0de47709248df36f72622ead9a374
parent 220603 a41c0b832692c099c407ace2d8e3b2fa9cfcf7a9
child 220605 f0c35946500cd4eb40b2f0af4595de677d7358ad
push id11
push userryanvm@gmail.com
push dateTue, 14 Oct 2014 20:30:27 +0000
treeherdermozilla-b2g34_v2_1@cb72824df1d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersferjm, bajaj
bugs1045738
milestone34.0
Bug 1045738 - Allow FxA to sign an assertion while offline if its certificate is viable. r=ferjm, a=bajaj
services/fxaccounts/FxAccounts.jsm
services/fxaccounts/FxAccountsManager.jsm
services/fxaccounts/tests/xpcshell/test_accounts.js
services/fxaccounts/tests/xpcshell/test_manager.js
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -141,17 +141,21 @@ AccountState.prototype = {
       // check with param substitutions added in bug 966674
       log.debug("getCertificate" + JSON.stringify(this.signedInUser));
     }
     // TODO: get the lifetime from the cert's .exp field
     if (this.cert && this.cert.validUntil > mustBeValidUntil) {
       log.debug(" getCertificate already had one");
       return this.resolve(this.cert.cert);
     }
-    // else get our cert signed
+
+    if (Services.io.offline) {
+      return this.reject(new Error(ERROR_OFFLINE));
+    }
+
     let willBeValidUntil = this.fxaInternal.now() + CERT_LIFETIME;
     return this.fxaInternal.getCertificateSigned(data.sessionToken,
                                                  keyPair.serializedPublicKey,
                                                  CERT_LIFETIME).then(
       cert => {
         log.debug("getCertificate got a new one: " + !!cert);
         this.cert = {
           cert: cert,
--- a/services/fxaccounts/FxAccountsManager.jsm
+++ b/services/fxaccounts/FxAccountsManager.jsm
@@ -175,16 +175,17 @@ this.FxAccountsManager = {
    * As of May 2014, the only HTTP call triggered by this._getAssertion()
    * is to /certificate/sign via:
    *   FxAccounts.getAssertion()
    *     FxAccountsInternal.getCertificateSigned()
    *       FxAccountsClient.signCertificate()
    * See the latter method for possible (error code, errno) pairs.
    */
   _handleGetAssertionError: function(reason, aAudience, aPrincipal) {
+    log.debug("FxAccountsManager._handleGetAssertionError()");
     let errno = (reason ? reason.errno : NaN) || NaN;
     // If the previously valid email/password pair is no longer valid ...
     if (errno == ERRNO_INVALID_AUTH_TOKEN) {
       return this._fxAccounts.accountStatus().then(
         (exists) => {
           // ... if the email still maps to an account, the password
           // must have changed, so ask the user to enter the new one ...
           if (exists) {
@@ -302,16 +303,19 @@ this.FxAccountsManager = {
             return this._serverError(reason);
           }
         );
       }
     );
   },
 
   _uiRequest: function(aRequest, aAudience, aPrincipal, aParams) {
+    if (Services.io.offline) {
+      return this._error(ERROR_OFFLINE);
+    }
     let ui = Cc["@mozilla.org/fxaccounts/fxaccounts-ui-glue;1"]
                .createInstance(Ci.nsIFxAccountsUIGlue);
     if (!ui[aRequest]) {
       return this._error(ERROR_UI_REQUEST);
     }
 
     if (!aParams || !Array.isArray(aParams)) {
       aParams = [aParams];
@@ -383,38 +387,35 @@ this.FxAccountsManager = {
     );
   },
 
   getAccount: function() {
     // We check first if we have session details cached.
     if (this._activeSession) {
       // If our cache says that the account is not yet verified,
       // we kick off verification before returning what we have.
-      if (this._activeSession && !this._activeSession.verified &&
-          !Services.io.offline) {
+      if (!this._activeSession.verified) {
         this.verificationStatus(this._activeSession);
       }
-
       log.debug("Account " + JSON.stringify(this._user));
       return Promise.resolve(this._user);
     }
 
     // If no cached information, we try to get it from the persistent storage.
     return this._fxAccounts.getSignedInUser().then(
       user => {
         if (!user || !user.email) {
           log.debug("No signed in account");
           return Promise.resolve(null);
         }
 
         this._activeSession = user;
         // If we get a stored information of a not yet verified account,
         // we kick off verification before returning what we have.
-        if (!user.verified && !Services.io.offline) {
-          log.debug("Unverified account");
+        if (!user.verified) {
           this.verificationStatus(user);
         }
 
         log.debug("Account " + JSON.stringify(this._user));
         return Promise.resolve(this._user);
       }
     );
   },
@@ -457,17 +458,18 @@ this.FxAccountsManager = {
     // There is no way to unverify an already verified account, so we just
     // return the account details of a verified account
     if (this._activeSession.verified) {
       log.debug("Account already verified");
       return;
     }
 
     if (Services.io.offline) {
-      this._error(ERROR_OFFLINE);
+      log.warn("Offline; skipping verification.");
+      return;
     }
 
     let client = this._getFxAccountsClient();
     client.recoveryEmailStatus(this._activeSession.sessionToken).then(
       data => {
         let error = this._getError(data);
         if (error) {
           this._error(error, data);
@@ -503,23 +505,23 @@ this.FxAccountsManager = {
    *   refreshAuthentication  - (bool) Force re-auth.
    *   silent                 - (bool) Prevent any UI interaction.
    *                            I.e., try to get an automatic assertion.
    */
   getAssertion: function(aAudience, aPrincipal, aOptions) {
     if (!aAudience) {
       return this._error(ERROR_INVALID_AUDIENCE);
     }
-    if (Services.io.offline) {
-      return this._error(ERROR_OFFLINE);
-    }
 
     let secMan = Cc["@mozilla.org/scriptsecuritymanager;1"]
                    .getService(Ci.nsIScriptSecurityManager);
     let uri = Services.io.newURI(aPrincipal.origin, null, null);
+    log.debug("FxAccountsManager.getAssertion() aPrincipal: ",
+              aPrincipal.origin, aPrincipal.appId,
+              aPrincipal.isInBrowserElement);
     let principal = secMan.getAppCodebasePrincipal(uri,
       aPrincipal.appId, aPrincipal.isInBrowserElement);
 
     return this.getAccount().then(
       user => {
         if (user) {
           // Three have-user cases to consider. First: are we unverified?
           if (!user.verified) {
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -179,16 +179,55 @@ add_task(function test_get_signed_in_use
   let localOnly = true;
   yield account.signOut(localOnly);
 
   // user should be undefined after sign out
   let result = yield account.getSignedInUser();
   do_check_eq(result, null);
 });
 
+add_task(function test_getCertificate() {
+  _("getCertificate()");
+  // This test, unlike the rest, uses an un-mocked FxAccounts instance.
+  // However, we still need to pass an object to the constructor to
+  // force it to expose "internal".
+  let fxa = new FxAccounts({onlySetInternal: true})
+  let credentials = {
+    email: "foo@example.com",
+    uid: "1234@lcip.org",
+    assertion: "foobar",
+    sessionToken: "dead",
+    kA: "beef",
+    kB: "cafe",
+    verified: true
+  };
+  yield fxa.setSignedInUser(credentials);
+
+  // Test that an expired cert throws if we're offline.
+  fxa.internal.currentAccountState.cert = {
+    validUntil: Date.parse("Mon, 13 Jan 2000 21:45:06 GMT")
+  };
+  let offline = Services.io.offline;
+  Services.io.offline = true;
+  // This call would break from missing parameters ...
+  fxa.internal.currentAccountState.getCertificate().then(
+    result => {
+      Services.io.offline = offline;
+      do_throw("Unexpected success");
+    },
+    err => {
+      Services.io.offline = offline;
+      // ... so we have to check the error string.
+      do_check_eq(err, "Error: OFFLINE");
+    }
+  );
+  _("----- DONE ----\n");
+});
+
+
 // Sanity-check that our mocked client is working correctly
 add_test(function test_client_mock() {
   do_test_pending();
 
   let fxa = new MockFxAccounts();
   let client = fxa.internal.fxAccountsClient;
   do_check_eq(client._verified, false);
   do_check_eq(typeof client.signIn, "function");
--- a/services/fxaccounts/tests/xpcshell/test_manager.js
+++ b/services/fxaccounts/tests/xpcshell/test_manager.js
@@ -11,16 +11,19 @@ Cu.import("resource://gre/modules/FxAcco
 Cu.import("resource://gre/modules/Promise.jsm");
 
 // === Mocks ===
 
 // Globals representing server state
 let passwordResetOnServer = false;
 let deletedOnServer = false;
 
+// Global representing FxAccounts state
+let certExpired = false;
+
 // Override FxAccountsUIGlue.
 const kFxAccountsUIGlueUUID = "{8f6d5d87-41ed-4bb5-aa28-625de57564c5}";
 const kFxAccountsUIGlueContractID =
   "@mozilla.org/fxaccounts/fxaccounts-ui-glue;1";
 
 // Save original FxAccountsUIGlue factory.
 const kFxAccountsUIGlueFactory =
   Cm.getClassObject(Cc[kFxAccountsUIGlueContractID], Ci.nsIFactory);
@@ -119,16 +122,18 @@ FxAccountsManager._fxAccounts = {
   getAssertion: function() {
     if (!this._signedInUser) {
       return null;
     }
 
     let deferred = Promise.defer();
     if (passwordResetOnServer || deletedOnServer) {
       deferred.reject({errno: ERRNO_INVALID_AUTH_TOKEN});
+    } else if (Services.io.offline && certExpired) {
+      deferred.reject(new Error(ERROR_OFFLINE));
     } else {
       deferred.resolve(this._assertion);
     }
     return deferred.promise;
   },
 
   getSignedInUser: function() {
     this._getSignedInUserCalled = true;
@@ -358,16 +363,78 @@ add_test(function(test_getAssertion_acti
       run_next_test();
     },
     error => {
       do_throw("Unexpected error: " + error);
     }
   );
 });
 
+add_test(function() {
+  // getAssertion() succeeds if offline with valid cert
+  do_print("= getAssertion active session, valid cert, offline");
+  FxAccountsManager._fxAccounts._signedInUser.verified = true;
+  FxAccountsManager._activeSession.verified = true;
+  Services.io.offline = true;
+  FxAccountsManager.getAssertion("audience", principal).then(
+    result => {
+      FxAccountsManager._fxAccounts._reset();
+      Services.io.offline = false;
+      run_next_test();
+    },
+    error => {
+      Services.io.offline = false;
+      do_throw("Unexpected error: " + error);
+    }
+  );
+});
+
+add_test(function() {
+  // getAssertion() rejects if offline and cert expired.
+  do_print("= getAssertion active session, expired cert, offline");
+  FxAccountsManager._fxAccounts._signedInUser.verified = true;
+  FxAccountsManager._activeSession.verified = true;
+  Services.io.offline = true;
+  certExpired = true;
+  FxAccountsManager.getAssertion("audience", principal).then(
+    result => {
+      Services.io.offline = false;
+      certExpired = false;
+      do_throw("Unexpected success");
+    },
+    error => {
+      FxAccountsManager._fxAccounts._reset();
+      Services.io.offline = false;
+      certExpired = false;
+      run_next_test();
+    }
+  );
+});
+
+add_test(function() {
+  // getAssertion() rejects if offline and UI needed.
+  do_print("= getAssertion active session, trigger UI, offline");
+  let user = FxAccountsManager._fxAccounts._signedInUser;
+  FxAccountsManager._fxAccounts._signedInUser = null;
+  Services.io.offline = true;
+  FxAccountsManager.getAssertion("audience", principal).then(
+    result => {
+      Services.io.offline = false;
+      do_throw("Unexpected success");
+    },
+    error => {
+      do_check_false(FxAccountsUIGlue._signInFlowCalled);
+      FxAccountsManager._fxAccounts._reset();
+      FxAccountsManager._fxAccounts._signedInUser = user;
+      Services.io.offline = false;
+      run_next_test();
+    }
+  );
+});
+
 add_test(function(test_getAssertion_refreshAuth) {
   do_print("= getAssertion refreshAuth =");
   let gracePeriod = 1200;
   FxAccountsUIGlue._activeSession = {
     email: "user@domain.org",
     verified: true,
     sessionToken: "1234"
   };