Bug 1056523 - Ensure sync credentials are reset during reauth flow. r=markh
authorChris Karlof <ckarlof@mozilla.com>
Wed, 27 Aug 2014 16:14:58 -0700
changeset 223642 cc15ba9c697bc89605bebf0b4e793620813c74c3
parent 223641 21242c99fffe3b6388616dcb46070ce20cf5a27d
child 223643 07d046eea6349989e92f1c99ad3fb24b1fb59c39
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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 1056523 - Ensure sync credentials are reset during reauth flow. r=markh 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.
--- a/services/sync/modules/browserid_identity.js
+++ b/services/sync/modules/browserid_identity.js
@@ -183,17 +183,21 @@ this.BrowserIDManager.prototype = {
     // 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;
@@ -574,19 +578,20 @@ 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 || err));
           // 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