Bug 1337314 - Encrypt card number while normallizing field. r=lchang

MozReview-Commit-ID: 9HSpLzJMnoE

--HG--
extra : rebase_source : 99bba044c911da99dcbd94597641325f65ad0535

browser/extensions/formautofill/FormAutofillParent.jsm

@@ -176,7 +176,7 @@ FormAutofillParent.prototype = {
    * @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();
@@ -195,6 +195,7 @@ FormAutofillParent.prototype = {
         break;
       }
       case "FormAutofill:SaveCreditCard": {
+        await this.profileStorage.creditCards.normalizeCCNumberFields(data.creditcard);
         this.profileStorage.creditCards.add(data.creditcard);
         break;
       }

browser/extensions/formautofill/ProfileStorage.jsm

@@ -116,6 +116,8 @@ 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");
 
@@ -1471,26 +1473,9 @@ class CreditCards extends AutofillRecords {
   }
 
   _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
@@ -1529,6 +1514,38 @@ class CreditCards extends AutofillRecords {
       }
     }
   }
+
+  /**
+   * 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) {

browser/extensions/formautofill/test/browser/head.js

@@ -155,7 +155,10 @@ function saveAddress(address) {
 }
 
 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) {

browser/extensions/formautofill/test/unit/test_creditCardRecords.js

@@ -36,6 +36,7 @@ const TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR = {
 
 const TEST_CREDIT_CARD_WITH_INVALID_FIELD = {
   "cc-name": "John Doe",
+  "cc-number": "1234123412341234",
   invalidField: "INVALID",
 };
 
@@ -56,15 +57,24 @@ const TEST_CREDIT_CARD_WITH_INVALID_NUMBERS = {
   "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();
 };
 
@@ -73,8 +83,6 @@ 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);
@@ -163,7 +171,7 @@ add_task(async function test_getByFilter() {
   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);
@@ -191,7 +199,9 @@ add_task(async function test_add() {
   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\./);
 });
 
@@ -210,7 +220,7 @@ add_task(async function test_update() {
                                           (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();
@@ -229,8 +239,10 @@ add_task(async function test_update() {
     /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\./
   );
 });
@@ -241,8 +253,11 @@ add_task(async function test_validate() {
   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();
@@ -255,11 +270,20 @@ add_task(async function test_validate() {
     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() {

browser/extensions/formautofill/test/unit/test_storage_tombstones.js

@@ -40,10 +40,14 @@ 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);
     }
   });

browser/extensions/formautofill/test/unit/test_transformFields.js

@@ -488,6 +488,7 @@ const CREDIT_CARD_COMPUTE_TESTCASES = [
   {
     description: "Empty credit card",
     creditCard: {
+      "cc-number": "1234123412341234", // cc-number won't be verified
     },
     expectedResult: {
     },
@@ -498,6 +499,7 @@ const CREDIT_CARD_COMPUTE_TESTCASES = [
     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",
@@ -511,8 +513,9 @@ const CREDIT_CARD_COMPUTE_TESTCASES = [
 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: {
     },
@@ -525,6 +528,7 @@ const CREDIT_CARD_NORMALIZE_TESTCASES = [
       "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",
@@ -535,11 +539,21 @@ const CREDIT_CARD_NORMALIZE_TESTCASES = [
     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) => {
@@ -594,7 +608,11 @@ add_task(async function test_computeCreditCardFields() {
   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);
@@ -614,7 +632,11 @@ add_task(async function test_normalizeCreditCardFields() {
   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);