Bug 1468644 - cc-numbers are always masked in dialog content; tests to verify this and card details in the payment response. r?jaws draft
authorSam Foster <sfoster@mozilla.com>
Wed, 18 Jul 2018 12:45:45 -0700
changeset 820170 bcc14bf1f5d9edd182c0ff07f934cc347c62a13f
parent 819867 1174739835690f7f4024a913794e5cf35d1524a2
push id116738
push userbmo:sfoster@mozilla.com
push dateThu, 19 Jul 2018 01:29:05 +0000
reviewersjaws
bugs1468644
milestone63.0a1
Bug 1468644 - cc-numbers are always masked in dialog content; tests to verify this and card details in the payment response. r?jaws MozReview-Commit-ID: 34MkjyRBth7
browser/components/payments/content/paymentDialogWrapper.js
browser/components/payments/res/containers/payment-method-picker.js
browser/components/payments/test/browser/browser_card_edit.js
--- a/browser/components/payments/content/paymentDialogWrapper.js
+++ b/browser/components/payments/content/paymentDialogWrapper.js
@@ -159,28 +159,24 @@ var paymentDialogWrapper = {
   async _convertProfileBasicCardToPaymentMethodData(guid, cardSecurityCode) {
     let cardData = this.temporaryStore.creditCards.get(guid) ||
                    formAutofillStorage.creditCards.get(guid);
     if (!cardData) {
       throw new Error(`Basic card not found in storage: ${guid}`);
     }
 
     let cardNumber;
-    if (cardData.isTemporary) {
-      cardNumber = cardData["cc-number"];
-    } else {
-      try {
-        cardNumber = await MasterPassword.decrypt(cardData["cc-number-encrypted"], true);
-      } catch (ex) {
-        if (ex.result != Cr.NS_ERROR_ABORT) {
-          throw ex;
-        }
-        // User canceled master password entry
-        return null;
+    try {
+      cardNumber = await MasterPassword.decrypt(cardData["cc-number-encrypted"], true);
+    } catch (ex) {
+      if (ex.result != Cr.NS_ERROR_ABORT) {
+        throw ex;
       }
+      // User canceled master password entry
+      return null;
     }
 
     let billingAddressGUID = cardData.billingAddressGUID;
     let billingAddress;
     try {
       billingAddress = await this._convertProfileAddressToPaymentAddress(billingAddressGUID);
     } catch (ex) {
       // The referenced address may not exist if it was deleted or hasn't yet synced to this profile
@@ -568,17 +564,16 @@ var paymentDialogWrapper = {
       // that's only supported when we're adding a new record.
       // TODO: "MasterPassword.ensureLoggedIn" can be removed after the storage
       // APIs are refactored to be async functions (bug 1399367).
       if (!await MasterPassword.ensureLoggedIn()) {
         Cu.reportError("User canceled master password entry");
         return;
       }
     }
-
     let isTemporary = record.isTemporary;
     let collection = isTemporary ? this.temporaryStore[collectionName] :
                                    formAutofillStorage[collectionName];
 
     try {
       if (guid) {
         await collection.update(guid, record, preserveOldProperties);
       } else {
--- a/browser/components/payments/res/containers/payment-method-picker.js
+++ b/browser/components/payments/res/containers/payment-method-picker.js
@@ -18,16 +18,17 @@ export default class PaymentMethodPicker
     super();
     this.dropdown = new RichSelect();
     this.dropdown.addEventListener("change", this);
     this.dropdown.setAttribute("option-type", "basic-card-option");
     this.spacerText = document.createTextNode(" ");
     this.securityCodeInput = document.createElement("input");
     this.securityCodeInput.autocomplete = "off";
     this.securityCodeInput.size = 3;
+    this.securityCodeInput.classList.add("security-code");
     this.securityCodeInput.addEventListener("change", this);
     this.addLink = document.createElement("a");
     this.addLink.className = "add-link";
     this.addLink.href = "javascript:void(0)";
     this.addLink.textContent = this.dataset.addLinkLabel;
     this.addLink.addEventListener("click", this);
     this.editLink = document.createElement("a");
     this.editLink.className = "edit-link";
--- a/browser/components/payments/test/browser/browser_card_edit.js
+++ b/browser/components/payments/test/browser/browser_card_edit.js
@@ -139,44 +139,48 @@ async function add_link(aOptions = {}) {
       let billingAddressSelect = content.document.querySelector("#billingAddressGUID");
 
       is(billingAddressSelect.childElementCount, 3,
          "Three options should exist in the billingAddressSelect");
       let selectedOption =
         billingAddressSelect.children[billingAddressSelect.selectedIndex];
       let selectedAddressGuid = selectedOption.value;
       let lastAddress = Object.values(addressColn)[Object.keys(addressColn).length - 1];
-      if (testArgs.isPrivate) {
-        // guid property is added in later patch on bug 1463608
-        is(selectedAddressGuid, lastAddress.guid,
-           "The select should have the new address selected");
-      } else {
-        is(selectedAddressGuid, lastAddress.guid,
-           "The select should have the new address selected");
-      }
+      is(selectedAddressGuid, lastAddress.guid, "The select should have the new address selected");
     }, aOptions);
 
     await fillInCardForm(frame, PTU.BasicCards.JaneMasterCard, {
       isTemporary: aOptions.isPrivate,
       checkboxSelector: "basic-card-form .persist-checkbox",
     });
 
     await spawnPaymentDialogTask(frame, async (testArgs = {}) => {
       let {
         PaymentTestUtils: PTU,
       } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
 
+      content.document.querySelector("basic-card-form button:last-of-type").click();
+
+      await PTU.DialogContentUtils.waitForState(content, (state) => {
+        return state.page.id == "payment-summary";
+      }, "Check we are back on the sumamry page");
+    });
+
+    await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.setSecurityCode, {
+      securityCode: "123",
+    });
+
+    await spawnPaymentDialogTask(frame, async (testArgs = {}) => {
+      let {
+        PaymentTestUtils: PTU,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
       let {prefilledGuids} = testArgs;
       let card = Object.assign({}, PTU.BasicCards.JaneMasterCard);
-
-      content.document.querySelector("basic-card-form button:last-of-type").click();
-
-      let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-        return state.page.id == "payment-summary";
-      }, "Check we are back on the sumamry page");
+      let state = await PTU.DialogContentUtils.getCurrentState(content);
 
       let cardCount = Object.keys(state.savedBasicCards).length +
                          Object.keys(state.tempBasicCards).length;
       is(cardCount, 2, "Card was added");
       if (testArgs.isPrivate) {
         is(Object.keys(state.tempBasicCards).length, 1, "Card was added temporarily");
         is(Object.keys(state.savedBasicCards).length, 1, "No change to saved cards");
       } else {
@@ -187,48 +191,57 @@ async function add_link(aOptions = {}) {
       let cardCollection = testArgs.isPrivate ? state.tempBasicCards : state.savedBasicCards;
       let addressCollection = testArgs.isPrivate ? state.tempAddresses : state.savedAddresses;
       let savedCardGUID =
         Object.keys(cardCollection).find(key => key != prefilledGuids.card1GUID);
       let savedAddressGUID =
         Object.keys(addressCollection).find(key => key != prefilledGuids.address1GUID);
       let savedCard = savedCardGUID && cardCollection[savedCardGUID];
 
-      card["cc-number"] = "************4444"; // Card should be masked
-
+      // we should never have an un-masked cc-number in the state:
+      ok(Object.values(cardCollection).every(card => card["cc-number"].startsWith("************")),
+         "All cc-numbers in state are masked");
+      card["cc-number"] = "************4444"; // Expect card number to be masked at this point
       for (let [key, val] of Object.entries(card)) {
-        if (key == "cc-number" && testArgs.isPrivate) {
-          // cc-number is not yet masked for private/temporary cards
-          is(savedCard[key], val, "Check " + key);
-        } else {
-          is(savedCard[key], val, "Check " + key);
-        }
+        is(savedCard[key], val, "Check " + key);
       }
-      if (testArgs.isPrivate) {
-        ok(testArgs.isPrivate,
-           "Checking card/address from private window relies on guid property " +
-           "which isnt available yet");
-      } else {
-        is(savedCard.billingAddressGUID, savedAddressGUID,
-           "The saved card should be associated with the billing address");
-      }
+
+      is(savedCard.billingAddressGUID, savedAddressGUID,
+         "The saved card should be associated with the billing address");
     }, aOptions);
 
-    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
+    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.completePayment);
+
+    // Add a handler to complete the payment above.
+    info("acknowledging the completion from the merchant page");
+    let result = await ContentTask.spawn(browser, {}, PTU.ContentTasks.addCompletionHandler);
+
+    // Verify response has the expected properties
+    let expectedDetails = Object.assign({
+      "cc-security-code": "123",
+    }, PTU.BasicCards.JaneMasterCard);
+    let expectedBillingAddress = PTU.Addresses.TimBL2;
+    let cardDetails = result.response.details;
+
+    checkPaymentMethodDetailsMatchesCard(cardDetails, expectedDetails,
+                                         "Check response payment details");
+    checkPaymentAddressMatchesStorageAddress(cardDetails.billingAddress, expectedBillingAddress,
+                                             "Check response billing address");
+
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
 }
 
 add_task(async function test_add_link() {
   let prefilledGuids = await setup([PTU.Addresses.TimBL], [PTU.BasicCards.JohnDoe]);
   await add_link({
     isPrivate: false,
     prefilledGuids,
   });
-}).skip();
+});
 
 add_task(async function test_private_add_link() {
   let prefilledGuids = await setup([PTU.Addresses.TimBL], [PTU.BasicCards.JohnDoe]);
   await add_link({
     isPrivate: true,
     prefilledGuids,
   });
 });
@@ -381,17 +394,17 @@ add_task(async function test_edit_link()
     for (let [key, val] of Object.entries(card)) {
       is(savedCard[key], val, "Check updated " + key);
     }
 
     state = await PTU.DialogContentUtils.waitForState(content, (state) => {
       return state.page.id == "payment-summary";
     }, "Switched back to payment-summary");
   }, args);
-}).skip();
+});
 
 add_task(async function test_private_card_adding() {
   await setup([PTU.Addresses.TimBL], [PTU.BasicCards.JohnDoe]);
   const args = {
     methodData: [PTU.MethodData.basicCard],
     details: PTU.Details.total60USD,
   };
   let privateWin = await BrowserTestUtils.openNewBrowserWindow({private: true});
@@ -446,9 +459,9 @@ add_task(async function test_private_car
     is(tempCard["cc-given-name"], "John", "cc-given-name was computed");
     is(tempCard["cc-family-name"], "Doe", "cc-family-name was computed");
     ok(tempCard["cc-exp"], "cc-exp was computed");
     ok(tempCard["cc-number-encrypted"], "cc-number-encrypted was computed");
   }, args, {
     browser: privateWin.gBrowser,
   });
   await BrowserTestUtils.closeWindow(privateWin);
-}).skip();
+});