Bug 1334026 - Show the the insecure field warning on insecure password fields even if they're not marked. r=mconley
☠☠ backed out by dad46f412588 ☠ ☠
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 26 Jan 2017 18:03:16 -0800
changeset 331301 25c931f199e855302e93a295035804f617823348
parent 331300 4be138bdb7bf895d5d50f2884ff86c1a631c7a34
child 331302 245f6ee1f9bdace38bfade74015f027d16eb80a7
push id31265
push usercbook@mozilla.com
push dateFri, 27 Jan 2017 09:41:20 +0000
treeherdermozilla-central@dad46f412588 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1334026
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1334026 - Show the the insecure field warning on insecure password fields even if they're not marked. r=mconley MozReview-Commit-ID: JwEYWQmexj
toolkit/components/satchel/nsFormFillController.cpp
toolkit/components/satchel/test/mochitest.ini
toolkit/components/satchel/test/test_password_autocomplete.html
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -697,20 +697,29 @@ nsFormFillController::GetUserContextId(u
 ////////////////////////////////////////////////////////////////////////
 //// nsIAutoCompleteSearch
 
 NS_IMETHODIMP
 nsFormFillController::StartSearch(const nsAString &aSearchString, const nsAString &aSearchParam,
                                   nsIAutoCompleteResult *aPreviousResult, nsIAutoCompleteObserver *aListener)
 {
   nsresult rv;
+  nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(mFocusedInputNode);
 
   // If the login manager has indicated it's responsible for this field, let it
   // handle the autocomplete. Otherwise, handle with form history.
-  if (mPwmgrInputs.Get(mFocusedInputNode)) {
+  if (mFocusedInputNode && (mPwmgrInputs.Get(mFocusedInputNode) ||
+                            formControl->GetType() == NS_FORM_INPUT_PASSWORD)) {
+
+    // Handle the case where a password field is focused but
+    // MarkAsLoginManagerField wasn't called because password manager is disabled.
+    if (!mLoginManager) {
+      mLoginManager = do_GetService("@mozilla.org/login-manager;1");
+    }
+
     // XXX aPreviousResult shouldn't ever be a historyResult type, since we're not letting
     // satchel manage the field?
     mLastListener = aListener;
     rv = mLoginManager->AutoCompleteSearchAsync(aSearchString,
                                                 aPreviousResult,
                                                 mFocusedInput,
                                                 this);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -981,39 +990,42 @@ nsFormFillController::MaybeStartControll
   nsCOMPtr<nsIDOMHTMLElement> datalist;
   aInput->GetList(getter_AddRefs(datalist));
   bool hasList = datalist != nullptr;
 
   bool isPwmgrInput = false;
   if (mPwmgrInputs.Get(inputNode))
       isPwmgrInput = true;
 
-  // Don't show autocomplete on password fields regardless of datalist or
-  // autocomplete being enabled as we don't want to show form history on them.
-  // The GetType() check was more readable than !formControl->IsSingleLineTextControl(true)
-  // but this logic should be kept in-sync with that.
-  if (formControl->GetType() == NS_FORM_INPUT_PASSWORD && !isPwmgrInput) {
-    return;
-  }
-
   if (isPwmgrInput || hasList || autocomplete) {
     StartControllingInput(aInput);
   }
 }
 
 nsresult
 nsFormFillController::Focus(nsIDOMEvent* aEvent)
 {
   nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(
     aEvent->InternalDOMEvent()->GetTarget());
   MaybeStartControllingInput(input);
 
+  // Bail if we didn't start controlling the input.
+  if (!mFocusedInputNode) {
+    mContextMenuFiredBeforeFocus = false;
+    return NS_OK;
+  }
+
+  nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(mFocusedInputNode);
+  MOZ_ASSERT(formControl);
+
   // If this focus doesn't immediately follow a contextmenu event then show
   // the autocomplete popup
-  if (!mContextMenuFiredBeforeFocus && mPwmgrInputs.Get(mFocusedInputNode)) {
+  if (!mContextMenuFiredBeforeFocus &&
+      (mPwmgrInputs.Get(mFocusedInputNode) ||
+       formControl->GetType() == NS_FORM_INPUT_PASSWORD)) {
     ShowPopup();
   }
 
   mContextMenuFiredBeforeFocus = false;
   return NS_OK;
 }
 
 nsresult
--- a/toolkit/components/satchel/test/mochitest.ini
+++ b/toolkit/components/satchel/test/mochitest.ini
@@ -10,10 +10,11 @@ support-files =
 [test_bug_787624.html]
 [test_datalist_with_caching.html]
 [test_form_autocomplete.html]
 [test_form_autocomplete_with_list.html]
 [test_form_submission.html]
 [test_form_submission_cap.html]
 [test_form_submission_cap2.html]
 [test_password_autocomplete.html]
+scheme = https
 [test_popup_direction.html]
 [test_popup_enter_event.html]
--- a/toolkit/components/satchel/test/test_password_autocomplete.html
+++ b/toolkit/components/satchel/test/test_password_autocomplete.html
@@ -16,17 +16,24 @@
 <!-- we presumably can't hide the content for this test. -->
 <div id="content">
   <datalist id="datalist1">
     <option>value10</option>
     <option>value11</option>
     <option>value12</option>
   </datalist>
   <form id="form1" onsubmit="return false;">
-    <input  type="password" name="field1" list="datalist1">
+    <!-- Don't set the type to password until rememberSignons is false since we
+         want to test when rememberSignons is false. -->
+    <input  type="to-be-password" name="field1" list="datalist1">
+    <button type="submit">Submit</button>
+  </form>
+  <!-- Same as form1 but with an insecure HTTP action -->
+  <form id="form2" onsubmit="return false;" action="http://mochi.test/">
+    <input  type="to-be-password" name="field1" list="datalist1">
     <button type="submit">Submit</button>
   </form>
 </div>
 
 <pre id="test">
 <script class="testbody" type="text/javascript">
 /* import-globals-from satchel_common.js */
 
@@ -61,36 +68,42 @@ function expectPopupDoesNotOpen(triggerF
   let popupShown = waitForNextPopup();
   triggerFn();
   return Promise.race([
     popupShown.then(() => Promise.reject("Popup opened unexpectedly.")),
     new Promise(resolve => setTimeout(resolve, POPUP_RESPONSE_WAIT_TIME_MS)),
   ]);
 }
 
-let input = $_(1, "field1");
+add_task(function* test_initialize() {
+  yield SpecialPowers.pushPrefEnv({set: [["signon.rememberSignons", false]]});
 
-add_task(function* test_initialize() {
-  yield SpecialPowers.pushPrefEnv("set", [["signon.rememberSignons", false]]);
+  is(window.location.protocol, "https:", "This test must run on HTTPS");
+
+  // Now that rememberSignons is false, create the password fields.
+  $_(1, "field1").type = "password";
+  $_(2, "field1").type = "password";
 
   yield new Promise(resolve => updateFormHistory([
     { op : "remove" },
     { op : "add", fieldname : "field1", value : "value1" },
     { op : "add", fieldname : "field1", value : "value2" },
     { op : "add", fieldname : "field1", value : "value3" },
     { op : "add", fieldname : "field1", value : "value4" },
     { op : "add", fieldname : "field1", value : "value5" },
     { op : "add", fieldname : "field1", value : "value6" },
     { op : "add", fieldname : "field1", value : "value7" },
     { op : "add", fieldname : "field1", value : "value8" },
     { op : "add", fieldname : "field1", value : "value9" },
   ], resolve));
 });
 
-add_task(function* test_no_form_history() {
+add_task(function* test_secure_noFormHistoryOrWarning() {
+  let input = $_(1, "field1");
+
   // The autocomplete popup should not open under any circumstances on
   // type=password with password manager disabled.
   for (let triggerFn of [
     () => input.focus(),
     () => input.click(),
     () => doKey("down"),
     () => doKey("page_down"),
     () => doKey("return"),
@@ -102,12 +115,26 @@ add_task(function* test_no_form_history(
     // We must wait for the entire timeout for each individual test, because the
     // next event in the list might prevent the popup from opening.
     yield expectPopupDoesNotOpen(triggerFn);
   }
 
   // Close the popup.
   input.blur();
 });
+
+add_task(function* test_insecure_focusWarning() {
+  // Form 2 has an insecure action so should show the warning even if password manager is disabled.
+  let input = $_(2, "field1");
+  let shownPromise = waitForNextPopup();
+  input.focus();
+  yield shownPromise;
+
+  ok(getMenuEntries()[0].includes("Logins entered here could be compromised"),
+     "Check warning is first")
+
+  // Close the popup
+  input.blur();
+});
 </script>
 </pre>
 </body>
 </html>