Bug 1013064 (part 3) - only migrate data into the loginmgr when it is unlocked. r=ckarlof

From 86c67f9b9081ce905442c86b38575b3422c8dce3 Mon Sep 17 00:00:00 2001
---
 services/fxaccounts/FxAccounts.jsm                 |  25 ++++-
 .../tests/xpcshell/test_loginmgr_storage.js        | 113 +++++++++++++++++++++
 2 files changed, 137 insertions(+), 1 deletion(-)

services/fxaccounts/FxAccounts.jsm

@@ -1030,7 +1030,30 @@ LoginManagerStorage.prototype = {
     // if we have encryption keys it must have been saved before we
     // used the login manager, so re-save it.
     if (data.accountData.kA || data.accountData.kB || data.keyFetchToken) {
-      log.info("account data needs migration to the login manager.");
+      // We need to migrate, but the MP might be locked (eg, on the first run
+      // with this enabled, we will get here very soon after startup, so will
+      // certainly be locked.)  This means we can't actually store the data in
+      // the login manager (and thus might lose it if we migrated now)
+      // So if the MP is locked, we *don't* migrate, but still just return
+      // the subset of data we now store in the JSON.
+      // This will cause sync to notice the lack of keys, force an unlock then
+      // re-fetch the account data to see if the keys are there.  At *that*
+      // point we will end up back here, but because the MP is now unlocked
+      // we can actually perform the migration.
+      if (!this._isLoggedIn) {
+        // return the "safe" subset but leave the storage alone.
+        log.info("account data needs migration to the login manager but the MP is locked.");
+        let result = {
+          version: data.version,
+          accountData: {},
+        };
+        for (let fieldName of FXA_PWDMGR_PLAINTEXT_FIELDS) {
+          result.accountData[fieldName] = data.accountData[fieldName];
+        }
+        return result;
+      }
+      // actually migrate - just calling .set() will split everything up.
+      log.info("account data is being migrated to the login manager.");
       yield this.set(data);
     }
 

services/fxaccounts/tests/xpcshell/test_loginmgr_storage.js

@@ -103,6 +103,119 @@ add_task(function test_MPLocked() {
   yield fxa.signOut(/* localOnly = */ true)
 });
 
+add_task(function test_migrationMPUnlocked() {
+  // first manually save a signedInUser.json to simulate a first-run with
+  // pre-migrated data.
+  let fxa = new FxAccounts({});
+
+  let creds = {
+    email: "test@example.com",
+    sessionToken: "sessionToken",
+    kA: "the kA value",
+    kB: "the kB value",
+    verified: true
+  };
+  let toWrite = {
+    version: fxa.version,
+    accountData: creds,
+  }
+
+  let path = OS.Path.join(OS.Constants.Path.profileDir, "signedInUser.json");
+  yield CommonUtils.writeJSON(toWrite, path);
+
+  // now load it - it should migrate.
+  let data = yield fxa.getSignedInUser();
+  Assert.deepEqual(data, creds, "we got all the data back");
+
+  // and verify it was actually migrated - re-read signedInUser back.
+  let data = yield CommonUtils.readJSON(path);
+
+  Assert.strictEqual(data.accountData.email, creds.email, "correct email in the clear text");
+  Assert.strictEqual(data.accountData.sessionToken, creds.sessionToken, "correct sessionToken in the clear text");
+  Assert.strictEqual(data.accountData.verified, creds.verified, "correct verified flag");
+
+  Assert.ok(!("kA" in data.accountData), "kA not stored in clear text");
+  Assert.ok(!("kB" in data.accountData), "kB not stored in clear text");
+
+  let login = getLoginMgrData();
+  Assert.strictEqual(login.username, creds.email, "email matches");
+  let loginData = JSON.parse(login.password);
+  Assert.strictEqual(loginData.version, data.version, "same version flag in both places");
+  Assert.strictEqual(loginData.accountData.kA, creds.kA, "correct kA in the login mgr");
+  Assert.strictEqual(loginData.accountData.kB, creds.kB, "correct kB in the login mgr");
+
+  Assert.ok(!("email" in loginData), "email not stored in the login mgr json");
+  Assert.ok(!("sessionToken" in loginData), "sessionToken not stored in the login mgr json");
+  Assert.ok(!("verified" in loginData), "verified not stored in the login mgr json");
+
+  yield fxa.signOut(/* localOnly = */ true);
+  Assert.strictEqual(getLoginMgrData(), null, "login mgr data deleted on logout");
+});
+
+add_task(function test_migrationMPLocked() {
+  // first manually save a signedInUser.json to simulate a first-run with
+  // pre-migrated data.
+  let fxa = new FxAccounts({});
+
+  let creds = {
+    email: "test@example.com",
+    sessionToken: "sessionToken",
+    kA: "the kA value",
+    kB: "the kB value",
+    verified: true
+  };
+  let toWrite = {
+    version: fxa.version,
+    accountData: creds,
+  }
+
+  let path = OS.Path.join(OS.Constants.Path.profileDir, "signedInUser.json");
+  yield CommonUtils.writeJSON(toWrite, path);
+
+  // pretend the MP is locked.
+  fxa.internal.signedInUserStorage.__defineGetter__("_isLoggedIn", function() false);
+
+  // now load it - it should *not* migrate, but should only give the JSON-safe
+  // data back.
+  let data = yield fxa.getSignedInUser();
+  Assert.ok(!data.kA);
+  Assert.ok(!data.kB);
+
+  // and verify the data on disk wan't migrated.
+  data = yield CommonUtils.readJSON(path);
+  Assert.deepEqual(data, toWrite);
+
+  // Now "unlock" and re-ask for the signedInUser - it should migrate.
+  fxa.internal.signedInUserStorage.__defineGetter__("_isLoggedIn", function() true);
+  data = yield fxa.getSignedInUser();
+  // this time we should have got all the data, not just the JSON-safe fields.
+  Assert.strictEqual(data.kA, creds.kA);
+  Assert.strictEqual(data.kB, creds.kB);
+
+  // And verify the data in the JSON was migrated
+  data = yield CommonUtils.readJSON(path);
+  Assert.strictEqual(data.accountData.email, creds.email, "correct email in the clear text");
+  Assert.strictEqual(data.accountData.sessionToken, creds.sessionToken, "correct sessionToken in the clear text");
+  Assert.strictEqual(data.accountData.verified, creds.verified, "correct verified flag");
+
+  Assert.ok(!("kA" in data.accountData), "kA not stored in clear text");
+  Assert.ok(!("kB" in data.accountData), "kB not stored in clear text");
+
+  let login = getLoginMgrData();
+  Assert.strictEqual(login.username, creds.email, "email matches");
+  let loginData = JSON.parse(login.password);
+  Assert.strictEqual(loginData.version, data.version, "same version flag in both places");
+  Assert.strictEqual(loginData.accountData.kA, creds.kA, "correct kA in the login mgr");
+  Assert.strictEqual(loginData.accountData.kB, creds.kB, "correct kB in the login mgr");
+
+  Assert.ok(!("email" in loginData), "email not stored in the login mgr json");
+  Assert.ok(!("sessionToken" in loginData), "sessionToken not stored in the login mgr json");
+  Assert.ok(!("verified" in loginData), "verified not stored in the login mgr json");
+
+  yield fxa.signOut(/* localOnly = */ true);
+  Assert.strictEqual(getLoginMgrData(), null, "login mgr data deleted on logout");
+});
+
 add_task(function test_consistentWithMPEdgeCases() {
   let fxa = new FxAccounts({});