Bug 1468644 - cc-numbers are always masked in dialog content; tests to verify this and card details in the payment response. r=jaws
authorSam Foster <sfoster@mozilla.com>
Wed, 18 Jul 2018 12:45:45 -0700
changeset 427496 c552e6f6cc3ab580bfadf61fe78711beea0e6b6d
parent 427495 aefd790bff28457d7b2e99025aae60e2caec7ff1
child 427497 285ade739cd21e156a9236c3e4bd242b5837604c
push id34306
push usercsabou@mozilla.com
push dateFri, 20 Jul 2018 21:41:18 +0000
treeherdermozilla-central@d6a5e8aea651 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1468644
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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();
+});