Bug 1017433 (part 2) - allow for upload and download of encrypted sync sentinel. r=rnewman

services/sync/modules/FxaMigrator.jsm

@@ -249,11 +249,7 @@ Migrator.prototype = {
       verified: signedInUser.verified,
       prefs: this._getSentinelPrefs(),
     };
-    if (Weave.Service.setFxaMigrationSentinel) {
-      yield Weave.Service.setFxaMigrationSentinel(sentinel);
-    } else {
-      this.log.warn("Waiting on bug 1017433; no sync sentinel");
-    }
+    yield Weave.Service.setFxAMigrationSentinel(sentinel);
   }),
 
   /* Ask sync to upload the migration sentinal if we (or any other linked device)
@@ -269,11 +265,7 @@ Migrator.prototype = {
   /* Ask sync to return a migration sentinel if one exists, otherwise return null */
   _getSyncMigrationSentinel: Task.async(function* () {
     yield WeaveService.whenLoaded();
-    if (!Weave.Service.getFxaMigrationSentinel) {
-      this.log.warn("Waiting on bug 1017433; no sync sentinel");
-      return null;
-    }
-    let sentinel = yield Weave.Service.getFxaMigrationSentinel();
+    let sentinel = yield Weave.Service.getFxAMigrationSentinel();
     this.log.debug("got migration sentinel ${}", sentinel);
     return sentinel;
   }),

services/sync/modules/service.js

@@ -1280,7 +1280,16 @@ Sync11Service.prototype = {
       histogram = Services.telemetry.getHistogramById("WEAVE_COMPLETE_SUCCESS_COUNT");
       histogram.add(1);
 
-      // We successfully synchronized. Now let's update our declined engines.
+      // We successfully synchronized.
+      // Try and fetch the migration sentinel - it will end up in the recordManager
+      // cache, so a sync migration doesn't need a server round-trip.
+      // If we have no clusterURL, we are probably doing a node reassignment
+      // do don't attempt to get the credentials.
+      if (this.clusterURL) {
+        this.recordManager.get(this.storageURL + "meta/fxa_credentials");
+      }
+
+      // Now let's update our declined engines.
       let meta = this.recordManager.get(this.metaURL);
       if (!meta) {
         this._log.warn("No meta/global; can't update declined state.");
@@ -1316,6 +1325,92 @@ Sync11Service.prototype = {
     this.recordManager.set(this.metaURL, meta);
   },
 
+  /**
+   * Get a migration sentinel for the Firefox Accounts migration.
+   * Returns a JSON blob - it is up to callers of this to make sense of the
+   * data.
+   *
+   * Returns a promise that resolves with the sentinel, or null.
+   */
+  getFxAMigrationSentinel: function() {
+    if (this._shouldLogin()) {
+      this._log.debug("In getFxAMigrationSentinel: should login.");
+      if (!this.login()) {
+        this._log.debug("Can't get migration sentinel: login returned false.");
+        return Promise.resolve(null);
+      }
+    }
+    if (!this.identity.syncKeyBundle) {
+      this._log.error("Can't get migration sentinel: no syncKeyBundle.");
+      return Promise.resolve(null);
+    }
+    try {
+      let collectionURL = this.storageURL + "meta/fxa_credentials";
+      let cryptoWrapper = this.recordManager.get(collectionURL);
+      if (!cryptoWrapper.payload) {
+        // nothing to decrypt - .decrypt is noisy in that case, so just bail
+        // now.
+        return Promise.resolve(null);
+      }
+      // If the payload has a sentinel it means we must have put back the
+      // decrypted version last time we were called.
+      if (cryptoWrapper.payload.sentinel) {
+        return Promise.resolve(cryptoWrapper.payload.sentinel);
+      }
+      // If decryption fails it almost certainly means the key is wrong - but
+      // it's not clear if we need to take special action for that case?
+      let payload = cryptoWrapper.decrypt(this.identity.syncKeyBundle);
+      // After decrypting the ciphertext is lost, so we just stash the
+      // decrypted payload back into the wrapper.
+      cryptoWrapper.payload = payload;
+      return Promise.resolve(payload.sentinel);
+    } catch (ex) {
+      this._log.error("Failed to fetch the migration sentinel: ${}", ex);
+      return Promise.resolve(null);
+    }
+  },
+
+  /**
+   * Set a migration sentinel for the Firefox Accounts migration.
+   * Accepts a JSON blob - it is up to callers of this to make sense of the
+   * data.
+   *
+   * Returns a promise that resolves with a boolean which indicates if the
+   * sentinel was successfully written.
+   */
+  setFxAMigrationSentinel: function(sentinel) {
+    if (this._shouldLogin()) {
+      this._log.debug("In setFxAMigrationSentinel: should login.");
+      if (!this.login()) {
+        this._log.debug("Can't set migration sentinel: login returned false.");
+        return Promise.resolve(false);
+      }
+    }
+    if (!this.identity.syncKeyBundle) {
+      this._log.error("Can't set migration sentinel: no syncKeyBundle.");
+      return Promise.resolve(false);
+    }
+    try {
+      let collectionURL = this.storageURL + "meta/fxa_credentials";
+      let cryptoWrapper = new CryptoWrapper("meta", "fxa_credentials");
+      cryptoWrapper.cleartext.sentinel = sentinel;
+
+      cryptoWrapper.encrypt(this.identity.syncKeyBundle);
+
+      let res = this.resource(collectionURL);
+      let response = res.put(cryptoWrapper.toJSON());
+
+      if (!response.success) {
+        throw response;
+      }
+      this.recordManager.set(collectionURL, cryptoWrapper);
+    } catch (ex) {
+      this._log.error("Failed to set the migration sentinel: ${}", ex);
+      return Promise.resolve(false);
+    }
+    return Promise.resolve(true);
+  },
+
   /**
    * If we have a passphrase, rather than a 25-alphadigit sync key,
    * use the provided sync ID to bootstrap it using PBKDF2.

services/sync/tests/unit/test_fxa_migration.js

@@ -103,20 +103,18 @@ add_task(function *testMigration() {
 
   // monkey-patch the migration sentinel code so we know it was called.
   let haveStartedSentinel = false;
-// (This is waiting on bug 1017433)
-/**
-  let origSetFxaMigrationSentinel = Service.setFxaMigrationSentinel;
+  let origSetFxAMigrationSentinel = Service.setFxAMigrationSentinel;
   let promiseSentinelWritten = new Promise((resolve, reject) => {
-    Service.setFxaMigrationSentinel = function(arg) {
+    Service.setFxAMigrationSentinel = function(arg) {
       haveStartedSentinel = true;
-      return origSetFxaMigrationSentinel.call(Service, arg).then(result => {
-        Service.setFxaMigrationSentinel = origSetFxaMigrationSentinel;
+      return origSetFxAMigrationSentinel.call(Service, arg).then(result => {
+        Service.setFxAMigrationSentinel = origSetFxAMigrationSentinel;
         resolve(result);
         return result;
       });
     }
   });
-**/
+
   // We are now configured for legacy sync, but we aren't in an EOL state yet,
   // so should still be not waiting for a user.
   Assert.deepEqual((yield fxaMigrator._queueCurrentUserState()), null,
@@ -227,10 +225,7 @@ add_task(function *testMigration() {
   Assert.ok(Service.scheduler.isBlocked, "sync is blocked.");
 
   // We should see the migration sentinel written and it should return true.
-// (This is waiting on bug 1017433)
-/**
   Assert.ok((yield promiseSentinelWritten), "wrote the sentinel");
-**/
 
   // And we should see a new sync start
   yield promiseFinalSync;

services/sync/tests/unit/test_fxa_migration_sentinel.js

@@ -0,0 +1,150 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test the reading and writing of the sync migration sentinel.
+Cu.import("resource://gre/modules/Promise.jsm");
+Cu.import("resource://gre/modules/FxAccounts.jsm");
+Cu.import("resource://gre/modules/FxAccountsCommon.js");
+
+Cu.import("resource://testing-common/services/sync/utils.js");
+Cu.import("resource://testing-common/services/common/logging.js");
+
+Cu.import("resource://services-sync/record.js");
+
+// Set our username pref early so sync initializes with the legacy provider.
+Services.prefs.setCharPref("services.sync.username", "foo");
+
+// Now import sync
+Cu.import("resource://services-sync/service.js");
+
+const USER = "foo";
+const PASSPHRASE = "abcdeabcdeabcdeabcdeabcdea";
+
+function promiseStopServer(server) {
+  return new Promise((resolve, reject) => {
+    server.stop(resolve);
+  });
+}
+
+let numServerRequests = 0;
+
+// Helpers
+function configureLegacySync() {
+  let contents = {
+    meta: {global: {}},
+    crypto: {},
+  };
+
+  setBasicCredentials(USER, "password", PASSPHRASE);
+
+  numServerRequests = 0;
+  let server = new SyncServer({
+    onRequest: () => {
+      ++numServerRequests
+    }
+  });
+  server.registerUser(USER, "password");
+  server.createContents(USER, contents);
+  server.start();
+
+  Service.serverURL = server.baseURI;
+  Service.clusterURL = server.baseURI;
+  Service.identity.username = USER;
+  Service._updateCachedURLs();
+
+  return server;
+}
+
+// Test a simple round-trip of the get/set functions.
+add_task(function *() {
+  // Arrange for a legacy sync user.
+  let server = configureLegacySync();
+
+  Assert.equal((yield Service.getFxAMigrationSentinel()), null, "no sentinel to start");
+
+  let sentinel = {foo: "bar"};
+  yield Service.setFxAMigrationSentinel(sentinel);
+
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got the sentinel back");
+
+  yield promiseStopServer(server);
+});
+
+// Test the records are cached by the record manager.
+add_task(function *() {
+  // Arrange for a legacy sync user.
+  let server = configureLegacySync();
+  Service.login();
+
+  // Reset the request count here as the login would have made some.
+  numServerRequests = 0;
+
+  Assert.equal((yield Service.getFxAMigrationSentinel()), null, "no sentinel to start");
+  Assert.equal(numServerRequests, 1, "first fetch should hit the server");
+
+  let sentinel = {foo: "bar"};
+  yield Service.setFxAMigrationSentinel(sentinel);
+  Assert.equal(numServerRequests, 2, "setting sentinel should hit the server");
+
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got the sentinel back");
+  Assert.equal(numServerRequests, 2, "second fetch should not should hit the server");
+
+  // Clobber the caches and ensure we still get the correct value back when we
+  // do hit the server.
+  Service.recordManager.clearCache();
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got the sentinel back");
+  Assert.equal(numServerRequests, 3, "should have re-hit the server with empty caches");
+
+  yield promiseStopServer(server);
+});
+
+// Test the records are cached by a sync.
+add_task(function* () {
+  let server = configureLegacySync();
+
+  // A first sync clobbers meta/global due to it being empty, so we first
+  // do a sync which forces a good set of data on the server.
+  Service.sync();
+
+  // Now create a sentinel exists on the server.  It's encrypted, so we need to
+  // put an encrypted version.
+  let cryptoWrapper = new CryptoWrapper("meta", "fxa_credentials");
+  let sentinel = {foo: "bar"};
+  cryptoWrapper.cleartext = {
+    id: "fxa_credentials",
+    sentinel: sentinel,
+    deleted: false,
+  }
+  cryptoWrapper.encrypt(Service.identity.syncKeyBundle);
+  let payload = {
+    ciphertext: cryptoWrapper.ciphertext,
+    IV:         cryptoWrapper.IV,
+    hmac:       cryptoWrapper.hmac,
+  };
+
+  server.createContents(USER, {
+    meta: {fxa_credentials: payload},
+    crypto: {},
+  });
+
+  // Another sync - this will cause the encrypted record to be fetched.
+  Service.sync();
+  // Reset the request count here as the sync will have made many!
+  numServerRequests = 0;
+
+  // Asking for the sentinel should use the copy cached in the record manager.
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got it");
+  Assert.equal(numServerRequests, 0, "should not have hit the server");
+
+  // And asking for it again should work (we have to work around the fact the
+  // ciphertext is clobbered on first decrypt...)
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got it again");
+  Assert.equal(numServerRequests, 0, "should not have hit the server");
+
+  yield promiseStopServer(server);
+});
+
+function run_test() {
+  initTestLogging();
+  run_next_test();
+}

services/sync/tests/unit/xpcshell.ini

@@ -176,3 +176,4 @@ skip-if = ! healthreport
 # FxA migration
 [test_block_sync.js]
 [test_fxa_migration.js]
+[test_fxa_migration_sentinel.js]