Bug 1388674 - Only prompt to save logins if a login field was modified by the user. r=MattN

Depends on D51718

Differential Revision: https://phabricator.services.mozilla.com/D24556

--HG--
extra : moz-landing-system : lando

modules/libpref/init/all.js

@@ -3895,6 +3895,7 @@ pref("signon.generation.available",         false);
 pref("signon.generation.enabled",           false);
 pref("signon.privateBrowsingCapture.enabled", false);
 pref("signon.storeWhenAutocompleteOff",     true);
+pref("signon.userInputRequiredToCapture.enabled", true);
 pref("signon.debug",                        false);
 pref("signon.recipes.path",                 "chrome://passwordmgr/content/recipes.json");
 pref("signon.schemeUpgrades",               false);

toolkit/components/passwordmgr/LoginHelper.jsm

@@ -82,6 +82,9 @@ this.LoginHelper = {
     this.storeWhenAutocompleteOff = Services.prefs.getBoolPref(
       "signon.storeWhenAutocompleteOff"
     );
+    this.userInputRequiredToCapture = Services.prefs.getBoolPref(
+      "signon.userInputRequiredToCapture.enabled"
+    );
   },
 
   createLogger(aLogPrefix) {

toolkit/components/passwordmgr/LoginManagerChild.jsm

@@ -247,11 +247,20 @@ const observer = {
 
       // Used to watch for changes to fields filled with generated passwords.
       case "input": {
-        if (docState.generatedPasswordFields.has(aEvent.target)) {
+        let field = aEvent.target;
+        if (docState.generatedPasswordFields.has(field)) {
           LoginManagerChild.forWindow(
             window
           )._maybeStopTreatingAsGeneratedPasswordField(aEvent);
         }
+        if (
+          field.hasBeenTypePassword ||
+          LoginHelper.isUsernameFieldType(field)
+        ) {
+          // flag this form as user-modified
+          let formLikeRoot = FormLikeFactory.findRootForField(field);
+          docState.fieldModificationsByRootElement.set(formLikeRoot, true);
+        }
         break;
       }
 
@@ -870,6 +879,12 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
       return;
     }
 
+    // set up input event listeners so we know if the user has interacted with these fields
+    form.rootElement.addEventListener("input", observer, {
+      capture: true,
+      mozSystemGroup: true,
+    });
+
     this._getLoginDataFromParent(form, { showMasterPassword: true })
       .then(this.loginsFound.bind(this))
       .catch(Cu.reportError);
@@ -895,6 +910,7 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
          * Keeps track of logins that were last submitted.
          */
         lastSubmittedValuesByRootElement: new WeakMap(),
+        fieldModificationsByRootElement: new WeakMap(),
         loginFormRootElements: new WeakSet(),
       };
       this._loginFormStateByDocument.set(document, loginFormState);
@@ -1557,9 +1573,17 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
       return;
     }
 
-    let autoFilledLogin = this.stateForDocument(doc).fillsByRootElement.get(
-      form.rootElement
-    );
+    let docState = this.stateForDocument(doc);
+    let fieldsModified = this._formHasModifiedFields(formLikeRoot);
+    if (!fieldsModified && LoginHelper.userInputRequiredToCapture) {
+      // we know no fields in this form had user modifications, so don't prompt
+      log(
+        "(form submission ignored -- submitting values that are not changed by the user)"
+      );
+      return;
+    }
+
+    let autoFilledLogin = docState.fillsByRootElement.get(form.rootElement);
 
     let detail = {
       origin,
@@ -1816,7 +1840,6 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
     }
 
     log("_fillForm", form.elements);
-    let usernameField;
     // Will be set to one of AUTOFILL_RESULT in the `try` block.
     let autofillResult = -1;
     const AUTOFILL_RESULT = {
@@ -1834,6 +1857,16 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
       PASSWORD_AUTOCOMPLETE_NEW_PASSWORD: 11,
     };
 
+    // Heuristically determine what the user/pass fields are
+    // We do this before checking to see if logins are stored,
+    // so that the user isn't prompted for a master password
+    // without need.
+    let [usernameField, passwordField] = this._getFormFields(
+      form,
+      false,
+      recipes
+    );
+
     try {
       // Nothing to do if we have no matching (excluding form action
       // checks) logins available, and there isn't a need to show
@@ -1848,17 +1881,6 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
         return;
       }
 
-      // Heuristically determine what the user/pass fields are
-      // We do this before checking to see if logins are stored,
-      // so that the user isn't prompted for a master password
-      // without need.
-      let passwordField;
-      [usernameField, passwordField] = this._getFormFields(
-        form,
-        false,
-        recipes
-      );
-
       // If we have a password inputElement parameter and it's not
       // the same as the one heuristically found, use the parameter
       // one instead.
@@ -2151,6 +2173,14 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
     }
   }
 
+  _formHasModifiedFields(formLikeRoot) {
+    let state = this.stateForDocument(formLikeRoot.ownerDocument);
+    let fieldsModified = state.fieldModificationsByRootElement.get(
+      formLikeRoot
+    );
+    return fieldsModified;
+  }
+
   /**
    * Given a field, determine whether that field was last filled as a username
    * field AND whether the username is still filled in with the username AND

toolkit/components/passwordmgr/test/mochitest/mochitest.ini

@@ -160,6 +160,7 @@ skip-if = os == "linux" || toolkit == 'android' # Tests desktop prompts
 [test_prompt_promptAuth_proxy.html]
 skip-if = e10s || os == "linux" || toolkit == 'android' # Tests desktop prompts
 [test_recipe_login_fields.html]
+[test_submit_without_field_modifications.html]
 [test_username_focus.html]
 skip-if = toolkit == 'android' # android:autocomplete.
 [test_xhr.html]

toolkit/components/passwordmgr/test/mochitest/test_submit_without_field_modifications.html

@@ -0,0 +1,227 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Don't send onFormSubmit message on navigation if the user did not interact
+    with the login fields</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="text/javascript" src="pwmgr_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<p id="display"></p>
+
+<div id="content">
+  <iframe id="loginFrame">
+  </iframe>
+</div>
+
+<pre id="test"></pre>
+<script>
+const { TestUtils } = SpecialPowers.Cu.import("resource://testing-common/TestUtils.jsm");
+SimpleTest.requestFlakyTimeout("Giving a chance for the unexpected popup to show");
+let iframe = document.getElementById("loginFrame");
+
+function waitForLoad() {
+  return new Promise(resolve => {
+    function handleLoad() {
+      iframe.removeEventListener("load", handleLoad);
+      resolve();
+    }
+    iframe.addEventListener("load", handleLoad);
+  });
+}
+
+ async function setup(pageUrl) {
+  let loadPromise = waitForLoad();
+  let processedFormPromise = promiseFormsProcessed();
+  iframe.src = pageUrl || "https://example.org/tests/toolkit/components/passwordmgr/test/mochitest/formless_basic.html";
+
+  await processedFormPromise;
+  info("initial form processed");
+  await loadPromise;
+  await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
+    let doc = this.content.document;
+    let link = doc.createElement("a");
+    link.setAttribute("href", "http://mochi.test:8888");
+    doc.body.appendChild(link);
+  });
+}
+
+async function setFieldValues() {
+  await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
+    // assign to .value directly, we deliberately don't emulate user input
+    let doc = this.content.document;
+    doc.getElementById("form-basic-username").value = "user";
+    doc.getElementById("form-basic-password").value = "pass";
+  });
+}
+
+async function clickLink() {
+  let loadPromise = waitForLoad();
+  await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
+    let doc = this.content.document;
+    doc.querySelector("a[href]").click();
+  });
+  await loadPromise;
+}
+
+async function userInput(selector, value) {
+  await SpecialPowers.spawn(getIframeBrowsingContext(window), [selector, value], function(sel, val) {
+    this.content.document.querySelector(sel).setUserInput(val);
+  });
+}
+
+add_task(async function test_init() {
+  await SpecialPowers.pushPrefEnv({"set": [
+    ["signon.userInputRequiredToCapture.enabled", true],
+  ]});
+});
+
+add_task(async function test_no_message_on_navigation() {
+  // If login field values were set by the website, we don't message to save the
+  // login values if the user did not interact with the fields before submiting.
+  await setup();
+  await setFieldValues();
+
+  let submitMessageSent = false;
+  getSubmitMessage().then(value => {
+    submitMessageSent = true;
+  });
+  await clickLink();
+
+  // allow time to pass before concluding no onFormSubmit message was sent
+  await new Promise(res => setTimeout(res, 1000));
+  ok(!submitMessageSent, "onFormSubmit message is not sent on navigation since the login fields were not modified");
+});
+
+add_task(async function test_prefd_off_message_on_navigation() {
+  // Confirm the pref controls capture behavior with non-user-set field values.
+  await SpecialPowers.pushPrefEnv({"set": [
+    ["signon.userInputRequiredToCapture.enabled", false],
+  ]});
+  await setup();
+  await setFieldValues();
+
+  let promiseSubmitMessage = getSubmitMessage();
+  await clickLink();
+  await promiseSubmitMessage;
+  info("onFormSubmit message was sent as expected after navigation");
+
+  SpecialPowers.popPrefEnv();
+});
+
+add_task(async function test_message_with_user_interaction_on_navigation() {
+  await setup();
+  await setFieldValues();
+  await userInput("#form-basic-username", "foo");
+
+  let promiseSubmitMessage = getSubmitMessage();
+  await clickLink();
+  await promiseSubmitMessage;
+  info("onFormSubmit message was sent as expected after user interaction");
+});
+
+add_task(async function test_empty_form_with_input_handler() {
+  await setup();
+  await userInput("#form-basic-username", "user");
+  await userInput("#form-basic-password", "pass");
+
+  let promiseSubmitMessage = getSubmitMessage();
+  await clickLink();
+  await promiseSubmitMessage;
+  info("onFormSubmit message was sent as expected after user interaction");
+});
+
+add_task(async function test_message_on_autofill_without_user_interaction() {
+  runInParent(function addLogin() {
+    const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+    let login = Cc["@mozilla.org/login-manager/loginInfo;1"]
+                        .createInstance(Ci.nsILoginInfo);
+    login.init("https://example.org", "https://example.org", null,
+               "user1", "pass1", "", "");
+
+    Services.logins.addLogin(login);
+  });
+
+  await setup();
+  // Check for autofilled values.
+  await checkLoginFormInChildFrame(getIframeBrowsingContext(window, 0),
+                                   "form-basic-username", "user1",
+                                   "form-basic-password", "pass1");
+
+  let promiseSubmitMessage = getSubmitMessage();
+  await clickLink();
+
+  let messageData = await promiseSubmitMessage;
+  ok(messageData.autoFilledLoginGuid, "Message was sent with autoFilledLoginGuid");
+});
+
+add_task(async function test_message_on_autofill_with_user_interaction() {
+  runInParent(function addLogin() {
+    const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+    let login = Cc["@mozilla.org/login-manager/loginInfo;1"]
+                        .createInstance(Ci.nsILoginInfo);
+    login.init("https://example.org", "https://example.org", null,
+               "user1", "pass1", "", "");
+
+    Services.logins.addLogin(login);
+  });
+
+  await setup();
+  // Check for autofilled values.
+  await checkLoginFormInChildFrame(getIframeBrowsingContext(window, 0),
+                                   "form-basic-username", "user1",
+                                   "form-basic-password", "pass1");
+
+  userInput("#form-basic-username", "newuser");
+  let promiseSubmitMessage = getSubmitMessage();
+  await clickLink();
+
+  let messageData = await promiseSubmitMessage;
+  ok(messageData.autoFilledLoginGuid, "Message was sent with autoFilledLoginGuid");
+  is(messageData.usernameField.value, "newuser", "Message was sent with correct usernameField.value");
+  info("Message was sent as expected after user interaction");
+});
+
+add_task(async function test_no_message_on_user_input_from_other_form() {
+  // ensure input into unrelated fields on the page don't change login form modified-ness
+  await setup("http://example.com/tests/toolkit/components/passwordmgr/test/mochitest/form_basic.html");
+
+  // Add a form which will not be submitted and an input associated with that form
+  await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
+    let doc = this.content.document;
+    let loginForm = doc.querySelector("form");
+    let fragment = doc.createDocumentFragment();
+    let otherForm = doc.createElement("form");
+    otherForm.id ="otherForm";
+    fragment.appendChild(otherForm);
+
+    let alienField = doc.createElement("input");
+    alienField.id = "alienField";
+    alienField.type = "text"; // not a password field
+    alienField.setAttribute("form", "otherForm");
+    // new field is child of the login, but a member of different non-login form via its .form property
+    loginForm.appendChild(alienField);
+    doc.body.appendChild(fragment);
+  });
+  await userInput("#alienField", "something");
+
+  let submitMessageSent = false;
+  getSubmitMessage().then(data => {
+    info("submit mesage data: " + JSON.stringify(data));
+    submitMessageSent = true;
+  });
+
+  info("submitting the form");
+  await clickLink();
+
+  // allow time to pass before concluding no onFormSubmit message was sent
+  await new Promise(res => setTimeout(res, 1000));
+  ok(!submitMessageSent, "onFormSubmit message is not sent on navigation since no login fields were modified");
+});
+
+</script>
+</body>
+</html>