Bug 1207889 - handle the webchannel changepassword command and update the signed in user. r=kitcambridge
authorMark Hammond <mhammond@skippinet.com.au>
Wed, 23 Mar 2016 12:02:29 +1100
changeset 289920 a1cbd97d7b433593bf16f933f58342473e858944
parent 289919 f9e8d838c9ee9b5f71984b82e6cf4f312fa108a4
child 289921 0220f77f4cd48c201645aedac03b435d4f6affbc
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskitcambridge
bugs1207889
milestone48.0a1
Bug 1207889 - handle the webchannel changepassword command and update the signed in user. r=kitcambridge
services/fxaccounts/FxAccounts.jsm
services/fxaccounts/FxAccountsStorage.jsm
services/fxaccounts/FxAccountsWebChannel.jsm
services/fxaccounts/tests/xpcshell/test_accounts.js
services/fxaccounts/tests/xpcshell/test_web_channel.js
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -51,16 +51,17 @@ var publicProperties = [
   "now",
   "promiseAccountsForceSigninURI",
   "promiseAccountsChangeProfileURI",
   "promiseAccountsManageURI",
   "removeCachedOAuthToken",
   "resendVerificationEmail",
   "setSignedInUser",
   "signOut",
+  "updateUserAccountData",
   "updateDeviceRegistration",
   "whenVerified"
 ];
 
 // An AccountState object holds all state related to one specific account.
 // Only one AccountState is ever "current" in the FxAccountsInternal object -
 // whenever a user logs out or logs in, the current AccountState is discarded,
 // making it impossible for the wrong state or state data to be accidentally
@@ -517,16 +518,44 @@ FxAccountsInternal.prototype = {
         Services.telemetry.getHistogramById("FXA_CONFIGURED").add(1);
         this.notifyObservers(ONLOGIN_NOTIFICATION);
       }).then(() => {
         return currentAccountState.resolve();
       });
     })
   },
 
+  /**
+   * Update account data for the currently signed in user.
+   *
+   * @param credentials
+   *        The credentials object containing the fields to be updated.
+   *        This object must contain |email| and |uid| fields and they must
+   *        match the currently signed in user.
+   */
+  updateUserAccountData(credentials) {
+    log.debug("updateUserAccountData called with fields", Object.keys(credentials));
+    if (logPII) {
+      log.debug("updateUserAccountData called with data", credentials);
+    }
+    let currentAccountState = this.currentAccountState;
+    return currentAccountState.promiseInitialized.then(() => {
+      return currentAccountState.getUserAccountData(["email", "uid"]);
+    }).then(existing => {
+      if (existing.email != credentials.email || existing.uid != credentials.uid) {
+        throw new Error("The specified credentials aren't for the current user");
+      }
+      // We need to nuke email and uid as storage will complain if we try and
+      // update them (even when the value is the same)
+      credentials = Cu.cloneInto(credentials, {}); // clone it first
+      delete credentials.email;
+      delete credentials.uid;
+      return currentAccountState.updateUserAccountData(credentials);
+    });
+  },
 
   /**
    * returns a promise that fires with the assertion.  If there is no verified
    * signed-in user, fires with null.
    */
   getAssertion: function getAssertion(audience) {
     return this._getAssertion(audience);
   },
--- a/services/fxaccounts/FxAccountsStorage.jsm
+++ b/services/fxaccounts/FxAccountsStorage.jsm
@@ -1,26 +1,36 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 this.EXPORTED_SYMBOLS = [
+  "FxAccountsStorageManagerCanStoreField",
   "FxAccountsStorageManager",
 ];
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/FxAccountsCommon.js");
 Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://services-common/utils.js");
 
+// A helper function so code can check what fields are able to be stored by
+// the storage manager without having a reference to a manager instance.
+function FxAccountsStorageManagerCanStoreField(fieldName) {
+  return FXA_PWDMGR_MEMORY_FIELDS.has(fieldName) ||
+         FXA_PWDMGR_PLAINTEXT_FIELDS.has(fieldName) ||
+         FXA_PWDMGR_SECURE_FIELDS.has(fieldName);
+}
+
+// The storage manager object.
 this.FxAccountsStorageManager = function(options = {}) {
   this.options = {
     filename: options.filename || DEFAULT_STORAGE_FILENAME,
     baseDir: options.baseDir || OS.Constants.Path.profileDir,
   }
   this.plainStorage = new JSONStorage(this.options);
   // On b2g we have no loginManager for secure storage, and tests may want
   // to pretend secure storage isn't available.
--- a/services/fxaccounts/FxAccountsWebChannel.jsm
+++ b/services/fxaccounts/FxAccountsWebChannel.jsm
@@ -17,25 +17,28 @@ Cu.import("resource://gre/modules/XPCOMU
 Cu.import("resource://gre/modules/FxAccountsCommon.js");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebChannel",
                                   "resource://gre/modules/WebChannel.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
                                   "resource://gre/modules/FxAccounts.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "FxAccountsStorageManagerCanStoreField",
+                                  "resource://gre/modules/FxAccountsStorage.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Weave",
                                   "resource://services-sync/main.js");
 
 const COMMAND_PROFILE_CHANGE       = "profile:change";
 const COMMAND_CAN_LINK_ACCOUNT     = "fxaccounts:can_link_account";
 const COMMAND_LOGIN                = "fxaccounts:login";
 const COMMAND_LOGOUT               = "fxaccounts:logout";
 const COMMAND_DELETE               = "fxaccounts:delete";
 const COMMAND_SYNC_PREFERENCES     = "fxaccounts:sync_preferences";
+const COMMAND_CHANGE_PASSWORD      = "fxaccounts:change_password";
 
 const PREF_LAST_FXA_USER           = "identity.fxaccounts.lastSignedInUserHash";
 const PREF_SYNC_SHOW_CUSTOMIZATION = "services.sync-setup.ui.showCustomizationDialog";
 
 /**
  * Create a new FxAccountsWebChannel to listen for account updates
  *
  * @param {Object} options Options
@@ -167,16 +170,19 @@ this.FxAccountsWebChannel.prototype = {
             };
 
             log.debug("FxAccountsWebChannel response", response);
             this._channel.send(response, sendingContext);
             break;
           case COMMAND_SYNC_PREFERENCES:
             this._helpers.openSyncPreferences(sendingContext.browser, data.entryPoint);
             break;
+          case COMMAND_CHANGE_PASSWORD:
+            this._helpers.changePassword(data);
+            break;
           default:
             log.warn("Unrecognized FxAccountsWebChannel command", command);
             break;
         }
       }
     };
 
     this._channelCallback = listener;
@@ -270,16 +276,39 @@ this.FxAccountsWebChannelHelpers.prototy
       if (userData.uid === uid) {
         // true argument is `localOnly`, because server-side stuff
         // has already been taken care of by the content server
         return fxAccounts.signOut(true);
       }
     });
   },
 
+  changePassword(credentials) {
+    // If |credentials| has fields that aren't handled by accounts storage,
+    // updateUserAccountData will throw - mainly to prevent errors in code
+    // that hard-codes field names.
+    // However, in this case the field names aren't really in our control.
+    // We *could* still insist the server know what fields names are valid,
+    // but that makes life difficult for the server when Firefox adds new
+    // features (ie, new fields) - forcing the server to track a map of
+    // versions to supported field names doesn't buy us much.
+    // So we just remove field names we know aren't handled.
+    let newCredentials = {};
+    for (let name of Object.keys(credentials)) {
+      if (name == "email" || name == "uid" || FxAccountsStorageManagerCanStoreField(name)) {
+        newCredentials[name] = credentials[name];
+      } else {
+        log.info("changePassword ignoring unsupported field", name);
+      }
+    }
+    this._fxAccounts.updateUserAccountData(newCredentials).catch(err => {
+      log.error("Failed to update account data on password change", err);
+    });
+  },
+
   /**
    * Get the hash of account name of the previously signed in account
    */
   getPreviousAccountNameHashPref() {
     try {
       return Services.prefs.getComplexValue(PREF_LAST_FXA_USER, Ci.nsISupportsString).data;
     } catch (_) {
       return "";
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -257,16 +257,68 @@ add_task(function* test_get_signed_in_us
   let localOnly = true;
   yield account.signOut(localOnly);
 
   // user should be undefined after sign out
   result = yield account.getSignedInUser();
   do_check_eq(result, null);
 });
 
+add_task(function* test_update_account_data() {
+  _("Check updateUserAccountData does the right thing.");
+  let account = MakeFxAccounts();
+  let credentials = {
+    email: "foo@example.com",
+    uid: "1234@lcip.org",
+    assertion: "foobar",
+    sessionToken: "dead",
+    kA: "beef",
+    kB: "cafe",
+    verified: true
+  };
+  yield account.setSignedInUser(credentials);
+
+  let newCreds = {
+    email: credentials.email,
+    uid: credentials.uid,
+    assertion: "new_assertion",
+  }
+  yield account.updateUserAccountData(newCreds);
+  do_check_eq((yield account.getSignedInUser()).assertion, "new_assertion",
+              "new field value was saved");
+
+  // but we should fail attempting to change email or uid.
+  newCreds = {
+    email: "someoneelse@example.com",
+    uid: credentials.uid,
+    assertion: "new_assertion",
+  }
+  yield Assert.rejects(account.updateUserAccountData(newCreds));
+  newCreds = {
+    email: credentials.email,
+    uid: "another_uid",
+    assertion: "new_assertion",
+  }
+  yield Assert.rejects(account.updateUserAccountData(newCreds));
+
+  // should fail without email or uid.
+  newCreds = {
+    assertion: "new_assertion",
+  }
+  yield Assert.rejects(account.updateUserAccountData(newCreds));
+
+  // and should fail with a field name that's not known by storage.
+  newCreds = {
+    email: credentials.email,
+    uid: "another_uid",
+    foo: "bar",
+  }
+  yield Assert.rejects(account.updateUserAccountData(newCreds));
+});
+
 add_task(function* test_getCertificateOffline() {
   _("getCertificateOffline()");
   let fxa = MakeFxAccounts();
   let credentials = {
     email: "foo@example.com",
     uid: "1234@lcip.org",
     sessionToken: "dead",
     verified: true,
--- a/services/fxaccounts/tests/xpcshell/test_web_channel.js
+++ b/services/fxaccounts/tests/xpcshell/test_web_channel.js
@@ -314,16 +314,36 @@ add_test(function test_helpers_open_sync
       do_check_eq(uri, "about:preferences?entrypoint=fxa%3Averification_complete#sync");
       run_next_test();
     }
   };
 
   helpers.openSyncPreferences(mockBrowser, "fxa:verification_complete");
 });
 
+add_test(function test_helpers_change_password() {
+  let updateCalled = false;
+  let helpers = new FxAccountsWebChannelHelpers({
+    fxAccounts: {
+      updateUserAccountData(credentials) {
+        do_check_true(credentials.hasOwnProperty("email"));
+        do_check_true(credentials.hasOwnProperty("uid"));
+        do_check_true(credentials.hasOwnProperty("kA"));
+        // "foo" isn't a field known by storage, so should be dropped.
+        do_check_false(credentials.hasOwnProperty("foo"));
+        updateCalled = true;
+        return Promise.resolve();
+      }
+    }
+  });
+  helpers.changePassword({ email: "email", uid: "uid", kA: "kA", foo: "foo" });
+  do_check_true(updateCalled);
+  run_next_test();
+});
+
 function run_test() {
   run_next_test();
 }
 
 function makeObserver(aObserveTopic, aObserveFunc) {
   let callback = function (aSubject, aTopic, aData) {
     log.debug("observed " + aTopic + " " + aData);
     if (aTopic == aObserveTopic) {