Bug 1349709 - Ensure `PasswordEngine` conforms to the new `pullAllChanges` interface. r?tcsc draft
authorKit Cambridge <kit@yakshaving.ninja>
Wed, 22 Mar 2017 19:21:08 -0700
changeset 503316 114170fd9874c8d582acab64d35d2d5a0a35cb67
parent 502995 deff5eb74dcbc29d7e7556b4ce6e15590ee4d4a0
child 550387 7561c82b820a90839ff302ede137a47b30f2cfd7
push id50534
push userbmo:kit@mozilla.com
push dateThu, 23 Mar 2017 02:25:03 +0000
reviewerstcsc
bugs1349709, 1317223
milestone55.0a1
Bug 1349709 - Ensure `PasswordEngine` conforms to the new `pullAllChanges` interface. r?tcsc This was a regression from bug 1317223. This commit also adds a password engine sync test to catch future regressions. MozReview-Commit-ID: 9dq8K39jLwB
services/sync/modules/engines/passwords.js
services/sync/tests/unit/test_password_engine.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/modules/engines/passwords.js
+++ b/services/sync/modules/engines/passwords.js
@@ -94,21 +94,21 @@ PasswordEngine.prototype = {
         return local.guid;
       }
     }
 
     return null;
   },
 
   pullAllChanges() {
-    let changeset = new Changeset();
+    let changes = {};
     for (let [id, info] of Object.entries(this._store.getAllIDs())) {
-      changeset.set(id, info.timePasswordChanged);
+      changes[id] = info.timePasswordChanged / 1000;
     }
-    return changeset;
+    return changes;
   }
 };
 
 function PasswordStore(name, engine) {
   Store.call(this, name, engine);
   this._nsLoginInfo = new Components.Constructor("@mozilla.org/login-manager/loginInfo;1", Ci.nsILoginInfo, "init");
 }
 PasswordStore.prototype = {
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_password_engine.js
@@ -0,0 +1,135 @@
+Cu.import("resource://services-sync/engines/passwords.js");
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://testing-common/services/sync/utils.js");
+
+const LoginInfo = Components.Constructor(
+  "@mozilla.org/login-manager/loginInfo;1", Ci.nsILoginInfo, "init");
+
+const PropertyBag = Components.Constructor(
+  "@mozilla.org/hash-property-bag;1", Ci.nsIWritablePropertyBag);
+
+function serverForEngine(engine) {
+  let clientsEngine = Service.clientsEngine;
+  return serverForUsers({"foo": "password"}, {
+    meta: {
+      global: {
+        syncID: Service.syncID,
+        storageVersion: STORAGE_VERSION,
+        engines: {
+          clients: {
+            version: clientsEngine.version,
+            syncID: clientsEngine.syncID,
+          },
+          [engine.name]: {
+            version: engine.version,
+            syncID: engine.syncID,
+          },
+        },
+      },
+    },
+    crypto: {
+      keys: encryptPayload({
+        id: "keys",
+        // Generate a fake default key bundle to avoid resetting the client
+        // before the first sync.
+        default: [
+          Svc.Crypto.generateRandomKey(),
+          Svc.Crypto.generateRandomKey(),
+        ],
+      }),
+    },
+    [engine.name]: {},
+  });
+}
+
+add_task(async function test_password_engine() {
+  _("Basic password sync test");
+
+  let engine = Service.engineManager.get("passwords");
+  let store = engine._store;
+
+  let server = serverForEngine(engine);
+  await SyncTestingInfrastructure(server);
+  let collection = server.user("foo").collection("passwords");
+
+  generateNewKeys(Service.collectionKeys);
+  enableValidationPrefs();
+
+  _("Add new login to upload during first sync");
+  let newLogin;
+  {
+    let login = new LoginInfo("https://example.com", "", null, "username",
+      "password", "", "");
+    Services.logins.addLogin(login);
+
+    let logins = Services.logins.findLogins({}, "https://example.com", "", "");
+    equal(logins.length, 1, "Should find new login in login manager");
+    newLogin = logins[0].QueryInterface(Ci.nsILoginMetaInfo);
+
+    // Insert a server record that's older, so that we prefer the local one.
+    let rec = new LoginRec("passwords", newLogin.guid);
+    rec.formSubmitURL = newLogin.formSubmitURL;
+    rec.httpRealm = newLogin.httpRealm;
+    rec.hostname = newLogin.hostname;
+    rec.username = newLogin.username;
+    rec.password = "sekrit";
+    let remotePasswordChangeTime = Date.now() - 1 * 60 * 60 * 24 * 1000;
+    rec.timeCreated = remotePasswordChangeTime;
+    rec.timePasswordChanged = remotePasswordChangeTime;
+    collection.insert(newLogin.guid, encryptPayload(rec.cleartext),
+      remotePasswordChangeTime / 1000);
+  }
+
+  _("Add login with older password change time to replace during first sync");
+  let oldLogin;
+  {
+    let login = new LoginInfo("https://mozilla.com", "", null, "us3r",
+      "0ldpa55", "", "");
+    Services.logins.addLogin(login);
+
+    let props = new PropertyBag();
+    let localPasswordChangeTime = Date.now() - 1 * 60 * 60 * 24 * 1000;
+    props.setProperty("timePasswordChanged", localPasswordChangeTime);
+    Services.logins.modifyLogin(login, props);
+
+    let logins = Services.logins.findLogins({}, "https://mozilla.com", "", "");
+    equal(logins.length, 1, "Should find old login in login manager");
+    oldLogin = logins[0].QueryInterface(Ci.nsILoginMetaInfo);
+    equal(oldLogin.timePasswordChanged, localPasswordChangeTime);
+
+    let rec = new LoginRec("passwords", oldLogin.guid);
+    rec.hostname = oldLogin.hostname;
+    rec.formSubmitURL = oldLogin.formSubmitURL;
+    rec.httpRealm = oldLogin.httpRealm;
+    rec.username = oldLogin.username;
+    // Change the password and bump the password change time to ensure we prefer
+    // the remote one during reconciliation.
+    rec.password = "n3wpa55";
+    rec.usernameField = oldLogin.usernameField;
+    rec.passwordField = oldLogin.usernameField;
+    rec.timeCreated = oldLogin.timeCreated;
+    rec.timePasswordChanged = Date.now();
+    collection.insert(oldLogin.guid, encryptPayload(rec.cleartext));
+  }
+
+  Svc.Obs.notify("weave:engine:start-tracking");
+
+  try {
+    await sync_engine_and_validate_telem(engine, false);
+
+    let newRec = JSON.parse(JSON.parse(
+      collection.payload(newLogin.guid)).ciphertext);
+    equal(newRec.password, "password",
+      "Should update remote password for newer login");
+
+    let logins = Services.logins.findLogins({}, "https://mozilla.com", "", "");
+    equal(logins[0].password, "n3wpa55",
+      "Should update local password for older login");
+  } finally {
+    store.wipe();
+    Svc.Prefs.resetBranch("");
+    Service.recordManager.clearCache();
+    await promiseStopServer(server);
+    Svc.Obs.notify("weave:engine:stop-tracking");
+  }
+});
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -155,16 +155,17 @@ requesttimeoutfactor = 4
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_history_engine.js]
 [test_history_store.js]
 [test_history_tracker.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_places_guid_downgrade.js]
+[test_password_engine.js]
 [test_password_store.js]
 [test_password_validator.js]
 [test_password_tracker.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_prefs_store.js]
 support-files = prefs_test_prefs_store.js
 [test_prefs_tracker.js]