Bug 1390433 - (From 1337314)Encrypt card number while normallizing field. draft
authorsteveck-chung <schung@mozilla.com>
Mon, 17 Jul 2017 17:14:54 +0800
changeset 648830 309f591ed66c381f893d32c9282f0e6167f6b2ab
parent 648829 ec9d5f3435ceae650932196d79a8284d88d35f0a
child 648831 f9ab71432c920ef8842746a143b3a457083e6f80
push id74896
push userschung@mozilla.com
push dateFri, 18 Aug 2017 10:48:05 +0000
bugs1390433, 1337314
milestone56.0
Bug 1390433 - (From 1337314)Encrypt card number while normallizing field. MozReview-Commit-ID: 9HSpLzJMnoE
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/browser/head.js
browser/extensions/formautofill/test/unit/test_creditCardRecords.js
browser/extensions/formautofill/test/unit/test_storage_tombstones.js
browser/extensions/formautofill/test/unit/test_transformFields.js
--- 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) {