Commit 334c6a3d authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Do not fill passwords on passwords.google.com authentication form

Bug: 747828
Change-Id: I1d531f2288017b64b6c43f43f855c02e6ad74732
Reviewed-on: https://chromium-review.googlesource.com/588790Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490769}
parent d36a373e
...@@ -3028,4 +3028,33 @@ TEST_F(PasswordAutofillAgentTest, NoForm_MultipleAJAXEventsWithoutSubmission) { ...@@ -3028,4 +3028,33 @@ TEST_F(PasswordAutofillAgentTest, NoForm_MultipleAJAXEventsWithoutSubmission) {
ASSERT_FALSE(static_cast<bool>(fake_driver_.password_form_submitted())); ASSERT_FALSE(static_cast<bool>(fake_driver_.password_form_submitted()));
} }
// Tests that information about Gaia reauthentication form is not sent to the
// browser, nor on load nor on user click.
TEST_F(PasswordAutofillAgentTest, GaiaReauthenticationFormIgnored) {
// HTML is already loaded in test SetUp method, so information about password
// forms was already sent to the |fake_drive_|. Hence it should be reset.
fake_driver_.reset_password_forms_calls();
const char kGaiaReauthenticationFormHTML[] =
"<FORM id='ReauthenticationForm'>"
" <INPUT type='hidden' name='continue' "
"value='https://passwords.google.com/'>"
" <INPUT type='hidden' name='rart'>"
" <INPUT type='password' id='password'/>"
" <INPUT type='submit' value='Login'/>"
"</FORM>";
LoadHTMLWithUrlOverride(kGaiaReauthenticationFormHTML,
"https://accounts.google.com");
UpdateOnlyPasswordElement();
// Simulate a user clicking on the password element.
static_cast<PageClickListener*>(autofill_agent_)
->FormControlElementClicked(password_element_, false);
// Check that no information about Gaia reauthentication is not sent.
EXPECT_FALSE(fake_driver_.called_password_forms_parsed());
EXPECT_FALSE(fake_driver_.called_password_forms_rendered());
}
} // namespace autofill } // namespace autofill
...@@ -1201,6 +1201,11 @@ void PasswordAutofillAgent::SendPasswordForms(bool only_visible) { ...@@ -1201,6 +1201,11 @@ void PasswordAutofillAgent::SendPasswordForms(bool only_visible) {
std::vector<PasswordForm> password_forms; std::vector<PasswordForm> password_forms;
for (const blink::WebFormElement& form : forms) { for (const blink::WebFormElement& form : forms) {
if (IsGaiaReauthenticationForm(form)) {
// Bail if this is a GAIA passwords site reauthentication form, so that
// page will be ignored.
return;
}
if (only_visible) { if (only_visible) {
bool is_form_visible = form_util::AreFormContentsVisible(form); bool is_form_visible = form_util::AreFormContentsVisible(form);
if (logger) { if (logger) {
......
...@@ -306,8 +306,6 @@ void FindPredictedElements( ...@@ -306,8 +306,6 @@ void FindPredictedElements(
} }
} }
// TODO(crbug.com/543085): Move the reauthentication recognition code to the
// browser.
const char kPasswordSiteUrlRegex[] = const char kPasswordSiteUrlRegex[] =
"passwords(?:-[a-z-]+\\.corp)?\\.google\\.com"; "passwords(?:-[a-z-]+\\.corp)?\\.google\\.com";
...@@ -394,15 +392,6 @@ bool GetPasswordForm( ...@@ -394,15 +392,6 @@ bool GetPasswordForm(
last_text_input_before_password; last_text_input_before_password;
autofill::PossibleUsernamesVector other_possible_usernames; autofill::PossibleUsernamesVector other_possible_usernames;
// Bail if this is a GAIA passwords site reauthentication form, so that
// the form will be ignored.
// TODO(msramek): Move this logic to the browser, and disable filling only
// for the sync credential and if passwords are being synced.
if (IsGaiaReauthenticationForm(GURL(form.document.Url()).GetOrigin(),
form.control_elements)) {
return false;
}
std::map<WebInputElement, PasswordFormFieldPredictionType> predicted_elements; std::map<WebInputElement, PasswordFormFieldPredictionType> predicted_elements;
if (form_predictions) { if (form_predictions) {
FindPredictedElements(form, password_form->form_data, *form_predictions, FindPredictedElements(form, password_form->form_data, *form_predictions,
...@@ -649,16 +638,19 @@ bool GetPasswordForm( ...@@ -649,16 +638,19 @@ bool GetPasswordForm(
} // namespace } // namespace
bool IsGaiaReauthenticationForm( bool IsGaiaReauthenticationForm(const blink::WebFormElement& form) {
const GURL& origin, if (GURL(form.GetDocument().Url()).GetOrigin() !=
const std::vector<blink::WebFormControlElement>& control_elements) { GaiaUrls::GetInstance()->gaia_url().GetOrigin()) {
if (origin != GaiaUrls::GetInstance()->gaia_url().GetOrigin())
return false; return false;
}
bool has_rart_field = false; bool has_rart_field = false;
bool has_continue_field = false; bool has_continue_field = false;
for (const blink::WebFormControlElement& element : control_elements) { blink::WebVector<blink::WebFormControlElement> web_control_elements;
form.GetFormControlElements(web_control_elements);
for (const blink::WebFormControlElement& element : web_control_elements) {
// We're only interested in the presence // We're only interested in the presence
// of <input type="hidden" /> elements. // of <input type="hidden" /> elements.
CR_DEFINE_STATIC_LOCAL(WebString, kHidden, ("hidden")); CR_DEFINE_STATIC_LOCAL(WebString, kHidden, ("hidden"));
......
...@@ -28,13 +28,8 @@ namespace autofill { ...@@ -28,13 +28,8 @@ namespace autofill {
struct PasswordForm; struct PasswordForm;
// Tests whether the given form is a GAIA reauthentication form. The form is // Tests whether the given form is a GAIA reauthentication form.
// not passed directly as WebFormElement, but by specifying its |origin| and bool IsGaiaReauthenticationForm(const blink::WebFormElement& form);
// |control_elements|. This is for better performance and easier testing.
// TODO(msramek): Move this logic to the browser.
bool IsGaiaReauthenticationForm(
const GURL& origin,
const std::vector<blink::WebFormControlElement>& control_elements);
typedef std::map< typedef std::map<
const blink::WebFormControlElement, const blink::WebFormControlElement,
......
...@@ -191,7 +191,7 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest { ...@@ -191,7 +191,7 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest {
FormsPredictionsMap* predictions, FormsPredictionsMap* predictions,
bool with_user_input) { bool with_user_input) {
WebFormElement form; WebFormElement form;
LoadWebFormFromHTML(html, &form); LoadWebFormFromHTML(html, &form, nullptr);
WebVector<WebFormControlElement> control_elements; WebVector<WebFormControlElement> control_elements;
form.GetFormControlElements(control_elements); form.GetFormControlElements(control_elements);
...@@ -218,7 +218,7 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest { ...@@ -218,7 +218,7 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest {
const std::map<int, PasswordFormFieldPredictionType>& const std::map<int, PasswordFormFieldPredictionType>&
predictions_positions) { predictions_positions) {
WebFormElement form; WebFormElement form;
LoadWebFormFromHTML(html, &form); LoadWebFormFromHTML(html, &form, nullptr);
FormData form_data; FormData form_data;
ASSERT_TRUE(form_util::WebFormElementToFormData( ASSERT_TRUE(form_util::WebFormElementToFormData(
...@@ -239,8 +239,13 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest { ...@@ -239,8 +239,13 @@ class MAYBE_PasswordFormConversionUtilsTest : public content::RenderViewTest {
} }
// Loads the given |html| and retrieves the sole WebFormElement from it. // Loads the given |html| and retrieves the sole WebFormElement from it.
void LoadWebFormFromHTML(const std::string& html, WebFormElement* form) { void LoadWebFormFromHTML(const std::string& html,
LoadHTML(html.c_str()); WebFormElement* form,
const char* origin) {
if (origin)
LoadHTMLWithUrlOverride(html.c_str(), origin);
else
LoadHTML(html.c_str());
WebLocalFrame* frame = GetMainFrame(); WebLocalFrame* frame = GetMainFrame();
ASSERT_NE(nullptr, frame); ASSERT_NE(nullptr, frame);
...@@ -1354,12 +1359,6 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, IsGaiaReauthFormIgnored) { ...@@ -1354,12 +1359,6 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, IsGaiaReauthFormIgnored) {
{TestCase::KeyValue("continue", "passwords.google.com:443"), {TestCase::KeyValue("continue", "passwords.google.com:443"),
TestCase::KeyValue("rart", "")}, TestCase::KeyValue("rart", "")},
true}, true},
// Encoded URL is considered the same.
{"https://accounts.google.com",
{TestCase::KeyValue("continue",
"https://passwords.%67oogle.com/settings"),
TestCase::KeyValue("rart", "")},
true},
// Fully qualified domain should work as well. // Fully qualified domain should work as well.
{"https://accounts.google.com", {"https://accounts.google.com",
{TestCase::KeyValue("continue", {TestCase::KeyValue("continue",
...@@ -1391,15 +1390,9 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, IsGaiaReauthFormIgnored) { ...@@ -1391,15 +1390,9 @@ TEST_F(MAYBE_PasswordFormConversionUtilsTest, IsGaiaReauthFormIgnored) {
} }
std::string html = builder->ProduceHTML(); std::string html = builder->ProduceHTML();
WebFormElement form; WebFormElement form;
LoadWebFormFromHTML(html, &form); LoadWebFormFromHTML(html, &form, test_case.origin);
std::vector<WebFormControlElement> control_elements;
WebVector<blink::WebFormControlElement> web_control_elements;
form.GetFormControlElements(web_control_elements);
control_elements.assign(web_control_elements.begin(),
web_control_elements.end());
EXPECT_EQ(test_case.expected_form_is_reauth, EXPECT_EQ(test_case.expected_form_is_reauth,
IsGaiaReauthenticationForm(GURL(test_case.origin).GetOrigin(), IsGaiaReauthenticationForm(form));
control_elements));
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment