Fill on account select in the password manager

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 9e3e0d87fef189c727540f73b52222a8dd24e80b..869f121bba7e17680f2c42b84b8637675a4bf3c8 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -5,6 +5,7 @@
 #include "components/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
@@ -13,6 +14,7 @@
 #include "components/autofill/content/renderer/form_autofill_util.h"
 #include "components/autofill/content/renderer/password_form_conversion_utils.h"
 #include "components/autofill/content/renderer/renderer_save_password_progress_logger.h"
+#include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/autofill/core/common/password_autofill_util.h"
 #include "components/autofill/core/common/password_form.h"
@@ -62,6 +64,23 @@ struct FormElements {
 
 typedef std::vector<FormElements*> FormElementsList;
 
+bool ShouldFillOnAccountSelect() {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableFillOnAccountSelect)) {
+    return true;
+  }
+
+  // This is made explicit in anticiption of experimental groups being added.
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableFillOnAccountSelect)) {
+    return false;
+  }
+
+  // TODO(jww): Add experimental groups check here.
+
+  return false;
+}
+
 // Helper to search the given form element for the specified input elements
 // in |data|, and add results to |result|.
 static bool FindFormInputElements(blink::WebFormElement* fe,
@@ -303,11 +322,8 @@ bool PasswordAutofillAgent::TextFieldDidEndEditing(
 
   // Do not set selection when ending an editing session, otherwise it can
   // mess with focus.
-  FillUserNameAndPassword(&username,
-                          &password,
-                          fill_data,
-                          true /* exact_username_match */,
-                          false /* set_selection */);
+  FillUserNameAndPassword(
+      &username, &password, fill_data, EXACT_USERNAME_MATCH);
   return true;
 }
 
@@ -814,7 +830,7 @@ void PasswordAutofillAgent::OnFillPasswordForm(
     // If wait_for_username is true, we don't want to initially fill the form
     // until the user types in a valid username.
     if (!form_data.wait_for_username)
-      FillFormOnPasswordRecieved(form_data, username_element, password_element);
+      FillFormOnPasswordReceived(form_data, username_element, password_element);
 
     // We might have already filled this form if there are two <form> elements
     // with identical markup.
@@ -920,7 +936,7 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
   return !suggestions.empty();
 }
 
-void PasswordAutofillAgent::FillFormOnPasswordRecieved(
+void PasswordAutofillAgent::FillFormOnPasswordReceived(
     const PasswordFormFillData& fill_data,
     blink::WebInputElement username_element,
     blink::WebInputElement password_element) {
@@ -937,8 +953,13 @@ void PasswordAutofillAgent::FillFormOnPasswordRecieved(
   if (!IsElementAutocompletable(password_element))
     return;
 
-  // Try to set the username to the preferred name, but only if the field
-  // can be set and isn't prefilled.
+  if (ShouldFillOnAccountSelect()) {
+    username_element.setAutofilled(true);
+    return;
+  }

[Garrett Casto, 2014/09/12 19:08:28]

This is going to break sites that hide the username field, like
accounts.google.com. If the username is hidden and prefilled we don't want to
delay filling passwords. I would think that this is still pretty safe in the
face of XSS as the attacker would need to prefill the username field with your
username, which seems reasonably difficult. Even if we want to introduce some
other way around this, for the moment I would put this clause inside the next
one, so that we only  abort filling if we would have filled in the username
ourselves.

[jww, 2014/09/15 22:48:00]

Hm, this is a tough one. I don't think this is safe at all in the XSS case.
Generally, we consider usernames public knowledge, and especially in the case
where an attacker has an XSS to lots of sites (which almost certainly have
usernames accessible and there's likely overlap).

I feel pretty strongly we should come up with a way around this. Why don't we
provide the accounts popup menu when the user clicks on the password field
anyway? Is that something we can do?

+
+  // Try to set the username to the preferred name, but only if the field can be
+  // set and isn't prefilled.
   if (IsElementAutocompletable(username_element) &&
       username_element.value().isEmpty()) {
     // TODO(tkent): Check maxlength and pattern.
@@ -947,19 +968,17 @@ void PasswordAutofillAgent::FillFormOnPasswordRecieved(
 
   // Fill if we have an exact match for the username. Note that this sets
   // username to autofilled.
-  FillUserNameAndPassword(&username_element,
-                          &password_element,
-                          fill_data,
-                          true /* exact_username_match */,
-                          false /* set_selection */);
+  FillUserNameAndPassword(
+      &username_element, &password_element, fill_data, EXACT_USERNAME_MATCH);
 }
 
 bool PasswordAutofillAgent::FillUserNameAndPassword(
     blink::WebInputElement* username_element,
     blink::WebInputElement* password_element,
     const PasswordFormFillData& fill_data,
-    bool exact_username_match,
-    bool set_selection) {
+    const int options) {
+  bool exact_username_match = (options & EXACT_USERNAME_MATCH) != 0;
+  bool set_selection = (options & SET_SELECTION) != 0;
   base::string16 current_username = username_element->value();
   // username and password will contain the match found if any.
   base::string16 username;
@@ -1064,11 +1083,7 @@ void PasswordAutofillAgent::PerformInlineAutocomplete(
 #if !defined(OS_ANDROID)
   // Fill the user and password field with the most relevant match. Android
   // only fills in the fields after the user clicks on the suggestion popup.
-  FillUserNameAndPassword(&username,
-                          &password,
-                          fill_data,
-                          false /* exact_username_match */,
-                          true /* set_selection */);
+  FillUserNameAndPassword(&username, &password, fill_data, SET_SELECTION);
 #endif
 }