Commit 02f40d91 authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Fix false saving positives on accounts.google.com.

In the following case false positive on accounts.google.com happens:

1.Suppose that the user has 2 Gaia credentials (with usernames and passwords
u1/p1 and u2/p2). Let u1 be saved in Password Manager.

2.The user go to accounts.google.com, u1/p1 are autofilled by CPM (a username
field is visible, a password field is invisible).

3.The user is typing u2 in the username field and is clicking next button.

4.At that moment the page removes password form from the DOM and Password
Manager incorrectly thinks that it was successful submission with u2/p1

Video is on bug 764663 (actual_bubble).

This CL fixes this by ignoring for saving accounts.google.com forms without
a visible password field.

Similar issues might be on different sites, this CL fixes only
accounts.google.com, because
1.A general solution is unlikely without more complex changes and server-side
support.
2.accounts.google.com is crucial for Chrome, in particular because it serves
the Chrome sign-in page.


Bug: 758155, 764663, 880876

Change-Id: I57bbe94ea0a6d101c6a5a33cf354b42f0d0353fc
Reviewed-on: https://chromium-review.googlesource.com/1206476
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588907}
parent 92be1b27
...@@ -250,6 +250,16 @@ bool ShouldPromptUserToSavePassword( ...@@ -250,6 +250,16 @@ bool ShouldPromptUserToSavePassword(
return manager.IsNewLogin(); return manager.IsNewLogin();
} }
// Checks that |form| has visible password fields. It should be used only for
// GAIA forms.
bool IsThereVisiblePasswordField(const FormData& form) {
for (const autofill::FormFieldData& field : form.fields) {
if (field.form_control_type == "password" && field.is_focusable)
return true;
}
return false;
}
} // namespace } // namespace
// static // static
...@@ -515,6 +525,16 @@ void PasswordManager::OnPasswordFormSubmittedNoChecks( ...@@ -515,6 +525,16 @@ void PasswordManager::OnPasswordFormSubmittedNoChecks(
logger.LogMessage(Logger::STRING_ON_SAME_DOCUMENT_NAVIGATION); logger.LogMessage(Logger::STRING_ON_SAME_DOCUMENT_NAVIGATION);
} }
if (gaia::IsGaiaSignonRealm(GURL(password_form.signon_realm)) &&
!IsThereVisiblePasswordField(password_form.form_data)) {
// Gaia form without visible password fields is found.
// It might happen only when Password Manager autofilled a username
// (visible) and a password (invisible) fields. Then the user typed a new
// username. A page removed the form. As result a form is inconsistent - the
// username from one account, the password from another. Skip such form.
return;
}
if (is_new_form_parsing_for_saving_enabled_) if (is_new_form_parsing_for_saving_enabled_)
ProcessSubmittedForm(password_form.form_data, driver); ProcessSubmittedForm(password_form.form_data, driver);
......
...@@ -2627,4 +2627,26 @@ TEST_F(PasswordManagerTest, ProcessingOtherSubmissionTypes) { ...@@ -2627,4 +2627,26 @@ TEST_F(PasswordManagerTest, ProcessingOtherSubmissionTypes) {
manager()->OnPasswordFormSubmittedNoChecks(&driver_, submitted_form); manager()->OnPasswordFormSubmittedNoChecks(&driver_, submitted_form);
} }
TEST_F(PasswordManagerTest, SubmittedGaiaFormWithoutVisiblePasswordField) {
// Tests that a submitted GAIA sign-in form which does not contain a visible
// password field is skipped.
std::vector<PasswordForm> observed;
PasswordForm form(MakeSimpleGAIAForm());
observed.push_back(form);
EXPECT_CALL(*store_, GetLogins(_, _))
.WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms()));
manager()->OnPasswordFormsParsed(&driver_, observed);
manager()->OnPasswordFormsRendered(&driver_, observed, true);
EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
.WillRepeatedly(Return(true));
form.username_value = ASCIIToUTF16("username");
form.password_value = ASCIIToUTF16("password");
form.form_data.fields[1].is_focusable = false;
EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0);
manager()->OnPasswordFormSubmittedNoChecks(&driver_, form);
}
} // namespace password_manager } // namespace password_manager
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