From 1676f316279bcad381185b5faa33f6030dc39255 Mon Sep 17 00:00:00 2001
From: Jared Wein <jwein@mozilla.com>
Date: Tue, 2 Apr 2019 18:24:47 +0000
Subject: [PATCH] Bug 1147563 - Provide autocomplete experience when
 formSubmitURL does not match. r=MattN

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

--HG--
rename : toolkit/components/passwordmgr/test/mochitest/test_autofill_https_upgrade.html => toolkit/components/passwordmgr/test/mochitest/test_autofill_different_formSubmitURL.html
extra : moz-landing-system : lando
---
 .../components/passwordmgr/LoginHelper.jsm    |  22 +++-
 .../passwordmgr/LoginManagerContent.jsm       |  23 +++-
 .../passwordmgr/LoginManagerParent.jsm        |  24 ++--
 .../passwordmgr/test/mochitest/mochitest.ini  |   6 +
 ...test_autofill_different_formSubmitURL.html |  78 ++++++++++++
 ...basic_form_autocomplete_formSubmitURL.html | 112 ++++++++++++++++++
 6 files changed, 251 insertions(+), 14 deletions(-)
 create mode 100644 toolkit/components/passwordmgr/test/mochitest/test_autofill_different_formSubmitURL.html
 create mode 100644 toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete_formSubmitURL.html

diff --git a/toolkit/components/passwordmgr/LoginHelper.jsm b/toolkit/components/passwordmgr/LoginHelper.jsm
index 06d22fe7faf3..79a1857b0561 100644
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@@ -272,6 +272,7 @@ var LoginHelper = {
    */
   isOriginMatching(aLoginOrigin, aSearchOrigin, aOptions = {
     schemeUpgrades: false,
+    acceptWildcardMatch: false,
   }) {
     if (aLoginOrigin == aSearchOrigin) {
       return true;
@@ -281,6 +282,10 @@ var LoginHelper = {
       return false;
     }
 
+    if (aOptions.acceptWildcardMatch && aLoginOrigin == "") {
+      return true;
+    }
+
     if (aOptions.schemeUpgrades) {
       try {
         let loginURI = Services.io.newURI(aLoginOrigin);
@@ -480,12 +485,17 @@ var LoginHelper = {
    *        String representing the origin to use for preferring one login over
    *        another when they are dupes. This is used with "scheme" for
    *        `resolveBy` so the scheme from this origin will be preferred.
+   * @param {string} [preferredFormActionOrigin = undefined]
+   *        String representing the action origin to use for preferring one login over
+   *        another when they are dupes. This is used with "actionOrigin" for
+   *        `resolveBy` so the scheme from this action origin will be preferred.
    *
    * @returns {nsILoginInfo[]} list of unique logins.
    */
   dedupeLogins(logins, uniqueKeys = ["username", "password"],
                resolveBy = ["timeLastUsed"],
-               preferredOrigin = undefined) {
+               preferredOrigin = undefined,
+               preferredFormActionOrigin = undefined) {
     const KEY_DELIMITER = ":";
 
     if (!preferredOrigin && resolveBy.includes("scheme")) {
@@ -530,6 +540,16 @@ var LoginHelper = {
 
       for (let preference of resolveBy) {
         switch (preference) {
+          case "actionOrigin": {
+            if (!preferredFormActionOrigin) {
+              break;
+            }
+            if (LoginHelper.isOriginMatching(existingLogin.formSubmitURL, preferredFormActionOrigin, {schemeUpgrades: LoginHelper.schemeUpgrades}) &&
+                !LoginHelper.isOriginMatching(login.formSubmitURL, preferredFormActionOrigin, {schemeUpgrades: LoginHelper.schemeUpgrades})) {
+              return false;
+            }
+            break;
+          }
           case "scheme": {
             if (!preferredOriginScheme) {
               break;
diff --git a/toolkit/components/passwordmgr/LoginManagerContent.jsm b/toolkit/components/passwordmgr/LoginManagerContent.jsm
index a1306e4d0e87..e61472b9a874 100644
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -1179,9 +1179,10 @@ var LoginManagerContent = {
    *
    * @param {LoginForm} form
    * @param {nsILoginInfo[]} foundLogins an array of nsILoginInfo that could be
-            used for the form
+   *        used for the form, including ones with a different form action origin
+   *        which are only used when the fill is userTriggered
    * @param {Set} recipes a set of recipes that could be used to affect how the
-            form is filled
+   *        form is filled
    * @param {Object} [options = {}] a list of options for this method
    * @param {HTMLInputElement} [options.inputElement = null] an optional target
    *        input element we want to fill
@@ -1229,8 +1230,9 @@ var LoginManagerContent = {
     };
 
     try {
-      // Nothing to do if we have no matching logins available,
-      // and there isn't a need to show the insecure form warning.
+      // Nothing to do if we have no matching (excluding form action
+      // checks) logins available, and there isn't a need to show
+      // the insecure form warning.
       if (foundLogins.length == 0 &&
           (InsecurePasswordUtils.isFormSecure(form) ||
           !LoginHelper.showInsecureFieldWarning)) {
@@ -1286,6 +1288,19 @@ var LoginManagerContent = {
         usernameField.addEventListener("keydown", observer);
       }
 
+      if (!userTriggered) {
+        // Only autofill logins that match the form's action. In the above code
+        // we have attached autocomplete for logins that don't match the form action.
+        foundLogins = foundLogins.filter(l => {
+          return LoginHelper.isOriginMatching(l.formSubmitURL,
+                                              LoginHelper.getFormActionOrigin(form),
+                                              {
+                                                schemeUpgrades: LoginHelper.schemeUpgrades,
+                                                acceptWildcardMatch: true,
+                                              });
+        });
+      }
+
       // Nothing to do if we have no matching logins available.
       // Only insecure pages reach this block and logs the same
       // telemetry flag.
diff --git a/toolkit/components/passwordmgr/LoginManagerParent.jsm b/toolkit/components/passwordmgr/LoginManagerParent.jsm
index 45b37b5635ae..d915b477346a 100644
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -40,14 +40,17 @@ var LoginManagerParent = {
   // to avoid spamming master password prompts on autocomplete searches.
   _lastMPLoginCancelled: Math.NEGATIVE_INFINITY,
 
-  _searchAndDedupeLogins(formOrigin, actionOrigin) {
+  _searchAndDedupeLogins(formOrigin, actionOrigin, {looseActionOriginMatch} = {}) {
     let logins;
+    let matchData = {
+      hostname: formOrigin,
+      schemeUpgrades: LoginHelper.schemeUpgrades,
+    };
+    if (!looseActionOriginMatch) {
+      matchData.formSubmitURL = actionOrigin;
+    }
     try {
-      logins = LoginHelper.searchLoginsWithObject({
-        hostname: formOrigin,
-        formSubmitURL: actionOrigin,
-        schemeUpgrades: LoginHelper.schemeUpgrades,
-      });
+      logins = LoginHelper.searchLoginsWithObject(matchData);
     } catch (e) {
       // Record the last time the user cancelled the MP prompt
       // to avoid spamming them with MP prompts for autocomplete.
@@ -61,10 +64,11 @@ var LoginManagerParent = {
 
     // Dedupe so the length checks below still make sense with scheme upgrades.
     let resolveBy = [
+      "actionOrigin",
       "scheme",
       "timePasswordChanged",
     ];
-    return LoginHelper.dedupeLogins(logins, ["username"], resolveBy, formOrigin);
+    return LoginHelper.dedupeLogins(logins, ["username"], resolveBy, formOrigin, actionOrigin);
   },
 
   // Listeners are added in BrowserGlue.jsm on desktop
@@ -222,7 +226,8 @@ var LoginManagerParent = {
       return;
     }
 
-    let logins = this._searchAndDedupeLogins(formOrigin, actionOrigin);
+    // Autocomplete results do not need to match actionOrigin.
+    let logins = this._searchAndDedupeLogins(formOrigin, actionOrigin, {looseActionOriginMatch: true});
 
     log("sendLoginDataToChild:", logins.length, "deduped logins");
     // Convert the array of nsILoginInfo to vanilla JS objects since nsILoginInfo
@@ -270,7 +275,8 @@ var LoginManagerParent = {
     } else {
       log("Creating new autocomplete search result.");
 
-      logins = this._searchAndDedupeLogins(formOrigin, actionOrigin);
+      // Autocomplete results do not need to match actionOrigin.
+      logins = this._searchAndDedupeLogins(formOrigin, actionOrigin, {looseActionOriginMatch: true});
     }
 
     let matchingLogins = logins.filter(function(fullMatch) {
diff --git a/toolkit/components/passwordmgr/test/mochitest/mochitest.ini b/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
index 1974d0737995..9c9b4b3a8ed8 100644
--- a/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
+++ b/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
@@ -38,6 +38,9 @@ skip-if = toolkit == 'android' # autocomplete
 [test_autofill_autocomplete_types.html]
 scheme = https
 skip-if = toolkit == 'android' # bug 1533965
+[test_autofill_different_formSubmitURL.html]
+scheme = https
+skip-if = toolkit == 'android' # Bug 1259768
 [test_autofill_from_bfcache.html]
 scheme = https
 skip-if = toolkit == 'android' # bug 1527403
@@ -61,6 +64,9 @@ skip-if = toolkit == 'android' # autocomplete
 [test_basic_form_3pw_1.html]
 [test_basic_form_autocomplete.html]
 skip-if = toolkit == 'android' # android:autocomplete.
+[test_basic_form_autocomplete_formSubmitURL.html]
+skip-if = toolkit == 'android' # android:autocomplete.
+scheme = https
 [test_basic_form_honor_autocomplete_off.html]
 scheme = https
 skip-if = toolkit == 'android' # android:autocomplete.
diff --git a/toolkit/components/passwordmgr/test/mochitest/test_autofill_different_formSubmitURL.html b/toolkit/components/passwordmgr/test/mochitest/test_autofill_different_formSubmitURL.html
new file mode 100644
index 000000000000..809cf751bbf2
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/mochitest/test_autofill_different_formSubmitURL.html
@@ -0,0 +1,78 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test autofill on an HTTPS page using upgraded HTTP logins wtih different formSubmitURL</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.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>
+<script>
+const MISSING_ACTION_PATH = TESTS_DIR + "mochitest/form_basic.html";
+
+const chromeScript = runChecksAfterCommonInit(false);
+
+let nsLoginInfo = SpecialPowers.wrap(SpecialPowers.Components).Constructor("@mozilla.org/login-manager/loginInfo;1",
+                                                                           SpecialPowers.Ci.nsILoginInfo,
+                                                                           "init");
+</script>
+<p id="display"></p>
+
+<!-- we presumably can't hide the content for this test. -->
+<div id="content">
+  <iframe></iframe>
+</div>
+
+<pre id="test">
+<script class="testbody" type="text/javascript">
+let iframe = SpecialPowers.wrap(document.getElementsByTagName("iframe")[0]);
+
+// Check for expected username/password in form.
+function checkACForm(expectedUsername, expectedPassword) {
+  let iframeDoc = iframe.contentDocument;
+  let uname = iframeDoc.getElementById("form-basic-username");
+  let pword = iframeDoc.getElementById("form-basic-password");
+  let formID = uname.parentNode.id;
+  is(uname.value, expectedUsername, "Checking " + formID + " username");
+  is(pword.value, expectedPassword, "Checking " + formID + " password");
+}
+async function prepareLoginsAndProcessForm(url, logins = []) {
+  LoginManager.removeAllLogins();
+
+  let dates = Date.now();
+  for (let login of logins) {
+    SpecialPowers.do_QueryInterface(login, SpecialPowers.Ci.nsILoginMetaInfo);
+    // Force all dates to be the same so they don't affect things like deduping.
+    login.timeCreated = login.timePasswordChanged = login.timeLastUsed = dates;
+    LoginManager.addLogin(login);
+  }
+
+  iframe.src = url;
+  await promiseFormsProcessed();
+}
+
+add_task(async function test_formSubmitURL_wildcard_should_autofill() {
+  await prepareLoginsAndProcessForm("https://example.com" + MISSING_ACTION_PATH, [
+    new nsLoginInfo("https://example.com", "", null,
+                    "name2", "pass2", "uname", "pword"),
+  ]);
+
+  checkACForm("name2", "pass2");
+});
+
+add_task(async function test_formSubmitURL_different_shouldnt_autofill() {
+  await prepareLoginsAndProcessForm("https://example.com" + MISSING_ACTION_PATH, [
+    new nsLoginInfo("https://example.com", "https://another.domain", null,
+                    "name2", "pass2", "uname", "pword"),
+  ]);
+
+  checkACForm("", "");
+});
+
+</script>
+</pre>
+</body>
+</html>
diff --git a/toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete_formSubmitURL.html b/toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete_formSubmitURL.html
new file mode 100644
index 000000000000..72d9ca05d93e
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete_formSubmitURL.html
@@ -0,0 +1,112 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test that logins with non-matching formSubmitURL appear in autocomplete dropdown</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="text/javascript" src="../../../satchel/test/satchel_common.js"></script>
+  <script type="text/javascript" src="pwmgr_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+Login Manager test: logins with non-matching formSubmitURL appear in autocomplete dropdown
+
+<script>
+var chromeScript = runChecksAfterCommonInit();
+
+runInParent(function setup() {
+  const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+  // Create some logins just for this form, since we'll be deleting them.
+  var nsLoginInfo = Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
+                                           Ci.nsILoginInfo, "init");
+  assert.ok(nsLoginInfo != null, "nsLoginInfo constructor");
+
+  var login1 = new nsLoginInfo("https://example.com", "https://differentFormSubmitURL", null,
+                               "dfsu1", "dfsp1", "uname", "pword");
+
+  Services.logins.addLogin(login1);
+});
+</script>
+<p id="display"></p>
+
+<!-- we presumably can't hide the content for this test. -->
+<div id="content">
+
+  <!-- form1 tests multiple matching logins -->
+  <form id="form1" action="https://autocomplete:8888/formtest.js" onsubmit="return false;">
+    <input  type="text"       name="uname">
+    <input  type="password"   name="pword">
+    <button type="submit">Submit</button>
+  </form>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Login Manager: multiple login autocomplete. **/
+
+var uname = $_(1, "uname");
+var pword = $_(1, "pword");
+
+// Restore the form to the default state.
+function restoreForm() {
+  uname.value = "";
+  pword.value = "";
+  uname.focus();
+}
+
+// Check for expected username/password in form.
+function checkACForm(expectedUsername, expectedPassword) {
+  var formID = uname.parentNode.id;
+  is(uname.value, expectedUsername, "Checking " + formID + " username is: " + expectedUsername);
+  is(pword.value, expectedPassword, "Checking " + formID + " password is: " + expectedPassword);
+}
+
+function spinEventLoop() {
+  return Promise.resolve();
+}
+
+add_task(async function setup() {
+  listenForUnexpectedPopupShown();
+});
+
+add_task(async function test_form1_initial_empty() {
+  await SimpleTest.promiseFocus(window);
+
+  // Make sure initial form is empty.
+  checkACForm("", "");
+  let popupState = await getPopupState();
+  is(popupState.open, false, "Check popup is initially closed");
+});
+
+/* For this testcase, the only login that exists for this origin
+ * is one with a different formSubmitURL, so the login will appear
+ * in the autocomplete popup.
+ */
+add_task(async function test_form1_menu_shows_logins_for_different_formSubmitURL() {
+  await SimpleTest.promiseFocus(window);
+  // Trigger autocomplete popup
+  restoreForm();
+  let shownPromise = promiseACShown();
+  synthesizeKey("KEY_ArrowDown"); // open
+  let results = await shownPromise;
+
+  let popupState = await getPopupState();
+  is(popupState.selectedIndex, -1, "Check no entries are selected upon opening");
+
+  let expectedMenuItems = ["dfsu1"];
+  checkAutoCompleteResults(results, expectedMenuItems, "example.com", "Check all menuitems are displayed correctly.");
+
+  synthesizeKey("KEY_ArrowDown"); // first item
+  checkACForm("", ""); // value shouldn't update just by selecting
+
+  synthesizeKey("KEY_Enter");
+  await promiseFormsProcessed();
+  checkACForm("dfsu1", "dfsp1");
+});
+</script>
+</pre>
+</body>
+</html>