Bug 1156752 - explicitly list where each FxA field is stored. r=zaach
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 27 Jul 2015 08:58:53 +1000
changeset 286363 2873123a1a3282dbceb30be30eb53305010262dd
parent 286362 c6a3767c30fc845c43988b1870ed343430fe40b3
child 286364 c7d1dba2f44d4680bf8353545f85d525e11d7605
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszaach
bugs1156752
milestone42.0a1
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 1156752 - explicitly list where each FxA field is stored. r=zaach
browser/base/content/aboutaccounts/aboutaccounts.js
browser/components/customizableui/test/browser_946320_tabs_from_other_computers.js
services/fxaccounts/FxAccounts.jsm
services/fxaccounts/FxAccountsCommon.js
services/fxaccounts/FxAccountsStorage.jsm
services/fxaccounts/tests/xpcshell/test_accounts.js
services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js
services/fxaccounts/tests/xpcshell/test_storage_manager.js
services/sync/modules-testing/utils.js
services/sync/tests/unit/test_browserid_identity.js
--- a/browser/base/content/aboutaccounts/aboutaccounts.js
+++ b/browser/base/content/aboutaccounts/aboutaccounts.js
@@ -143,18 +143,23 @@ let wrapper = {
    *
    * @param accountData the user's account data and credentials
    */
   onLogin: function (accountData) {
     log("Received: 'login'. Data:" + JSON.stringify(accountData));
 
     if (accountData.customizeSync) {
       Services.prefs.setBoolPref(PREF_SYNC_SHOW_CUSTOMIZATION, true);
-      delete accountData.customizeSync;
     }
+    delete accountData.customizeSync;
+    // sessionTokenContext is erroneously sent by the content server.
+    // https://github.com/mozilla/fxa-content-server/issues/2766
+    // To avoid having the FxA storage manager not knowing what to do with
+    // it we delete it here.
+    delete accountData.sessionTokenContext;
 
     // We need to confirm a relink - see shouldAllowRelink for more
     let newAccountEmail = accountData.email;
     // The hosted code may have already checked for the relink situation
     // by sending the can_link_account command. If it did, then
     // it will indicate we don't need to ask twice.
     if (!accountData.verifiedCanLinkAccount && !shouldAllowRelink(newAccountEmail)) {
       // we need to tell the page we successfully received the message, but
--- a/browser/components/customizableui/test/browser_946320_tabs_from_other_computers.js
+++ b/browser/components/customizableui/test/browser_946320_tabs_from_other_computers.js
@@ -114,17 +114,17 @@ function configureFxAccountIdentity() {
   };
 
   let MockInternal = {
     newAccountState(credentials) {
       isnot(credentials, "not expecting credentials");
       let storageManager = new MockFxaStorageManager();
       // and init storage with our user.
       storageManager.initialize(user);
-      return new AccountState(this, storageManager);
+      return new AccountState(storageManager);
     },
     getCertificate(data, keyPair, mustBeValidUntil) {
       this.cert = {
         validUntil: this.now() + 10000,
         cert: "certificate",
       };
       return Promise.resolve(this.cert.cert);
     },
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -67,35 +67,33 @@ let publicProperties = [
 // somePromiseBasedFunction: function() {
 //   let currentState = this.currentAccountState;
 //   return someOtherPromiseFunction().then(
 //     data => currentState.resolve(data)
 //   );
 // }
 // If the state has changed between the function being called and the promise
 // being resolved, the .resolve() call will actually be rejected.
-let AccountState = this.AccountState = function(fxaInternal, storageManager) {
-  this.fxaInternal = fxaInternal;
+let AccountState = this.AccountState = function(storageManager) {
   this.storageManager = storageManager;
   this.promiseInitialized = this.storageManager.getAccountData().then(data => {
     this.oauthTokens = data && data.oauthTokens ? data.oauthTokens : {};
   }).catch(err => {
     log.error("Failed to initialize the storage manager", err);
     // Things are going to fall apart, but not much we can do about it here.
   });
 };
 
 AccountState.prototype = {
-  cert: null,
-  keyPair: null,
   oauthTokens: null,
   whenVerifiedDeferred: null,
   whenKeysReadyDeferred: null,
 
-  get isCurrent() this.fxaInternal && this.fxaInternal.currentAccountState === this,
+  // If the storage manager has been nuked then we are no longer current.
+  get isCurrent() this.storageManager != null,
 
   abort() {
     if (this.whenVerifiedDeferred) {
       this.whenVerifiedDeferred.reject(
         new Error("Verification aborted; Another user signing in"));
       this.whenVerifiedDeferred = null;
     }
 
@@ -103,17 +101,16 @@ AccountState.prototype = {
       this.whenKeysReadyDeferred.reject(
         new Error("Verification aborted; Another user signing in"));
       this.whenKeysReadyDeferred = null;
     }
 
     this.cert = null;
     this.keyPair = null;
     this.oauthTokens = null;
-    this.fxaInternal = null;
     // Avoid finalizing the storageManager multiple times (ie, .signOut()
     // followed by .abort())
     if (!this.storageManager) {
       return Promise.resolve();
     }
     let storageManager = this.storageManager;
     this.storageManager = null;
     return storageManager.finalize();
@@ -126,92 +123,35 @@ AccountState.prototype = {
     this.oauthTokens = null;
     let storageManager = this.storageManager;
     this.storageManager = null;
     return storageManager.deleteAccountData().then(() => {
       return storageManager.finalize();
     });
   },
 
-  getUserAccountData() {
+  // Get user account data. Optionally specify explcit field names to fetch
+  // (and note that if you require an in-memory field you *must* specify the
+  // field name(s).)
+  getUserAccountData(fieldNames = null) {
     if (!this.isCurrent) {
       return Promise.reject(new Error("Another user has signed in"));
     }
-    return this.storageManager.getAccountData().then(result => {
+    return this.storageManager.getAccountData(fieldNames).then(result => {
       return this.resolve(result);
     });
   },
 
   updateUserAccountData(updatedFields) {
     if (!this.isCurrent) {
       return Promise.reject(new Error("Another user has signed in"));
     }
     return this.storageManager.updateAccountData(updatedFields);
   },
 
-  getCertificate: function(data, keyPair, mustBeValidUntil) {
-    // TODO: get the lifetime from the cert's .exp field
-    if (this.cert && this.cert.validUntil > mustBeValidUntil) {
-      log.debug(" getCertificate already had one");
-      return this.resolve(this.cert.cert);
-    }
-
-    if (Services.io.offline) {
-      return this.reject(new Error(ERROR_OFFLINE));
-    }
-
-    let willBeValidUntil = this.fxaInternal.now() + CERT_LIFETIME;
-    return this.fxaInternal.getCertificateSigned(data.sessionToken,
-                                                 keyPair.serializedPublicKey,
-                                                 CERT_LIFETIME).then(
-      cert => {
-        log.debug("getCertificate got a new one: " + !!cert);
-        this.cert = {
-          cert: cert,
-          validUntil: willBeValidUntil
-        };
-        return cert;
-      }
-    ).then(result => this.resolve(result));
-  },
-
-  getKeyPair: function(mustBeValidUntil) {
-    // If the debugging pref to ignore cached authentication credentials is set for Sync,
-    // then don't use any cached key pair, i.e., generate a new one and get it signed.
-    // The purpose of this pref is to expedite any auth errors as the result of a
-    // expired or revoked FxA session token, e.g., from resetting or changing the FxA
-    // password.
-    let ignoreCachedAuthCredentials = false;
-    try {
-      ignoreCachedAuthCredentials = Services.prefs.getBoolPref("services.sync.debug.ignoreCachedAuthCredentials");
-    } catch(e) {
-      // Pref doesn't exist
-    }
-    if (!ignoreCachedAuthCredentials && this.keyPair && (this.keyPair.validUntil > mustBeValidUntil)) {
-      log.debug("getKeyPair: already have a keyPair");
-      return this.resolve(this.keyPair.keyPair);
-    }
-    // Otherwse, create a keypair and set validity limit.
-    let willBeValidUntil = this.fxaInternal.now() + KEY_LIFETIME;
-    let d = Promise.defer();
-    jwcrypto.generateKeyPair("DS160", (err, kp) => {
-      if (err) {
-        return this.reject(err);
-      }
-      this.keyPair = {
-        keyPair: kp,
-        validUntil: willBeValidUntil
-      };
-      log.debug("got keyPair");
-      delete this.cert;
-      d.resolve(this.keyPair.keyPair);
-    });
-    return d.promise.then(result => this.resolve(result));
-  },
-
   resolve: function(result) {
     if (!this.isCurrent) {
       log.info("An accountState promise was resolved, but was actually rejected" +
                " due to a different user being signed in. Originally resolved" +
                " with", result);
       return Promise.reject(new Error("A different user signed in"));
     }
     return Promise.resolve(result);
@@ -422,17 +362,17 @@ FxAccountsInternal.prototype = {
     }
     return this._profile;
   },
 
   // A hook-point for tests who may want a mocked AccountState or mocked storage.
   newAccountState(credentials) {
     let storage = new FxAccountsStorageManager();
     storage.initialize(credentials);
-    return new AccountState(this, storage);
+    return new AccountState(storage);
   },
 
   /**
    * Return the current time in milliseconds as an integer.  Allows tests to
    * manipulate the date to simulate certificate expiration.
    */
   now: function() {
     return this.fxAccountsClient.now();
@@ -555,34 +495,80 @@ FxAccountsInternal.prototype = {
         }
       }).then(() => {
         return currentAccountState.resolve();
       });
     })
   },
 
   /**
+   * returns a promise that fires with the keypair.
+   */
+  getKeyPair: Task.async(function* (mustBeValidUntil) {
+    // If the debugging pref to ignore cached authentication credentials is set for Sync,
+    // then don't use any cached key pair, i.e., generate a new one and get it signed.
+    // The purpose of this pref is to expedite any auth errors as the result of a
+    // expired or revoked FxA session token, e.g., from resetting or changing the FxA
+    // password.
+    let ignoreCachedAuthCredentials = false;
+    try {
+      ignoreCachedAuthCredentials = Services.prefs.getBoolPref("services.sync.debug.ignoreCachedAuthCredentials");
+    } catch(e) {
+      // Pref doesn't exist
+    }
+    let currentState = this.currentAccountState;
+    let accountData = yield currentState.getUserAccountData("keyPair");
+    if (!ignoreCachedAuthCredentials && accountData.keyPair && (accountData.keyPair.validUntil > mustBeValidUntil)) {
+      log.debug("getKeyPair: already have a keyPair");
+      return accountData.keyPair.keyPair;
+    }
+    // Otherwse, create a keypair and set validity limit.
+    let willBeValidUntil = this.now() + KEY_LIFETIME;
+    let kp = yield new Promise((resolve, reject) => {
+      jwcrypto.generateKeyPair("DS160", (err, kp) => {
+        if (err) {
+          return reject(err);
+        }
+        log.debug("got keyPair");
+        let toUpdate = {
+          keyPair: {
+            keyPair: kp,
+            validUntil: willBeValidUntil
+          },
+          cert: null
+        };
+        currentState.updateUserAccountData(toUpdate).then(() => {
+          resolve(kp);
+        }).catch(err => {
+          log.error("Failed to update account data with keypair and cert");
+        });
+      });
+    });
+    return kp;
+  }),
+
+  /**
    * returns a promise that fires with the assertion.  If there is no verified
    * signed-in user, fires with null.
    */
   getAssertion: function getAssertion(audience) {
     log.debug("enter getAssertion()");
     let currentState = this.currentAccountState;
     let mustBeValidUntil = this.now() + ASSERTION_USE_PERIOD;
     return currentState.getUserAccountData().then(data => {
       if (!data) {
         // No signed-in user
         return null;
       }
       if (!this.isUserEmailVerified(data)) {
         // Signed-in user has not verified email
         return null;
       }
-      return currentState.getKeyPair(mustBeValidUntil).then(keyPair => {
-        return currentState.getCertificate(data, keyPair, mustBeValidUntil)
+      return this.getKeyPair(mustBeValidUntil).then(keyPair => {
+        return this.getCertificate(data, keyPair, mustBeValidUntil)
           .then(cert => {
             return this.getAssertionFromCert(data, keyPair, cert, audience);
           });
       });
     }).then(result => currentState.resolve(result));
   },
 
   /**
@@ -840,16 +826,47 @@ FxAccountsInternal.prototype = {
     }
     return this.fxAccountsClient.signCertificate(
       sessionToken,
       JSON.parse(serializedPublicKey),
       lifetime
     );
   },
 
+  /**
+   * returns a promise that fires with a certificate.
+   */
+  getCertificate: Task.async(function* (data, keyPair, mustBeValidUntil) {
+    // TODO: get the lifetime from the cert's .exp field
+    let currentState = this.currentAccountState;
+    let accountData = yield currentState.getUserAccountData("cert");
+    if (accountData.cert && accountData.cert.validUntil > mustBeValidUntil) {
+      log.debug(" getCertificate already had one");
+      return accountData.cert.cert;
+    }
+    if (Services.io.offline) {
+      throw new Error(ERROR_OFFLINE);
+    }
+    let willBeValidUntil = this.now() + CERT_LIFETIME;
+    let cert = yield this.getCertificateSigned(data.sessionToken,
+                                               keyPair.serializedPublicKey,
+                                               CERT_LIFETIME);
+    log.debug("getCertificate got a new one: " + !!cert);
+    if (cert) {
+      let toUpdate = {
+        cert: {
+          cert: cert,
+          validUntil: willBeValidUntil
+        }
+      };
+      yield currentState.updateUserAccountData(toUpdate);
+    }
+    return cert;
+  }),
+
   getUserAccountData: function() {
     return this.currentAccountState.getUserAccountData();
   },
 
   isUserEmailVerified: function isUserEmailVerified(data) {
     return !!(data && data.verified);
   },
 
--- a/services/fxaccounts/FxAccountsCommon.js
+++ b/services/fxaccounts/FxAccountsCommon.js
@@ -207,23 +207,32 @@ exports.ERROR_AUTH_ERROR                
 exports.ERROR_INVALID_PARAMETER              = "INVALID_PARAMETER";
 
 // Status code errors
 exports.ERROR_CODE_METHOD_NOT_ALLOWED        = 405;
 exports.ERROR_MSG_METHOD_NOT_ALLOWED         = "METHOD_NOT_ALLOWED";
 
 // FxAccounts has the ability to "split" the credentials between a plain-text
 // JSON file in the profile dir and in the login manager.
-// These constants relate to that.
+// In order to prevent new fields accidentally ending up in the "wrong" place,
+// all fields stored are listed here.
 
 // The fields we save in the plaintext JSON.
 // See bug 1013064 comments 23-25 for why the sessionToken is "safe"
-exports.FXA_PWDMGR_PLAINTEXT_FIELDS = ["email", "verified", "authAt",
-                                       "sessionToken", "uid", "oauthTokens",
-                                       "profile"];
+exports.FXA_PWDMGR_PLAINTEXT_FIELDS = new Set(
+  ["email", "verified", "authAt", "sessionToken", "uid", "oauthTokens", "profile"]);
+
+// Fields we store in secure storage if it exists.
+exports.FXA_PWDMGR_SECURE_FIELDS = new Set(
+  ["kA", "kB", "keyFetchToken", "unwrapBKey", "assertion"]);
+
+// Fields we keep in memory and don't persist anywhere.
+exports.FXA_PWDMGR_MEMORY_FIELDS = new Set(
+  ["cert", "keyPair"]);
+
 // The pseudo-host we use in the login manager
 exports.FXA_PWDMGR_HOST = "chrome://FirefoxAccounts";
 // The realm we use in the login manager.
 exports.FXA_PWDMGR_REALM = "Firefox Accounts credentials";
 
 // Error matching.
 exports.SERVER_ERRNO_TO_ERROR = {};
 
--- a/services/fxaccounts/FxAccountsStorage.jsm
+++ b/services/fxaccounts/FxAccountsStorage.jsm
@@ -57,20 +57,28 @@ this.FxAccountsStorageManager.prototype 
   _initialize: Task.async(function* (accountData) {
     log.trace("initializing new storage manager");
     try {
       if (accountData) {
         // If accountData is passed we don't need to read any storage.
         this._needToReadSecure = false;
         // split it into the 2 parts, write it and we are done.
         for (let [name, val] of Iterator(accountData)) {
-          if (FXA_PWDMGR_PLAINTEXT_FIELDS.indexOf(name) >= 0) {
+          if (FXA_PWDMGR_PLAINTEXT_FIELDS.has(name)) {
             this.cachedPlain[name] = val;
+          } else if (FXA_PWDMGR_SECURE_FIELDS.has(name)) {
+            this.cachedSecure[name] = val;
           } else {
-            this.cachedSecure[name] = val;
+            // Hopefully it's an "in memory" field. If it's not we log a warning
+            // but still treat it as such (so it will still be available in this
+            // session but isn't persisted anywhere.)
+            if (!FXA_PWDMGR_MEMORY_FIELDS.has(name)) {
+              log.warn("Unknown FxA field name in user data, treating as in-memory", name);
+            }
+            this.cachedMemory[name] = val;
           }
         }
         // write it out and we are done.
         yield this._write();
         return;
       }
       // So we were initialized without account data - that means we need to
       // read the state from storage. We try and read plain storage first and
@@ -116,40 +124,77 @@ this.FxAccountsStorageManager.prototype 
     // be in a rejected state)
     this._promiseStorageComplete = result.catch(err => {
       log.error("${func} failed: ${err}", {func, err});
     });
     return result;
   },
 
   // Get the account data by combining the plain and secure storage.
-  getAccountData: Task.async(function* () {
+  // If fieldNames is specified, it may be a string or an array of strings,
+  // and only those fields are returned. If not specified the entire account
+  // data is returned except for "in memory" fields. Note that not specifying
+  // field names will soon be deprecated/removed - we want all callers to
+  // specify the fields they care about.
+  getAccountData: Task.async(function* (fieldNames = null) {
     yield this._promiseInitialized;
     // We know we are initialized - this means our .cachedPlain is accurate
     // and doesn't need to be read (it was read if necessary by initialize).
     // So if there's no uid, there's no user signed in.
     if (!('uid' in this.cachedPlain)) {
       return null;
     }
     let result = {};
-    for (let [name, value] of Iterator(this.cachedPlain)) {
-      result[name] = value;
+    if (fieldNames === null) {
+      // The "old" deprecated way of fetching a logged in user.
+      for (let [name, value] of Iterator(this.cachedPlain)) {
+        result[name] = value;
+      }
+      // But the secure data may not have been read, so try that now.
+      yield this._maybeReadAndUpdateSecure();
+      // .cachedSecure now has as much as it possibly can (which is possibly
+      // nothing if (a) secure storage remains locked and (b) we've never updated
+      // a field to be stored in secure storage.)
+      for (let [name, value] of Iterator(this.cachedSecure)) {
+        result[name] = value;
+      }
+      // Note we don't return cachedMemory fields here - they must be explicitly
+      // requested.
+      return result;
+    }
+    // The new explicit way of getting attributes.
+    if (!Array.isArray(fieldNames)) {
+      fieldNames = [fieldNames];
     }
-    // But the secure data may not have been read, so try that now.
-    yield this._maybeReadAndUpdateSecure();
-    // .cachedSecure now has as much as it possibly can (which is possibly
-    // nothing if (a) secure storage remains locked and (b) we've never updated
-    // a field to be stored in secure storage.)
-    for (let [name, value] of Iterator(this.cachedSecure)) {
-      result[name] = value;
+    let checkedSecure = false;
+    for (let fieldName of fieldNames) {
+      if (FXA_PWDMGR_MEMORY_FIELDS.has(fieldName)) {
+        if (this.cachedMemory[fieldName] !== undefined) {
+          result[fieldName] = this.cachedMemory[fieldName];
+        }
+      } else if (FXA_PWDMGR_PLAINTEXT_FIELDS.has(fieldName)) {
+        if (this.cachedPlain[fieldName] !== undefined) {
+          result[fieldName] = this.cachedPlain[fieldName];
+        }
+      } else if (FXA_PWDMGR_SECURE_FIELDS.has(fieldName)) {
+        // We may not have read secure storage yet.
+        if (!checkedSecure) {
+          yield this._maybeReadAndUpdateSecure();
+          checkedSecure = true;
+        }
+        if (this.cachedSecure[fieldName] !== undefined) {
+          result[fieldName] = this.cachedSecure[fieldName];
+        }
+      } else {
+        throw new Error("unexpected field '" + name + "'");
+      }
     }
     return result;
   }),
 
-
   // Update just the specified fields. This DOES NOT allow you to change to
   // a different user, nor to set the user as signed-out.
   updateAccountData: Task.async(function* (newFields) {
     yield this._promiseInitialized;
     if (!('uid' in this.cachedPlain)) {
       // If this storage instance shows no logged in user, then you can't
       // update fields.
       throw new Error("No user is logged in");
@@ -158,38 +203,50 @@ this.FxAccountsStorageManager.prototype 
       // Once we support
       // user changing email address this may need to change, but it's not
       // clear how we would be told of such a change anyway...
       throw new Error("Can't change uid or email address");
     }
     log.debug("_updateAccountData with items", Object.keys(newFields));
     // work out what bucket.
     for (let [name, value] of Iterator(newFields)) {
-      if (FXA_PWDMGR_PLAINTEXT_FIELDS.indexOf(name) >= 0) {
+      if (FXA_PWDMGR_MEMORY_FIELDS.has(name)) {
+        if (value == null) {
+          delete this.cachedMemory[name];
+        } else {
+          this.cachedMemory[name] = value;
+        }
+      } else if (FXA_PWDMGR_PLAINTEXT_FIELDS.has(name)) {
         if (value == null) {
           delete this.cachedPlain[name];
         } else {
           this.cachedPlain[name] = value;
         }
-      } else {
+      } else if (FXA_PWDMGR_SECURE_FIELDS.has(name)) {
         // don't do the "delete on null" thing here - we need to keep it until
         // we have managed to read so we can nuke it on write.
         this.cachedSecure[name] = value;
+      } else {
+        // Throwing seems reasonable here as some client code has explicitly
+        // specified the field name, so it's either confused or needs to update
+        // how this field is to be treated.
+        throw new Error("unexpected field '" + name + "'");
       }
     }
     // If we haven't yet read the secure data, do so now, else we may write
     // out partial data.
     yield this._maybeReadAndUpdateSecure();
     // Now save it - but don't wait on the _write promise - it's queued up as
     // a storage operation, so .finalize() will wait for completion, but no need
     // for us to.
     this._write();
   }),
 
   _clearCachedData() {
+    this.cachedMemory = {};
     this.cachedPlain = {};
     // If we don't have secure storage available we have cachedPlain and
     // cachedSecure be the same object.
     this.cachedSecure = this.secureStorage == null ? this.cachedPlain : {};
   },
 
   /* Reads the plain storage and caches the read values in this.cachedPlain.
      Only ever called once and unlike the "secure" storage, is expected to never
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -150,17 +150,17 @@ function MockFxAccounts() {
     _now_is: new Date(),
     now: function () {
       return this._now_is;
     },
     newAccountState(credentials) {
       // we use a real accountState but mocked storage.
       let storage = new MockStorageManager();
       storage.initialize(credentials);
-      return new AccountState(this, storage);
+      return new AccountState(storage);
     },
     getCertificateSigned: function (sessionToken, serializedPublicKey) {
       _("mock getCertificateSigned\n");
       this._getCertificateSigned_calls.push([sessionToken, serializedPublicKey]);
       return this._d_signCertificate.promise;
     },
     fxAccountsClient: new MockFxAccountsClient()
   });
@@ -197,17 +197,17 @@ add_test(function test_non_https_remote_
 add_task(function test_get_signed_in_user_initially_unset() {
   // This test, unlike many of the the rest, uses a (largely) un-mocked
   // FxAccounts instance.
   let account = new FxAccounts({
     newAccountState(credentials) {
       // we use a real accountState but mocked storage.
       let storage = new MockStorageManager();
       storage.initialize(credentials);
-      return new AccountState(this, storage);
+      return new AccountState(storage);
     },
   });
   let credentials = {
     email: "foo@example.com",
     uid: "1234@lcip.org",
     assertion: "foobar",
     sessionToken: "dead",
     kA: "beef",
@@ -246,17 +246,17 @@ add_task(function* test_getCertificate()
   // This test, unlike many of the the rest, uses a (largely) un-mocked
   // FxAccounts instance.
   // We do mock the storage to keep the test fast on b2g.
   let fxa = new FxAccounts({
     newAccountState(credentials) {
       // we use a real accountState but mocked storage.
       let storage = new MockStorageManager();
       storage.initialize(credentials);
-      return new AccountState(this, storage);
+      return new AccountState(storage);
     },
   });
   let credentials = {
     email: "foo@example.com",
     uid: "1234@lcip.org",
     assertion: "foobar",
     sessionToken: "dead",
     kA: "beef",
@@ -267,17 +267,17 @@ add_task(function* test_getCertificate()
 
   // Test that an expired cert throws if we're offline.
   fxa.internal.currentAccountState.cert = {
     validUntil: Date.parse("Mon, 13 Jan 2000 21:45:06 GMT")
   };
   let offline = Services.io.offline;
   Services.io.offline = true;
   // This call would break from missing parameters ...
-  yield fxa.internal.currentAccountState.getCertificate().then(
+  yield fxa.internal.getCertificate().then(
     result => {
       Services.io.offline = offline;
       do_throw("Unexpected success");
     },
     err => {
       Services.io.offline = offline;
       // ... so we have to check the error string.
       do_check_eq(err, "Error: OFFLINE");
@@ -500,18 +500,19 @@ add_task(function test_getAssertion() {
   fxa.internal._d_signCertificate.resolve("cert1");
   let assertion = yield d;
   do_check_eq(fxa.internal._getCertificateSigned_calls.length, 1);
   do_check_eq(fxa.internal._getCertificateSigned_calls[0][0], "sessionToken");
   do_check_neq(assertion, null);
   _("ASSERTION: " + assertion + "\n");
   let pieces = assertion.split("~");
   do_check_eq(pieces[0], "cert1");
-  let keyPair = fxa.internal.currentAccountState.keyPair;
-  let cert = fxa.internal.currentAccountState.cert;
+  let userData = yield fxa.getSignedInUser();
+  let keyPair = userData.keyPair;
+  let cert = userData.cert;
   do_check_neq(keyPair, undefined);
   _(keyPair.validUntil + "\n");
   let p2 = pieces[1].split(".");
   let header = JSON.parse(atob(p2[0]));
   _("HEADER: " + JSON.stringify(header) + "\n");
   do_check_eq(header.alg, "DS128");
   let payload = JSON.parse(atob(p2[1]));
   _("PAYLOAD: " + JSON.stringify(payload) + "\n");
@@ -548,19 +549,20 @@ add_task(function test_getAssertion() {
   header = JSON.parse(atob(p2[0]));
   payload = JSON.parse(atob(p2[1]));
   do_check_eq(payload.aud, "third.example.com");
 
   // The keypair and cert should have the same validity as before, but the
   // expiration time of the assertion should be different.  We compare this to
   // the initial start time, to which they are relative, not the current value
   // of "now".
+  userData = yield fxa.getSignedInUser();
 
-  keyPair = fxa.internal.currentAccountState.keyPair;
-  cert = fxa.internal.currentAccountState.cert;
+  keyPair = userData.keyPair;
+  cert = userData.cert;
   do_check_eq(keyPair.validUntil, start + KEY_LIFETIME);
   do_check_eq(cert.validUntil, start + CERT_LIFETIME);
   exp = Number(payload.exp);
   do_check_eq(exp, now + ASSERTION_LIFETIME);
 
   // Now we wait even longer, and expect both assertion and cert to expire.  So
   // we will have to get a new keypair and cert.
   now += ONE_DAY_MS;
@@ -571,18 +573,19 @@ add_task(function test_getAssertion() {
   do_check_eq(fxa.internal._getCertificateSigned_calls.length, 2);
   do_check_eq(fxa.internal._getCertificateSigned_calls[1][0], "sessionToken");
   pieces = assertion.split("~");
   do_check_eq(pieces[0], "cert2");
   p2 = pieces[1].split(".");
   header = JSON.parse(atob(p2[0]));
   payload = JSON.parse(atob(p2[1]));
   do_check_eq(payload.aud, "fourth.example.com");
-  keyPair = fxa.internal.currentAccountState.keyPair;
-  cert = fxa.internal.currentAccountState.cert;
+  userData = yield fxa.getSignedInUser();
+  keyPair = userData.keyPair;
+  cert = userData.cert;
   do_check_eq(keyPair.validUntil, now + KEY_LIFETIME);
   do_check_eq(cert.validUntil, now + CERT_LIFETIME);
   exp = Number(payload.exp);
 
   do_check_eq(exp, now + ASSERTION_LIFETIME);
   _("----- DONE ----\n");
 });
 
--- a/services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js
+++ b/services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js
@@ -80,17 +80,17 @@ MockFxAccountsClient.prototype = {
 
 function MockFxAccounts() {
   return new FxAccounts({
     fxAccountsClient: new MockFxAccountsClient(),
     newAccountState(credentials) {
       // we use a real accountState but mocked storage.
       let storage = new MockStorageManager();
       storage.initialize(credentials);
-      return new AccountState(this, storage);
+      return new AccountState(storage);
     },
   });
 }
 
 function* createMockFxA() {
   let fxa = new MockFxAccounts();
   let credentials = {
     email: "foo@example.com",
--- a/services/fxaccounts/tests/xpcshell/test_storage_manager.js
+++ b/services/fxaccounts/tests/xpcshell/test_storage_manager.js
@@ -46,19 +46,21 @@ function MockedSecureStorage(accountData
       accountData: accountData,
     }
   }
   this.data = data;
   this.numReads = 0;
 }
 
 MockedSecureStorage.prototype = {
+  fetchCount: 0,
   locked: false,
   STORAGE_LOCKED: function() {},
   get: Task.async(function* (uid, email) {
+    this.fetchCount++;
     if (this.locked) {
       throw new this.STORAGE_LOCKED();
     }
     this.numReads++;
     Assert.equal(this.numReads, 1, "should only ever be 1 read of unlocked data");
     return this.data;
   }),
 
@@ -80,17 +82,17 @@ function add_storage_task(testFunction) 
 
 // initialized without account data and there's nothing to read. Not logged in.
 add_storage_task(function* checkInitializedEmpty(sm) {
   if (sm.secureStorage) {
     sm.secureStorage = new MockedSecureStorage(null);
   }
   yield sm.initialize();
   Assert.strictEqual((yield sm.getAccountData()), null);
-  Assert.rejects(sm.updateAccountData({foo: "bar"}), "No user is logged in")
+  Assert.rejects(sm.updateAccountData({kA: "kA"}), "No user is logged in")
 });
 
 // Initialized with account data (ie, simulating a new user being logged in).
 // Should reflect the initial data and be written to storage.
 add_storage_task(function* checkNewUser(sm) {
   let initialAccountData = {
     uid: "uid",
     email: "someone@somewhere.com",
@@ -125,31 +127,31 @@ add_storage_task(function* checkEverythi
   }
   yield sm.initialize();
   let accountData = yield sm.getAccountData();
   Assert.ok(accountData, "read account data");
   Assert.equal(accountData.uid, "uid");
   Assert.equal(accountData.email, "someone@somewhere.com");
   // Update the data - we should be able to fetch it back and it should appear
   // in our storage.
-  yield sm.updateAccountData({verified: true, foo: "bar", kA: "kA"});
+  yield sm.updateAccountData({verified: true, kA: "kA", kB: "kB"});
   accountData = yield sm.getAccountData();
-  Assert.equal(accountData.foo, "bar");
+  Assert.equal(accountData.kB, "kB");
   Assert.equal(accountData.kA, "kA");
   // Check the new value was written to storage.
   yield sm._promiseStorageComplete; // storage is written in the background.
   // "verified" is a plain-text field.
   Assert.equal(sm.plainStorage.data.accountData.verified, true);
   // "kA" and "foo" are secure
   if (sm.secureStorage) {
     Assert.equal(sm.secureStorage.data.accountData.kA, "kA");
-    Assert.equal(sm.secureStorage.data.accountData.foo, "bar");
+    Assert.equal(sm.secureStorage.data.accountData.kB, "kB");
   } else {
     Assert.equal(sm.plainStorage.data.accountData.kA, "kA");
-    Assert.equal(sm.plainStorage.data.accountData.foo, "bar");
+    Assert.equal(sm.plainStorage.data.accountData.kB, "kB");
   }
 });
 
 add_storage_task(function* checkInvalidUpdates(sm) {
   sm.plainStorage = new MockedPlainStorage({uid: "uid", email: "someone@somewhere.com"})
   if (sm.secureStorage) {
     sm.secureStorage = new MockedSecureStorage(null);
   }
@@ -226,16 +228,63 @@ add_task(function* checkEverythingReadSe
 
   let accountData = yield sm.getAccountData();
   Assert.ok(accountData, "read account data");
   Assert.equal(accountData.uid, "uid");
   Assert.equal(accountData.email, "someone@somewhere.com");
   Assert.equal(accountData.kA, "kA");
 });
 
+add_task(function* checkMemoryFieldsNotReturnedByDefault() {
+  let sm = new FxAccountsStorageManager();
+  sm.plainStorage = new MockedPlainStorage({uid: "uid", email: "someone@somewhere.com"})
+  sm.secureStorage = new MockedSecureStorage({kA: "kA"});
+  yield sm.initialize();
+
+  // keyPair is a memory field.
+  yield sm.updateAccountData({keyPair: "the keypair value"});
+  let accountData = yield sm.getAccountData();
+
+  // Requesting everything should *not* return in memory fields.
+  Assert.strictEqual(accountData.keyPair, undefined);
+  // But requesting them specifically does get them.
+  accountData = yield sm.getAccountData("keyPair");
+  Assert.strictEqual(accountData.keyPair, "the keypair value");
+});
+
+add_task(function* checkExplicitGet() {
+  let sm = new FxAccountsStorageManager();
+  sm.plainStorage = new MockedPlainStorage({uid: "uid", email: "someone@somewhere.com"})
+  sm.secureStorage = new MockedSecureStorage({kA: "kA"});
+  yield sm.initialize();
+
+  let accountData = yield sm.getAccountData(["uid", "kA"]);
+  Assert.ok(accountData, "read account data");
+  Assert.equal(accountData.uid, "uid");
+  Assert.equal(accountData.kA, "kA");
+  // We didn't ask for email so shouldn't have got it.
+  Assert.strictEqual(accountData.email, undefined);
+});
+
+add_task(function* checkExplicitGetNoSecureRead() {
+  let sm = new FxAccountsStorageManager();
+  sm.plainStorage = new MockedPlainStorage({uid: "uid", email: "someone@somewhere.com"})
+  sm.secureStorage = new MockedSecureStorage({kA: "kA"});
+  yield sm.initialize();
+
+  Assert.equal(sm.secureStorage.fetchCount, 0);
+  // request 2 fields in secure storage - it should have caused a single fetch.
+  let accountData = yield sm.getAccountData(["email", "uid"]);
+  Assert.ok(accountData, "read account data");
+  Assert.equal(accountData.uid, "uid");
+  Assert.equal(accountData.email, "someone@somewhere.com");
+  Assert.strictEqual(accountData.kA, undefined);
+  Assert.equal(sm.secureStorage.fetchCount, 1);
+});
+
 add_task(function* checkLockedUpdates() {
   let sm = new FxAccountsStorageManager();
   sm.plainStorage = new MockedPlainStorage({uid: "uid", email: "someone@somewhere.com"})
   sm.secureStorage = new MockedSecureStorage({kA: "old-kA", kB: "kB"});
   sm.secureStorage.locked = true;
   yield sm.initialize();
 
   let accountData = yield sm.getAccountData();
--- a/services/sync/modules-testing/utils.js
+++ b/services/sync/modules-testing/utils.js
@@ -176,27 +176,27 @@ this.configureFxAccountIdentity = functi
     newAccountState(credentials) {
       // We only expect this to be called with null indicating the (mock)
       // storage should be read.
       if (credentials) {
         throw new Error("Not expecting to have credentials passed");
       }
       let storageManager = new MockFxaStorageManager();
       storageManager.initialize(config.fxaccount.user);
-      let accountState = new AccountState(this, storageManager);
-      // mock getCertificate
-      accountState.getCertificate = function(data, keyPair, mustBeValidUntil) {
-        accountState.cert = {
-          validUntil: fxa.internal.now() + CERT_LIFETIME,
-          cert: "certificate",
-        };
-        return Promise.resolve(this.cert.cert);
-      }
+      let accountState = new AccountState(storageManager);
       return accountState;
-    }
+    },
+    getCertificate(data, keyPair, mustBeValidUntil) {
+      let cert = {
+        validUntil: this.now() + CERT_LIFETIME,
+        cert: "certificate",
+      };
+      this.currentAccountState.updateUserAccountData({cert: cert});
+      return Promise.resolve(cert.cert);
+    },
   };
   fxa = new FxAccounts(MockInternal);
 
   let mockTSC = { // TokenServerClient
     getTokenFromBrowserIDAssertion: function(uri, assertion, cb) {
       config.fxaccount.token.uid = config.username;
       cb(null, config.fxaccount.token);
     },
--- a/services/sync/tests/unit/test_browserid_identity.js
+++ b/services/sync/tests/unit/test_browserid_identity.js
@@ -590,17 +590,17 @@ add_task(function test_getKeysMissing() 
     newAccountState(credentials) {
       // We only expect this to be called with null indicating the (mock)
       // storage should be read.
       if (credentials) {
         throw new Error("Not expecting to have credentials passed");
       }
       let storageManager = new MockFxaStorageManager();
       storageManager.initialize(identityConfig.fxaccount.user);
-      return new AccountState(this, storageManager);
+      return new AccountState(storageManager);
     },
   });
 
   // Add a mock to the currentAccountState object.
   fxa.internal.currentAccountState.getCertificate = function(data, keyPair, mustBeValidUntil) {
     this.cert = {
       validUntil: fxa.internal.now() + CERT_LIFETIME,
       cert: "certificate",
@@ -669,17 +669,17 @@ function* initializeIdentityWithHAWKResp
     newAccountState(credentials) {
       // We only expect this to be called with null indicating the (mock)
       // storage should be read.
       if (credentials) {
         throw new Error("Not expecting to have credentials passed");
       }
       let storageManager = new MockFxaStorageManager();
       storageManager.initialize(config.fxaccount.user);
-      return new AccountState(this, storageManager);
+      return new AccountState(storageManager);
     },
   }
   let fxa = new FxAccounts(internal);
 
   browseridManager._fxaService = fxa;
   browseridManager._signedInUser = null;
   yield browseridManager.initializeWithCurrentIdentity();
   yield Assert.rejects(browseridManager.whenReadyToAuthenticate.promise,