Bug 1286918 - Implement sync validator for passwords engine and integrate with TPS r=markh
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Tue, 06 Sep 2016 13:38:49 -0400
changeset 313100 2c1d5f8602595b5228fdcc07a78ee044233e2589
parent 313099 db5d2a3899c084e73c49de970575f02dde5ccea2
child 313101 5dea4ee471a3ef68eb37749655ebb9c504204806
push id30671
push usercbook@mozilla.com
push dateThu, 08 Sep 2016 09:59:51 +0000
treeherdermozilla-central@bd28be90aed8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1286918
milestone51.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 1286918 - Implement sync validator for passwords engine and integrate with TPS r=markh MozReview-Commit-ID: 3cTvMmRFT8D
services/sync/modules/collection_validator.js
services/sync/modules/engines/passwords.js
services/sync/moz.build
services/sync/tests/unit/test_password_validator.js
services/sync/tests/unit/xpcshell.ini
services/sync/tps/extensions/tps/resource/tps.jsm
new file mode 100644
--- /dev/null
+++ b/services/sync/modules/collection_validator.js
@@ -0,0 +1,201 @@
+/* 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";
+
+const Cu = Components.utils;
+
+Cu.import("resource://services-sync/record.js");
+Cu.import("resource://services-sync/util.js");
+Cu.import("resource://services-sync/bookmark_utils.js");
+Cu.import("resource://services-common/async.js");
+Cu.import("resource://services-sync/main.js");
+
+this.EXPORTED_SYMBOLS = ["CollectionValidator", "CollectionProblemData"];
+
+class CollectionProblemData {
+  constructor() {
+    this.missingIDs = 0;
+    this.duplicates = [];
+    this.clientMissing = [];
+    this.serverMissing = [];
+    this.serverDeleted = [];
+    this.serverUnexpected = [];
+    this.differences = [];
+  }
+
+  /**
+   * Produce a list summarizing problems found. Each entry contains {name, count},
+   * where name is the field name for the problem, and count is the number of times
+   * the problem was encountered.
+   *
+   * Validation has failed if all counts are not 0.
+   */
+  getSummary() {
+    return [
+      { name: "clientMissing", count: this.clientMissing.length },
+      { name: "serverMissing", count: this.serverMissing.length },
+      { name: "serverDeleted", count: this.serverDeleted.length },
+      { name: "serverUnexpected", count: this.serverUnexpected.length },
+      { name: "differences", count: this.differences.length },
+      { name: "missingIDs", count: this.missingIDs },
+      { name: "duplicates", count: this.duplicates.length }
+    ];
+  }
+}
+
+class CollectionValidator {
+  // Construct a generic collection validator. This is intended to be called by
+  // subclasses.
+  // - name: Name of the engine
+  // - idProp: Property that identifies a record. That is, if a client and server
+  //   record have the same value for the idProp property, they should be
+  //   compared against eachother.
+  // - props: Array of properties that should be compared
+  constructor(name, idProp, props) {
+    this.name = name;
+    this.props = props;
+    this.idProp = idProp;
+  }
+
+  // Should a custom ProblemData type be needed, return it here.
+  emptyProblemData() {
+    return new CollectionProblemData();
+  }
+
+  getServerItems(engine) {
+    let collection = engine._itemSource();
+    let collectionKey = engine.service.collectionKeys.keyForCollection(engine.name);
+    collection.full = true;
+    let items = [];
+    collection.recordHandler = function(item) {
+      item.decrypt(collectionKey);
+      items.push(item.cleartext);
+    };
+    collection.get();
+    return items;
+  }
+
+  // Should return a promise that resolves to an array of client items.
+  getClientItems() {
+    return Promise.reject("Must implement");
+  }
+
+  // Turn the client item into something that can be compared with the server item,
+  // and is also safe to mutate.
+  normalizeClientItem(item) {
+    return Cu.cloneInto(item, {});
+  }
+
+  // Turn the server item into something that can be easily compared with the client
+  // items.
+  normalizeServerItem(item) {
+    return item;
+  }
+
+  // Return whether or not a server item should be present on the client. Expected
+  // to be overridden.
+  clientUnderstands(item) {
+    return true;
+  }
+
+  // Return whether or not a client item should be present on the server. Expected
+  // to be overridden
+  syncedByClient(item) {
+    return true;
+  }
+
+  // Compare the server item and the client item, and return a list of property
+  // names that are different. Can be overridden if needed.
+  getDifferences(client, server) {
+    let differences = [];
+    for (let prop of this.props) {
+      let clientProp = client[prop];
+      let serverProp = server[prop];
+      if ((clientProp || "") !== (serverProp || "")) {
+        differences.push(prop);
+      }
+    }
+    return differences;
+  }
+
+  // Returns an object containing
+  //   problemData: an instance of the class returned by emptyProblemData(),
+  //   clientRecords: Normalized client records
+  //   records: Normalized server records,
+  //   deletedRecords: Array of ids that were marked as deleted by the server.
+  compareClientWithServer(clientItems, serverItems) {
+    clientItems = clientItems.map(item => this.normalizeClientItem(item));
+    serverItems = serverItems.map(item => this.normalizeServerItem(item));
+    let problems = this.emptyProblemData();
+    let seenServer = new Map();
+    let serverDeleted = new Set();
+    let allRecords = new Map();
+
+    for (let record of serverItems) {
+      let id = record[this.idProp];
+      if (!id) {
+        ++problems.missingIDs;
+        continue;
+      }
+      if (record.deleted) {
+        serverDeleted.add(record);
+      } else {
+        let possibleDupe = seenServer.get(id);
+        if (possibleDupe) {
+          problems.duplicates.push(id);
+        } else {
+          seenServer.set(id, record);
+          allRecords.set(id, { server: record, client: null, });
+        }
+        record.understood = this.clientUnderstands(record);
+      }
+    }
+
+    let recordPairs = [];
+    let seenClient = new Map();
+    for (let record of clientItems) {
+      let id = record[this.idProp];
+      record.shouldSync = this.syncedByClient(record);
+      seenClient.set(id, record);
+      let combined = allRecords.get(id);
+      if (combined) {
+        combined.client = record;
+      } else {
+        allRecords.set(id,  { client: record, server: null });
+      }
+    }
+
+    for (let [id, { server, client }] of allRecords) {
+      if (!client && !server) {
+        throw new Error("Impossible: no client or server record for " + id);
+      } else if (server && !client) {
+        if (server.understood) {
+          problems.clientMissing.push(id);
+        }
+      } else if (client && !server) {
+        if (client.shouldSync) {
+          problems.serverMissing.push(id);
+        }
+      } else {
+        if (!client.shouldSync) {
+          if (!problems.serverUnexpected.includes(id)) {
+            problems.serverUnexpected.push(id);
+          }
+          continue;
+        }
+        let differences = this.getDifferences(client, server);
+        if (differences && differences.length) {
+          problems.differences.push({ id, differences });
+        }
+      }
+    }
+    return {
+      problemData: problems,
+      clientRecords: clientItems,
+      records: serverItems,
+      deletedRecords: [...serverDeleted]
+    };
+  }
+}
--- a/services/sync/modules/engines/passwords.js
+++ b/services/sync/modules/engines/passwords.js
@@ -1,18 +1,19 @@
 /* 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/. */
 
-this.EXPORTED_SYMBOLS = ['PasswordEngine', 'LoginRec'];
+this.EXPORTED_SYMBOLS = ['PasswordEngine', 'LoginRec', 'PasswordValidator'];
 
 var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/collection_validator.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-common/async.js");
 
 this.LoginRec = function LoginRec(collection, id) {
   CryptoWrapper.call(this, collection, id);
 }
 LoginRec.prototype = {
@@ -320,8 +321,51 @@ PasswordTracker.prototype = {
         break;
       case "removeAllLogins":
         this._log.trace(data);
         this.score += SCORE_INCREMENT_XLARGE;
         break;
     }
   },
 };
+
+class PasswordValidator extends CollectionValidator {
+  constructor() {
+    super("passwords", "id", [
+      "hostname",
+      "formSubmitURL",
+      "httpRealm",
+      "password",
+      "passwordField",
+      "username",
+      "usernameField",
+    ]);
+  }
+
+  getClientItems() {
+    let logins = Services.logins.getAllLogins({});
+    let syncHosts = Utils.getSyncCredentialsHosts()
+    let result = logins.map(l => l.QueryInterface(Ci.nsILoginMetaInfo))
+                       .filter(l => !syncHosts.has(l.hostname));
+    return Promise.resolve(result);
+  }
+
+  normalizeClientItem(item) {
+    return {
+      id: item.guid,
+      guid: item.guid,
+      hostname: item.hostname,
+      formSubmitURL: item.formSubmitURL,
+      httpRealm: item.httpRealm,
+      password: item.password,
+      passwordField: item.passwordField,
+      username: item.username,
+      usernameField: item.usernameField,
+      original: item,
+    }
+  }
+
+  normalizeServerItem(item) {
+    return Object.assign({ guid: item.id }, item);
+  }
+}
+
+
--- a/services/sync/moz.build
+++ b/services/sync/moz.build
@@ -17,16 +17,17 @@ EXTRA_COMPONENTS += [
 ]
 
 EXTRA_JS_MODULES['services-sync'] += [
     'modules/addonsreconciler.js',
     'modules/addonutils.js',
     'modules/bookmark_utils.js',
     'modules/bookmark_validator.js',
     'modules/browserid_identity.js',
+    'modules/collection_validator.js',
     'modules/engines.js',
     'modules/FxaMigrator.jsm',
     'modules/identity.js',
     'modules/jpakeclient.js',
     'modules/keys.js',
     'modules/main.js',
     'modules/policies.js',
     'modules/record.js',
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_password_validator.js
@@ -0,0 +1,158 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Components.utils.import("resource://services-sync/engines/passwords.js");
+
+function getDummyServerAndClient() {
+  return {
+    server: [
+      {
+        id: "11111",
+        guid: "11111",
+        hostname: "https://www.11111.com",
+        formSubmitURL: "https://www.11111.com/login",
+        password: "qwerty123",
+        passwordField: "pass",
+        username: "foobar",
+        usernameField: "user",
+        httpRealm: null,
+      },
+      {
+        id: "22222",
+        guid: "22222",
+        hostname: "https://www.22222.org",
+        formSubmitURL: "https://www.22222.org/login",
+        password: "hunter2",
+        passwordField: "passwd",
+        username: "baz12345",
+        usernameField: "user",
+        httpRealm: null,
+      },
+      {
+        id: "33333",
+        guid: "33333",
+        hostname: "https://www.33333.com",
+        formSubmitURL: "https://www.33333.com/login",
+        password: "p4ssw0rd",
+        passwordField: "passwad",
+        username: "quux",
+        usernameField: "user",
+        httpRealm: null,
+      },
+    ],
+    client: [
+      {
+        id: "11111",
+        guid: "11111",
+        hostname: "https://www.11111.com",
+        formSubmitURL: "https://www.11111.com/login",
+        password: "qwerty123",
+        passwordField: "pass",
+        username: "foobar",
+        usernameField: "user",
+        httpRealm: null,
+      },
+      {
+        id: "22222",
+        guid: "22222",
+        hostname: "https://www.22222.org",
+        formSubmitURL: "https://www.22222.org/login",
+        password: "hunter2",
+        passwordField: "passwd",
+        username: "baz12345",
+        usernameField: "user",
+        httpRealm: null,
+
+      },
+      {
+        id: "33333",
+        guid: "33333",
+        hostname: "https://www.33333.com",
+        formSubmitURL: "https://www.33333.com/login",
+        password: "p4ssw0rd",
+        passwordField: "passwad",
+        username: "quux",
+        usernameField: "user",
+        httpRealm: null,
+      }
+    ]
+  };
+}
+
+
+add_test(function test_valid() {
+  let { server, client } = getDummyServerAndClient();
+  let validator = new PasswordValidator();
+  let { problemData, clientRecords, records, deletedRecords } =
+      validator.compareClientWithServer(client, server);
+  equal(clientRecords.length, 3);
+  equal(records.length, 3)
+  equal(deletedRecords.length, 0);
+  deepEqual(problemData, validator.emptyProblemData());
+
+  run_next_test();
+});
+
+add_test(function test_missing() {
+  let validator = new PasswordValidator();
+  {
+    let { server, client } = getDummyServerAndClient();
+
+    client.pop();
+
+    let { problemData, clientRecords, records, deletedRecords } =
+        validator.compareClientWithServer(client, server);
+
+    equal(clientRecords.length, 2);
+    equal(records.length, 3)
+    equal(deletedRecords.length, 0);
+
+    let expected = validator.emptyProblemData();
+    expected.clientMissing.push("33333");
+    deepEqual(problemData, expected);
+  }
+  {
+    let { server, client } = getDummyServerAndClient();
+
+    server.pop();
+
+    let { problemData, clientRecords, records, deletedRecords } =
+        validator.compareClientWithServer(client, server);
+
+    equal(clientRecords.length, 3);
+    equal(records.length, 2)
+    equal(deletedRecords.length, 0);
+
+    let expected = validator.emptyProblemData();
+    expected.serverMissing.push("33333");
+    deepEqual(problemData, expected);
+  }
+
+  run_next_test();
+});
+
+
+add_test(function test_deleted() {
+  let { server, client } = getDummyServerAndClient();
+  let deletionRecord = { id: "444444", guid: "444444", deleted: true };
+
+  server.push(deletionRecord);
+  let validator = new PasswordValidator();
+
+  let { problemData, clientRecords, records, deletedRecords } =
+      validator.compareClientWithServer(client, server);
+
+  equal(clientRecords.length, 3);
+  equal(records.length, 4);
+  deepEqual(deletedRecords, [deletionRecord]);
+
+  let expected = validator.emptyProblemData();
+  deepEqual(problemData, expected);
+
+  run_next_test();
+});
+
+
+function run_test() {
+  run_next_test();
+}
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -165,16 +165,17 @@ skip-if = debug
 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_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]
 [test_tab_engine.js]
 [test_tab_store.js]
--- a/services/sync/tps/extensions/tps/resource/tps.jsm
+++ b/services/sync/tps/extensions/tps/resource/tps.jsm
@@ -19,16 +19,17 @@ Cu.import("resource://gre/modules/XPCOMU
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
 Cu.import("resource://services-common/async.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/main.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-sync/bookmark_validator.js");
+Cu.import("resource://services-sync/engines/passwords.js");
 // TPS modules
 Cu.import("resource://tps/logger.jsm");
 
 // Module wrappers for tests
 Cu.import("resource://tps/modules/addons.jsm");
 Cu.import("resource://tps/modules/bookmarks.jsm");
 Cu.import("resource://tps/modules/forms.jsm");
 Cu.import("resource://tps/modules/history.jsm");
@@ -107,16 +108,17 @@ var TPS = {
   _syncWipeAction: null,
   _tabsAdded: 0,
   _tabsFinished: 0,
   _test: null,
   _triggeredSync: false,
   _usSinceEpoch: 0,
   _requestedQuit: false,
   shouldValidateBookmarks: false,
+  shouldValidatePasswords: false,
 
   _init: function TPS__init() {
     // Check if Firefox Accounts is enabled
     let service = Cc["@mozilla.org/weave/service;1"]
                   .getService(Components.interfaces.nsISupports)
                   .wrappedJSObject;
     this.fxaccounts_enabled = service.fxAccountsEnabled;
 
@@ -411,16 +413,17 @@ var TPS = {
     }
     catch(e) {
       DumpHistory();
       throw(e);
     }
   },
 
   HandlePasswords: function (passwords, action) {
+    this.shouldValidatePasswords = true;
     try {
       for (let password of passwords) {
         let password_id = -1;
         Logger.logInfo("executing action " + action.toUpperCase() +
                       " on password " + JSON.stringify(password));
         let passwordOb = new Password(password);
         switch (action) {
           case ACTION_ADD:
@@ -651,24 +654,57 @@ var TPS = {
       if (serverRecordDumpStr) {
         Logger.logInfo("Server bookmark records:\n" + serverRecordDumpStr + "\n");
       }
       this.DumpError("Bookmark validation failed", e);
     }
     Logger.logInfo("Bookmark validation finished");
   },
 
+  ValidatePasswords() {
+    let serverRecordDumpStr;
+    try {
+      Logger.logInfo("About to perform password validation");
+      let pwEngine = Weave.Service.engineManager.get("passwords");
+      let validator = new PasswordValidator();
+      let serverRecords = validator.getServerItems(pwEngine);
+      let clientRecords = Async.promiseSpinningly(validator.getClientItems());
+      serverRecordDumpStr = JSON.stringify(serverRecords);
+
+      let { problemData } = validator.compareClientWithServer(clientRecords, serverRecords);
+
+      for (let { name, count } of problemData.getSummary()) {
+        if (count) {
+          Logger.logInfo(`Validation problem: "${name}": ${JSON.stringify(problemData[name])}`);
+        }
+        Logger.AssertEqual(count, 0, `Password validation error of type ${name}`);
+      }
+    } catch (e) {
+      // Dump the client records (should always be doable)
+      DumpPasswords();
+      // Dump the server records if gotten them already.
+      if (serverRecordDumpStr) {
+        Logger.logInfo("Server password records:\n" + serverRecordDumpStr + "\n");
+      }
+      this.DumpError("Password validation failed", e);
+    }
+    Logger.logInfo("Password validation finished");
+  },
+
   RunNextTestAction: function() {
     try {
       if (this._currentAction >=
           this._phaselist[this._currentPhase].length) {
+        // Run necessary validations and then finish up
         if (this.shouldValidateBookmarks) {
-          // Run bookmark validation and then finish up
           this.ValidateBookmarks();
         }
+        if (this.shouldValidatePasswords) {
+          this.ValidatePasswords();
+        }
         // we're all done
         Logger.logInfo("test phase " + this._currentPhase + ": " +
                        (this._errors ? "FAIL" : "PASS"));
         this._phaseFinished = true;
         this.quit();
         return;
       }
 
@@ -1095,16 +1131,19 @@ var Passwords = {
   delete: function Passwords__delete(passwords) {
     this.HandlePasswords(passwords, ACTION_DELETE);
   },
   verify: function Passwords__verify(passwords) {
     this.HandlePasswords(passwords, ACTION_VERIFY);
   },
   verifyNot: function Passwords__verifyNot(passwords) {
     this.HandlePasswords(passwords, ACTION_VERIFY_NOT);
+  },
+  skipValidation() {
+    TPS.shouldValidatePasswords = false;
   }
 };
 
 var Prefs = {
   modify: function Prefs__modify(prefs) {
     TPS.HandlePrefs(prefs, ACTION_MODIFY);
   },
   verify: function Prefs__verify(prefs) {