Skip to content

Commit

Permalink
Add re-auth opt-in PersonalDataManager API (2/4)
Browse files Browse the repository at this point in the history
This CL adds an API for the PersonalDataManager to communicate with the
autofill prefs that will be used for the re-auth opt-in flow. It
supports the functionality of opting in to re-auth, checking if we
should show the opt-in promo, and incrementing the max promo shown
counter.

Bug: 1427216
Change-Id: I021bc814d21c0530f0907087ae4c8416829b49c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555994
Commit-Queue: Vinny Persky <vinnypersky@google.com>
Reviewed-by: Siyu An <siyua@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Christoph Schwering <schwering@google.com>
Cr-Commit-Position: refs/heads/main@{#1153511}
  • Loading branch information
Vinny Persky authored and Chromium LUCI CQ committed Jun 5, 2023
1 parent c3713a2 commit d379384
Show file tree
Hide file tree
Showing 12 changed files with 160 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public boolean isPreferenceClickDisabled(Preference preference) {
mandatoryReauthSwitch.setSummary(
R.string.autofill_settings_page_enable_payment_method_mandatory_reauth_sublabel);
mandatoryReauthSwitch.setChecked(
PersonalDataManager.isAutofillPaymentMethodsMandatoryReauthEnabled());
PersonalDataManager.isPaymentMethodsMandatoryReauthEnabled());
mandatoryReauthSwitch.setKey(PREF_MANDATORY_REAUTH);
mandatoryReauthSwitch.setOnPreferenceChangeListener(
this::onMandatoryReauthSwitchToggled);
Expand Down Expand Up @@ -190,7 +190,7 @@ public boolean isPreferenceClickDisabled(Preference preference) {
if (card.getIsLocal()) {
if (ChromeFeatureList.isEnabled(
ChromeFeatureList.AUTOFILL_ENABLE_PAYMENTS_MANDATORY_REAUTH)
&& PersonalDataManager.isAutofillPaymentMethodsMandatoryReauthEnabled()) {
&& PersonalDataManager.isPaymentMethodsMandatoryReauthEnabled()) {
// When mandatory reauth is enabled, we require additional authentication before
// user can view/edit local card.
card_pref.setOnPreferenceClickListener(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,7 @@ public static void setAutofillCreditCardFidoAuthEnabled(boolean enable) {
/**
* @return Whether the Autofill feature for payment methods mandatory reauth is enabled.
*/
public static boolean isAutofillPaymentMethodsMandatoryReauthEnabled() {
public static boolean isPaymentMethodsMandatoryReauthEnabled() {
return getPrefService().getBoolean(Pref.AUTOFILL_PAYMENT_METHODS_MANDATORY_REAUTH);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ void AutofillPrivateAuthenticateUserAndFlipMandatoryAuthToggleFunction::
if (reauth_succeeded && browser_context()) {
PrefService* prefs =
Profile::FromBrowserContext(browser_context())->GetPrefs();
autofill::prefs::SetAutofillPaymentMethodsMandatoryReauth(
autofill::prefs::SetPaymentMethodsMandatoryReauthEnabled(
prefs, !prefs->GetBoolean(
autofill::prefs::kAutofillPaymentMethodsMandatoryReauth));
base::RecordAction(base::UserMetricsAction(
Expand Down Expand Up @@ -922,7 +922,7 @@ AutofillPrivateAuthenticateUserToEditLocalCardFunction::Run() {
if (!personal_data_manager || !personal_data_manager->IsDataLoaded()) {
return RespondNow(Error(kErrorDataUnavailable));
}
if (personal_data_manager->IsAutofillPaymentMethodsMandatoryReauthEnabled()) {
if (personal_data_manager->IsPaymentMethodsMandatoryReauthEnabled()) {
// If `device_authenticator` is not available, then don't do anything.
scoped_refptr<device_reauth::DeviceAuthenticator> device_authenticator =
client->GetDeviceAuthenticator();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,7 @@ IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest,
base::MakeRefCounted<device_reauth::MockDeviceAuthenticator>();
TestChromeAutofillClient* test_client = autofill_client();

test_personal_data_manager_->SetAutofillPaymentMethodsMandatoryReauthEnabled(
true);
test_personal_data_manager_->SetPaymentMethodsMandatoryReauthEnabled(true);
test_client->SetPersonalDataManger(test_personal_data_manager_.get());
test_client->SetDeviceAuthenticator(mock_device_authenticator);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1103,8 +1103,7 @@ void CreditCardAccessManager::FetchLocalOrFullServerCard() {

// Check if we need to authenticate the user before filling the local card
// or full server card.
if (personal_data_manager_
->IsAutofillPaymentMethodsMandatoryReauthEnabled()) {
if (personal_data_manager_->IsPaymentMethodsMandatoryReauthEnabled()) {
// `StartDeviceAuthenticationForFilling()` will asynchronously trigger
// the re-authentication flow, so we should avoid calling `Reset()`
// until the re-authentication flow is complete.
Expand Down Expand Up @@ -1158,8 +1157,7 @@ void CreditCardAccessManager::OnVirtualCardUnmaskResponseReceived(
base::UTF8ToUTF16(response_details.expiration_year));
// Check if we need to authenticate the user before filling the virtual
// card.
if (personal_data_manager_
->IsAutofillPaymentMethodsMandatoryReauthEnabled()) {
if (personal_data_manager_->IsPaymentMethodsMandatoryReauthEnabled()) {
// On some operating systems (for example, macOS and Windows), the
// device authentication prompt freezes Chrome. Thus we can only trigger
// the prompt after the progress dialog has been closed, which we can do
Expand Down
21 changes: 15 additions & 6 deletions components/autofill/core/browser/personal_data_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1928,13 +1928,22 @@ bool PersonalDataManager::IsSyncEnabledFor(
sync_service_->GetUserSettings()->GetSelectedTypes().Has(data_type);
}

bool PersonalDataManager::IsAutofillPaymentMethodsMandatoryReauthEnabled() {
if (!base::FeatureList::IsEnabled(
features::kAutofillEnablePaymentsMandatoryReauth)) {
return false;
}
void PersonalDataManager::SetPaymentMethodsMandatoryReauthEnabled(
bool enabled) {
prefs::SetPaymentMethodsMandatoryReauthEnabled(pref_service_, enabled);
}

bool PersonalDataManager::IsPaymentMethodsMandatoryReauthEnabled() {
return prefs::IsPaymentMethodsMandatoryReauthEnabled(pref_service_);
}

bool PersonalDataManager::ShouldShowPaymentMethodsMandatoryReauthPromo() {
return prefs::ShouldShowPaymentMethodsMandatoryReauthPromo(pref_service_);
}

return prefs::IsAutofillPaymentMethodsMandatoryReauthEnabled(pref_service_);
void PersonalDataManager::
IncrementPaymentMethodsMandatoryReauthPromoShownCounter() {
prefs::IncrementPaymentMethodsMandatoryReauthPromoShownCounter(pref_service_);
}

AutofillProfileMigrationStrikeDatabase*
Expand Down
20 changes: 18 additions & 2 deletions components/autofill/core/browser/personal_data_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,24 @@ class PersonalDataManager : public KeyedService,
// Returns true if Sync is enabled for `data_type`.
bool IsSyncEnabledFor(syncer::UserSelectableType data_type) const;

// Returns true if payments mandatory re-auth is enabled.
virtual bool IsAutofillPaymentMethodsMandatoryReauthEnabled();
// The functions below are related to the payments mandatory re-auth feature.
// All of this functionality is done through per-profile per-device prefs.
// `SetPaymentMethodsMandatoryReauthEnabled()` is used to update the opt-in
// status of the feature, and is called when a user successfully completes a
// full re-auth opt-in flow (with a successful authentication).
// `IsPaymentMethodsMandatoryReauthEnabled()` is checked before triggering the
// re-auth feature during a payments autofill flow.
// `ShouldShowPaymentMethodsMandatoryReauthPromo()` is used to check whether
// we should show the re-auth opt-in promo once a user submits a form, and
// there was no interactive authentication for the most recent payments
// autofill flow. `IncrementPaymentMethodsMandatoryReauthPromoShownCounter()`
// increments the counter that denotes the number of times that the promo has
// been shown, and this counter is used very similarly to a strike database
// when it comes time to check whether we should show the promo.
void SetPaymentMethodsMandatoryReauthEnabled(bool enabled);
virtual bool IsPaymentMethodsMandatoryReauthEnabled();
bool ShouldShowPaymentMethodsMandatoryReauthPromo();
void IncrementPaymentMethodsMandatoryReauthPromoShownCounter();

// Used to automatically import addresses without a prompt. Should only be
// set to true in tests.
Expand Down
78 changes: 78 additions & 0 deletions components/autofill/core/browser/personal_data_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,84 @@ TEST_F(PersonalDataManagerTest, AddUpdateRemoveProfiles) {
ExpectSameElements(profiles, personal_data_->GetProfiles());
}

#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
// Test that setting the `kAutofillEnablePaymentsMandatoryReauth` pref works
// correctly.
TEST_F(PersonalDataManagerTest, AutofillPaymentMethodsMandatoryReauthEnabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
features::kAutofillEnablePaymentsMandatoryReauth);
EXPECT_FALSE(personal_data_->IsPaymentMethodsMandatoryReauthEnabled());

personal_data_->SetPaymentMethodsMandatoryReauthEnabled(true);

EXPECT_TRUE(personal_data_->IsPaymentMethodsMandatoryReauthEnabled());

personal_data_->SetPaymentMethodsMandatoryReauthEnabled(false);

EXPECT_FALSE(personal_data_->IsPaymentMethodsMandatoryReauthEnabled());
}

// Test that setting the `kAutofillEnablePaymentsMandatoryReauth` does not
// enable the feature when the flag is off.
TEST_F(PersonalDataManagerTest,
AutofillPaymentMethodsMandatoryReauthEnabled_FlagOff) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(
features::kAutofillEnablePaymentsMandatoryReauth);
EXPECT_FALSE(personal_data_->IsPaymentMethodsMandatoryReauthEnabled());

personal_data_->SetPaymentMethodsMandatoryReauthEnabled(true);

EXPECT_FALSE(personal_data_->IsPaymentMethodsMandatoryReauthEnabled());
}

// Test that
// `PersonalDataManager::ShouldShowPaymentMethodsMandatoryReauthPromo()`
// only returns that we should show the promo when we are below the max counter
// limit for showing the promo.
TEST_F(
PersonalDataManagerTest,
ShouldShowPaymentMethodsMandatoryReauthPromo_MaxValueForPromoShownCounterReached) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
features::kAutofillEnablePaymentsMandatoryReauth);
for (int i = 0; i < prefs::kMaxValueForMandatoryReauthPromoShownCounter;
i++) {
EXPECT_TRUE(personal_data_->ShouldShowPaymentMethodsMandatoryReauthPromo());
personal_data_->IncrementPaymentMethodsMandatoryReauthPromoShownCounter();
}

EXPECT_FALSE(personal_data_->ShouldShowPaymentMethodsMandatoryReauthPromo());
}

// Test that
// `PersonalDataManager::ShouldShowPaymentMethodsMandatoryReauthPromo()`
// returns that we should not show the promo if the user has already made a
// decision.
TEST_F(
PersonalDataManagerTest,
ShouldShowPaymentMethodsMandatoryReauthPromo_UserHasMadeADecisionAlready) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
features::kAutofillEnablePaymentsMandatoryReauth);
personal_data_->SetPaymentMethodsMandatoryReauthEnabled(true);

EXPECT_FALSE(personal_data_->ShouldShowPaymentMethodsMandatoryReauthPromo());
}

// Test that
// `PersonalDataManager::ShouldShowPaymentMethodsMandatoryReauthPromo()`
// returns that we should not show the promo if the flag is off.
TEST_F(PersonalDataManagerTest,
ShouldShowPaymentMethodsMandatoryReauthPromo_FlagOff) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(
features::kAutofillEnablePaymentsMandatoryReauth);
EXPECT_FALSE(personal_data_->ShouldShowPaymentMethodsMandatoryReauthPromo());
}
#endif // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)

TEST_F(PersonalDataManagerTest, NoIBANsAddedIfDisabled) {
prefs::SetAutofillIBANEnabled(prefs_.get(), false);
personal_data_->AddIBAN(autofill::test::GetIBAN());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,11 +324,11 @@ TestPersonalDataManager::GetProfileUpdateStrikeDatabase() const {
return &inmemory_profile_update_strike_database_;
}

bool TestPersonalDataManager::IsAutofillPaymentMethodsMandatoryReauthEnabled() {
if (autofill_payment_methods_mandatory_reauth_enabled_) {
return true;
bool TestPersonalDataManager::IsPaymentMethodsMandatoryReauthEnabled() {
if (payment_methods_mandatory_reauth_enabled_.has_value()) {
return payment_methods_mandatory_reauth_enabled_.value();
}
return PersonalDataManager::IsAutofillPaymentMethodsMandatoryReauthEnabled();
return PersonalDataManager::IsPaymentMethodsMandatoryReauthEnabled();
}

void TestPersonalDataManager::ClearProfiles() {
Expand Down
8 changes: 4 additions & 4 deletions components/autofill/core/browser/test_personal_data_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class TestPersonalDataManager : public PersonalDataManager {
const override;
const AutofillProfileUpdateStrikeDatabase* GetProfileUpdateStrikeDatabase()
const override;
bool IsAutofillPaymentMethodsMandatoryReauthEnabled() override;
bool IsPaymentMethodsMandatoryReauthEnabled() override;

// Unique to TestPersonalDataManager:

Expand Down Expand Up @@ -172,8 +172,8 @@ class TestPersonalDataManager : public PersonalDataManager {

void ClearCreditCardArtImages() { credit_card_art_images_.clear(); }

void SetAutofillPaymentMethodsMandatoryReauthEnabled(bool val) {
autofill_payment_methods_mandatory_reauth_enabled_ = val;
void SetPaymentMethodsMandatoryReauthEnabled(bool val) {
payment_methods_mandatory_reauth_enabled_ = val;
}

private:
Expand All @@ -187,11 +187,11 @@ class TestPersonalDataManager : public PersonalDataManager {
absl::optional<bool> autofill_credit_card_enabled_;
absl::optional<bool> autofill_wallet_import_enabled_;
absl::optional<bool> eligible_for_account_storage_;
absl::optional<bool> payment_methods_mandatory_reauth_enabled_;
bool sync_feature_enabled_ = false;
AutofillSyncSigninState sync_and_signin_state_ =
AutofillSyncSigninState::kSignedInAndSyncFeatureEnabled;
CoreAccountInfo account_info_;
bool autofill_payment_methods_mandatory_reauth_enabled_ = false;

TestInMemoryStrikeDatabase inmemory_strike_database_;
AutofillProfileMigrationStrikeDatabase
Expand Down
38 changes: 28 additions & 10 deletions components/autofill/core/common/autofill_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#include "components/autofill/core/common/autofill_prefs.h"

#include "base/base64.h"
#include "base/feature_list.h"
#include "build/build_config.h"
#include "components/autofill/core/common/autofill_payments_features.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
Expand Down Expand Up @@ -245,24 +247,32 @@ void SetPaymentsIntegrationEnabled(PrefService* prefs, bool enabled) {
prefs->SetBoolean(kAutofillWalletImportEnabled, enabled);
}

bool IsAutofillPaymentMethodsMandatoryReauthEnabled(const PrefService* prefs) {
bool IsPaymentMethodsMandatoryReauthEnabled(const PrefService* prefs) {
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
if (!base::FeatureList::IsEnabled(
features::kAutofillEnablePaymentsMandatoryReauth)) {
return false;
}

return prefs->GetBoolean(kAutofillPaymentMethodsMandatoryReauth);
#else
return false;
#endif // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
}

void SetAutofillPaymentMethodsMandatoryReauth(PrefService* prefs,
bool enabled) {
void SetPaymentMethodsMandatoryReauthEnabled(PrefService* prefs, bool enabled) {
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
prefs->SetBoolean(kAutofillPaymentMethodsMandatoryReauth, enabled);
#endif // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
}

bool ShouldShowAutofillPaymentMethodsMandatoryReauthPromo(
const PrefService* prefs) {
bool ShouldShowPaymentMethodsMandatoryReauthPromo(const PrefService* prefs) {
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
if (!base::FeatureList::IsEnabled(
features::kAutofillEnablePaymentsMandatoryReauth)) {
return false;
}

// If the user has made a decision on this feature previously, then we should
// not show the opt-in promo.
if (prefs->GetUserPrefValue(kAutofillPaymentMethodsMandatoryReauth)) {
Expand All @@ -279,12 +289,20 @@ bool ShouldShowAutofillPaymentMethodsMandatoryReauthPromo(
#endif // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
}

void SetAutofillPaymentMethodsMandatoryReauthPromoShownCounter(
PrefService* prefs,
int count) {
void IncrementPaymentMethodsMandatoryReauthPromoShownCounter(
PrefService* prefs) {
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
prefs->SetInteger(kAutofillPaymentMethodsMandatoryReauthPromoShownCounter,
count);
if (prefs->GetInteger(
kAutofillPaymentMethodsMandatoryReauthPromoShownCounter) >=
kMaxValueForMandatoryReauthPromoShownCounter) {
return;
}

prefs->SetInteger(
kAutofillPaymentMethodsMandatoryReauthPromoShownCounter,
prefs->GetInteger(
kAutofillPaymentMethodsMandatoryReauthPromoShownCounter) +
1);
#endif // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
}

Expand Down
12 changes: 5 additions & 7 deletions components/autofill/core/common/autofill_prefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,14 @@ bool IsPaymentsIntegrationEnabled(const PrefService* prefs);

void SetPaymentsIntegrationEnabled(PrefService* prefs, bool enabled);

bool IsAutofillPaymentMethodsMandatoryReauthEnabled(const PrefService* prefs);
bool IsPaymentMethodsMandatoryReauthEnabled(const PrefService* prefs);

void SetAutofillPaymentMethodsMandatoryReauth(PrefService* prefs, bool enabled);
void SetPaymentMethodsMandatoryReauthEnabled(PrefService* prefs, bool enabled);

bool ShouldShowAutofillPaymentMethodsMandatoryReauthPromo(
const PrefService* prefs);
bool ShouldShowPaymentMethodsMandatoryReauthPromo(const PrefService* prefs);

void SetAutofillPaymentMethodsMandatoryReauthPromoShownCounter(
PrefService* prefs,
int count);
void IncrementPaymentMethodsMandatoryReauthPromoShownCounter(
PrefService* prefs);

void SetUserOptedInWalletSyncTransport(PrefService* prefs,
const CoreAccountId& account_id,
Expand Down

0 comments on commit d379384

Please sign in to comment.