Bug 1286918 - Implement sync validator for passwords engine and integrate with TPS r=markh

MozReview-Commit-ID: 3cTvMmRFT8D

--HG--
extra : rebase_source : 65bc4d593e16a347cf2acfce6d90a165e85a6554

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]
+    };
+  }
+}

services/sync/modules/engines/passwords.js

@@ -2,12 +2,13 @@
  * 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");
@@ -325,3 +326,46 @@ PasswordTracker.prototype = {
     }
   },
 };
+
+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);
+  }
+}
+
+

services/sync/moz.build

@@ -22,6 +22,7 @@ EXTRA_JS_MODULES['services-sync'] += [
     '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',

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();
+}

services/sync/tests/unit/xpcshell.ini

@@ -170,6 +170,7 @@ skip-if = debug
 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

services/sync/tps/extensions/tps/resource/tps.jsm

@@ -24,6 +24,7 @@ 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");
 
@@ -112,6 +113,7 @@ var TPS = {
   _usSinceEpoch: 0,
   _requestedQuit: false,
   shouldValidateBookmarks: false,
+  shouldValidatePasswords: false,
 
   _init: function TPS__init() {
     // Check if Firefox Accounts is enabled
@@ -416,6 +418,7 @@ var TPS = {
   },
 
   HandlePasswords: function (passwords, action) {
+    this.shouldValidatePasswords = true;
     try {
       for (let password of passwords) {
         let password_id = -1;
@@ -656,14 +659,47 @@ var TPS = {
     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"));
@@ -1100,6 +1136,9 @@ var Passwords = {
   },
   verifyNot: function Passwords__verifyNot(passwords) {
     this.HandlePasswords(passwords, ACTION_VERIFY_NOT);
+  },
+  skipValidation() {
+    TPS.shouldValidatePasswords = false;
   }
 };