Bug 1056523 - Ensure sync credentials are reset during reauth flow. r=markh, a=lmandel
authorChris Karlof <ckarlof@mozilla.com>
Wed, 27 Aug 2014 16:14:58 -0700
changeset 216671 8b409f2dfcb1
parent 216670 bca701646487
child 216672 ede2300e8733
push id3870
push userryanvm@gmail.com
push date2014-09-08 17:45 +0000
treeherdermozilla-beta@880228a5208a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh, lmandel
bugs1056523
milestone33.0
Bug 1056523 - Ensure sync credentials are reset during reauth flow. r=markh, a=lmandel This patch addresses a bug in the following scenario: User has browser 1 connected to sync and open, and resets her password on browser 2. Eventually the browser detects the need to reauthenticate the user, and prompts the user. When the user entered her new password, the browserid_identity module failed to re-derive a new syncKeyBundle from the new password and happily used the old one. Chaos ensued. This patch mitigate the problem by calling resetCredentials at the start of initializeWithCurrentIdentity(), which will clear the syncKeyBundle, along with other credentials. Previously this function was only resetting this._shouldHaveSyncKeyBundle. I also removed a misleading comment about the syncKeyBundle being cleared when it wasn't.
services/sync/modules/browserid_identity.js
--- a/services/sync/modules/browserid_identity.js
+++ b/services/sync/modules/browserid_identity.js
@@ -183,17 +183,21 @@ this.BrowserIDManager.prototype = {
     this._log.trace("initializeWithCurrentIdentity");
 
     // Reset the world before we do anything async.
     this.whenReadyToAuthenticate = Promise.defer();
     this.whenReadyToAuthenticate.promise.then(null, (err) => {
       this._log.error("Could not authenticate: " + err);
     });
 
-    this._shouldHaveSyncKeyBundle = false;
+    // initializeWithCurrentIdentity() can be called after the
+    // identity module was first initialized, e.g., after the
+    // user completes a force authentication, so we should make
+    // sure all credentials are reset before proceeding.
+    this.resetCredentials();
     this._authFailureReason = null;
 
     return this._fxaService.getSignedInUser().then(accountData => {
       if (!accountData) {
         this._log.info("initializeWithCurrentIdentity has no user logged in");
         this.account = null;
         // and we are as ready as we can ever be for auth.
         this._shouldHaveSyncKeyBundle = true;
@@ -523,19 +527,20 @@ this.BrowserIDManager.prototype = {
           this._log.error("Authentication error in _fetchTokenForUser: " + err);
           // 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);
           // for now assume it is just a transient network related problem.
           this._authFailureReason = LOGIN_FAILED_NETWORK_ERROR;
         }
-        // Drop the sync key bundle, but still expect to have one.
-        // This will arrange for us to be in the right 'currentAuthState'
-        // such that UI will show the right error.
+        // this._authFailureReason being set to be non-null in the above if clause
+        // ensures we are in the correct currentAuthState, and
+        // this._shouldHaveSyncKeyBundle being true ensures everything that cares knows
+        // that there is no authentication dance still under way.
         this._shouldHaveSyncKeyBundle = true;
         Weave.Status.login = this._authFailureReason;
         Services.obs.notifyObservers(null, "weave:service:login:error", null);
         throw err;
       });
   },
 
   // Returns a promise that is resolved when we have a valid token for the