Bug 1627658 - Avoid login capture until user interacts with the document. r=MattN

Differential Revision: https://phabricator.services.mozilla.com/D70290 --HG-- extra : moz-landing-system : lando
2020-04-14 19:32:26 +00:00 · 2020-04-14 19:32:26 +00:00 · 76b0d3e957
--- a/browser/tools/mozscreenshots/permissionPrompts/browser.ini
+++ b/browser/tools/mozscreenshots/permissionPrompts/browser.ini
@ -2,6 +2,9 @@
 subsuite = screenshots
 support-files =
  ../head.js
-  
+prefs =
+  signon.testOnlyUserHasInteractedByPrefValue=true
+  signon.testOnlyUserHasInteractedWithDocument=true
+
 [browser_permissionPrompts.js]
 skip-if = os == 'mac' # times out on macosx1014, see 1570098
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@ -37,6 +37,8 @@ this.LoginHelper = {
  privateBrowsingCaptureEnabled: null,
  schemeUpgrades: null,
  showAutoCompleteFooter: null,
+  testOnlyUserHasInteractedWithDocument: null,
+  userInputRequiredToCapture: null,

  init() {
    // Watch for pref changes to update cached pref values.
@ -75,6 +77,9 @@ this.LoginHelper = {
    this.includeOtherSubdomainsInLookup = Services.prefs.getBoolPref(
      "signon.includeOtherSubdomainsInLookup"
    );
+    this.passwordEditCaptureEnabled = Services.prefs.getBoolPref(
+      "signon.passwordEditCapture.enabled"
+    );
    this.privateBrowsingCaptureEnabled = Services.prefs.getBoolPref(
      "signon.privateBrowsingCapture.enabled"
    );
@ -85,12 +90,28 @@ this.LoginHelper = {
    this.storeWhenAutocompleteOff = Services.prefs.getBoolPref(
      "signon.storeWhenAutocompleteOff"
    );
+
+    if (
+      Services.prefs.getBoolPref(
+        "signon.testOnlyUserHasInteractedByPrefValue",
+        false
+      )
+    ) {
+      this.testOnlyUserHasInteractedWithDocument = Services.prefs.getBoolPref(
+        "signon.testOnlyUserHasInteractedWithDocument",
+        false
+      );
+      log.debug(
+        "updateSignonPrefs, using pref value for testOnlyUserHasInteractedWithDocument",
+        this.testOnlyUserHasInteractedWithDocument
+      );
+    } else {
+      this.testOnlyUserHasInteractedWithDocument = null;
+    }
+
    this.userInputRequiredToCapture = Services.prefs.getBoolPref(
      "signon.userInputRequiredToCapture.enabled"
    );
-    this.passwordEditCaptureEnabled = Services.prefs.getBoolPref(
-      "signon.passwordEditCapture.enabled"
-    );
  },

  createLogger(aLogPrefix) {
@ -1162,8 +1183,6 @@ this.LoginHelper = {
  },
 };

-LoginHelper.init();
-
 XPCOMUtils.defineLazyPreferenceGetter(
  LoginHelper,
  "showInsecureFieldWarning",
@ -1171,5 +1190,11 @@ XPCOMUtils.defineLazyPreferenceGetter(
 );

 XPCOMUtils.defineLazyGetter(this, "log", () => {
-  return LoginHelper.createLogger("LoginHelper");
+  let processName =
+    Services.appinfo.processType === Services.appinfo.PROCESS_TYPE_DEFAULT
+      ? "Main"
+      : "Content";
+  return LoginHelper.createLogger(`LoginHelper(${processName})`);
 });
+
+LoginHelper.init();
--- a/toolkit/components/passwordmgr/LoginManagerChild.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerChild.jsm
@ -1564,6 +1564,19 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
      dismissedPrompt = true;
    }

+    let docState = this.stateForDocument(doc);
+    let fieldsModified = this._formHasModifiedFields(form);
+    if (!fieldsModified && LoginHelper.userInputRequiredToCapture) {
+      if (targetField) {
+        throw new Error("No user input on targetField");
+      }
+      // we know no fields in this form had user modifications, so don't prompt
+      log(
+        `(${logMessagePrefix} ignored -- submitting values that are not changed by the user)`
+      );
+      return;
+    }
+
    if (
      this._compareAndUpdatePreviouslySentValues(
        form.rootElement,
@ -1578,19 +1591,6 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
      return;
    }

-    let docState = this.stateForDocument(doc);
-    let fieldsModified = this._formHasModifiedFields(form);
-    if (!fieldsModified && LoginHelper.userInputRequiredToCapture) {
-      if (targetField) {
-        throw new Error("No user input on targetField");
-      }
-      // we know no fields in this form had user modifications, so don't prompt
-      log(
-        `(${logMessagePrefix} ignored -- submitting values that are not changed by the user)`
-      );
-      return;
-    }
-
    let { login: autoFilledLogin } =
      docState.fillsByRootElement.get(form.rootElement) || {};
    let browsingContextId = win.windowGlobalChild.browsingContext.id;
@ -2181,7 +2181,23 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
  }

  _formHasModifiedFields(form) {
-    let state = this.stateForDocument(form.rootElement.ownerDocument);
+    let doc = form.rootElement.ownerDocument;
+    let userHasInteracted;
+    let testOnlyUserHasInteracted =
+      LoginHelper.testOnlyUserHasInteractedWithDocument;
+    if (Cu.isInAutomation && testOnlyUserHasInteracted !== null) {
+      userHasInteracted = testOnlyUserHasInteracted;
+    } else {
+      userHasInteracted = doc.userHasInteracted;
+    }
+
+    log("_formHasModifiedFields, userHasInteracted:", userHasInteracted);
+
+    // If the user hasn't interacted at all with the page, we don't need to check futher
+    if (!userHasInteracted) {
+      return false;
+    }
+    let state = this.stateForDocument(doc);
    // check for user inputs to the form fields
    let fieldsModified = state.fieldModificationsByRootElement.get(
      form.rootElement
--- a/toolkit/components/passwordmgr/test/browser/head.js
+++ b/toolkit/components/passwordmgr/test/browser/head.js
@ -12,6 +12,8 @@ add_task(async function common_initialize() {
  await SpecialPowers.pushPrefEnv({
    set: [
      ["signon.rememberSignons", true],
+      ["signon.testOnlyUserHasInteractedByPrefValue", true],
+      ["signon.testOnlyUserHasInteractedWithDocument", true],
      ["toolkit.telemetry.ipcBatchTimeout", 0],
    ],
  });
--- a/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
+++ b/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
@ -3,6 +3,8 @@ prefs =
  signon.rememberSignons=true
  signon.autofillForms.http=true
  signon.showAutoCompleteFooter=true
+  signon.testOnlyUserHasInteractedByPrefValue=true
+  signon.testOnlyUserHasInteractedWithDocument=true
  security.insecure_field_warning.contextual.enabled=false
  network.auth.non-web-content-triggered-resources-http-auth-allow=true

@ -27,7 +29,7 @@ skip-if = toolkit == 'android' && !is_fennec # Don't run on GeckoView
 # Note: new tests should use scheme = https unless they have a specific reason not to

 [test_autocomplete_basic_form.html]
-skip-if = toolkit == 'android' || debug && webrender && (os == 'linux' || os == 'win') # android:autocomplete. Bug 1541945
+skip-if = toolkit == 'android' || debug && (os == 'linux' || os == 'win') # android:autocomplete. Bug 1541945
 scheme = https
 [test_autocomplete_basic_form_insecure.html]
 skip-if = toolkit == 'android' || os == 'linux' # android:autocomplete., linux: bug 1325778
--- a/toolkit/components/passwordmgr/test/mochitest/test_submit_without_field_modifications.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_submit_without_field_modifications.html
@ -37,6 +37,31 @@ function waitForLoad() {
  });
 }

+async function setupWithOneLogin(pageUrl) {
+  let chromeScript = runInParent(function testSetup() {
+    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.com", "https://example.com", null,
+               "user1", "pass1");
+
+    Services.logins.addLogin(login);
+    for (let l of Services.logins.getAllLogins()) {
+      info("Got login: " + l.username + ", " + l.password);
+    }
+  });
+  await setup(pageUrl);
+  return chromeScript;
+}
+
+function resetSavedLogins() {
+  let chromeScript = runInParent(function testTeardown() {
+    const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+    Services.logins.removeAllLogins();
+  });
+  chromeScript.destroy();
+}
+
 async function setup(pageUrl) {
  let loadPromise = waitForLoad();
  let processedFormPromise = promiseFormsProcessed();
@ -53,25 +78,63 @@ async function setup(pageUrl) {
  });
 }

-async function clickLink() {
+async function navigateWithoutUserInteraction() {
  let loadPromise = waitForLoad();
  await SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
    let doc = this.content.document;
-    doc.querySelector("a[href]").click();
+    let hadInteracted = doc.userHasInteracted;
+    let target = doc.querySelector("a[href]");
+    if (target) {
+      target.click();
+    } else {
+      target = doc.querySelector("form");
+      target.submit();
+    }
+    is(doc.userHasInteracted, hadInteracted, "document.userHasInteracted shouldn't have changed");
  });
  await loadPromise;
 }

 async function userInput(selector, value) {
-  await SpecialPowers.spawn(getIframeBrowsingContext(window), [selector, value], function(sel, val) {
-    this.content.document.querySelector(sel).setUserInput(val);
+  await SpecialPowers.spawn(getIframeBrowsingContext(window), [selector, value], async function(sel, val) {
+    // use "real" synthesized events rather than setUserInput to ensure
+    // document.userHasInteracted is flipped true
+    let EventUtils = ContentTaskUtils.getEventUtils(content);
+    let target = this.content.document.querySelector(sel);
+    target.focus();
+    target.select();
+    await EventUtils.synthesizeKey("KEY_Backspace", {}, this.content);
+    await EventUtils.sendString(val, this.content);
+    info(
+      `userInput: new target.value: ${target.value}`
+    );
+    target.blur();
+    return Promise.resolve();
+  });
+}
+
+function checkDocumentUserHasInteracted() {
+  return SpecialPowers.spawn(getIframeBrowsingContext(window), [], function() {
+    return this.content.document.userHasInteracted;
  });
 }

 add_task(async function test_init() {
+  // For this test, we'll be testing with & without user document interaction.
+  // So we'll reset the pref which dictates the behavior of LMC._formHasModifiedFields in automation
+  // and ensure all interactions are properly emulated
+  ok(SpecialPowers.getBoolPref("signon.testOnlyUserHasInteractedByPrefValue"), "signon.testOnlyUserHasInteractedByPrefValue should default to true");
+  info("test_init, flipping the signon.testOnlyUserHasInteractedByPrefValue pref");
  await SpecialPowers.pushPrefEnv({"set": [
-    ["signon.userInputRequiredToCapture.enabled", true],
+    ["signon.testOnlyUserHasInteractedByPrefValue", false],
  ]});
+  SimpleTest.registerCleanupFunction(async function cleanup_pref() {
+    await SpecialPowers.popPrefEnv();
+  });
+
+  await SimpleTest.promiseWaitForCondition(() => LoginHelper.testOnlyUserHasInteractedWithDocument === null);
+  is(LoginHelper.testOnlyUserHasInteractedWithDocument, null,
+    "LoginHelper.testOnlyUserHasInteractedWithDocument should be null for this set of tests");
 });

 add_task(async function test_no_message_on_navigation() {
@ -83,7 +146,7 @@ add_task(async function test_no_message_on_navigation() {
  getSubmitMessage().then(value => {
    submitMessageSent = true;
  });
-  await clickLink();
+  await navigateWithoutUserInteraction();

  // allow time to pass before concluding no onFormSubmit message was sent
  await new Promise(res => setTimeout(res, 1000));
@ -98,7 +161,7 @@ add_task(async function test_prefd_off_message_on_navigation() {
  await setup(PREFILLED_FORM_URL);

  let promiseSubmitMessage = getSubmitMessage();
-  await clickLink();
+  await navigateWithoutUserInteraction();
  await promiseSubmitMessage;
  info("onFormSubmit message was sent as expected after navigation");

@ -110,7 +173,7 @@ add_task(async function test_message_with_user_interaction_on_navigation() {
  await userInput("#form-basic-username", "foo");

  let promiseSubmitMessage = getSubmitMessage();
-  await clickLink();
+  await navigateWithoutUserInteraction();
  await promiseSubmitMessage;
  info("onFormSubmit message was sent as expected after user interaction");
 });
@ -121,47 +184,77 @@ add_task(async function test_empty_form_with_input_handler() {
  await userInput("#form-basic-password", "pass");

  let promiseSubmitMessage = getSubmitMessage();
-  await clickLink();
+  await navigateWithoutUserInteraction();
  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.com", "https://example.com", null,
-               "user1", "pass1", "", "");
-
-    Services.logins.addLogin(login);
-  });
-
-  await setup(EXAMPLE_COM + "form_basic.html");
+add_task(async function test_no_message_on_autofill_without_user_interaction() {
+  let chromeScript = await setupWithOneLogin(EXAMPLE_COM + "form_basic.html");
  // Check for autofilled values.
  await checkLoginFormInChildFrame(getIframeBrowsingContext(window, 0),
                                   "form-basic-username", "user1",
                                   "form-basic-password", "pass1");

+  info("LoginHelper.testOnlyUserHasInteractedWithDocument:" +
+      LoginHelper.testOnlyUserHasInteractedWithDocument
+  );
+  ok(!(await checkDocumentUserHasInteracted()), "document.userHasInteracted should be initially false");
+  let submitMessageSent = false;
+  getSubmitMessage().then(value => {
+    submitMessageSent = true;
+  });
+  info("Navigating the page")
+  await navigateWithoutUserInteraction();
+
+  // allow time to pass before concluding no onFormSubmit message was sent
+  await new Promise(res => setTimeout(res, 1000));
+
+  chromeScript.destroy();
+  resetSavedLogins();
+
+  ok(!submitMessageSent, "onFormSubmit message is not sent on navigation since the document had no user interaction");
+});
+
+add_task(async function test_message_on_autofill_with_document_interaction() {
+  // We expect that as long as the form values !== their defaultValues,
+  // any document interaction allows the submit message to be sent
+
+  let chromeScript = await setupWithOneLogin(EXAMPLE_COM + "form_basic.html");
+  // Check for autofilled values.
+  await checkLoginFormInChildFrame(getIframeBrowsingContext(window, 0),
+                                   "form-basic-username", "user1",
+                                   "form-basic-password", "pass1");
+
+  let userInteracted = await checkDocumentUserHasInteracted();
+  ok(!userInteracted, "document.userHasInteracted should be initially false");
+
+  await SpecialPowers.spawn(getIframeBrowsingContext(window), ["#form-basic-username"], async function(sel) {
+    // Click somewhere in the document to ensure document.userHasInteracted is flipped to true
+    let EventUtils = ContentTaskUtils.getEventUtils(content);
+    let target = this.content.document.querySelector(sel);
+
+    await EventUtils.synthesizeMouseAtCenter(target, {}, this.content);
+  });
+
+  userInteracted = await checkDocumentUserHasInteracted();
+  ok(userInteracted, "After synthesizeMouseAtCenter, document.userHasInteracted should be true");
+
  let promiseSubmitMessage = getSubmitMessage();
-  await clickLink();
+  await navigateWithoutUserInteraction();

  let messageData = await promiseSubmitMessage;
  ok(messageData.autoFilledLoginGuid, "Message was sent with autoFilledLoginGuid");
+  info("Message was sent as expected after document user interaction");
+
+  chromeScript.destroy();
+  resetSavedLogins();
 });

 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.com", "https://example.com", null,
-               "user1", "pass1", "", "");
-
-    Services.logins.addLogin(login);
-  });
-
-  await setup(EXAMPLE_COM + "form_basic.html");
+  // Editing a field value causes the submit message to be sent as
+  // there is both document interaction and field modification
+  let chromeScript = await setupWithOneLogin(EXAMPLE_COM + "form_basic.html");
  // Check for autofilled values.
  await checkLoginFormInChildFrame(getIframeBrowsingContext(window, 0),
                                   "form-basic-username", "user1",
@ -169,12 +262,15 @@ add_task(async function test_message_on_autofill_with_user_interaction() {

  userInput("#form-basic-username", "newuser");
  let promiseSubmitMessage = getSubmitMessage();
-  await clickLink();
+  await navigateWithoutUserInteraction();

  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");
+  info("Message was sent as expected after user form interaction");
+
+  chromeScript.destroy();
+  resetSavedLogins();
 });

 add_task(async function test_no_message_on_user_input_from_other_form() {
@ -207,7 +303,7 @@ add_task(async function test_no_message_on_user_input_from_other_form() {
  });

  info("submitting the form");
-  await clickLink();
+  await navigateWithoutUserInteraction();

  // allow time to pass before concluding no onFormSubmit message was sent
  await new Promise(res => setTimeout(res, 1000));