Skip to content

Commit

Permalink
[Passwords] Add finch flag for fallback filling of credit card fields
Browse files Browse the repository at this point in the history
Finch gate https://crrev.com/c/4349933 that is disabled by default and
is a kill switch.


Bug: 1425028
Change-Id: I2b4a1612ed30a4d99ef6d38e75a00584c5d44d48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4370150
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1122088}
  • Loading branch information
Din Nezametdinov authored and Chromium LUCI CQ committed Mar 25, 2023
1 parent 20d8b73 commit 48ee3ab
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 1 deletion.
Expand Up @@ -615,7 +615,9 @@ std::vector<const FormFieldData*> GetRelevantPasswords(
// don't keep CC fields even for fallback filling and let non-password
// Autofill handle these fields. Fallback saving is fine because saving UIs of
// Autofill and the password manager are not mutually exclusive.
if (mode == FormDataParser::Mode::kFilling) {
if (mode == FormDataParser::Mode::kFilling &&
base::FeatureList::IsEnabled(
password_manager::features::kDisablePasswordsDropdownForCvcFields)) {
base::EraseIf(passwords, [](const ProcessedField* processed_field) {
// TODO(crbug/1425423): This code does not use |StringMatchesCVC| because
// the underlying regex has a high false positive rate, i.e. matches many
Expand Down
Expand Up @@ -1021,6 +1021,9 @@ TEST(FormParserTest, DisabledFields) {
}

TEST(FormParserTest, SkippingFieldsWithCreditCardFields) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
password_manager::features::kDisablePasswordsDropdownForCvcFields);
CheckTestData({
{
.description_for_logging =
Expand Down Expand Up @@ -1771,6 +1774,9 @@ TEST(FormParserTest, ComplementingResults) {

// The parser should avoid identifying CVC fields as passwords.
TEST(FormParserTest, IgnoreCvcFields) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
password_manager::features::kDisablePasswordsDropdownForCvcFields);
CheckTestData({
{
.description_for_logging =
Expand Down Expand Up @@ -1876,6 +1882,9 @@ TEST(FormParserTest, ServerHintsForCvcFieldsOverrideAutocomplete) {
// relatively safe as it should be unlikely that the server misclassifies a
// field as a CC Number field.
TEST(FormParserTest, CCNumber) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
password_manager::features::kDisablePasswordsDropdownForCvcFields);
CheckTestData({
{
.description_for_logging = "Server hints: CREDIT_CARD_NUMBER.",
Expand Down
Expand Up @@ -3381,6 +3381,9 @@ TEST_P(PasswordManagerTest, FillingAndSavingFallbacksOnNonPasswordForm) {
// overwritten with a server override, but it is not tested in this test). For
// saving, only the fallback is available.
TEST_P(PasswordManagerTest, FillingAndSavingFallbacksOnCreditCardForm) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
password_manager::features::kDisablePasswordsDropdownForCvcFields);
PasswordFormManager::set_wait_for_server_predictions_for_filling(false);
EXPECT_CALL(client_, IsSavingAndFillingEnabled(_))
.WillRepeatedly(Return(true));
Expand Down Expand Up @@ -3415,6 +3418,54 @@ TEST_P(PasswordManagerTest, FillingAndSavingFallbacksOnCreditCardForm) {
task_environment_.RunUntilIdle();
}

// TODO(crbug.com/1425028): Remove the test once
// |kDisablePasswordsDropdownForCvcFields| is launched.
// Same as |FillingAndSavingFallbacksOnCreditCardForm|, but password filling is
// suggested because |kDisablePasswordsDropdownForCvcFields| is disabled.
TEST_P(PasswordManagerTest,
FillingAndSavingFallbacksOnCreditCardForm_OldBehavior) {
PasswordFormManager::set_wait_for_server_predictions_for_filling(false);
EXPECT_CALL(client_, IsSavingAndFillingEnabled(_))
.WillRepeatedly(Return(true));

PasswordForm saved_match(MakeSimpleForm());
store_->AddLogin(saved_match);
PasswordForm credit_card_form(MakeSimpleCreditCardForm());
credit_card_form.only_for_fallback = true;

PasswordFormFillData form_data;
EXPECT_CALL(driver_, SetPasswordFillData)
.WillRepeatedly(SaveArg<0>(&form_data));

manager()->OnPasswordFormsParsed(&driver_, {credit_card_form.form_data});
task_environment_.RunUntilIdle();

// Check that manual filling fallback available.
EXPECT_EQ(saved_match.username_value,
form_data.preferred_login.username_value);
EXPECT_EQ(saved_match.password_value,
form_data.preferred_login.password_value);
// Check that no automatic filling available.
EXPECT_TRUE(form_data.username_element_renderer_id.is_null());
EXPECT_TRUE(form_data.password_element_renderer_id.is_null());

// Check that saving fallback is available.
std::unique_ptr<PasswordFormManagerForUI> form_manager_to_save;
EXPECT_CALL(client_, ShowManualFallbackForSavingPtr(_, false, false))
.WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save)));
manager()->OnInformAboutUserInput(&driver_, credit_card_form.form_data);
ASSERT_TRUE(form_manager_to_save);
EXPECT_THAT(form_manager_to_save->GetPendingCredentials(),
FormMatches(credit_card_form));

// Check that no automatic save prompt is shown.
OnPasswordFormSubmitted(credit_card_form.form_data);
EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0);
manager()->DidNavigateMainFrame(true);
manager()->OnPasswordFormsRendered(&driver_, {});
task_environment_.RunUntilIdle();
}

// Check that on successful login the credentials are checked for leak.
TEST_P(PasswordManagerTest, StartLeakDetection) {
auto mock_factory =
Expand Down
Expand Up @@ -30,6 +30,12 @@ BASE_FEATURE(kBiometricTouchToFill,
"BiometricTouchToFill",
base::FEATURE_DISABLED_BY_DEFAULT);

// Disables fallback filling if the server or the autocomplete attribute says it
// is a credit card field.
BASE_FEATURE(kDisablePasswordsDropdownForCvcFields,
"DisablePasswordsDropdownForCvcFields",
base::FEATURE_DISABLED_BY_DEFAULT);

// Enables the overwriting of prefilled username fields if the server predicted
// the field to contain a placeholder value.
BASE_FEATURE(kEnableOverwritingPlaceholderUsernames,
Expand Down
Expand Up @@ -29,6 +29,7 @@ BASE_DECLARE_FEATURE(kBiometricAuthenticationForFilling);
BASE_DECLARE_FEATURE(kBiometricAuthenticationInSettings);
#endif
BASE_DECLARE_FEATURE(kBiometricTouchToFill);
BASE_DECLARE_FEATURE(kDisablePasswordsDropdownForCvcFields);
BASE_DECLARE_FEATURE(kEnableOverwritingPlaceholderUsernames);

BASE_DECLARE_FEATURE(kEnablePasswordsAccountStorage);
Expand Down
20 changes: 20 additions & 0 deletions testing/variations/fieldtrial_testing_config.json
Expand Up @@ -4245,6 +4245,26 @@
]
}
],
"DisablePasswordsDropdownForCvcFields": [
{
"platforms": [
"android",
"chromeos",
"ios",
"linux",
"mac",
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"DisablePasswordsDropdownForCvcFields"
]
}
]
}
],
"DisableResourceScheduler": [
{
"platforms": [
Expand Down

0 comments on commit 48ee3ab

Please sign in to comment.