Commit d9f324df authored by Maria Kazinova's avatar Maria Kazinova Committed by Commit Bot

Submission detection on change password form fields clearing.

Unlike login forms, change password forms are often kept on page after
successful submission. This CL allows to detect submission for change
password forms that are cleared field by field with
HTMLInputElement::SetValue(), by setting an empty value.

Bug: 1149948, 1147363
Change-Id: Ia5d8e27f91b4a9e36784517e87fff41167551ac4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537697
Commit-Queue: Maria Kazinova <kazinova@google.com>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarChristoph Schwering <schwering@google.com>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829855}
parent 97ea8201
......@@ -315,6 +315,7 @@ class PasswordManagerInteractiveTestSubmissionDetectionOnFormClear
base::test::ScopedFeatureList feature_list_;
};
// Tests that submission is detected when change password form is reset.
IN_PROC_BROWSER_TEST_F(
PasswordManagerInteractiveTestSubmissionDetectionOnFormClear,
ChangePwdFormCleared) {
......@@ -331,7 +332,7 @@ IN_PROC_BROWSER_TEST_F(
signin_form.password_value = base::ASCIIToUTF16("old_pw");
password_store->AddLogin(signin_form);
NavigateToFile("/password/password_form.html");
NavigateToFile("/password/cleared_change_password_forms.html");
// Fill a form and submit through a <input type="submit"> button.
std::unique_ptr<BubbleObserver> prompt_observer(
......@@ -358,6 +359,125 @@ IN_PROC_BROWSER_TEST_F(
1);
}
// Tests that submission is detected when all password fields in a change
// password form are cleared and not detected when only some fields are cleared.
IN_PROC_BROWSER_TEST_F(
PasswordManagerInteractiveTestSubmissionDetectionOnFormClear,
ChangePwdFormFieldsCleared) {
// At first let us save credentials to the PasswordManager.
scoped_refptr<password_manager::TestPasswordStore> password_store =
static_cast<password_manager::TestPasswordStore*>(
PasswordStoreFactory::GetForProfile(
browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS)
.get());
password_manager::PasswordForm signin_form;
signin_form.signon_realm = embedded_test_server()->base_url().spec();
signin_form.username_value = base::ASCIIToUTF16("temp");
signin_form.password_value = base::ASCIIToUTF16("old_pw");
password_store->AddLogin(signin_form);
for (bool all_fields_cleared : {false, true}) {
base::HistogramTester histogram_tester;
SCOPED_TRACE(testing::Message("#all_fields_cleared = ")
<< all_fields_cleared);
NavigateToFile("/password/cleared_change_password_forms.html");
// Fill a form and submit through a <input type="submit"> button.
std::unique_ptr<BubbleObserver> prompt_observer(
new BubbleObserver(WebContents()));
FillElementWithValue("chg_new_password_1", "new_pw", "new_pw");
FillElementWithValue("chg_new_password_2", "new_pw", "new_pw");
std::string submit =
all_fields_cleared
? "document.getElementById('chg_clear_all_fields_button').click();"
: "document.getElementById('chg_clear_some_fields_button').click()"
";";
ASSERT_TRUE(content::ExecuteScript(WebContents(), submit));
if (all_fields_cleared)
EXPECT_TRUE(prompt_observer->IsUpdatePromptShownAutomatically());
else
EXPECT_FALSE(prompt_observer->IsUpdatePromptShownAutomatically());
if (all_fields_cleared) {
// We emulate that the user clicks "Update" button.
prompt_observer->AcceptUpdatePrompt();
// Check that credentials are stored.
WaitForPasswordStore();
CheckThatCredentialsStored("temp", "new_pw");
histogram_tester.ExpectUniqueSample(
"PasswordManager.SuccessfulSubmissionIndicatorEvent",
autofill::mojom::SubmissionIndicatorEvent::
CHANGE_PASSWORD_FORM_CLEARED,
1);
}
}
}
// Tests that submission is detected when the new password field outside the
// form tag is cleared not detected when other password fields are cleared.
IN_PROC_BROWSER_TEST_F(
PasswordManagerInteractiveTestSubmissionDetectionOnFormClear,
ChangePwdFormRelevantFormlessFieldsCleared) {
base::HistogramTester histogram_tester;
// At first let us save credentials to the PasswordManager.
scoped_refptr<password_manager::TestPasswordStore> password_store =
static_cast<password_manager::TestPasswordStore*>(
PasswordStoreFactory::GetForProfile(
browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS)
.get());
password_manager::PasswordForm signin_form;
signin_form.signon_realm = embedded_test_server()->base_url().spec();
signin_form.username_value = base::ASCIIToUTF16("temp");
signin_form.password_value = base::ASCIIToUTF16("old_pw");
password_store->AddLogin(signin_form);
for (bool relevant_fields_cleared : {false, true}) {
SCOPED_TRACE(testing::Message("#relevant_fields_cleared = ")
<< relevant_fields_cleared);
NavigateToFile("/password/cleared_change_password_forms.html");
// Fill a form and submit through a <input type="submit"> button.
std::unique_ptr<BubbleObserver> prompt_observer(
new BubbleObserver(WebContents()));
FillElementWithValue("formless_chg_new_password_1", "new_pw", "new_pw");
FillElementWithValue("formless_chg_new_password_2", "new_pw", "new_pw");
std::string submit = relevant_fields_cleared
? "document.getElementById('chg_clear_all_"
"formless_fields_button').click();"
: "document.getElementById('chg_clear_some_"
"formless_fields_button').click();";
ASSERT_TRUE(content::ExecuteScript(WebContents(), submit));
if (relevant_fields_cleared) {
prompt_observer->WaitForAutomaticUpdatePrompt();
EXPECT_TRUE(prompt_observer->IsUpdatePromptShownAutomatically());
} else {
EXPECT_FALSE(prompt_observer->IsUpdatePromptShownAutomatically());
}
if (relevant_fields_cleared) {
// We emulate that the user clicks "Update" button.
prompt_observer->AcceptUpdatePrompt();
// Check that credentials are stored.
WaitForPasswordStore();
CheckThatCredentialsStored("temp", "new_pw");
histogram_tester.ExpectUniqueSample(
"PasswordManager.SuccessfulSubmissionIndicatorEvent",
autofill::mojom::SubmissionIndicatorEvent::
CHANGE_PASSWORD_FORM_CLEARED,
1);
}
}
}
#if BUILDFLAG(ENABLE_DICE_SUPPORT)
// This test suite only applies to Gaia signin page, and checks that the
// signin interception bubble and the password bubbles never conflict.
......
<html>
<body>
<!-- Change password form with username. -->
<form action="done.html" id="chg_testform">
<input type="text" id="chg_username_field" name="chg_username_field">
<input type="password" id="chg_password_field" name="chg_password_field">
<input type="password" id="chg_new_password_1" name="chg_new_password_1">
<input type="password" id="chg_new_password_2" name="chg_new_password_2">
<input type="submit" id="chg_submit_button" name="chg_submit_button">
</form>
<button id="chg_clear_button" name="chg_clear_button"
onclick="document.getElementById('chg_testform').reset()">
Clear the form on successful JS submission
</button>
<button id="chg_clear_all_fields_button" name="chg_clear_all_fields_button"
onclick="document.getElementById('chg_password_field').value = '';document.getElementById('chg_new_password_1').value = '';document.getElementById('chg_new_password_2').value = '';">
Clear all form fields on successful JS submission
</button>
<button id="chg_clear_some_fields_button" name="chg_clear_some_fields_button"
onclick="document.getElementById('chg_password_field').value = '';document.getElementById('chg_new_password_1').value = '';">
Clear some form fields
</button>
<!-- Formless change password fields. -->
<input type="password" id="formless_chg_password_field">
<input type="password" id="formless_chg_new_password_1" autocomplete="new-password">
<input type="password" id="formless_chg_new_password_2">
<button id="chg_clear_all_formless_fields_button"
onclick="document.getElementById('formless_chg_password_field').value = '';document.getElementById('formless_chg_new_password_1').value = '';document.getElementById('formless_chg_new_password_2').value = '';">
Clear relevant formless fields on successful submission
</button>
<button id="chg_clear_some_formless_fields_button"
onclick="document.getElementById('formless_chg_password_field').value = '';document.getElementById('formless_chg_new_password_2').value = '';">
Clear some formless fields
</button>
</body>
</html>
......@@ -74,7 +74,6 @@ needs to access them by their offsets in the array of the form children.
<!-- Change password form with username. -->
<form action="done.html" id="chg_testform">
<input type="text" id="chg_username_field" name="chg_username_field">
<input type="password" id="chg_password_field" name="chg_password_field">
<input type="password" id="chg_new_password_1" name="chg_new_password_1">
......@@ -82,11 +81,6 @@ needs to access them by their offsets in the array of the form children.
<input type="submit" id="chg_submit_button" name="chg_submit_button">
</form>
<button id="chg_clear_button" name="chg_clear_button"
onclick="document.getElementById('chg_testform').reset()">
Clear the form on successful JS submission!
</button>
<!-- Change password form without the username. -->
<form action="done.html" id="chg_testform_wo_username">
<input type="password" id="chg_password_wo_username_field" name="chg_password_wo_username_field">
......
......@@ -931,6 +931,10 @@ void AutofillAgent::FormElementReset(const WebFormElement& form) {
password_autofill_agent_->InformAboutFormClearing(form);
}
void AutofillAgent::PasswordFieldReset(const WebInputElement& element) {
password_autofill_agent_->InformAboutFieldClearing(element);
}
void AutofillAgent::SelectWasUpdated(
const blink::WebFormControlElement& element) {
// Look for the form and field associated with the select element. If they are
......
......@@ -196,6 +196,7 @@ class AutofillAgent : public content::RenderFrameObserver,
bool ShouldSuppressKeyboard(
const blink::WebFormControlElement& element) override;
void FormElementReset(const blink::WebFormElement& form) override;
void PasswordFieldReset(const blink::WebInputElement& element) override;
void HandleFocusChangeComplete();
......
......@@ -1384,23 +1384,47 @@ void PasswordAutofillAgent::InformAboutFormClearing(
FieldRendererId element_id(element.UniqueRendererFormControlId());
// Notify PasswordManager if |form| has password fields that have user typed
// input or input autofilled on user trigger.
if (element.FormControlTypeForAutofill() == "password" &&
(field_data_manager_->DidUserType(element_id) ||
field_data_manager_->WasAutofilledOnUserTrigger(element_id))) {
const form_util::ExtractMask extract_mask =
static_cast<form_util::ExtractMask>(form_util::EXTRACT_VALUE |
form_util::EXTRACT_OPTIONS);
FormData form_data;
if (WebFormElementToFormData(form, WebFormControlElement(),
field_data_manager_.get(), extract_mask,
&form_data, nullptr)) {
GetPasswordManagerDriver()->PasswordFormCleared(form_data);
}
if (IsPasswordFieldFilledByUser(element)) {
NotifyPasswordManagerAboutClearedForm(form);
return;
}
}
}
void PasswordAutofillAgent::InformAboutFieldClearing(
const WebInputElement& cleared_element) {
if (!FrameCanAccessPasswordManager())
return;
DCHECK(cleared_element.Value().IsEmpty());
if (!base::FeatureList::IsEnabled(
password_manager::features::kDetectFormSubmissionOnFormClear)) {
return;
}
FieldRendererId field_id(cleared_element.UniqueRendererFormControlId());
// Ignore fields that had no user input or autofill on user trigger.
if (!field_data_manager_->DidUserType(field_id) &&
!field_data_manager_->WasAutofilledOnUserTrigger(field_id)) {
return;
}
WebFormElement form = cleared_element.Form();
if (form.IsNull()) {
// Process password field clearing for fields outside the <form> tag.
GetPasswordManagerDriver()->PasswordFormCleared(
*GetFormDataFromUnownedInputElements());
return;
}
// Process field clearing for a form under a <form> tag.
// Only notify PasswordManager in case all user filled password fields were
// cleared.
bool cleared_all_password_fields = base::ranges::all_of(
form.GetFormControlElements(), [this](const auto& el) {
return !IsPasswordFieldFilledByUser(el) || el.Value().IsEmpty();
});
if (cleared_all_password_fields)
NotifyPasswordManagerAboutClearedForm(form);
}
////////////////////////////////////////////////////////////////////////////////
// PasswordAutofillAgent, private:
......@@ -1889,4 +1913,24 @@ bool PasswordAutofillAgent::CanShowPopupWithoutPasswords(
IsElementEditable(password_element);
}
bool PasswordAutofillAgent::IsPasswordFieldFilledByUser(
const WebFormControlElement& element) const {
FieldRendererId element_id(element.UniqueRendererFormControlId());
return element.FormControlTypeForAutofill() == "password" &&
(field_data_manager_->DidUserType(element_id) ||
field_data_manager_->WasAutofilledOnUserTrigger(element_id));
}
void PasswordAutofillAgent::NotifyPasswordManagerAboutClearedForm(
const WebFormElement& cleared_form) {
const auto extract_mask = static_cast<form_util::ExtractMask>(
form_util::EXTRACT_VALUE | form_util::EXTRACT_OPTIONS);
FormData form_data;
if (WebFormElementToFormData(cleared_form, WebFormControlElement(),
field_data_manager_.get(), extract_mask,
&form_data, nullptr)) {
GetPasswordManagerDriver()->PasswordFormCleared(form_data);
}
}
} // namespace autofill
......@@ -223,10 +223,16 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
std::unique_ptr<FormData> GetFormDataFromUnownedInputElements();
// Notification that form was cleared. This can be used as a signal of
// a successful submission for change password forms.
// Notification that form element was cleared by HTMLFormElement::reset()
// method. This can be used as a signal of a successful submission for change
// password forms.
void InformAboutFormClearing(const blink::WebFormElement& form);
// Notification that input element was cleared by HTMLInputValue::SetValue()
// method by setting an empty value. This can be used as a signal of a
// successful submission for change password forms.
void InformAboutFieldClearing(const blink::WebInputElement& element);
bool logging_state_active() const { return logging_state_active_; }
// Determine whether the current frame is allowed to access the password
......@@ -471,6 +477,15 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
bool CanShowPopupWithoutPasswords(
const blink::WebInputElement& password_element) const;
// Returns true if the element is of type 'password' and has either user typed
// input or input autofilled on user trigger.
bool IsPasswordFieldFilledByUser(
const blink::WebFormControlElement& element) const;
// Extracts and sends the form data of |cleared_form| to PasswordManager.
void NotifyPasswordManagerAboutClearedForm(
const blink::WebFormElement& cleared_form);
// The logins we have filled so far with their associated info.
WebInputToPasswordInfoMap web_input_to_password_info_;
// A (sort-of) reverse map to |web_input_to_password_info_|.
......
......@@ -14,6 +14,7 @@
#include "base/feature_list.h"
#include "base/metrics/field_trial.h"
#include "base/metrics/histogram_macros.h"
#include "base/ranges/algorithm.h"
#include "base/strings/strcat.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
......@@ -61,6 +62,7 @@ using autofill::NOT_USERNAME;
using autofill::SINGLE_USERNAME;
using autofill::UNKNOWN_TYPE;
using autofill::USERNAME;
using autofill::mojom::SubmissionIndicatorEvent;
using base::NumberToString;
using BlacklistedStatus =
password_manager::OriginCredentialStore::BlacklistedStatus;
......@@ -1231,16 +1233,27 @@ void PasswordManager::ResetPendingCredentials() {
void PasswordManager::OnPasswordFormCleared(
PasswordManagerDriver* driver,
const autofill::FormData& form_data) {
// Find a form with corresponding renderer id.
auto it = base::ranges::find_if(form_managers_, [&](auto& manager) {
return manager->DoesManageAccordingToRendererId(
form_data.unique_renderer_id, driver);
});
if (it != form_managers_.end() && (*it)->is_submitted() &&
(*it)->GetSubmittedForm()->IsPossibleChangePasswordForm()) {
(*it)->UpdateSubmissionIndicatorEvent(
autofill::mojom::SubmissionIndicatorEvent::
CHANGE_PASSWORD_FORM_CLEARED);
PasswordFormManager* manager = GetMatchedManager(driver, form_data);
if (!manager || !manager->is_submitted() ||
!manager->GetSubmittedForm()->IsPossibleChangePasswordForm()) {
return;
}
// If a password form was cleared, login is successful.
if (form_data.is_form_tag) {
manager->UpdateSubmissionIndicatorEvent(
SubmissionIndicatorEvent::CHANGE_PASSWORD_FORM_CLEARED);
OnLoginSuccessful();
return;
}
// If password fields outside the <form> tag were cleared, it should be
// verified that fields are relevant.
FieldRendererId new_password_field_id =
manager->GetSubmittedForm()->new_password_element_renderer_id;
auto it = base::ranges::find(form_data.fields, new_password_field_id,
&autofill::FormFieldData::unique_renderer_id);
if (it != form_data.fields.end() && it->value.empty()) {
manager->UpdateSubmissionIndicatorEvent(
SubmissionIndicatorEvent::CHANGE_PASSWORD_FORM_CLEARED);
OnLoginSuccessful();
}
}
......
......@@ -3739,8 +3739,6 @@ TEST_P(PasswordManagerTest, SubmissionDetectedOnClearedForm) {
old_password_field.value = ASCIIToUTF16("oldpass");
form_data.fields.push_back(old_password_field);
// Form changes: new and confirmation password fields are added by the
// website's scripts.
FormFieldData new_password_field;
new_password_field.form_control_type = "password";
new_password_field.unique_renderer_id = FieldRendererId(2);
......@@ -3767,6 +3765,67 @@ TEST_P(PasswordManagerTest, SubmissionDetectedOnClearedForm) {
.WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save)));
manager()->OnPasswordFormCleared(&driver_, form_data);
}
TEST_P(PasswordManagerTest, SubmissionDetectedOnClearedFormlessFields) {
EXPECT_CALL(client_, IsSavingAndFillingEnabled).WillRepeatedly(Return(true));
PasswordForm saved_match(MakeSavedForm());
EXPECT_CALL(*store_, GetLogins)
.WillRepeatedly(WithArg<1>(InvokeConsumer(store_.get(), saved_match)));
for (bool new_password_field_was_cleared : {true, false}) {
SCOPED_TRACE(testing::Message("#new password field was cleared = ")
<< new_password_field_was_cleared);
// Create FormData for a form with 1 password field and process it.
FormData form_data;
form_data.is_form_tag = false;
form_data.unique_renderer_id = FormRendererId(0);
form_data.url = GURL("http://www.google.com/a/LoginAuth");
FormFieldData old_password_field;
old_password_field.form_control_type = "password";
old_password_field.unique_renderer_id = FieldRendererId(1);
old_password_field.name = ASCIIToUTF16("oldpass");
old_password_field.value = ASCIIToUTF16("oldpass");
form_data.fields.push_back(old_password_field);
FormFieldData new_password_field;
new_password_field.form_control_type = "password";
new_password_field.unique_renderer_id = FieldRendererId(2);
new_password_field.name = ASCIIToUTF16("newpass");
new_password_field.autocomplete_attribute = "new-password";
form_data.fields.push_back(new_password_field);
FormFieldData confirm_password_field;
confirm_password_field.form_control_type = "password";
confirm_password_field.unique_renderer_id = FieldRendererId(3);
confirm_password_field.name = ASCIIToUTF16("confpass");
form_data.fields.push_back(confirm_password_field);
manager()->OnPasswordFormsParsed(&driver_, {form_data});
form_data.fields[0].value = ASCIIToUTF16("oldpass");
form_data.fields[1].value = ASCIIToUTF16("newpass");
form_data.fields[2].value = ASCIIToUTF16("newpass");
manager()->OnInformAboutUserInput(&driver_, form_data);
form_data.fields[0].value = base::string16();
form_data.fields[2].value = base::string16();
if (new_password_field_was_cleared)
form_data.fields[1].value = base::string16();
std::unique_ptr<PasswordFormManagerForUI> form_manager_to_save;
if (new_password_field_was_cleared) {
EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr)
.WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save)));
} else {
EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr).Times(0);
}
manager()->OnPasswordFormCleared(&driver_, form_data);
}
}
#endif // !defined(OS_IOS)
TEST_P(PasswordManagerTest, IsFormManagerPendingPasswordUpdate) {
......
......@@ -75,6 +75,10 @@ class WebAutofillClient {
// Called when the given form element is reset.
virtual void FormElementReset(const WebFormElement&) {}
// Called when the empty value is set for the given input element, which is
// or has been a password field.
virtual void PasswordFieldReset(const WebInputElement& element) {}
protected:
virtual ~WebAutofillClient() = default;
};
......
......@@ -1193,8 +1193,13 @@ void HTMLInputElement::setValue(const String& value,
selection);
input_type_view_->DidSetValue(sanitized_value, value_changed);
if (value_changed)
if (value_changed) {
NotifyFormStateChanged();
if (value.IsEmpty() && HasBeenPasswordField()) {
GetDocument().GetFrame()->GetPage()->GetChromeClient().PasswordFieldReset(
*this);
}
}
}
void HTMLInputElement::SetNonAttributeValue(const String& sanitized_value) {
......
......@@ -528,6 +528,8 @@ class CORE_EXPORT ChromeClient : public GarbageCollected<ChromeClient> {
virtual void FormElementReset(HTMLFormElement& element) {}
virtual void PasswordFieldReset(HTMLInputElement& element) {}
protected:
ChromeClient() = default;
......
......@@ -1186,4 +1186,11 @@ void ChromeClientImpl::FormElementReset(HTMLFormElement& element) {
fill_client->FormElementReset(WebFormElement(&element));
}
void ChromeClientImpl::PasswordFieldReset(HTMLInputElement& element) {
if (auto* fill_client =
AutofillClientFromFrame(element.GetDocument().GetFrame())) {
fill_client->PasswordFieldReset(WebInputElement(&element));
}
}
} // namespace blink
......@@ -289,6 +289,8 @@ class CORE_EXPORT ChromeClientImpl final : public ChromeClient {
void FormElementReset(HTMLFormElement& element) override;
void PasswordFieldReset(HTMLInputElement& element) override;
private:
bool IsChromeClientImpl() const override { return true; }
......
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