From 497abd940b0b47a75ce39cc3d8e8d6b318d98f3a Mon Sep 17 00:00:00 2001 From: prathiksha Date: Fri, 12 Apr 2019 21:54:06 +0000 Subject: [PATCH] Bug 1185000 - Password manager should not offer to save credit card numbers. r=jaws Password manager should not offer to save credit card numbers in certain straight-forward cases. Differential Revision: https://phabricator.services.mozilla.com/D25485 --HG-- extra : moz-landing-system : lando --- .../components/LoginManagerPrompter.js | 19 +-- .../passwordmgr/LoginManagerContent.jsm | 13 ++ .../passwordmgr/LoginManagerParent.jsm | 12 +- .../passwordmgr/LoginManagerPrompter.jsm | 18 ++- .../passwordmgr/nsILoginManagerPrompter.idl | 11 +- .../passwordmgr/test/browser/browser.ini | 1 + ...owser_doorhanger_dismissed_for_ccnumber.js | 115 ++++++++++++++++++ 7 files changed, 167 insertions(+), 22 deletions(-) create mode 100644 toolkit/components/passwordmgr/test/browser/browser_doorhanger_dismissed_for_ccnumber.js diff --git a/mobile/android/components/LoginManagerPrompter.js b/mobile/android/components/LoginManagerPrompter.js index 5df48d895a21..e2c4078ca649 100644 --- a/mobile/android/components/LoginManagerPrompter.js +++ b/mobile/android/components/LoginManagerPrompter.js @@ -107,8 +107,8 @@ LoginManagerPrompter.prototype = { * promptToSavePassword * */ - promptToSavePassword: function(aLogin) { - this._showSaveLoginNotification(aLogin); + promptToSavePassword: function(aLogin, dismissed) { + this._showSaveLoginNotification(aLogin, dismissed); Services.telemetry.getHistogramById("PWMGR_PROMPT_REMEMBER_ACTION").add(PROMPT_DISPLAYED); Services.obs.notifyObservers(aLogin, "passwordmgr-prompt-save"); }, @@ -125,8 +125,10 @@ LoginManagerPrompter.prototype = { * Username string used in creating a doorhanger action * @param aPassword * Password string used in creating a doorhanger action + * @param dismissed + * A boolean indicating if a prompt is dismissed by default. */ - _showLoginNotification: function(aBody, aButtons, aUsername, aPassword) { + _showLoginNotification: function(aBody, aButtons, aUsername, aPassword, dismissed = false) { let actionText = { text: aUsername, type: "EDIT", @@ -145,6 +147,7 @@ LoginManagerPrompter.prototype = { persistWhileVisible: true, timeout: Date.now() + 10000, actionText: actionText, + dismissed, }; let win = (this._browser && this._browser.contentWindow) || this._window; @@ -159,7 +162,7 @@ LoginManagerPrompter.prototype = { * their login, and only save a login which they know worked. * */ - _showSaveLoginNotification: function(aLogin) { + _showSaveLoginNotification: function(aLogin, dismissed) { let brandShortName = this._strBundle.brand.GetStringFromName("brandShortName"); let notificationText = this._getLocalizedString("saveLogin", [brandShortName]); @@ -191,7 +194,7 @@ LoginManagerPrompter.prototype = { }, ]; - this._showLoginNotification(notificationText, buttons, aLogin.username, aLogin.password); + this._showLoginNotification(notificationText, buttons, aLogin.username, aLogin.password, dismissed); }, /* @@ -202,7 +205,7 @@ LoginManagerPrompter.prototype = { * fields. * */ - promptToChangePassword: function(aOldLogin, aNewLogin) { + promptToChangePassword: function(aOldLogin, aNewLogin, dismissed) { this._showChangeLoginNotification(aOldLogin, aNewLogin.password); Services.telemetry.getHistogramById("PWMGR_PROMPT_UPDATE_ACTION").add(PROMPT_DISPLAYED); let oldGUID = aOldLogin.QueryInterface(Ci.nsILoginMetaInfo).guid; @@ -215,7 +218,7 @@ LoginManagerPrompter.prototype = { * Shows the Change Password notification doorhanger. * */ - _showChangeLoginNotification: function(aOldLogin, aNewPassword) { + _showChangeLoginNotification: function(aOldLogin, aNewPassword, dismissed) { var notificationText; if (aOldLogin.username) { let displayUser = this._sanitizeUsername(aOldLogin.username); @@ -247,7 +250,7 @@ LoginManagerPrompter.prototype = { }, ]; - this._showLoginNotification(notificationText, buttons, aOldLogin.username, aNewPassword); + this._showLoginNotification(notificationText, buttons, aOldLogin.username, aNewPassword, dismissed); }, /* diff --git a/toolkit/components/passwordmgr/LoginManagerContent.jsm b/toolkit/components/passwordmgr/LoginManagerContent.jsm index e61472b9a874..a420ef387f04 100644 --- a/toolkit/components/passwordmgr/LoginManagerContent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm @@ -21,6 +21,7 @@ const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm") const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm"); const {PrivateBrowsingUtils} = ChromeUtils.import("resource://gre/modules/PrivateBrowsingUtils.jsm"); const {PromiseUtils} = ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm"); +const {CreditCard} = ChromeUtils.import("resource://gre/modules/CreditCard.jsm"); ChromeUtils.defineModuleGetter(this, "DeferredTask", "resource://gre/modules/DeferredTask.jsm"); ChromeUtils.defineModuleGetter(this, "FormLikeFactory", @@ -1139,6 +1140,17 @@ var LoginManagerContent = { openerTopWindowID = win.opener.top.windowUtils.outerWindowID; } + // Dismiss prompt if the username field is a credit card number AND + // if the password field is a three digit number. Also dismiss prompt if + // the password is a credit card number and the password field has attribute + // autocomplete="cc-number". + let dismissedPrompt = false; + let newPasswordFieldValue = newPasswordField.value; + if ((CreditCard.isValidNumber(usernameValue) && newPasswordFieldValue.trim().match(/^[0-9]{3}$/)) || + (CreditCard.isValidNumber(newPasswordFieldValue) && newPasswordField.getAutocompleteInfo().fieldName == "cc-number")) { + dismissedPrompt = true; + } + let autoFilledLogin = this.stateForDocument(doc).fillsByRootElement.get(form.rootElement); messageManager.sendAsyncMessage("PasswordManager:onFormSubmit", { hostname, @@ -1148,6 +1160,7 @@ var LoginManagerContent = { newPasswordField: mockPassword, oldPasswordField: mockOldPassword, openerTopWindowID, + dismissedPrompt, }); }, diff --git a/toolkit/components/passwordmgr/LoginManagerParent.jsm b/toolkit/components/passwordmgr/LoginManagerParent.jsm index d915b477346a..4d371757f42e 100644 --- a/toolkit/components/passwordmgr/LoginManagerParent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm @@ -100,6 +100,7 @@ var LoginManagerParent = { newPasswordField: data.newPasswordField, oldPasswordField: data.oldPasswordField, openerTopWindowID: data.openerTopWindowID, + dismissedPrompt: data.dismissedPrompt, target: msg.target}); break; } @@ -303,7 +304,7 @@ var LoginManagerParent = { onFormSubmit({hostname, formSubmitURL, autoFilledLoginGuid, usernameField, newPasswordField, oldPasswordField, openerTopWindowID, - target}) { + dismissedPrompt, target}) { function getPrompter() { var prompterSvc = Cc["@mozilla.org/login-manager/prompter;1"]. createInstance(Ci.nsILoginManagerPrompter); @@ -391,7 +392,7 @@ var LoginManagerParent = { formLogin.username = oldLogin.username; formLogin.usernameField = oldLogin.usernameField; - prompter.promptToChangePassword(oldLogin, formLogin); + prompter.promptToChangePassword(oldLogin, formLogin, dismissedPrompt); } else { // Note: It's possible that that we already have the correct u+p saved // but since we don't have the username, we don't know if the user is @@ -449,11 +450,11 @@ var LoginManagerParent = { if (existingLogin.password != formLogin.password) { log("...passwords differ, prompting to change."); prompter = getPrompter(); - prompter.promptToChangePassword(existingLogin, formLogin); + prompter.promptToChangePassword(existingLogin, formLogin, dismissedPrompt); } else if (!existingLogin.username && formLogin.username) { log("...empty username update, prompting to change."); prompter = getPrompter(); - prompter.promptToChangePassword(existingLogin, formLogin); + prompter.promptToChangePassword(existingLogin, formLogin, dismissedPrompt); } else { recordLoginUse(existingLogin); } @@ -461,10 +462,9 @@ var LoginManagerParent = { return; } - // Prompt user to save login (via dialog or notification bar) prompter = getPrompter(); - prompter.promptToSavePassword(formLogin); + prompter.promptToSavePassword(formLogin, dismissedPrompt); }, /** diff --git a/toolkit/components/passwordmgr/LoginManagerPrompter.jsm b/toolkit/components/passwordmgr/LoginManagerPrompter.jsm index bef8b965da16..6f87080d6ced 100644 --- a/toolkit/components/passwordmgr/LoginManagerPrompter.jsm +++ b/toolkit/components/passwordmgr/LoginManagerPrompter.jsm @@ -812,12 +812,12 @@ LoginManagerPrompter.prototype = { this._openerBrowser = aOpenerBrowser; }, - promptToSavePassword(aLogin) { + promptToSavePassword(aLogin, dismissed) { this.log("promptToSavePassword"); var notifyObj = this._getPopupNote(); if (notifyObj) { this._showLoginCaptureDoorhanger(aLogin, "password-save", { - dismissed: this._inPrivateBrowsing, + dismissed: this._inPrivateBrowsing || dismissed, }); Services.obs.notifyObservers(aLogin, "passwordmgr-prompt-save"); } else { @@ -1176,14 +1176,17 @@ LoginManagerPrompter.prototype = { * The old login we may want to update. * @param {nsILoginInfo} aNewLogin * The new login from the page form. + * @param dismissed + * A boolean indicating if the prompt should be automatically + * dismissed on being shown. */ - promptToChangePassword(aOldLogin, aNewLogin) { + promptToChangePassword(aOldLogin, aNewLogin, dismissed) { this.log("promptToChangePassword"); let notifyObj = this._getPopupNote(); if (notifyObj) { this._showChangeLoginNotification(notifyObj, aOldLogin, - aNewLogin); + aNewLogin, dismissed); } else { this._showChangeLoginDialog(aOldLogin, aNewLogin); } @@ -1200,13 +1203,16 @@ LoginManagerPrompter.prototype = { * * @param aNewLogin * The login object with the changes we want to make. + * @param dismissed + * A boolean indicating if the prompt should be automatically + * dismissed on being shown. */ - _showChangeLoginNotification(aNotifyObj, aOldLogin, aNewLogin) { + _showChangeLoginNotification(aNotifyObj, aOldLogin, aNewLogin, dismissed = false) { aOldLogin.hostname = aNewLogin.hostname; aOldLogin.formSubmitURL = aNewLogin.formSubmitURL; aOldLogin.password = aNewLogin.password; aOldLogin.username = aNewLogin.username; - this._showLoginCaptureDoorhanger(aOldLogin, "password-change"); + this._showLoginCaptureDoorhanger(aOldLogin, "password-change", {dismissed}); let oldGUID = aOldLogin.QueryInterface(Ci.nsILoginMetaInfo).guid; Services.obs.notifyObservers(aNewLogin, "passwordmgr-prompt-change", oldGUID); diff --git a/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl b/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl index 7edb2b28b19e..ca1d32b63e06 100644 --- a/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl +++ b/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl @@ -49,8 +49,11 @@ interface nsILoginManagerPrompter : nsISupports { * * @param aLogin * The login to be saved. + * @param dismissed + * A boolean value indicating whether the save logins doorhanger should + * be dismissed automatically when shown. */ - void promptToSavePassword(in nsILoginInfo aLogin); + void promptToSavePassword(in nsILoginInfo aLogin, in boolean dismissed); /** * Ask the user if they want to change a login's password or username. @@ -60,9 +63,13 @@ interface nsILoginManagerPrompter : nsISupports { * The existing login (with the old password). * @param aNewLogin * The new login. + * @param dismissed + * A boolean value indicating whether the save logins doorhanger should + * be dismissed automatically when shown. */ void promptToChangePassword(in nsILoginInfo aOldLogin, - in nsILoginInfo aNewLogin); + in nsILoginInfo aNewLogin, + in boolean dismissed); /** * Ask the user if they want to change the password for one of diff --git a/toolkit/components/passwordmgr/test/browser/browser.ini b/toolkit/components/passwordmgr/test/browser/browser.ini index 53dc36cfa95e..fd0d0ca4e334 100644 --- a/toolkit/components/passwordmgr/test/browser/browser.ini +++ b/toolkit/components/passwordmgr/test/browser/browser.ini @@ -49,6 +49,7 @@ skip-if = verify [browser_username_select_dialog.js] support-files = subtst_notifications_change_p.html +[browser_doorhanger_dismissed_for_ccnumber.js] [browser_DOMFormHasPassword.js] [browser_DOMInputPasswordAdded.js] skip-if = (os == "linux") || (os == "mac") # Bug 1337606 diff --git a/toolkit/components/passwordmgr/test/browser/browser_doorhanger_dismissed_for_ccnumber.js b/toolkit/components/passwordmgr/test/browser/browser_doorhanger_dismissed_for_ccnumber.js new file mode 100644 index 000000000000..edd82d615b1a --- /dev/null +++ b/toolkit/components/passwordmgr/test/browser/browser_doorhanger_dismissed_for_ccnumber.js @@ -0,0 +1,115 @@ +"use strict"; + +const TEST_HOSTNAME = "https://example.com"; +const BASIC_FORM_PAGE_PATH = DIRECTORY_PATH + "form_basic.html"; + +function getSubmitMessage() { + info("getSubmitMessage"); + return new Promise((resolve, reject) => { + Services.mm.addMessageListener("PasswordManager:onFormSubmit", function onFormSubmit() { + Services.mm.removeMessageListener("PasswordManager:onFormSubmit", onFormSubmit); + resolve(); + }); + }); +} + +add_task(async function test_doorhanger_dismissal_un() { + let url = TEST_HOSTNAME + BASIC_FORM_PAGE_PATH; + await BrowserTestUtils.withNewTab({ + gBrowser, + url, + }, async function test_un_value_as_ccnumber(browser) { + // If the username field has a credit card number and if + // the password field is a three digit numberic value, + // we automatically dismiss the save logins prompt on submission. + + let processedPromise = getSubmitMessage(); + ContentTask.spawn(browser, null, async () => { + content.document.getElementById("form-basic-username").setUserInput("4111111111111111"); + content.document.getElementById("form-basic-password").setUserInput("123"); + + content.document.getElementById("form-basic-submit").click(); + }); + await processedPromise; + + let notif = getCaptureDoorhanger("password-save"); + ok(notif, "got notification popup"); + ok(notif.dismissed, "notification popup was automatically dismissed"); + }); +}); + +add_task(async function test_doorhanger_dismissal_pw() { + let url = TEST_HOSTNAME + BASIC_FORM_PAGE_PATH; + await BrowserTestUtils.withNewTab({ + gBrowser, + url, + }, async function test_pw_value_as_ccnumber(browser) { + // If the password field has a credit card number and if + // the password field is also tagged autocomplete="cc-number", + // we automatically dismiss the save logins prompt on submission. + + let processedPromise = getSubmitMessage(); + ContentTask.spawn(browser, null, async () => { + content.document.getElementById("form-basic-username").setUserInput("aaa"); + content.document.getElementById("form-basic-password").setUserInput("4111111111111111"); + content.document.getElementById("form-basic-password").setAttribute("autocomplete", "cc-number"); + + content.document.getElementById("form-basic-submit").click(); + }); + await processedPromise; + + let notif = getCaptureDoorhanger("password-save"); + ok(notif, "got notification popup"); + ok(notif.dismissed, "notification popup was automatically dismissed"); + }); +}); + +add_task(async function test_doorhanger_shown_on_un_with_invalid_ccnumber() { + let url = TEST_HOSTNAME + BASIC_FORM_PAGE_PATH; + await BrowserTestUtils.withNewTab({ + gBrowser, + url, + }, async function test_un_with_invalid_cc_number(browser) { + // If the username field has a CC number that is invalid, + // we show the doorhanger to save logins like we usually do. + + let processedPromise = getSubmitMessage(); + ContentTask.spawn(browser, null, async () => { + content.document.getElementById("form-basic-username").value = "1234123412341234"; + content.document.getElementById("form-basic-password").value = "411"; + + content.document.getElementById("form-basic-submit").click(); + }); + await processedPromise; + + let notif = getCaptureDoorhanger("password-save"); + ok(notif, "got notification popup"); + ok(!notif.dismissed, "notification popup was not automatically dismissed"); + }); +}); + +add_task(async function test_doorhanger_dismissal_on_change() { + let url = TEST_HOSTNAME + BASIC_FORM_PAGE_PATH; + await BrowserTestUtils.withNewTab({ + gBrowser, + url, + }, async function test_change_in_pw(browser) { + let nsLoginInfo = new Components.Constructor("@mozilla.org/login-manager/loginInfo;1", + Ci.nsILoginInfo, "init"); + let login = new nsLoginInfo("https://example.org", "https://example.org", null, + "4111111111111111", "111", "form-basic-username", "form-basic-password"); + Services.logins.addLogin(login); + + let processedPromise = getSubmitMessage(); + ContentTask.spawn(browser, null, async () => { + content.document.getElementById("form-basic-password").setUserInput("111"); + content.document.getElementById("form-basic-submit").click(); + }); + await processedPromise; + + let notif = getCaptureDoorhanger("password-save"); + ok(notif, "got notification popup"); + ok(notif.dismissed, "notification popup was automatically dismissed"); + }); +}); +