Bug 1146346 - Fix sync login when master-password was unlocked by something other than sync. r=ckarlof, a=sledru
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 02 Apr 2015 09:51:05 +1100
changeset 258254 edf4fa83d569
parent 258253 b28b502e4aca
child 258255 dad86e3e53cd
push id4628
push userryanvm@gmail.com
push date2015-04-03 20:32 +0000
treeherdermozilla-beta@e4566e5991e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckarlof, sledru
bugs1146346
milestone38.0
Bug 1146346 - Fix sync login when master-password was unlocked by something other than sync. r=ckarlof, a=sledru
services/sync/modules/browserid_identity.js
--- a/services/sync/modules/browserid_identity.js
+++ b/services/sync/modules/browserid_identity.js
@@ -415,43 +415,33 @@ this.BrowserIDManager.prototype = {
   _getSyncCredentialsHosts: function() {
     return Utils.getSyncCredentialsHostsFxA();
   },
 
   /**
    * The current state of the auth credentials.
    *
    * This essentially validates that enough credentials are available to use
-   * 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.
+   * Sync. It doesn't check we have all the keys we need as the master-password
+   * may have been locked when we tried to get them - we rely on
+   * unlockAndVerifyAuthState to check that for us.
    */
   get currentAuthState() {
     if (this._authFailureReason) {
       this._log.info("currentAuthState returning " + this._authFailureReason +
                      " due to previous failure");
       return this._authFailureReason;
     }
     // TODO: need to revisit this. Currently this isn't ready to go until
     // both the username and syncKeyBundle are both configured and having no
     // username seems to make things fail fast so that's good.
     if (!this.username) {
       return LOGIN_FAILED_NO_USERNAME;
     }
 
-    // No need to check this.syncKey as our getter for that attribute
-    // uses this.syncKeyBundle
-    // 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.
@@ -462,32 +452,36 @@ this.BrowserIDManager.prototype = {
   /**
    * 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()) {
+      log.debug("unlockAndVerifyAuthState already has (or can fetch) sync keys");
       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.
+      log.debug("unlockAndVerifyAuthState: user declined to unlock master-password");
       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;
+        let result = this._canFetchKeys() ? STATUS_OK : LOGIN_FAILED_LOGIN_REJECTED;
+        log.debug("unlockAndVerifyAuthState re-fetched credentials and is returning", result);
+        return result;
       }
     );
   },
 
   /**
    * Do we have a non-null, not yet expired token for the user currently
    * signed in?
    */
@@ -524,16 +518,17 @@ this.BrowserIDManager.prototype = {
     let client = this._tokenServerClient;
     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("Unable to fetch keys (master-password locked?), so aborting token fetch");
       return Promise.resolve(null);
     }
 
     let maybeFetchKeys = () => {
       // This is called at login time and every time we need a new token - in
       // the latter case we already have kA and kB, so optimise that case.
       if (userData.kA && userData.kB) {
         return;