From 0f7e6a5e25cf18b52c69caf5d2bb1ae99fadc068 Mon Sep 17 00:00:00 2001 From: Tim Giles Date: Mon, 2 May 2022 18:18:39 +0000 Subject: [PATCH] Bug 1764709 - Always normalize credit card expiration month and year when creating a new record. r=geckoview-reviewers,dimi,sgalich,agi Differential Revision: https://phabricator.services.mozilla.com/D144055 --- .../test/browser/creditCard/browser.ini | 2 + ...rowser_creditCard_submission_normalized.js | 113 ++++++++++++++++++ .../test/unit/test_createRecords.js | 60 ++++++++-- .../geckoview/test/AutocompleteTest.kt | 20 ++-- .../formautofill/FormAutofillHandler.jsm | 24 ++-- 5 files changed, 189 insertions(+), 30 deletions(-) create mode 100644 browser/extensions/formautofill/test/browser/creditCard/browser_creditCard_submission_normalized.js diff --git a/browser/extensions/formautofill/test/browser/creditCard/browser.ini b/browser/extensions/formautofill/test/browser/creditCard/browser.ini index 91ff50a3f048..0df3dd03dd5f 100644 --- a/browser/extensions/formautofill/test/browser/creditCard/browser.ini +++ b/browser/extensions/formautofill/test/browser/creditCard/browser.ini @@ -18,6 +18,8 @@ skip-if = (!debug && os == "mac") || (os == "win" && ccov) # perma-fail see Bug skip-if = ((os == "mac") || (os == 'linux') || (os == 'win')) [browser_creditCard_fill_cancel_login.js] skip-if = ((!debug && os == "mac") || (os == 'linux') || (os == 'win')) +[browser_creditCard_submission_normalized.js] +skip-if = apple_silicon && !debug [browser_editCreditCardDialog.js] skip-if = ((os == 'linux') || (os == "mac") || (os == 'win')) # perma-fail see Bug 1600059 [browser_insecure_form.js] diff --git a/browser/extensions/formautofill/test/browser/creditCard/browser_creditCard_submission_normalized.js b/browser/extensions/formautofill/test/browser/creditCard/browser_creditCard_submission_normalized.js new file mode 100644 index 000000000000..b63f471c541a --- /dev/null +++ b/browser/extensions/formautofill/test/browser/creditCard/browser_creditCard_submission_normalized.js @@ -0,0 +1,113 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// We want to ensure that non-normalized credit card data is normalized +// correctly as part of the save credit card flow +add_task(async function test_new_submitted_card_is_normalized() { + await SpecialPowers.pushPrefEnv({ + set: [[CREDITCARDS_USED_STATUS_PREF, 0]], + }); + const testCard = { + "cc-name": "Test User", + "cc-number": "5038146897157463", + "cc-exp-month": "4", + "cc-exp-year": "25", + }; + const expectedData = { + "cc-name": "Test User", + "cc-number": "5038146897157463", + "cc-exp-month": "4", + // cc-exp-year should be normalized to 2025 + "cc-exp-year": "2025", + }; + let onChanged = waitForStorageChangedEvents("add"); + + await BrowserTestUtils.withNewTab( + { gBrowser, url: CREDITCARD_FORM_URL }, + async function(browser) { + let promiseShown = promiseNotificationShown(); + await focusUpdateSubmitForm(browser, { + focusSelector: "#cc-name", + newValues: { + "#cc-name": testCard["cc-name"], + "#cc-number": testCard["cc-number"], + "#cc-exp-month": testCard["cc-exp-month"], + "#cc-exp-year": testCard["cc-exp-year"], + }, + }); + + await promiseShown; + await clickDoorhangerButton(MAIN_BUTTON); + } + ); + + await onChanged; + + let creditCards = await getCreditCards(); + let savedCreditCard = creditCards[0]; + let decryptedNumber = await OSKeyStore.decrypt( + savedCreditCard["cc-number-encrypted"] + ); + savedCreditCard["cc-number"] = decryptedNumber; + for (let key in testCard) { + let expected = expectedData[key]; + let actual = savedCreditCard[key]; + Assert.equal(expected, actual, `${key} should match`); + } +}); + +add_task(async function test_updated_card_is_normalized() { + await SpecialPowers.pushPrefEnv({ + set: [[CREDITCARDS_USED_STATUS_PREF, 0]], + }); + const testCard = { + "cc-name": "Test User", + "cc-number": "5038146897157463", + "cc-exp-month": "11", + "cc-exp-year": "20", + }; + await saveCreditCard(testCard); + const expectedData = { + "cc-name": "Test User", + "cc-number": "5038146897157463", + "cc-exp-month": "10", + // cc-exp-year should be normalized to 2027 + "cc-exp-year": "2027", + }; + let onChanged = waitForStorageChangedEvents("update"); + + await BrowserTestUtils.withNewTab( + { gBrowser, url: CREDITCARD_FORM_URL }, + async function(browser) { + let promiseShown = promiseNotificationShown(); + await focusUpdateSubmitForm(browser, { + focusSelector: "#cc-name", + newValues: { + "#cc-name": testCard["cc-name"], + "#cc-number": testCard["cc-number"], + "#cc-exp-month": "10", + "#cc-exp-year": "27", + }, + }); + + await promiseShown; + await clickDoorhangerButton(MAIN_BUTTON); + } + ); + + await onChanged; + + let creditCards = await getCreditCards(); + let savedCreditCard = creditCards[0]; + savedCreditCard["cc-number"] = await OSKeyStore.decrypt( + savedCreditCard["cc-number-encrypted"] + ); + + for (let key in testCard) { + let expected = expectedData[key]; + let actual = savedCreditCard[key]; + Assert.equal(expected, actual, `${key} should match`); + } +}); diff --git a/browser/extensions/formautofill/test/unit/test_createRecords.js b/browser/extensions/formautofill/test/unit/test_createRecords.js index 8fe0fddd7f15..7ed814cc1f44 100644 --- a/browser/extensions/formautofill/test/unit/test_createRecords.js +++ b/browser/extensions/formautofill/test/unit/test_createRecords.js @@ -65,8 +65,7 @@ const TESTCASES = [ }, }, { - description: - '"country" using @autocomplete shouldn\'t be identified aggressively', + description: `"country" using @autocomplete shouldn't be identified aggressively`, document: `
@@ -84,7 +83,7 @@ const TESTCASES = [ }, }, { - description: '"country" using heuristics should be identified aggressively', + description: `"country" using heuristics should be identified aggressively`, document: ` @@ -107,7 +106,7 @@ const TESTCASES = [ }, }, { - description: '"tel" related fields should be concatenated', + description: `"tel" related fields should be concatenated`, document: ` @@ -132,7 +131,7 @@ const TESTCASES = [ }, }, { - description: '"tel" should be removed if it\'s too short', + description: `"tel" should be removed if it's too short`, document: ` @@ -158,7 +157,7 @@ const TESTCASES = [ }, }, { - description: '"tel" should be removed if it\'s too long', + description: `"tel" should be removed if it's too long`, document: ` @@ -184,7 +183,7 @@ const TESTCASES = [ }, }, { - description: '"tel" should be removed if it contains invalid characters', + description: `"tel" should be removed if it contains invalid characters`, document: ` @@ -446,6 +445,53 @@ const TESTCASES = [ ], }, }, + { + description: + "A credit card form with separate expiry fields should have normalized expiry data.", + document: ` + + + +
`, + formValue: { + "cc-number": "5105105105105100", + "cc-exp-month": "05", + "cc-exp-year": "26", + }, + expectedRecord: { + address: [], + creditCard: [ + { + "cc-number": "5105105105105100", + "cc-exp-month": "5", + "cc-exp-year": "2026", + }, + ], + }, + }, + { + description: + "A credit card form with combined expiry fields should have normalized expiry data.", + document: `
+ + +
`, + formValue: { + "cc-number": "5105105105105100", + "cc-exp": "07/27", + }, + expectedRecord: { + address: [], + creditCard: [ + { + "cc-number": "5105105105105100", + "cc-exp": "07/27", + "cc-exp-month": "7", + "cc-exp-year": "2027", + }, + ], + }, + }, ]; for (let testcase of TESTCASES) { diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AutocompleteTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AutocompleteTest.kt index ffb6d16312b5..556169de467d 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AutocompleteTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AutocompleteTest.kt @@ -697,8 +697,8 @@ class AutocompleteTest : BaseSessionTest() { fun creditCardSaveAccept() { val ccName = "MyCard" val ccNumber = "5105105105105100" - val ccExpMonth = "06" - val ccExpYear = "24" + val ccExpMonth = "6" + val ccExpYear = "2024" mainSession.loadTestPath(CC_FORM_HTML_PATH) mainSession.waitForPageStop() @@ -843,8 +843,8 @@ class AutocompleteTest : BaseSessionTest() { fun creditCardSaveDismiss() { val ccName = "MyCard" val ccNumber = "5105105105105100" - val ccExpMonth = "06" - val ccExpYear = "24" + val ccExpMonth = "6" + val ccExpYear = "2024" mainSession.loadTestPath(CC_FORM_HTML_PATH) mainSession.waitForPageStop() @@ -913,9 +913,9 @@ class AutocompleteTest : BaseSessionTest() { fun creditCardSaveModifyAccept() { val ccName = "MyCard" val ccNumber = "5105105105105100" - val ccExpMonth = "06" - val ccExpYearNew = "26" - val ccExpYear = "24" + val ccExpMonth = "6" + val ccExpYearNew = "2026" + val ccExpYear = "2024" mainSession.loadTestPath(CC_FORM_HTML_PATH) mainSession.waitForPageStop() @@ -994,11 +994,11 @@ class AutocompleteTest : BaseSessionTest() { fun creditCardUpdateAccept() { val ccName = "MyCard" val ccNumber1 = "5105105105105100" - val ccExpMonth1 = "06" - val ccExpYear1 = "24" + val ccExpMonth1 = "6" + val ccExpYear1 = "2024" val ccNumber2 = "4111111111111111" val ccExpMonth2 = "11" - val ccExpYear2 = "21" + val ccExpYear2 = "2021" val savedCreditCards = mutableListOf() mainSession.loadTestPath(CC_FORM_HTML_PATH) diff --git a/toolkit/components/formautofill/FormAutofillHandler.jsm b/toolkit/components/formautofill/FormAutofillHandler.jsm index 9b39b1b70408..29285678d0ce 100644 --- a/toolkit/components/formautofill/FormAutofillHandler.jsm +++ b/toolkit/components/formautofill/FormAutofillHandler.jsm @@ -1331,19 +1331,17 @@ class FormAutofillCreditCardSection extends FormAutofillSection { creditCard.record["cc-number"] ); - // Normalize cc-exp-month and cc-exp-year using the cc-exp field - if (creditCard.record["cc-exp"]) { - let { month, year } = CreditCard.normalizeExpiration({ - expirationString: creditCard.record["cc-exp"], - expirationMonth: creditCard.record["cc-exp-month"], - expirationYear: creditCard.record["cc-exp-year"], - }); - if (month) { - creditCard.record["cc-exp-month"] = month; - } - if (year) { - creditCard.record["cc-exp-year"] = year; - } + // Normalize cc-exp-month and cc-exp-year + let { month, year } = CreditCard.normalizeExpiration({ + expirationString: creditCard.record["cc-exp"], + expirationMonth: creditCard.record["cc-exp-month"], + expirationYear: creditCard.record["cc-exp-year"], + }); + if (month) { + creditCard.record["cc-exp-month"] = month; + } + if (year) { + creditCard.record["cc-exp-year"] = year; } } }