Bug 1390433 - (From 1337314)Encrypt card number while normallizing field.
MozReview-Commit-ID: 9HSpLzJMnoE
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -171,17 +171,17 @@ FormAutofillParent.prototype = {
/**
* Handles the message coming from FormAutofillContent.
*
* @param {string} message.name The name of the message.
* @param {object} message.data The data of the message.
* @param {nsIFrameMessageManager} message.target Caller's message manager.
*/
- receiveMessage({name, data, target}) {
+ async receiveMessage({name, data, target}) {
switch (name) {
case "FormAutofill:InitStorage": {
this.profileStorage.initialize();
break;
}
case "FormAutofill:GetRecords": {
this._getRecords(data, target);
break;
@@ -190,16 +190,17 @@ 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);
this.profileStorage.creditCards.add(data.creditcard);
break;
}
case "FormAutofill:RemoveAddresses": {
data.guids.forEach(guid => this.profileStorage.addresses.remove(guid));
break;
}
case "FormAutofill:OnFormSubmit": {
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -111,16 +111,18 @@ Cu.import("resource://gre/modules/Servic
Cu.import("resource://gre/modules/osfile.jsm");
Cu.import("resource://formautofill/FormAutofillUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "JSONFile",
"resource://gre/modules/JSONFile.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "FormAutofillNameUtils",
"resource://formautofill/FormAutofillNameUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "MasterPassword",
+ "resource://formautofill/MasterPassword.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PhoneNumber",
"resource://formautofill/phonenumberutils/PhoneNumber.jsm");
XPCOMUtils.defineLazyServiceGetter(this, "gUUIDGenerator",
"@mozilla.org/uuid-generator;1",
"nsIUUIDGenerator");
XPCOMUtils.defineLazyGetter(this, "REGION_NAMES", function() {
@@ -1466,36 +1468,19 @@ class CreditCards extends AutofillRecord
creditCard["cc-family-name"] = nameParts.family;
hasNewComputedFields = true;
}
return hasNewComputedFields;
}
_normalizeFields(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 (!/^\d+$/.test(ccNumber)) {
- throw new Error("Credit card number contains invalid characters.");
- }
-
- // TODO: Encrypt cc-number here (bug 1337314).
- // e.g. creditCard["cc-number-encrypted"] = Encrypt(creditCard["cc-number"]);
-
- if (ccNumber.length > 4) {
- creditCard["cc-number"] = "*".repeat(ccNumber.length - 4) + ccNumber.substr(-4);
- } else {
- creditCard["cc-number"] = ccNumber;
- }
+ // 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"],
@@ -1524,16 +1509,48 @@ 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 (!/^\d+$/.test(ccNumber)) {
+ throw new Error("Credit card number contains invalid characters.");
+ }
+
+ // Based on the information on wiki[1], the shortest valid length should be
+ // 12 digits(Maestro).
+ // [1] https://en.wikipedia.org/wiki/Payment_card_number
+ if (ccNumber.length < 12) {
+ throw new Error("Invalid credit card number because length is under 12 digits.");
+ }
+
+ creditCard["cc-number-encrypted"] = await MasterPassword.encrypt(creditCard["cc-number"]);
+ 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/test/browser/head.js
+++ b/browser/extensions/formautofill/test/browser/head.js
@@ -150,17 +150,20 @@ function getAddresses() {
}
function saveAddress(address) {
Services.cpmm.sendAsyncMessage("FormAutofill:SaveAddress", {address});
return TestUtils.topicObserved("formautofill-storage-changed");
}
function saveCreditCard(creditcard) {
- Services.cpmm.sendAsyncMessage("FormAutofill:SaveCreditCard", {creditcard});
+ let creditcardClone = Object.assign({}, creditcard);
+ Services.cpmm.sendAsyncMessage("FormAutofill:SaveCreditCard", {
+ creditcard: creditcardClone,
+ });
return TestUtils.topicObserved("formautofill-storage-changed");
}
function removeAddresses(guids) {
Services.cpmm.sendAsyncMessage("FormAutofill:RemoveAddresses", {guids});
return TestUtils.topicObserved("formautofill-storage-changed");
}
/**
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -31,16 +31,17 @@ const TEST_CREDIT_CARD_3 = {
const TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR = {
"cc-number": "1234123412341234",
"cc-exp-month": 1,
"cc-exp-year": 12,
};
const TEST_CREDIT_CARD_WITH_INVALID_FIELD = {
"cc-name": "John Doe",
+ "cc-number": "1234123412341234",
invalidField: "INVALID",
};
const TEST_CREDIT_CARD_WITH_INVALID_EXPIRY_DATE = {
"cc-name": "John Doe",
"cc-number": "1111222233334444",
"cc-exp-month": 13,
"cc-exp-year": -3,
@@ -51,35 +52,42 @@ const TEST_CREDIT_CARD_WITH_SPACES_BETWE
"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");
- do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_1));
+ let encryptedCC_1 = Object.assign({}, TEST_CREDIT_CARD_1);
+ await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_1);
+ do_check_true(profileStorage.creditCards.add(encryptedCC_1));
await onChanged;
- do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_2));
+ let encryptedCC_2 = Object.assign({}, TEST_CREDIT_CARD_2);
+ await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_2);
+ do_check_true(profileStorage.creditCards.add(encryptedCC_2));
await profileStorage._saveImmediately();
};
let reCCNumber = /^(\*+)(.{4})$/;
let do_check_credit_card_matches = (creditCardWithMeta, creditCard) => {
for (let key in creditCard) {
if (key == "cc-number") {
- // check "cc-number-encrypted" after encryption lands (bug 1337314).
-
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);
} else {
do_check_eq(creditCardWithMeta[key], creditCard[key]);
}
}
@@ -158,17 +166,17 @@ add_task(async function test_getByFilter
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
let filter = {info: {fieldName: "cc-name"}, searchString: "Tim"};
let creditCards = profileStorage.creditCards.getByFilter(filter);
do_check_eq(creditCards.length, 1);
do_check_credit_card_matches(creditCards[0], TEST_CREDIT_CARD_2);
- // TODO: Uncomment this after decryption lands (bug 1337314).
+ // TODO: Uncomment this after decryption lands (bug 1389413).
// filter = {info: {fieldName: "cc-number"}, searchString: "11"};
// creditCards = profileStorage.creditCards.getByFilter(filter);
// do_check_eq(creditCards.length, 1);
// do_check_credit_card_matches(creditCards[0], TEST_CREDIT_CARD_2);
});
add_task(async function test_add() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
@@ -186,17 +194,19 @@ 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);
- Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_FIELD),
+ let encryptedCC_invalid = Object.assign({}, TEST_CREDIT_CARD_WITH_INVALID_FIELD);
+ await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC_invalid);
+ Assert.throws(() => profileStorage.creditCards.add(encryptedCC_invalid),
/"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);
@@ -205,17 +215,17 @@ 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);
@@ -224,47 +234,61 @@ 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, TEST_CREDIT_CARD_WITH_INVALID_FIELD),
+ () => profileStorage.creditCards.update(guid, encryptedCC_invalid),
/"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[1]["cc-exp-month"], TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR["cc-exp-month"]);
do_check_eq(creditCards[1]["cc-exp-year"],
parseInt(TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR["cc-exp-year"], 10) + 2000);
do_check_eq(creditCards[2]["cc-number"].length, 16);
- // TODO: Check the decrypted numbers should not contain spaces after
- // decryption lands (bug 1337314).
- Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_NUMBERS),
- /Credit card number contains invalid characters\./);
+ 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.");
+ }
+
+ 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, "Invalid credit card number because length 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_storage_tombstones.js
+++ b/browser/extensions/formautofill/test/unit/test_storage_tombstones.js
@@ -35,20 +35,24 @@ let do_check_tombstone_record = (profile
["guid", "timeLastModified", "deleted"].sort());
};
// Like add_task, but actually adds 2 - one for addresses and one for cards.
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, TEST_CC_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
@@ -483,68 +483,82 @@ const ADDRESS_NORMALIZE_TESTCASES = [
},
];
const CREDIT_CARD_COMPUTE_TESTCASES = [
// Empty
{
description: "Empty credit card",
creditCard: {
+ "cc-number": "1234123412341234", // cc-number won't be verified
},
expectedResult: {
},
},
// Name
{
description: "Has \"cc-name\"",
creditCard: {
"cc-name": "Timothy John Berners-Lee",
+ "cc-number": "1234123412341234", // cc-number won't be verified
},
expectedResult: {
"cc-name": "Timothy John Berners-Lee",
"cc-given-name": "Timothy",
"cc-additional-name": "John",
"cc-family-name": "Berners-Lee",
},
},
];
const CREDIT_CARD_NORMALIZE_TESTCASES = [
// Empty
{
- description: "Empty credit card",
+ description: "No normalizable field",
creditCard: {
+ "cc-number": "1234123412341234", // cc-number won't be verified
},
expectedResult: {
},
},
// Name
{
description: "Has both \"cc-name\" and the split name fields",
creditCard: {
"cc-name": "Timothy John Berners-Lee",
"cc-given-name": "John",
"cc-family-name": "Doe",
+ "cc-number": "1234123412341234", // cc-number won't be verified
},
expectedResult: {
"cc-name": "Timothy John Berners-Lee",
},
},
{
description: "Has only the split name fields",
creditCard: {
"cc-given-name": "John",
"cc-family-name": "Doe",
+ "cc-number": "1234123412341234", // cc-number won't be verified
},
expectedResult: {
"cc-name": "John Doe",
},
},
+ {
+ description: "Number should be encrypted and masked",
+ creditCard: {
+ "cc-number": "1234123412341234",
+ },
+ expectedResult: {
+ "cc-number": "************1234",
+ },
+ },
];
let do_check_record_matches = (expectedRecord, record) => {
for (let key in expectedRecord) {
do_check_eq(expectedRecord[key], record[key]);
}
};
@@ -589,17 +603,21 @@ 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();
- CREDIT_CARD_COMPUTE_TESTCASES.forEach(testcase => profileStorage.creditCards.add(testcase.creditCard));
+ for (let testcase of CREDIT_CARD_COMPUTE_TESTCASES) {
+ let encryptedCC = Object.assign({}, testcase.creditCard);
+ await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC);
+ profileStorage.creditCards.add(encryptedCC);
+ }
await profileStorage._saveImmediately();
profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
let creditCards = profileStorage.creditCards.getAll();
for (let i in creditCards) {
@@ -609,17 +627,21 @@ 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();
- CREDIT_CARD_NORMALIZE_TESTCASES.forEach(testcase => profileStorage.creditCards.add(testcase.creditCard));
+ for (let testcase of CREDIT_CARD_NORMALIZE_TESTCASES) {
+ let encryptedCC = Object.assign({}, testcase.creditCard);
+ await profileStorage.creditCards.normalizeCCNumberFields(encryptedCC);
+ profileStorage.creditCards.add(encryptedCC);
+ }
await profileStorage._saveImmediately();
profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
let creditCards = profileStorage.creditCards.getAll();
for (let i in creditCards) {