Bug 1013064 (part 4) - browserid_identity and sync changes to support FxA credentials in the login manager. r=ckarlof,rnewman

From 9717484083e66b78eedfa14e539d51382aba760f Mon Sep 17 00:00:00 2001
---
 services/sync/modules/browserid_identity.js        | 61 ++++++++++++++++++++--
 services/sync/modules/identity.js                  | 19 +++++++
 services/sync/modules/service.js                   | 20 ++++---
 .../sync/tests/unit/test_browserid_identity.js     | 15 ++++++
 4 files changed, 102 insertions(+), 13 deletions(-)

services/sync/modules/browserid_identity.js

@@ -399,7 +399,10 @@ this.BrowserIDManager.prototype = {
    * The current state of the auth credentials.
    *
    * This essentially validates that enough credentials are available to use
-   * Sync.
+   * Sync, although it effectively ignores the state of the master-password -
+   * if that's locked and that's the only problem we can see, say everything
+   * is OK - unlockAndVerifyAuthState will be used to perform the unlock
+   * and re-verification if necessary.
    */
   get currentAuthState() {
     if (this._authFailureReason) {
@@ -416,14 +419,53 @@ this.BrowserIDManager.prototype = {
 
     // No need to check this.syncKey as our getter for that attribute
     // uses this.syncKeyBundle
-    // If bundle creation started, but failed.
-    if (this._shouldHaveSyncKeyBundle && !this.syncKeyBundle) {
-      return LOGIN_FAILED_NO_PASSPHRASE;
+    // If bundle creation started, but failed due to any reason other than
+    // the MP being locked...
+    if (this._shouldHaveSyncKeyBundle && !this.syncKeyBundle && !Utils.mpLocked()) {
+      // Return a state that says a re-auth is necessary so we can get keys.
+      return LOGIN_FAILED_LOGIN_REJECTED;
     }
 
     return STATUS_OK;
   },
 
+  // Do we currently have keys, or do we have enough that we should be able
+  // to successfully fetch them?
+  _canFetchKeys: function() {
+    let userData = this._signedInUser;
+    // a keyFetchToken means we can almost certainly grab them.
+    // kA and kB means we already have them.
+    return userData && (userData.keyFetchToken || (userData.kA && userData.kB));
+  },
+
+  /**
+   * Verify the current auth state, unlocking the master-password if necessary.
+   *
+   * Returns a promise that resolves with the current auth state after
+   * attempting to unlock.
+   */
+  unlockAndVerifyAuthState: function() {
+    if (this._canFetchKeys()) {
+      return Promise.resolve(STATUS_OK);
+    }
+    // so no keys - ensure MP unlocked.
+    if (!Utils.ensureMPUnlocked()) {
+      // user declined to unlock, so we don't know if they are stored there.
+      return Promise.resolve(MASTER_PASSWORD_LOCKED);
+    }
+    // now we are unlocked we must re-fetch the user data as we may now have
+    // the details that were previously locked away.
+    return this._fxaService.getSignedInUser().then(
+      accountData => {
+        this._updateSignedInUser(accountData);
+        // If we still can't get keys it probably means the user authenticated
+        // without unlocking the MP or cleared the saved logins, so we've now
+        // lost them - the user will need to reauth before continuing.
+        return this._canFetchKeys() ? STATUS_OK : LOGIN_FAILED_LOGIN_REJECTED;
+      }
+    );
+  },
+
   /**
    * Do we have a non-null, not yet expired token for the user currently
    * signed in?
@@ -449,6 +491,14 @@ this.BrowserIDManager.prototype = {
     let fxa = this._fxaService;
     let userData = this._signedInUser;
 
+    // We need kA and kB for things to work.  If we don't have them, just
+    // return null for the token - sync calling unlockAndVerifyAuthState()
+    // before actually syncing will setup the error states if necessary.
+    if (!this._canFetchKeys()) {
+      log.info("_fetchTokenForUser has no keys to use.");
+      return null;
+    }
+
     log.info("Fetching assertion and token from: " + tokenServerURI);
 
     let maybeFetchKeys = () => {
@@ -524,7 +574,8 @@ this.BrowserIDManager.prototype = {
           // set it to the "fatal" LOGIN_FAILED_LOGIN_REJECTED reason.
           this._authFailureReason = LOGIN_FAILED_LOGIN_REJECTED;
         } else {
-          this._log.error("Non-authentication error in _fetchTokenForUser: " + err.message);
+          this._log.error("Non-authentication error in _fetchTokenForUser: "
+                          + (err.message || err));
           // for now assume it is just a transient network related problem.
           this._authFailureReason = LOGIN_FAILED_NETWORK_ERROR;
         }

services/sync/modules/identity.js

@@ -377,6 +377,25 @@ IdentityManager.prototype = {
     return STATUS_OK;
   },
 
+  /**
+   * Verify the current auth state, unlocking the master-password if necessary.
+   *
+   * Returns a promise that resolves with the current auth state after
+   * attempting to unlock.
+   */
+  unlockAndVerifyAuthState: function() {
+    // Try to fetch the passphrase - this will prompt for MP unlock as a
+    // side-effect...
+    try {
+      this.syncKey;
+    } catch (ex) {
+      this._log.debug("Fetching passphrase threw " + ex +
+                      "; assuming master password locked.");
+      return Promise.resolve(MASTER_PASSWORD_LOCKED);
+    }
+    return Promise.resolve(STATUS_OK);
+  },
+
   /**
    * Persist credentials to password store.
    *

services/sync/modules/service.js

@@ -684,17 +684,21 @@ Sync11Service.prototype = {
       return false;
     }
 
-    // Unlock master password, or return.
     // Attaching auth credentials to a request requires access to
     // passwords, which means that Resource.get can throw MP-related
     // exceptions!
-    // Try to fetch the passphrase first, while we still have control.
-    try {
-      this.identity.syncKey;
-    } catch (ex) {
-      this._log.debug("Fetching passphrase threw " + ex +
-                      "; assuming master password locked.");
-      this.status.login = MASTER_PASSWORD_LOCKED;
+    // So we ask the identity to verify the login state after unlocking the
+    // master password (ie, this call is expected to prompt for MP unlock
+    // if necessary) while we still have control.
+    let cb = Async.makeSpinningCallback();
+    this.identity.unlockAndVerifyAuthState().then(
+      result => cb(null, result),
+      cb
+    );
+    let unlockedState = cb.wait();
+    this._log.debug("Fetching unlocked auth state returned " + unlockedState);
+    if (unlockedState != STATUS_OK) {
+      this.status.login = unlockedState;
       return false;
     }
 

services/sync/tests/unit/test_browserid_identity.js

@@ -83,9 +83,23 @@ add_task(function test_initialializeWithCurrentIdentity() {
   }
 );
 
+add_task(function test_initialializeWithNoKeys() {
+    _("Verify start after initializeWithCurrentIdentity without kA, kB or keyFetchToken");
+    let identityConfig = makeIdentityConfig();
+    delete identityConfig.fxaccount.user.kA;
+    delete identityConfig.fxaccount.user.kB;
+    // there's no keyFetchToken by default, so the initialize should fail.
+    configureFxAccountIdentity(browseridManager, identityConfig);
+
+    yield browseridManager.initializeWithCurrentIdentity();
+    yield browseridManager.whenReadyToAuthenticate.promise;
+    do_check_eq(Status.login, LOGIN_SUCCEEDED, "login succeeded even without keys");
+    do_check_false(browseridManager._canFetchKeys(), "_canFetchKeys reflects lack of keys");
+});
 
 add_test(function test_getResourceAuthenticator() {
     _("BrowserIDManager supplies a Resource Authenticator callback which returns a Hawk header.");
+    configureFxAccountIdentity(browseridManager);
     let authenticator = browseridManager.getResourceAuthenticator();
     do_check_true(!!authenticator);
     let req = {uri: CommonUtils.makeURI(
@@ -240,6 +254,7 @@ add_test(function test_RESTResourceAuthenticatorSkew() {
 add_task(function test_ensureLoggedIn() {
   configureFxAccountIdentity(browseridManager);
   yield browseridManager.initializeWithCurrentIdentity();
+  yield browseridManager.whenReadyToAuthenticate.promise;
   Assert.equal(Status.login, LOGIN_SUCCEEDED, "original initialize worked");
   yield browseridManager.ensureLoggedIn();
   Assert.equal(Status.login, LOGIN_SUCCEEDED, "original ensureLoggedIn worked");