[Autofill Auth] Reset rate limiter for GDFGRP on page refresh.

On page refresh, signal that GDFGRP request can be made again,
regardless of timeout.

Bug: 949269
Change-Id: Idcf75c8125e6fd0ddd8c1ae06ccbb80385f57c65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2276408Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Commit-Queue: Manas Verma <manasverma@google.com>
Cr-Commit-Position: refs/heads/master@{#791465}

components/autofill/content/browser/content_autofill_driver.cc

@@ -59,7 +59,7 @@ ContentAutofillDriver::ContentAutofillDriver(
   }
 }
 
-ContentAutofillDriver::~ContentAutofillDriver() {}
+ContentAutofillDriver::~ContentAutofillDriver() = default;
 
 // static
 ContentAutofillDriver* ContentAutofillDriver::GetForRenderFrameHost(
@@ -293,20 +293,32 @@ void ContentAutofillDriver::SelectFieldOptionsDidChange(const FormData& form) {
 
 void ContentAutofillDriver::DidNavigateFrame(
     content::NavigationHandle* navigation_handle) {
-  if (navigation_handle->IsSameDocument())
+  if (navigation_handle->IsSameDocument()) {
+    // On page refresh, reset the rate limiter for fetching authentication
+    // details for credit card unmasking.
+    if (autofill_manager_) {
+      autofill_manager_->credit_card_access_manager()
+          ->SignalCanFetchUnmaskDetails();
+    }
     return;
+  }
 
   autofill_handler_->Reset();
 }
 
 void ContentAutofillDriver::SetAutofillManager(
     std::unique_ptr<AutofillManager> manager) {
-  CHECK(autofill_manager_);
   autofill_handler_ = std::move(manager);
   autofill_manager_ = static_cast<AutofillManager*>(autofill_handler_.get());
   autofill_manager_->SetExternalDelegate(autofill_external_delegate_.get());
 }
 
+ContentAutofillDriver::ContentAutofillDriver()
+    : render_frame_host_(nullptr),
+      autofill_manager_(nullptr),
+      key_press_handler_manager_(this),
+      log_manager_(nullptr) {}
+
 const mojo::AssociatedRemote<mojom::AutofillAgent>&
 ContentAutofillDriver::GetAutofillAgent() {
   // Here is a lazy binding, and will not reconnect after connection error.

components/autofill/content/browser/content_autofill_driver.h

@@ -134,11 +134,14 @@ class ContentAutofillDriver : public AutofillDriver,
 
   void SetAutofillProviderForTesting(AutofillProvider* provider);
 
- protected:
   // Sets the manager to |manager| and sets |manager|'s external delegate
   // to |autofill_external_delegate_|. Takes ownership of |manager|.
   void SetAutofillManager(std::unique_ptr<AutofillManager> manager);
 
+ protected:
+  // Constructor for tests.
+  ContentAutofillDriver();
+
  private:
   // KeyPressHandlerManager::Delegate:
   void AddHandler(

components/autofill/core/browser/BUILD.gn

@@ -515,6 +515,8 @@ jumbo_static_library("test_support") {
       "payments/test_internal_authenticator.h",
     ]
 
+    public_deps += [ "//components/autofill/content/browser" ]
+
     deps += [ "//third_party/blink/public/common" ]
   }
 
@@ -709,7 +711,10 @@ source_set("unit_tests") {
   ]
 
   if (!is_ios) {
-    deps += [ "//third_party/blink/public/common" ]
+    deps += [
+      "//content/test:test_support",
+      "//third_party/blink/public/common",
+    ]
   }
 }
 

components/autofill/core/browser/DEPS

@@ -14,6 +14,7 @@ include_rules = [
   "+components/version_info",
   "+components/webdata/common",
   "+components/webdata_services",
+  "+content/public/test",
   "+crypto/hkdf.h",
   "+crypto/random.h",
   "+google_apis/gaia",
@@ -52,6 +53,9 @@ specific_include_rules = {
   "autofill_driver\.h": [
     "+third_party/blink/public/mojom/webauthn/internal_authenticator.mojom.h"
   ],
+  "test_autofill_driver\.h": [
+    "+components/autofill/content/browser/content_autofill_driver.h"
+  ],
   "credit_card_save_manager_unittest\.cc": [
     "+components/ukm",
   ],

components/autofill/core/browser/payments/credit_card_access_manager.cc

@@ -377,6 +377,10 @@ void CreditCardAccessManager::OnSettingsPageFIDOAuthToggled(bool opt_in) {
 #endif
 }
 
+void CreditCardAccessManager::SignalCanFetchUnmaskDetails() {
+  can_fetch_unmask_details_.Signal();
+}
+
 void CreditCardAccessManager::CacheUnmaskedCardInfo(const CreditCard& card,
                                                     const base::string16& cvc) {
   DCHECK_EQ(card.record_type(), CreditCard::FULL_SERVER_CARD);
@@ -722,10 +726,6 @@ void CreditCardAccessManager::HandleDialogUserResponse(
 }
 #endif
 
-void CreditCardAccessManager::SignalCanFetchUnmaskDetails() {
-  can_fetch_unmask_details_.Signal();
-}
-
 void CreditCardAccessManager::AdditionallyPerformFidoAuth(
     const CreditCardCVCAuthenticator::CVCAuthenticationResponse& response,
     base::Value request_options) {

components/autofill/core/browser/payments/credit_card_access_manager.h

@@ -122,6 +122,11 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester,
   // TODO(crbug/949269): Add a rate limiter to counter spam clicking.
   void OnSettingsPageFIDOAuthToggled(bool opt_in);
 
+  // Resets the rate limiter for fetching unmask deatils. Used with
+  // PostTaskWithDelay() with a timeout, and also called by AutofillDriver on
+  // page refresh.
+  void SignalCanFetchUnmaskDetails();
+
   // Caches CreditCard and corresponding CVC for unmasked card so that
   // card info can later be filled without attempting to auth again.
   // TODO(crbug/1069929): Add browsertests for this.
@@ -136,6 +141,8 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester,
  private:
   FRIEND_TEST_ALL_PREFIXES(CreditCardAccessManagerBrowserTest,
                            NavigateFromPage_UnmaskedCardCacheResets);
+  FRIEND_TEST_ALL_PREFIXES(CreditCardAccessManagerTest,
+                           PreflightCallRateLimited);
   friend class AutofillAssistantTest;
   friend class AutofillManagerTest;
   friend class AutofillMetricsTest;
@@ -226,10 +233,6 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester,
   void HandleDialogUserResponse(WebauthnDialogCallbackType type);
 #endif
 
-  // Used with PostTaskWithDelay() to signal |can_fetch_unmask_details_| event
-  // after a timeout.
-  void SignalCanFetchUnmaskDetails();
-
   // Additionlly authorizes the card with FIDO. It also delays the form filling.
   // It should only be called when registering a new card or opting-in from
   // Android.

components/autofill/core/browser/payments/credit_card_access_manager_unittest.cc

@@ -73,6 +73,7 @@
 #include "components/autofill/core/browser/payments/fido_authentication_strike_database.h"
 #include "components/autofill/core/browser/payments/test_credit_card_fido_authenticator.h"
 #include "components/autofill/core/browser/payments/test_internal_authenticator.h"
+#include "content/public/test/mock_navigation_handle.h"
 #endif
 
 using base::ASCIIToUTF16;
@@ -166,9 +167,8 @@ class CreditCardAccessManagerTest : public testing::Test {
     autocomplete_history_manager_ =
         std::make_unique<MockAutocompleteHistoryManager>();
 
-    accessor_.reset(new TestAccessor());
-    autofill_driver_ =
-        std::make_unique<testing::NiceMock<TestAutofillDriver>>();
+    accessor_ = std::make_unique<TestAccessor>();
+    autofill_driver_ = std::make_unique<TestAutofillDriver>();
 
     payments_client_ = new payments::TestPaymentsClient(
         autofill_driver_->GetURLLoaderFactory(),
@@ -184,6 +184,7 @@ class CreditCardAccessManagerTest : public testing::Test {
         autofill_manager_->credit_card_access_manager();
 
 #if !defined(OS_IOS)
+    autofill_driver_->SetAutofillManager(std::move(autofill_manager_));
     autofill_driver_->SetAuthenticator(new TestInternalAuthenticator());
     credit_card_access_manager_->set_fido_authenticator_for_testing(
         std::make_unique<TestCreditCardFIDOAuthenticator>(
@@ -1789,6 +1790,46 @@ TEST_F(CreditCardAccessManagerTest, IntentToOptOut_OptOutFailure) {
   OptChange(AutofillClient::PERMANENT_FAILURE, false);
   EXPECT_FALSE(IsCreditCardFIDOAuthEnabled());
 }
+
+// TODO(crbug.com/1109296) Debug issues and re-enable this test on MacOS.
+#if !defined(OS_MACOSX)
+// Ensures that PrepareToFetchCreditCard() is properly rate limited.
+TEST_F(CreditCardAccessManagerTest, PreflightCallRateLimited) {
+  // Create server card and set user as eligible for FIDO auth.
+  base::HistogramTester histogram_tester;
+  std::string preflight_call_metric =
+      "Autofill.BetterAuth.CardUnmaskPreflightCalled";
+
+  ClearCards();
+  CreateServerCard(kTestGUID, kTestNumber);
+  GetFIDOAuthenticator()->SetUserVerifiable(true);
+  ResetFetchCreditCard();
+
+  // First call to PrepareToFetchCreditCard() should make RPC.
+  credit_card_access_manager_->PrepareToFetchCreditCard();
+  histogram_tester.ExpectTotalCount(preflight_call_metric, 1);
+
+  // The above call should automatically reset the flag.
+  EXPECT_FALSE(
+      credit_card_access_manager_->can_fetch_unmask_details_.IsSignaled());
+
+  // Any subsequent calls should not make a RPC.
+  credit_card_access_manager_->PrepareToFetchCreditCard();
+  histogram_tester.ExpectTotalCount(preflight_call_metric, 1);
+
+  // Mock a page refresh, and make a third request.
+  std::unique_ptr<content::MockNavigationHandle> mock_navigation_handle =
+      std::make_unique<content::MockNavigationHandle>();
+  mock_navigation_handle->set_is_same_document(true);
+  autofill_driver_->DidNavigateFrame(mock_navigation_handle.get());
+
+  credit_card_access_manager_->PrepareToFetchCreditCard();
+
+  // Since the page was refreshed, rate limiter is reset and new RPC should be
+  // logged.
+  histogram_tester.ExpectTotalCount(preflight_call_metric, 2);
+}
+#endif  // !defined(OS_MACOSX)
 #endif  // !defined(OS_IOS)
 
 // Ensures that |is_authentication_in_progress_| is set correctly.
@@ -1829,7 +1870,4 @@ TEST_F(CreditCardAccessManagerTest, FetchCreditCardUsesUnmaskedCardCache) {
   histogram_tester.ExpectBucketCount("Autofill.UsedCachedServerCard", 2, 1);
 }
 
-// TODO(crbug/949269): Once metrics are added, create test to ensure that
-// PrepareToFetchCreditCard() is properly rate limited.
-
 }  // namespace autofill

components/autofill/core/browser/test_autofill_driver.h

@@ -14,13 +14,18 @@
 #include "services/network/test/test_url_loader_factory.h"
 
 #if !defined(OS_IOS)
+#include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/core/browser/payments/internal_authenticator.h"
 #endif
 
 namespace autofill {
 
 // This class is only for easier writing of tests.
+#if defined(OS_IOS)
 class TestAutofillDriver : public AutofillDriver {
+#else
+class TestAutofillDriver : public ContentAutofillDriver {
+#endif
  public:
   TestAutofillDriver();
   ~TestAutofillDriver() override;