--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -198,17 +198,22 @@ FormAutofillParent.prototype = {
if (data.guid) {
this.profileStorage.addresses.update(data.guid, data.address);
} else {
this.profileStorage.addresses.add(data.address);
}
break;
}
case "FormAutofill:SaveCreditCard": {
- await this.profileStorage.creditCards.normalizeCCNumberFields(data.creditcard);
+ // TODO: "MasterPassword.ensureLoggedIn" can be removed after the storage
+ // APIs are refactored to be async functions (bug 1399367).
+ if (!await MasterPassword.ensureLoggedIn()) {
+ log.warn("User canceled master password entry");
+ return;
+ }
this.profileStorage.creditCards.add(data.creditcard);
break;
}
case "FormAutofill:RemoveAddresses": {
data.guids.forEach(guid => this.profileStorage.addresses.remove(guid));
break;
}
case "FormAutofill:RemoveCreditCards": {
@@ -449,25 +454,24 @@ FormAutofillParent.prototype = {
return;
}
if (state == "disable") {
Services.prefs.setBoolPref("extensions.formautofill.creditCards.enabled", false);
return;
}
- try {
- await this.profileStorage.creditCards.normalizeCCNumberFields(creditCard.record);
- this.profileStorage.creditCards.add(creditCard.record);
- } catch (e) {
- if (e.result != Cr.NS_ERROR_ABORT) {
- throw e;
- }
+ // TODO: "MasterPassword.ensureLoggedIn" can be removed after the storage
+ // APIs are refactored to be async functions (bug 1399367).
+ if (!await MasterPassword.ensureLoggedIn()) {
log.warn("User canceled master password entry");
+ return;
}
+
+ this.profileStorage.creditCards.add(creditCard.record);
},
_onFormSubmit(data, target) {
let {profile: {address, creditCard}, timeStartedFillingMS} = data;
if (address) {
this._onAddressSubmit(address, target, timeStartedFillingMS);
}
--- a/browser/extensions/formautofill/MasterPassword.jsm
+++ b/browser/extensions/formautofill/MasterPassword.jsm
@@ -31,29 +31,52 @@ this.MasterPassword = {
/**
* @returns {boolean} True if a master password is set and false otherwise.
*/
get isEnabled() {
return this._token.hasPassword;
},
/**
- * Display the master password login prompt no matter it's logged in or not.
- * If an existing MP prompt is already open, the result from it will be used instead.
+ * @returns {boolean} True if master password is logged in and false if not.
+ */
+ get isLoggedIn() {
+ return Services.logins.isLoggedIn;
+ },
+
+ /**
+ * @returns {boolean} True if there is another master password login dialog
+ * existing and false otherwise.
+ */
+ get isUIBusy() {
+ return Services.logins.uiBusy;
+ },
+
+ /**
+ * Ensure the master password is logged in. It will display the master password
+ * login prompt or do nothing if it's logged in already. If an existing MP
+ * prompt is already prompted, the result from it will be used instead.
*
- * @returns {Promise<boolean>} True if it's logged in or no password is set and false
- * if it's still not logged in (prompt canceled or other error).
+ * @param {boolean} reauth Prompt the login dialog no matter it's logged in
+ * or not if it's set to true.
+ * @returns {Promise<boolean>} True if it's logged in or no password is set
+ * and false if it's still not logged in (prompt
+ * canceled or other error).
*/
- async prompt() {
+ async ensureLoggedIn(reauth = false) {
if (!this.isEnabled) {
return true;
}
+ if (this.isLoggedIn && !reauth) {
+ return true;
+ }
+
// If a prompt is already showing then wait for and focus it.
- if (Services.logins.uiBusy) {
+ if (this.isUIBusy) {
return this.waitForExistingDialog();
}
let token = this._token;
try {
// 'true' means always prompt for token password. User will be prompted until
// clicking 'Cancel' or entering the correct password.
token.login(true);
@@ -76,57 +99,86 @@ this.MasterPassword = {
* Decrypts cipherText.
*
* @param {string} cipherText Encrypted string including the algorithm details.
* @param {boolean} reauth True if we want to force the prompt to show up
* even if the user is already logged in.
* @returns {Promise<string>} resolves to the decrypted string, or rejects otherwise.
*/
async decrypt(cipherText, reauth = false) {
- let loggedIn = false;
- if (reauth) {
- loggedIn = await this.prompt();
- } else {
- loggedIn = await this.waitForExistingDialog();
- }
-
- if (!loggedIn) {
+ if (!await this.ensureLoggedIn(reauth)) {
throw Components.Exception("User canceled master password entry", Cr.NS_ERROR_ABORT);
}
+ return cryptoSDR.decrypt(cipherText);
+ },
+ /**
+ * Decrypts cipherText synchronously. "ensureLoggedIn()" needs to be called
+ * outside in case another dialog is showing.
+ *
+ * NOTE: This method will be removed soon once the ProfileStorage APIs are
+ * refactored to be async functions (bug 1399367). Please use async
+ * version instead.
+ *
+ * @deprecated
+ * @param {string} cipherText Encrypted string including the algorithm details.
+ * @returns {string} The decrypted string.
+ */
+ decryptSync(cipherText) {
+ if (this.isUIBusy) {
+ throw Components.Exception("\"ensureLoggedIn()\" should be called first", Cr.NS_ERROR_UNEXPECTED);
+ }
return cryptoSDR.decrypt(cipherText);
},
/**
* Encrypts a string and returns cipher text containing algorithm information used for decryption.
*
* @param {string} plainText Original string without encryption.
* @returns {Promise<string>} resolves to the encrypted string (with algorithm), otherwise rejects.
*/
async encrypt(plainText) {
- if (Services.logins.uiBusy && !await this.waitForExistingDialog()) {
+ if (!await this.ensureLoggedIn()) {
throw Components.Exception("User canceled master password entry", Cr.NS_ERROR_ABORT);
}
return cryptoSDR.encrypt(plainText);
},
/**
+ * Encrypts plainText synchronously. "ensureLoggedIn()" needs to be called
+ * outside in case another dialog is showing.
+ *
+ * NOTE: This method will be removed soon once the ProfileStorage APIs are
+ * refactored to be async functions (bug 1399367). Please use async
+ * version instead.
+ *
+ * @deprecated
+ * @param {string} plainText A plain string to be encrypted.
+ * @returns {string} The encrypted cipher string.
+ */
+ encryptSync(plainText) {
+ if (this.isUIBusy) {
+ throw Components.Exception("\"ensureLoggedIn()\" should be called first", Cr.NS_ERROR_UNEXPECTED);
+ }
+ return cryptoSDR.encrypt(plainText);
+ },
+
+ /**
* Resolve when master password dialogs are closed, immediately if none are open.
*
* An existing MP dialog will be focused and will request attention.
*
* @returns {Promise<boolean>}
* Resolves with whether the user is logged in to MP.
*/
async waitForExistingDialog() {
- if (!Services.logins.uiBusy) {
- log.debug("waitForExistingDialog: Dialog isn't showing. isLoggedIn:",
- Services.logins.isLoggedIn);
- return Services.logins.isLoggedIn;
+ if (!this.isUIBusy) {
+ log.debug("waitForExistingDialog: Dialog isn't showing. isLoggedIn:", this.isLoggedIn);
+ return this.isLoggedIn;
}
return new Promise((resolve) => {
log.debug("waitForExistingDialog: Observing the open dialog");
let observer = {
QueryInterface: XPCOMUtils.generateQI([
Ci.nsIObserver,
Ci.nsISupportsWeakReference,
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -53,39 +53,57 @@
* ],
* creditCards: [
* {
* guid, // 12 characters
* version, // schema version in integer
*
* // credit card fields
* cc-name,
- * cc-number, // e.g. ************1234
- * cc-number-encrypted,
+ * cc-number, // will be stored in masked format (************1234)
+ * // (see details below)
* cc-exp-month,
* cc-exp-year, // 2-digit year will be converted to 4 digits
* // upon saving
*
* // computed fields (These fields are computed based on the above fields
* // and are not allowed to be modified directly.)
* cc-given-name,
* cc-additional-name,
* cc-family-name,
+ * cc-number-encrypted, // encrypted from the original unmasked "cc-number"
+ * // (see details below)
* cc-exp,
*
* // metadata
* timeCreated, // in ms
* timeLastUsed, // in ms
* timeLastModified, // in ms
* timesUsed
* _sync: { ... optional sync metadata },
* }
* ]
* }
*
+ *
+ * Encrypt-related Credit Card Fields (cc-number & cc-number-encrypted):
+ *
+ * When saving or updating a credit-card record, the storage will encrypt the
+ * value of "cc-number", store the encrypted number in "cc-number-encrypted"
+ * field, and replace "cc-number" field with the masked number. These all happen
+ * in "_computeFields". We do reverse actions in "_stripComputedFields", which
+ * decrypts "cc-number-encrypted", restores it to "cc-number", and deletes
+ * "cc-number-encrypted". Therefore, calling "_stripComputedFields" followed by
+ * "_computeFields" can make sure the encrypt-related fields are up-to-date.
+ *
+ * In general, you have to decrypt the number by your own outside ProfileStorage
+ * when necessary. However, you will get the decrypted records when querying
+ * data with "rawData=true" to ensure they're ready to sync.
+ *
+ *
* Sync Metadata:
*
* Records may also have a _sync field, which consists of:
* {
* changeCounter, // integer - the number of changes made since the last
* // sync.
* lastSyncedFields, // object - hashes of the original values for fields
* // changed since the last sync.
@@ -177,25 +195,25 @@ const TEL_COMPONENTS = [
const VALID_ADDRESS_COMPUTED_FIELDS = [
"name",
"country-name",
].concat(STREET_ADDRESS_COMPONENTS, TEL_COMPONENTS);
const VALID_CREDIT_CARD_FIELDS = [
"cc-name",
"cc-number",
- "cc-number-encrypted",
"cc-exp-month",
"cc-exp-year",
];
const VALID_CREDIT_CARD_COMPUTED_FIELDS = [
"cc-given-name",
"cc-additional-name",
"cc-family-name",
+ "cc-number-encrypted",
"cc-exp",
];
const INTERNAL_FIELDS = [
"guid",
"version",
"timeCreated",
"timeLastUsed",
@@ -242,31 +260,41 @@ class AutofillRecords {
this.VALID_FIELDS = validFields;
this.VALID_COMPUTED_FIELDS = validComputedFields;
this._store = store;
this._collectionName = collectionName;
this._schemaVersion = schemaVersion;
let hasChanges = (result, record) => this._migrateRecord(record) || result;
- if (this._store.data[this._collectionName].reduce(hasChanges, false)) {
+ if (this.data.reduce(hasChanges, false)) {
this._store.saveSoon();
}
}
/**
* Gets the schema version number.
*
* @returns {number}
* The current schema version number.
*/
get version() {
return this._schemaVersion;
}
+ /**
+ * Gets the data of this collection.
+ *
+ * @returns {array}
+ * The data object.
+ */
+ get data() {
+ return this._store.data[this._collectionName];
+ }
+
// Ensures that we don't try to apply synced records with newer schema
// versions. This is a temporary measure to ensure we don't accidentally
// bump the schema version without a syncing strategy in place (bug 1377204).
_ensureMatchingVersion(record) {
if (record.version != this.version) {
throw new Error(`Got unknown record version ${
record.version}; want ${this.version}`);
}
@@ -287,19 +315,19 @@ class AutofillRecords {
if (sourceSync) {
// Remove tombstones for incoming items that were changed on another
// device. Local deletions always lose to avoid data loss.
let index = this._findIndexByGUID(record.guid, {
includeDeleted: true,
});
if (index > -1) {
- let existing = this._store.data[this._collectionName][index];
+ let existing = this.data[index];
if (existing.deleted) {
- this._store.data[this._collectionName].splice(index, 1);
+ this.data.splice(index, 1);
} else {
throw new Error(`Record ${record.guid} already exists`);
}
}
let recordToSave = this._clone(record);
return this._saveRecord(recordToSave, {sourceSync});
}
@@ -344,17 +372,17 @@ class AutofillRecords {
this._computeFields(recordToSave);
}
if (sourceSync) {
let sync = this._getSyncMetaData(recordToSave, true);
sync.changeCounter = 0;
}
- this._store.data[this._collectionName].push(recordToSave);
+ this.data.push(recordToSave);
this._store.saveSoon();
Services.obs.notifyObservers({wrappedJSObject: {sourceSync}}, "formautofill-storage-changed", "add");
return recordToSave.guid;
}
_generateGUID() {
@@ -374,23 +402,26 @@ class AutofillRecords {
* @param {Object} record
* The new record used to overwrite the old one.
* @param {boolean} [preserveOldProperties = false]
* Preserve old record's properties if they don't exist in new record.
*/
update(guid, record, preserveOldProperties = false) {
this.log.debug("update:", guid, record);
- let recordFound = this._findByGUID(guid);
- if (!recordFound) {
+ let recordFoundIndex = this._findIndexByGUID(guid);
+ if (recordFoundIndex == -1) {
throw new Error("No matching record.");
}
- // Clone the record by Object assign API to preserve the property with empty string.
- let recordToUpdate = Object.assign({}, record);
+ // Clone the record before modifying it to avoid exposing incomplete changes.
+ let recordFound = this._clone(this.data[recordFoundIndex]);
+ this._stripComputedFields(recordFound);
+
+ let recordToUpdate = this._clone(record);
this._normalizeRecord(recordToUpdate);
for (let field of this.VALID_FIELDS) {
let oldValue = recordFound[field];
let newValue = recordToUpdate[field];
// Resume the old field value in the perserve case
if (preserveOldProperties && newValue === undefined) {
@@ -407,18 +438,18 @@ class AutofillRecords {
}
recordFound.timeLastModified = Date.now();
let syncMetadata = this._getSyncMetaData(recordFound);
if (syncMetadata) {
syncMetadata.changeCounter += 1;
}
- this._stripComputedFields(recordFound);
this._computeFields(recordFound);
+ this.data[recordFoundIndex] = recordFound;
this._store.saveSoon();
Services.obs.notifyObservers(null, "formautofill-storage-changed", "update");
}
/**
* Notifies the storage of the use of the specified record, so we can update
* the metadata accordingly. This does not bump the Sync change counter, since
@@ -456,35 +487,35 @@ class AutofillRecords {
if (sourceSync) {
this._removeSyncedRecord(guid);
} else {
let index = this._findIndexByGUID(guid, {includeDeleted: false});
if (index == -1) {
this.log.warn("attempting to remove non-existing entry", guid);
return;
}
- let existing = this._store.data[this._collectionName][index];
+ let existing = this.data[index];
if (existing.deleted) {
return; // already a tombstone - don't touch it.
}
let existingSync = this._getSyncMetaData(existing);
if (existingSync) {
// existing sync metadata means it has been synced. This means we must
// leave a tombstone behind.
- this._store.data[this._collectionName][index] = {
+ this.data[index] = {
guid,
timeLastModified: Date.now(),
deleted: true,
_sync: existingSync,
};
existingSync.changeCounter++;
} else {
// If there's no sync meta-data, this record has never been synced, so
// we can delete it.
- this._store.data[this._collectionName].splice(index, 1);
+ this.data.splice(index, 1);
}
}
this._store.saveSoon();
Services.obs.notifyObservers({wrappedJSObject: {sourceSync}}, "formautofill-storage-changed", "remove");
}
/**
@@ -502,17 +533,17 @@ class AutofillRecords {
this.log.debug("get:", guid, rawData);
let recordFound = this._findByGUID(guid);
if (!recordFound) {
return null;
}
// The record is cloned to avoid accidental modifications from outside.
- let clonedRecord = this._clone(recordFound);
+ let clonedRecord = this._cloneAndCleanUp(recordFound);
if (rawData) {
this._stripComputedFields(clonedRecord);
} else {
this._recordReadProcessor(clonedRecord);
}
return clonedRecord;
}
@@ -524,19 +555,19 @@ class AutofillRecords {
* @param {boolean} [options.includeDeleted = false]
* Also return any tombstone records.
* @returns {Array.<Object>}
* An array containing clones of all records.
*/
getAll({rawData = false, includeDeleted = false} = {}) {
this.log.debug("getAll", rawData, includeDeleted);
- let records = this._store.data[this._collectionName].filter(r => !r.deleted || includeDeleted);
+ let records = this.data.filter(r => !r.deleted || includeDeleted);
// Records are cloned to avoid accidental modifications from outside.
- let clonedRecords = records.map(r => this._clone(r));
+ let clonedRecords = records.map(r => this._cloneAndCleanUp(r));
clonedRecords.forEach(record => {
if (rawData) {
this._stripComputedFields(record);
} else {
this._recordReadProcessor(record);
}
});
return clonedRecords;
@@ -590,26 +621,27 @@ class AutofillRecords {
}
}
/**
* Attempts a three-way merge between a changed local record, an incoming
* remote record, and the shared parent that we synthesize from the last
* synced fields - see _maybeStoreLastSyncedField.
*
- * @param {Object} localRecord
- * The changed local record, currently in storage.
+ * @param {Object} strippedLocalRecord
+ * The changed local record, currently in storage. Computed fields
+ * are stripped.
* @param {Object} remoteRecord
* The remote record.
* @returns {Object|null}
* The merged record, or `null` if there are conflicts and the
* records can't be merged.
*/
- _mergeSyncedRecords(localRecord, remoteRecord) {
- let sync = this._getSyncMetaData(localRecord, true);
+ _mergeSyncedRecords(strippedLocalRecord, remoteRecord) {
+ let sync = this._getSyncMetaData(strippedLocalRecord, true);
// Copy all internal fields from the remote record. We'll update their
// values in `_replaceRecordAt`.
let mergedRecord = {};
for (let field of INTERNAL_FIELDS) {
if (remoteRecord[field] != null) {
mergedRecord[field] = remoteRecord[field];
}
@@ -618,40 +650,40 @@ class AutofillRecords {
for (let field of this.VALID_FIELDS) {
let isLocalSame = false;
let isRemoteSame = false;
if (field in sync.lastSyncedFields) {
// If the field has changed since the last sync, compare hashes to
// determine if the local and remote values are different. Hashing is
// expensive, but we don't expect this to happen frequently.
let lastSyncedValue = sync.lastSyncedFields[field];
- isLocalSame = lastSyncedValue == sha512(localRecord[field]);
+ isLocalSame = lastSyncedValue == sha512(strippedLocalRecord[field]);
isRemoteSame = lastSyncedValue == sha512(remoteRecord[field]);
} else {
// Otherwise, if the field hasn't changed since the last sync, we know
// it's the same locally.
isLocalSame = true;
- isRemoteSame = localRecord[field] == remoteRecord[field];
+ isRemoteSame = strippedLocalRecord[field] == remoteRecord[field];
}
let value;
if (isLocalSame && isRemoteSame) {
// Local and remote are the same; doesn't matter which one we pick.
- value = localRecord[field];
+ value = strippedLocalRecord[field];
} else if (isLocalSame && !isRemoteSame) {
value = remoteRecord[field];
} else if (!isLocalSame && isRemoteSame) {
// We don't need to bump the change counter when taking the local
// change, because the counter must already be > 0 if we're attempting
// a three-way merge.
- value = localRecord[field];
- } else if (localRecord[field] == remoteRecord[field]) {
+ value = strippedLocalRecord[field];
+ } else if (strippedLocalRecord[field] == remoteRecord[field]) {
// Shared parent doesn't match either local or remote, but the values
// are identical, so there's no conflict.
- value = localRecord[field];
+ value = strippedLocalRecord[field];
} else {
// Both local and remote changed to different values. We'll need to fork
// the local record to resolve the conflict.
return null;
}
if (value != null) {
mergedRecord[field] = value;
@@ -671,22 +703,22 @@ class AutofillRecords {
* Should we copy Sync metadata? This is true if `remoteRecord` is a
* merged record with local changes that we need to upload. Passing
* `keepSyncMetadata` retains the record's change counter and
* last synced fields, so that we don't clobber the local change if
* the sync is interrupted after the record is merged, but before
* it's uploaded.
*/
_replaceRecordAt(index, remoteRecord, {keepSyncMetadata = false} = {}) {
- let localRecord = this._store.data[this._collectionName][index];
+ let localRecord = this.data[index];
let newRecord = this._clone(remoteRecord);
this._stripComputedFields(newRecord);
- this._store.data[this._collectionName][index] = newRecord;
+ this.data[index] = newRecord;
if (keepSyncMetadata) {
// It's safe to move the Sync metadata from the old record to the new
// record, since we always clone records when we return them, and we
// never hand out references to the metadata object via public methods.
newRecord._sync = localRecord._sync;
} else {
// As a side effect, `_getSyncMetaData` marks the record as syncing if the
@@ -715,36 +747,33 @@ class AutofillRecords {
this._computeFields(newRecord);
}
/**
* Clones a local record, giving the clone a new GUID and Sync metadata. The
* original record remains unchanged in storage.
*
- * @param {Object} localRecord
- * The local record.
+ * @param {Object} strippedLocalRecord
+ * The local record. Computed fields are stripped.
* @returns {string}
* A clone of the local record with a new GUID.
*/
- _forkLocalRecord(localRecord) {
- let forkedLocalRecord = this._clone(localRecord);
-
- this._stripComputedFields(forkedLocalRecord);
-
+ _forkLocalRecord(strippedLocalRecord) {
+ let forkedLocalRecord = this._cloneAndCleanUp(strippedLocalRecord);
forkedLocalRecord.guid = this._generateGUID();
- this._store.data[this._collectionName].push(forkedLocalRecord);
// Give the record fresh Sync metadata and bump its change counter as a
// side effect. This also excludes the forked record from de-duping on the
// next sync, if the current sync is interrupted before the record can be
// uploaded.
this._getSyncMetaData(forkedLocalRecord, true);
this._computeFields(forkedLocalRecord);
+ this.data.push(forkedLocalRecord);
return forkedLocalRecord;
}
/**
* Reconciles an incoming remote record into the matching local record. This
* method is only used by Sync; other callers should use `merge`.
*
@@ -765,38 +794,41 @@ class AutofillRecords {
throw new Error(`Can't reconcile tombstone ${remoteRecord.guid}`);
}
let localIndex = this._findIndexByGUID(remoteRecord.guid);
if (localIndex < 0) {
throw new Error(`Record ${remoteRecord.guid} not found`);
}
- let localRecord = this._store.data[this._collectionName][localIndex];
+ let localRecord = this.data[localIndex];
let sync = this._getSyncMetaData(localRecord, true);
let forkedGUID = null;
if (sync.changeCounter === 0) {
// Local not modified. Replace local with remote.
this._replaceRecordAt(localIndex, remoteRecord, {
keepSyncMetadata: false,
});
} else {
- let mergedRecord = this._mergeSyncedRecords(localRecord, remoteRecord);
+ let strippedLocalRecord = this._clone(localRecord);
+ this._stripComputedFields(strippedLocalRecord);
+
+ let mergedRecord = this._mergeSyncedRecords(strippedLocalRecord, remoteRecord);
if (mergedRecord) {
// Local and remote modified, but we were able to merge. Replace the
// local record with the merged record.
this._replaceRecordAt(localIndex, mergedRecord, {
keepSyncMetadata: true,
});
} else {
// Merge conflict. Fork the local record, then replace the original
// with the merged record.
- let forkedLocalRecord = this._forkLocalRecord(localRecord);
+ let forkedLocalRecord = this._forkLocalRecord(strippedLocalRecord);
forkedGUID = forkedLocalRecord.guid;
this._replaceRecordAt(localIndex, remoteRecord, {
keepSyncMetadata: false,
});
}
}
this._store.saveSoon();
@@ -816,38 +848,38 @@ class AutofillRecords {
let tombstone = {
guid,
timeLastModified: Date.now(),
deleted: true,
};
let sync = this._getSyncMetaData(tombstone, true);
sync.changeCounter = 0;
- this._store.data[this._collectionName].push(tombstone);
+ this.data.push(tombstone);
return;
}
- let existing = this._store.data[this._collectionName][index];
+ let existing = this.data[index];
let sync = this._getSyncMetaData(existing, true);
if (sync.changeCounter > 0) {
// Deleting a record with unsynced local changes. To avoid potential
// data loss, we ignore the deletion in favor of the changed record.
this.log.info("Ignoring deletion for record with local changes",
existing);
return;
}
if (existing.deleted) {
this.log.info("Ignoring deletion for tombstone", existing);
return;
}
// Removing a record that's not changed locally, and that's not already
// deleted. Replace the record with a synced tombstone.
- this._store.data[this._collectionName][index] = {
+ this.data[index] = {
guid,
timeLastModified: Date.now(),
deleted: true,
_sync: sync,
};
}
/**
@@ -859,17 +891,17 @@ class AutofillRecords {
* the object to pushSyncChanges, which will apply the changes to the store.
*
* @returns {object}
* An object describing the changes to sync.
*/
pullSyncChanges() {
let changes = {};
- let profiles = this._store.data[this._collectionName];
+ let profiles = this.data;
for (let profile of profiles) {
let sync = this._getSyncMetaData(profile, true);
if (sync.changeCounter < 1) {
if (sync.changeCounter != 0) {
this.log.error("negative change counter", profile);
}
continue;
}
@@ -916,17 +948,17 @@ class AutofillRecords {
/**
* Reset all sync metadata for all items.
*
* This is called when Sync is disconnected from this device. All sync
* metadata for all items is removed.
*/
resetSync() {
- for (let record of this._store.data[this._collectionName]) {
+ for (let record of this.data) {
delete record._sync;
}
// XXX - we should probably also delete all tombstones?
this.log.info("All sync metadata was reset");
}
/**
* Changes the GUID of an item. This should be called only by Sync. There
@@ -946,17 +978,17 @@ class AutofillRecords {
if (oldID == newID) {
throw new Error("changeGUID: old and new IDs are the same");
}
if (this._findIndexByGUID(newID) >= 0) {
throw new Error("changeGUID: record with destination id exists already");
}
let index = this._findIndexByGUID(oldID);
- let profile = this._store.data[this._collectionName][index];
+ let profile = this.data[index];
if (!profile) {
throw new Error("changeGUID: no source record");
}
if (this._getSyncMetaData(profile)) {
throw new Error("changeGUID: existing record has already been synced");
}
profile.guid = newID;
@@ -980,105 +1012,110 @@ class AutofillRecords {
}
/**
* Finds a local record with matching common fields and a different GUID.
* Sync uses this method to find and update unsynced local records with
* fields that match incoming remote records. This avoids creating
* duplicate profiles with the same information.
*
- * @param {Object} record
+ * @param {Object} remoteRecord
* The remote record.
* @returns {string|null}
* The GUID of the matching local record, or `null` if no records
* match.
*/
- findDuplicateGUID(record) {
- if (!record.guid) {
+ findDuplicateGUID(remoteRecord) {
+ if (!remoteRecord.guid) {
throw new Error("Record missing GUID");
}
- this._ensureMatchingVersion(record);
- if (record.deleted) {
+ this._ensureMatchingVersion(remoteRecord);
+ if (remoteRecord.deleted) {
// Tombstones don't carry enough info to de-dupe, and we should have
// handled them separately when applying the record.
throw new Error("Tombstones can't have duplicates");
}
- let records = this._store.data[this._collectionName];
- for (let profile of records) {
- if (profile.deleted) {
+ let localRecords = this.data;
+ for (let localRecord of localRecords) {
+ if (localRecord.deleted) {
continue;
}
- if (profile.guid == record.guid) {
- throw new Error(`Record ${record.guid} already exists`);
+ if (localRecord.guid == remoteRecord.guid) {
+ throw new Error(`Record ${remoteRecord.guid} already exists`);
}
- if (this._getSyncMetaData(profile)) {
- // This record has already been uploaded, so it can't be a dupe of
+ if (this._getSyncMetaData(localRecord)) {
+ // This local record has already been uploaded, so it can't be a dupe of
// another incoming item.
continue;
}
- let keys = new Set(Object.keys(record));
- for (let key of Object.keys(profile)) {
+
+ // Ignore computed fields when matching records as they aren't synced at all.
+ let strippedLocalRecord = this._clone(localRecord);
+ this._stripComputedFields(strippedLocalRecord);
+
+ let keys = new Set(Object.keys(remoteRecord));
+ for (let key of Object.keys(strippedLocalRecord)) {
keys.add(key);
}
- // Ignore internal and computed fields when matching records. Internal
- // fields are synced, but almost certainly have different values than the
- // local record, and we'll update them in `reconcile`. Computed fields
- // aren't synced at all.
+ // Ignore internal fields when matching records. Internal fields are synced,
+ // but almost certainly have different values than the local record, and
+ // we'll update them in `reconcile`.
for (let field of INTERNAL_FIELDS) {
keys.delete(field);
}
- for (let field of this.VALID_COMPUTED_FIELDS) {
- keys.delete(field);
- }
if (!keys.size) {
// This shouldn't ever happen; a valid record will always have fields
// that aren't computed or internal. Sync can't do anything about that,
// so we ignore the dubious local record instead of throwing.
continue;
}
let same = true;
for (let key of keys) {
// For now, we ensure that both (or neither) records have the field
// with matching values. This doesn't account for the version yet
// (bug 1377204).
- same = key in profile == key in record && profile[key] == record[key];
+ same = key in strippedLocalRecord == key in remoteRecord && strippedLocalRecord[key] == remoteRecord[key];
if (!same) {
break;
}
}
if (same) {
- return profile.guid;
+ return strippedLocalRecord.guid;
}
}
return null;
}
/**
* Internal helper functions.
*/
_clone(record) {
+ return Object.assign({}, record);
+ }
+
+ _cloneAndCleanUp(record) {
let result = {};
for (let key in record) {
// Do not expose hidden fields and fields with empty value (mainly used
// as placeholders of the computed fields).
if (!key.startsWith("_") && record[key] !== "") {
result[key] = record[key];
}
}
return result;
}
_findByGUID(guid, {includeDeleted = false} = {}) {
let found = this._findIndexByGUID(guid, {includeDeleted});
- return found < 0 ? undefined : this._store.data[this._collectionName][found];
+ return found < 0 ? undefined : this.data[found];
}
_findIndexByGUID(guid, {includeDeleted = false} = {}) {
- return this._store.data[this._collectionName].findIndex(record => {
+ return this.data.findIndex(record => {
return record.guid == guid && (!record.deleted || includeDeleted);
});
}
_migrateRecord(record) {
let hasChanges = false;
if (!record.version || isNaN(record.version) || record.version < 1) {
@@ -1405,31 +1442,38 @@ class Addresses extends AutofillRecords
* Merge the address if storage has multiple mergeable records.
* @param {Object} targetAddress
* The address for merge.
* @returns {Array.<string>}
* Return an array of the merged GUID string.
*/
mergeToStorage(targetAddress) {
let mergedGUIDs = [];
- for (let address of this._store.data[this._collectionName]) {
+ for (let address of this.data) {
if (!address.deleted && this.mergeIfPossible(address.guid, targetAddress)) {
mergedGUIDs.push(address.guid);
}
}
this.log.debug("Existing records matching and merging count is", mergedGUIDs.length);
return mergedGUIDs;
}
}
class CreditCards extends AutofillRecords {
constructor(store) {
super(store, "creditCards", VALID_CREDIT_CARD_FIELDS, VALID_CREDIT_CARD_COMPUTED_FIELDS, CREDIT_CARD_SCHEMA_VERSION);
}
+ _getMaskedCCNumber(ccNumber) {
+ if (ccNumber.length <= 4) {
+ throw new Error(`Invalid credit card number`);
+ }
+ return "*".repeat(ccNumber.length - 4) + ccNumber.substr(-4);
+ }
+
_computeFields(creditCard) {
// NOTE: Remember to bump the schema version number if any of the existing
// computing algorithm changes. (No need to bump when just adding new
// computed fields)
let hasNewComputedFields = false;
// Compute split names
@@ -1443,25 +1487,41 @@ class CreditCards extends AutofillRecord
let year = creditCard["cc-exp-year"];
let month = creditCard["cc-exp-month"];
if (!creditCard["cc-exp"] && month && year) {
creditCard["cc-exp"] = String(year) + "-" + String(month).padStart(2, "0");
hasNewComputedFields = true;
}
+ // Encrypt credit card number
+ if (!("cc-number-encrypted" in creditCard)) {
+ let ccNumber = (creditCard["cc-number"] || "").replace(/\s/g, "");
+ if (FormAutofillUtils.isCCNumber(ccNumber)) {
+ creditCard["cc-number"] = this._getMaskedCCNumber(ccNumber);
+ creditCard["cc-number-encrypted"] = MasterPassword.encryptSync(ccNumber);
+ } else {
+ delete creditCard["cc-number"];
+ // Computed fields are always present in the storage no matter it's
+ // empty or not.
+ creditCard["cc-number-encrypted"] = "";
+ }
+ }
+
return hasNewComputedFields;
}
+ _stripComputedFields(creditCard) {
+ if (creditCard["cc-number-encrypted"]) {
+ creditCard["cc-number"] = MasterPassword.decryptSync(creditCard["cc-number-encrypted"]);
+ }
+ super._stripComputedFields(creditCard);
+ }
+
_normalizeFields(creditCard) {
- // Check if cc-number is normalized(normalizeCCNumberFields should be called first).
- if (!creditCard["cc-number-encrypted"] || !creditCard["cc-number"].includes("*")) {
- throw new Error("Credit card number needs to be normalized first.");
- }
-
// Normalize name
if (creditCard["cc-given-name"] || creditCard["cc-additional-name"] || creditCard["cc-family-name"]) {
if (!creditCard["cc-name"]) {
creditCard["cc-name"] = FormAutofillNameUtils.joinNameParts({
given: creditCard["cc-given-name"],
middle: creditCard["cc-additional-name"],
family: creditCard["cc-family-name"],
});
@@ -1488,41 +1548,16 @@ class CreditCards extends AutofillRecord
} else if (expYear < 100) {
// Enforce 4 digits years.
creditCard["cc-exp-year"] = expYear + 2000;
} else {
creditCard["cc-exp-year"] = expYear;
}
}
}
-
- /**
- * Normalize credit card number related field for saving. It should always be
- * called before adding/updating credit card records.
- *
- * @param {Object} creditCard
- * The creditCard record with plaintext number only.
- */
- async normalizeCCNumberFields(creditCard) {
- // Fields that should not be set by content.
- delete creditCard["cc-number-encrypted"];
-
- // Validate and encrypt credit card numbers, and calculate the masked numbers
- if (creditCard["cc-number"]) {
- let ccNumber = creditCard["cc-number"].replace(/\s/g, "");
- delete creditCard["cc-number"];
-
- if (!FormAutofillUtils.isCCNumber(ccNumber)) {
- throw new Error("Credit card number contains invalid characters or is under 12 digits.");
- }
-
- creditCard["cc-number-encrypted"] = await MasterPassword.encrypt(ccNumber);
- creditCard["cc-number"] = "*".repeat(ccNumber.length - 4) + ccNumber.substr(-4);
- }
- }
}
function ProfileStorage(path) {
this._path = path;
this._initializePromise = null;
this.INTERNAL_FIELDS = INTERNAL_FIELDS;
}
--- a/browser/extensions/formautofill/content/editDialog.js
+++ b/browser/extensions/formautofill/content/editDialog.js
@@ -258,19 +258,22 @@ class EditCreditCard extends EditDialog
async handleSubmit() {
let creditCard = this.buildFormObject();
// Show error on the cc-number field if it's empty or invalid
if (!FormAutofillUtils.isCCNumber(creditCard["cc-number"])) {
this._elements.ccNumber.setCustomValidity(true);
return;
}
- let storage = await this.getStorage();
- await storage.normalizeCCNumberFields(creditCard);
- await this.saveRecord(creditCard, this._record ? this._record.guid : null);
+
+ // TODO: "MasterPassword.ensureLoggedIn" can be removed after the storage
+ // APIs are refactored to be async functions (bug 1399367).
+ if (await MasterPassword.ensureLoggedIn()) {
+ await this.saveRecord(creditCard, this._record ? this._record.guid : null);
+ }
window.close();
}
handleInput(event) {
// Clear the error message if cc-number is valid
if (event.target == this._elements.ccNumber &&
FormAutofillUtils.isCCNumber(this._elements.ccNumber.value)) {
this._elements.ccNumber.setCustomValidity("");
--- a/browser/extensions/formautofill/content/manageDialog.js
+++ b/browser/extensions/formautofill/content/manageDialog.js
@@ -343,17 +343,17 @@ class ManageCreditCards extends ManageRe
/**
* Open the edit address dialog to create/edit a credit card.
*
* @param {object} creditCard [optional]
*/
async openEditDialog(creditCard) {
// If master password is set, ask for password if user is trying to edit an
// existing credit card.
- if (!this._hasMasterPassword || !creditCard || await MasterPassword.prompt()) {
+ if (!creditCard || !this._hasMasterPassword || await MasterPassword.ensureLoggedIn(true)) {
this.prefWin.gSubDialog.open(EDIT_CREDIT_CARD_URL, null, creditCard);
}
}
/**
* Get credit card display label. It should display masked numbers and the
* cardholder's name, separated by a comma. If `showCreditCards` is set to
* true, decrypted credit card numbers are shown instead.
--- a/browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
+++ b/browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
@@ -46,17 +46,16 @@ add_task(async function test_submit_cred
// Wait 1000ms before submission to make sure the input value applied
await new Promise(resolve => setTimeout(resolve, 1000));
form.querySelector("input[type=submit]").click();
});
await promiseShown;
await clickDoorhangerButton(MAIN_BUTTON);
- await TestUtils.topicObserved("formautofill-storage-changed");
}
);
let creditCards = await getCreditCards();
is(creditCards.length, 1, "1 credit card in storage");
is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
});
--- a/browser/extensions/formautofill/test/unit/head.js
+++ b/browser/extensions/formautofill/test/unit/head.js
@@ -85,30 +85,30 @@ function getTempFile(leafName) {
if (file.exists()) {
file.remove(false);
}
});
return file;
}
-async function initProfileStorage(fileName, records) {
+async function initProfileStorage(fileName, records, collectionName = "addresses") {
let {ProfileStorage} = Cu.import("resource://formautofill/ProfileStorage.jsm", {});
let path = getTempFile(fileName).path;
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
if (!records || !Array.isArray(records)) {
return profileStorage;
}
let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
(subject, data) => data == "add");
for (let record of records) {
- do_check_true(profileStorage.addresses.add(record));
+ do_check_true(profileStorage[collectionName].add(record));
await onChanged;
}
await profileStorage._saveImmediately();
return profileStorage;
}
function runHeuristicsTest(patterns, fixturePathPrefix) {
Cu.import("resource://formautofill/FormAutofillHeuristics.jsm");
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -47,51 +47,39 @@ const TEST_CREDIT_CARD_WITH_INVALID_EXPI
"cc-exp-year": -3,
};
const TEST_CREDIT_CARD_WITH_SPACES_BETWEEN_DIGITS = {
"cc-name": "John Doe",
"cc-number": "1111 2222 3333 4444",
};
-const TEST_CREDIT_CARD_WITH_INVALID_NUMBERS = {
- "cc-name": "John Doe",
- "cc-number": "abcdefg",
-};
-
-const TEST_CREDIT_CARD_WITH_SHORT_NUMBERS = {
- "cc-name": "John Doe",
- "cc-number": "1234567890",
-};
-
let prepareTestCreditCards = async function(path) {
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
(subject, data) => data == "add");
- let encryptedCC_1 = Object.assign({}, TEST_CREDIT_CARD_1);
- await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_1);
- do_check_true(profileStorage.creditCards.add(encryptedCC_1));
+ do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_1));
await onChanged;
- let encryptedCC_2 = Object.assign({}, TEST_CREDIT_CARD_2);
- await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_2);
- do_check_true(profileStorage.creditCards.add(encryptedCC_2));
+ do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_2));
+ await onChanged;
await profileStorage._saveImmediately();
};
let reCCNumber = /^(\*+)(.{4})$/;
let do_check_credit_card_matches = (creditCardWithMeta, creditCard) => {
for (let key in creditCard) {
if (key == "cc-number") {
let matches = reCCNumber.exec(creditCardWithMeta["cc-number"]);
do_check_neq(matches, null);
do_check_eq(creditCardWithMeta["cc-number"].length, creditCard["cc-number"].length);
do_check_eq(creditCard["cc-number"].endsWith(matches[2]), true);
+ do_check_neq(creditCard["cc-number-encrypted"], "");
} else {
do_check_eq(creditCardWithMeta[key], creditCard[key]);
}
}
};
add_task(async function test_initialize() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
@@ -177,19 +165,17 @@ add_task(async function test_add() {
do_check_neq(creditCards[0].guid, undefined);
do_check_eq(creditCards[0].version, 1);
do_check_neq(creditCards[0].timeCreated, undefined);
do_check_eq(creditCards[0].timeLastModified, creditCards[0].timeCreated);
do_check_eq(creditCards[0].timeLastUsed, 0);
do_check_eq(creditCards[0].timesUsed, 0);
- let encryptedCC_invalid = Object.assign({}, TEST_CREDIT_CARD_WITH_INVALID_FIELD);
- await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_invalid);
- Assert.throws(() => profileStorage.creditCards.add(encryptedCC_invalid),
+ Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_FIELD),
/"invalidField" is not a valid field\./);
});
add_task(async function test_update() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
await prepareTestCreditCards(path);
let profileStorage = new ProfileStorage(path);
@@ -198,17 +184,16 @@ add_task(async function test_update() {
let creditCards = profileStorage.creditCards.getAll();
let guid = creditCards[1].guid;
let timeLastModified = creditCards[1].timeLastModified;
let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
(subject, data) => data == "update");
do_check_neq(creditCards[1]["cc-name"], undefined);
- await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_3);
profileStorage.creditCards.update(guid, TEST_CREDIT_CARD_3);
await onChanged;
await profileStorage._saveImmediately();
profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
let creditCard = profileStorage.creditCards.get(guid);
@@ -217,64 +202,45 @@ add_task(async function test_update() {
do_check_neq(creditCard.timeLastModified, timeLastModified);
do_check_credit_card_matches(creditCard, TEST_CREDIT_CARD_3);
Assert.throws(
() => profileStorage.creditCards.update("INVALID_GUID", TEST_CREDIT_CARD_3),
/No matching record\./
);
- let encryptedCC_invalid = Object.assign({}, TEST_CREDIT_CARD_WITH_INVALID_FIELD);
- await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_invalid);
Assert.throws(
- () => profileStorage.creditCards.update(guid, encryptedCC_invalid),
+ () => profileStorage.creditCards.update(guid, TEST_CREDIT_CARD_WITH_INVALID_FIELD),
/"invalidField" is not a valid field\./
);
});
add_task(async function test_validate() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
- await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_INVALID_EXPIRY_DATE);
profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_EXPIRY_DATE);
- await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR);
profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR);
- await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_SPACES_BETWEEN_DIGITS);
profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_SPACES_BETWEEN_DIGITS);
let creditCards = profileStorage.creditCards.getAll();
do_check_eq(creditCards[0]["cc-exp-month"], undefined);
do_check_eq(creditCards[0]["cc-exp-year"], undefined);
do_check_eq(creditCards[0]["cc-exp"], undefined);
let month = TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR["cc-exp-month"];
let year = parseInt(TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR["cc-exp-year"], 10) + 2000;
do_check_eq(creditCards[1]["cc-exp-month"], month);
do_check_eq(creditCards[1]["cc-exp-year"], year);
do_check_eq(creditCards[1]["cc-exp"], year + "-" + month.toString().padStart(2, "0"));
do_check_eq(creditCards[2]["cc-number"].length, 16);
-
- try {
- await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_INVALID_NUMBERS);
- throw new Error("Not receiving invalid characters error");
- } catch (e) {
- Assert.equal(e.message, "Credit card number contains invalid characters or is under 12 digits.");
- }
-
- try {
- await profileStorage.creditCards.normalizeCCNumberFields(TEST_CREDIT_CARD_WITH_SHORT_NUMBERS);
- throw new Error("Not receiving invalid characters error");
- } catch (e) {
- Assert.equal(e.message, "Credit card number contains invalid characters or is under 12 digits.");
- }
});
add_task(async function test_notifyUsed() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
await prepareTestCreditCards(path);
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
--- a/browser/extensions/formautofill/test/unit/test_getRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_getRecords.js
@@ -1,15 +1,16 @@
/*
* Test for make sure getRecords can retrieve right collection from storage.
*/
"use strict";
Cu.import("resource://formautofill/FormAutofillParent.jsm");
+Cu.import("resource://formautofill/MasterPassword.jsm");
Cu.import("resource://formautofill/ProfileStorage.jsm");
const TEST_ADDRESS_1 = {
"given-name": "Timothy",
"additional-name": "John",
"family-name": "Berners-Lee",
organization: "World Wide Web Consortium",
"street-address": "32 Vassar Street\nMIT Room 32-G524",
@@ -161,23 +162,26 @@ add_task(async function test_getRecords_
});
add_task(async function test_getRecords_creditCards() {
let formAutofillParent = new FormAutofillParent();
await formAutofillParent.init();
await formAutofillParent.profileStorage.initialize();
let collection = profileStorage.creditCards;
- let decryptedCCNumber = [TEST_CREDIT_CARD_1["cc-number"], TEST_CREDIT_CARD_2["cc-number"]];
- await collection.normalizeCCNumberFields(TEST_CREDIT_CARD_1);
- await collection.normalizeCCNumberFields(TEST_CREDIT_CARD_2);
- sinon.stub(collection, "getAll", () => [Object.assign({}, TEST_CREDIT_CARD_1), Object.assign({}, TEST_CREDIT_CARD_2)]);
+ let encryptedCCRecords = [TEST_CREDIT_CARD_1, TEST_CREDIT_CARD_2].map(record => {
+ let clonedRecord = Object.assign({}, record);
+ clonedRecord["cc-number"] = collection._getMaskedCCNumber(record["cc-number"]);
+ clonedRecord["cc-number-encrypted"] = MasterPassword.encryptSync(record["cc-number"]);
+ return clonedRecord;
+ });
+ sinon.stub(collection, "getAll", () => [Object.assign({}, encryptedCCRecords[0]), Object.assign({}, encryptedCCRecords[1])]);
let CreditCardsWithDecryptedNumber = [
- Object.assign({}, TEST_CREDIT_CARD_1, {"cc-number-decrypted": decryptedCCNumber[0]}),
- Object.assign({}, TEST_CREDIT_CARD_2, {"cc-number-decrypted": decryptedCCNumber[1]}),
+ Object.assign({}, encryptedCCRecords[0], {"cc-number-decrypted": TEST_CREDIT_CARD_1["cc-number"]}),
+ Object.assign({}, encryptedCCRecords[1], {"cc-number-decrypted": TEST_CREDIT_CARD_2["cc-number"]}),
];
let testCases = [
{
description: "If the search string could match 1 creditCard (without masterpassword)",
filter: {
collectionName: "creditCards",
info: {fieldName: "cc-name"},
@@ -224,27 +228,27 @@ add_task(async function test_getRecords_
{
description: "If the search string could match 1 creditCard (with masterpassword)",
filter: {
collectionName: "creditCards",
info: {fieldName: "cc-name"},
searchString: "John Doe",
},
mpEnabled: true,
- expectedResult: [TEST_CREDIT_CARD_1],
+ expectedResult: encryptedCCRecords.slice(0, 1),
},
{
description: "Return all creditCards if focused field is cc number (with masterpassword)",
filter: {
collectionName: "creditCards",
info: {fieldName: "cc-number"},
searchString: "123",
},
mpEnabled: true,
- expectedResult: [TEST_CREDIT_CARD_1, TEST_CREDIT_CARD_2],
+ expectedResult: encryptedCCRecords,
},
];
for (let testCase of testCases) {
do_print("Starting testcase: " + testCase.description);
if (testCase.mpEnabled) {
let tokendb = Cc["@mozilla.org/security/pk11tokendb;1"].createInstance(Ci.nsIPK11TokenDB);
let token = tokendb.getInternalKeyToken();
--- a/browser/extensions/formautofill/test/unit/test_reconcile.js
+++ b/browser/extensions/formautofill/test/unit/test_reconcile.js
@@ -6,17 +6,17 @@ const TEST_STORE_FILE_NAME = "test-profi
// parent: What the local record looked like the last time we wrote the
// record to the Sync server.
// local: What the local record looks like now. IOW, the differences between
// 'parent' and 'local' are changes recently made which we wish to sync.
// remote: An incoming record we need to apply (ie, a record that was possibly
// changed on a remote device)
//
// To further help understanding this, a few of the testcases are annotated.
-const RECONCILE_TESTCASES = [
+const ADDRESS_RECONCILE_TESTCASES = [
{
description: "Local change",
parent: {
// So when we last wrote the record to the server, it had these values.
"guid": "2bbd2d8fbc6b",
"version": 1,
"given-name": "Mark",
"family-name": "Hammond",
@@ -459,16 +459,468 @@ const RECONCILE_TESTCASES = [
"family-name": "Hammond",
"timeCreated": 1234,
"timeLastUsed": 5678,
"timesUsed": 6,
},
},
];
+const CREDIT_CARD_RECONCILE_TESTCASES = [
+ {
+ description: "Local change",
+ parent: {
+ // So when we last wrote the record to the server, it had these values.
+ "guid": "2bbd2d8fbc6b",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ local: [{
+ // The current local record - by comparing against parent we can see that
+ // only the cc-number has changed locally.
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ }],
+ remote: {
+ // This is the incoming record. It has the same values as "parent", so
+ // we can deduce the record hasn't actually been changed remotely so we
+ // can safely ignore the incoming record and write our local changes.
+ "guid": "2bbd2d8fbc6b",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ reconciled: {
+ "guid": "2bbd2d8fbc6b",
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ },
+ },
+ {
+ description: "Remote change",
+ parent: {
+ "guid": "e3680e9f890d",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ }],
+ remote: {
+ "guid": "e3680e9f890d",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ },
+ reconciled: {
+ "guid": "e3680e9f890d",
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ },
+ },
+
+ {
+ description: "New local field",
+ parent: {
+ "guid": "0cba738b1be0",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ }],
+ remote: {
+ "guid": "0cba738b1be0",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ reconciled: {
+ "guid": "0cba738b1be0",
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ },
+ {
+ description: "New remote field",
+ parent: {
+ "guid": "be3ef97f8285",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ }],
+ remote: {
+ "guid": "be3ef97f8285",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ reconciled: {
+ "guid": "be3ef97f8285",
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ },
+ {
+ description: "Deleted field locally",
+ parent: {
+ "guid": "9627322248ec",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ }],
+ remote: {
+ "guid": "9627322248ec",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ reconciled: {
+ "guid": "9627322248ec",
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ },
+ {
+ description: "Deleted field remotely",
+ parent: {
+ "guid": "7d7509f3eeb2",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ }],
+ remote: {
+ "guid": "7d7509f3eeb2",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ reconciled: {
+ "guid": "7d7509f3eeb2",
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ },
+ {
+ description: "Local and remote changes to unrelated fields",
+ parent: {
+ // The last time we wrote this to the server, "cc-exp-month" was 12.
+ "guid": "e087a06dfc57",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ local: [{
+ // The current local record - so locally we've changed "cc-number".
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ "cc-exp-month": 12,
+ }],
+ remote: {
+ // Remotely, we've changed "cc-exp-month" to 1.
+ "guid": "e087a06dfc57",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 1,
+ },
+ reconciled: {
+ "guid": "e087a06dfc57",
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ "cc-exp-month": 1,
+ },
+ },
+ {
+ description: "Multiple local changes",
+ parent: {
+ "guid": "340a078c596f",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ local: [{
+ "cc-name": "Skip",
+ "cc-number": "1111222233334444",
+ }, {
+ "cc-name": "Skip",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ }],
+ remote: {
+ "guid": "340a078c596f",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-year": 2000,
+ },
+ reconciled: {
+ "guid": "340a078c596f",
+ "cc-name": "Skip",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ "cc-exp-year": 2000,
+ },
+ },
+ {
+ // Local and remote diverged from the shared parent, but the values are the
+ // same, so we shouldn't fork.
+ description: "Same change to local and remote",
+ parent: {
+ "guid": "0b3a72a1bea2",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ }],
+ remote: {
+ "guid": "0b3a72a1bea2",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ },
+ reconciled: {
+ "guid": "0b3a72a1bea2",
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ },
+ },
+ {
+ description: "Conflicting changes to single field",
+ parent: {
+ // This is what we last wrote to the sync server.
+ "guid": "62068784d089",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ local: [{
+ // The current version of the local record - the cc-number has changed locally.
+ "cc-name": "John Doe",
+ "cc-number": "1111111111111111",
+ }],
+ remote: {
+ // An incoming record has a different cc-number than any of the above!
+ "guid": "62068784d089",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ },
+ forked: {
+ // So we've forked the local record to a new GUID (and the next sync is
+ // going to write this as a new record)
+ "cc-name": "John Doe",
+ "cc-number": "1111111111111111",
+ },
+ reconciled: {
+ // And we've updated the local version of the record to be the remote version.
+ guid: "62068784d089",
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ },
+ },
+ {
+ description: "Conflicting changes to multiple fields",
+ parent: {
+ "guid": "244dbb692e94",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111111111111111",
+ "cc-exp-month": 1,
+ }],
+ remote: {
+ "guid": "244dbb692e94",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ "cc-exp-month": 3,
+ },
+ forked: {
+ "cc-name": "John Doe",
+ "cc-number": "1111111111111111",
+ "cc-exp-month": 1,
+ },
+ reconciled: {
+ "guid": "244dbb692e94",
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ "cc-exp-month": 3,
+ },
+ },
+ {
+ description: "Field deleted locally, changed remotely",
+ parent: {
+ "guid": "6fc45e03d19a",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ }],
+ remote: {
+ "guid": "6fc45e03d19a",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 3,
+ },
+ forked: {
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ reconciled: {
+ "guid": "6fc45e03d19a",
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 3,
+ },
+ },
+ {
+ description: "Field changed locally, deleted remotely",
+ parent: {
+ "guid": "fff9fa27fa18",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 12,
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 3,
+ }],
+ remote: {
+ "guid": "fff9fa27fa18",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ forked: {
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "cc-exp-month": 3,
+ },
+ reconciled: {
+ "guid": "fff9fa27fa18",
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ },
+ },
+ {
+ // Created, last modified should be synced; last used and times used should
+ // be local. Remote created time older than local, remote modified time
+ // newer than local.
+ description: "Created, last modified time reconciliation without local changes",
+ parent: {
+ "guid": "5113f329c42f",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "timeCreated": 1234,
+ "timeLastModified": 5678,
+ "timeLastUsed": 5678,
+ "timesUsed": 6,
+ },
+ local: [],
+ remote: {
+ "guid": "5113f329c42f",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "timeCreated": 1200,
+ "timeLastModified": 5700,
+ "timeLastUsed": 5700,
+ "timesUsed": 3,
+ },
+ reconciled: {
+ "guid": "5113f329c42f",
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "timeCreated": 1200,
+ "timeLastModified": 5700,
+ "timeLastUsed": 5678,
+ "timesUsed": 6,
+ },
+ },
+ {
+ // Local changes, remote created time newer than local, remote modified time
+ // older than local.
+ description: "Created, last modified time reconciliation with local changes",
+ parent: {
+ "guid": "791e5608b80a",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "timeCreated": 1234,
+ "timeLastModified": 5678,
+ "timeLastUsed": 5678,
+ "timesUsed": 6,
+ },
+ local: [{
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ }],
+ remote: {
+ "guid": "791e5608b80a",
+ "version": 1,
+ "cc-name": "John Doe",
+ "cc-number": "1111222233334444",
+ "timeCreated": 1300,
+ "timeLastModified": 5000,
+ "timeLastUsed": 5000,
+ "timesUsed": 3,
+ },
+ reconciled: {
+ "guid": "791e5608b80a",
+ "cc-name": "John Doe",
+ "cc-number": "4444333322221111",
+ "timeCreated": 1234,
+ "timeLastUsed": 5678,
+ "timesUsed": 6,
+ },
+ },
+];
+
add_task(async function test_reconcile_unknown_version() {
let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME);
// Cross-version reconciliation isn't supported yet. See bug 1377204.
throws(() => {
profileStorage.addresses.reconcile({
"guid": "31d83d2725ec",
"version": 2,
@@ -531,43 +983,52 @@ add_task(async function test_reconcile_i
"family-name": "Hammond",
"organization": "Mozilla",
"tel": "123456",
}), "Second merge should not change record");
}
});
add_task(async function test_reconcile_three_way_merge() {
- let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME);
-
- for (let test of RECONCILE_TESTCASES) {
- do_print(test.description);
+ let TESTCASES = {
+ addresses: ADDRESS_RECONCILE_TESTCASES,
+ creditCards: CREDIT_CARD_RECONCILE_TESTCASES,
+ };
- profileStorage.addresses.add(test.parent, {sourceSync: true});
+ for (let collectionName in TESTCASES) {
+ do_print(`Start to test reconcile on ${collectionName}`);
- for (let updatedRecord of test.local) {
- profileStorage.addresses.update(test.parent.guid, updatedRecord);
- }
+ let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, null, collectionName);
- let localRecord = profileStorage.addresses.get(test.parent.guid, {
- rawData: true,
- });
+ for (let test of TESTCASES[collectionName]) {
+ do_print(test.description);
+
+ profileStorage[collectionName].add(test.parent, {sourceSync: true});
- let {forkedGUID} = profileStorage.addresses.reconcile(test.remote);
- let reconciledRecord = profileStorage.addresses.get(test.parent.guid, {
- rawData: true,
- });
- if (forkedGUID) {
- let forkedRecord = profileStorage.addresses.get(forkedGUID, {
+ for (let updatedRecord of test.local) {
+ profileStorage[collectionName].update(test.parent.guid, updatedRecord);
+ }
+
+ let localRecord = profileStorage[collectionName].get(test.parent.guid, {
rawData: true,
});
- notEqual(forkedRecord.guid, reconciledRecord.guid);
- equal(forkedRecord.timeLastModified, localRecord.timeLastModified);
- ok(objectMatches(forkedRecord, test.forked),
- `${test.description} should fork record`);
- } else {
- ok(!test.forked, `${test.description} should not fork record`);
+ let {forkedGUID} = profileStorage[collectionName].reconcile(test.remote);
+ let reconciledRecord = profileStorage[collectionName].get(test.parent.guid, {
+ rawData: true,
+ });
+ if (forkedGUID) {
+ let forkedRecord = profileStorage[collectionName].get(forkedGUID, {
+ rawData: true,
+ });
+
+ notEqual(forkedRecord.guid, reconciledRecord.guid);
+ equal(forkedRecord.timeLastModified, localRecord.timeLastModified);
+ ok(objectMatches(forkedRecord, test.forked),
+ `${test.description} should fork record`);
+ } else {
+ ok(!test.forked, `${test.description} should not fork record`);
+ }
+
+ ok(objectMatches(reconciledRecord, test.reconciled));
}
-
- ok(objectMatches(reconciledRecord, test.reconciled));
}
});
--- a/browser/extensions/formautofill/test/unit/test_storage_tombstones.js
+++ b/browser/extensions/formautofill/test/unit/test_storage_tombstones.js
@@ -40,19 +40,16 @@ function add_storage_task(test_function)
add_task(async function() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
let profileStorage = new ProfileStorage(path);
let testCC1 = Object.assign({}, TEST_CC_1);
await profileStorage.initialize();
for (let [storage, record] of [[profileStorage.addresses, TEST_ADDRESS_1],
[profileStorage.creditCards, testCC1]]) {
- if (storage.normalizeCCNumberFields) {
- await storage.normalizeCCNumberFields(record);
- }
await test_function(storage, record);
}
});
}
add_storage_task(async function test_simple_tombstone(storage, record) {
do_print("check simple tombstone semantics");
--- a/browser/extensions/formautofill/test/unit/test_transformFields.js
+++ b/browser/extensions/formautofill/test/unit/test_transformFields.js
@@ -616,19 +616,17 @@ add_task(async function test_normalizeAd
add_task(async function test_computeCreditCardFields() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
for (let testcase of CREDIT_CARD_COMPUTE_TESTCASES) {
- let encryptedCC = Object.assign({}, testcase.creditCard);
- await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC);
- profileStorage.creditCards.add(encryptedCC);
+ profileStorage.creditCards.add(testcase.creditCard);
}
await profileStorage._saveImmediately();
profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
let creditCards = profileStorage.creditCards.getAll();
@@ -640,19 +638,17 @@ add_task(async function test_computeCred
add_task(async function test_normalizeCreditCardFields() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
for (let testcase of CREDIT_CARD_NORMALIZE_TESTCASES) {
- let encryptedCC = Object.assign({}, testcase.creditCard);
- await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC);
- profileStorage.creditCards.add(encryptedCC);
+ profileStorage.creditCards.add(testcase.creditCard);
}
await profileStorage._saveImmediately();
profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
let creditCards = profileStorage.creditCards.getAll();
