[Autofill Auth] Fixing visibility of settings page FIDO authentication toggle.

Previously the FIDO authentication toggle would not be visible, even if the
device was capable of user verification. This was because the IPC call to check
whether or not a platform authenticator is available would not return on time.

This CL changes the implementation of that check to use native javascript
instead.

Bug: 949269
Change-Id: I6307a7e54889178582ae12dd174599c441957812
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850596
Commit-Queue: Manas Verma <manasverma@google.com>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#705312}

chrome/browser/resources/settings/autofill_page/payments_section.html

@@ -42,8 +42,9 @@
         pref="{{prefs.autofill.credit_card_enabled}}">
     </settings-toggle-button>
     <template is="dom-if"
-        if="[[isUserFIDOVerifiable_(
-            prefs.autofill.credit_card_enabled.value)]]">
+        if="[[shouldShowFidoToggle_(
+            prefs.autofill.credit_card_enabled.value,
+            userIsFidoVerifiable_)]]">
       <settings-toggle-button
           class="settings-box first"
           id="autofillCreditCardFIDOAuthToggle"

chrome/browser/resources/settings/autofill_page/payments_section.js

@@ -141,12 +141,12 @@ Polymer({
      * Set to true if user can be verified through FIDO authentication.
      * @private
      */
-    userIsFIDOVerifiable_: {
+    userIsFidoVerifiable_: {
       type: Boolean,
       value: function() {
-        return loadTimeData.getBoolean('userIsFIDOVerifiable');
+        return loadTimeData.getBoolean(
+            'fidoAuthenticationAvailableForAutofill');
       },
-      readOnly: true,
     },
 
     /**
@@ -208,6 +208,15 @@ Polymer({
       this.creditCards = cardList;
     };
 
+    // Update |userIsFidoVerifiable_| based on the availability of a platform
+    // authenticator.
+    if (window.PublicKeyCredential) {
+      window.PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable()
+          .then(r => {
+            this.userIsFidoVerifiable_ = this.userIsFidoVerifiable_ && r;
+          });
+    }
+
     /**
      * @type {function(!Array<!AutofillManager.AddressEntry>,
      *     !Array<!PaymentsManager.CreditCardEntry>)}
@@ -352,8 +361,8 @@ Polymer({
    *     authentication.
    * @private
    */
-  isUserFIDOVerifiable_: function(creditCardEnabled) {
-    return creditCardEnabled && this.userIsFIDOVerifiable_;
+  shouldShowFidoToggle_: function(creditCardEnabled, userIsFidoVerifiable) {
+    return creditCardEnabled && userIsFidoVerifiable;
   },
 
   /**

chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc

@@ -1707,8 +1707,9 @@ void AddOnStartupStrings(content::WebUIDataSource* html_source) {
                           base::size(kLocalizedStrings));
 }
 
-bool isUserFIDOVerifiable(autofill::PersonalDataManager* personal_data,
-                          content::WebContents* web_contents) {
+bool IsFidoAuthenticationAvailable(autofill::PersonalDataManager* personal_data,
+                                   content::WebContents* web_contents) {
+  // Don't show toggle switch if user is unable to downstream cards.
   if (personal_data->GetSyncSigninState() !=
           autofill::AutofillSyncSigninState::
               kSignedInAndWalletSyncTransportEnabled &&
@@ -1717,6 +1718,7 @@ bool isUserFIDOVerifiable(autofill::PersonalDataManager* personal_data,
     return false;
   }
 
+  // If |autofill_manager| is not available, then don't show toggle switch.
   autofill::ContentAutofillDriverFactory* autofill_driver_factory =
       autofill::ContentAutofillDriverFactory::FromWebContents(web_contents);
   if (!autofill_driver_factory)
@@ -1730,9 +1732,11 @@ bool isUserFIDOVerifiable(autofill::PersonalDataManager* personal_data,
   if (!autofill_manager)
     return false;
 
-  return autofill_manager->credit_card_access_manager()
-      ->GetOrCreateFIDOAuthenticator()
-      ->IsUserVerifiable();
+  // Show the toggle switch only if the flag is enabled. Once returned, this
+  // decision may be overridden (from true to false) by the caller in the
+  // payments section if no platform authenticator is found.
+  return base::FeatureList::IsEnabled(
+      autofill::features::kAutofillCreditCardAuthentication);
 }
 
 void AddAutofillStrings(content::WebUIDataSource* html_source,
@@ -1869,8 +1873,9 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
                             ProfileSyncServiceFactory::GetForProfile(profile),
                             /*is_test_mode=*/false,
                             /*log_manager=*/nullptr));
-  html_source->AddBoolean("userIsFIDOVerifiable",
-                          isUserFIDOVerifiable(personal_data, web_contents));
+  html_source->AddBoolean(
+      "fidoAuthenticationAvailableForAutofill",
+      IsFidoAuthenticationAvailable(personal_data, web_contents));
 
   html_source->AddBoolean(
       "passwordsLeakDetectionEnabled",

chrome/test/data/webui/settings/payments_section_test.js

@@ -66,6 +66,20 @@ cr.define('settings_payments_section', function() {
       return section;
     }
 
+    // Fakes the existence of a platform authenticator.
+    function addFakePlatformAuthenticator() {
+      if (!window.PublicKeyCredential) {
+        window.PublicKeyCredential = {};
+      }
+      window.PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable =
+          function() {
+        return new Promise(callback => {
+          callback(true);
+        });
+      };
+    }
+
+
     /**
      * Returns an array containing the local and server credit card items.
      * @return {!Array<!chrome.autofillPrivate.CreditCardEntry>}
@@ -450,8 +464,10 @@ cr.define('settings_payments_section', function() {
     });
 
     test('verifyFIDOAuthToggleShownIfUserIsVerifiable', function() {
-      // Set |userIsFIDOVerifiable| to true.
-      loadTimeData.overrideValues({userIsFIDOVerifiable: true});
+      // Set |fidoAuthenticationAvailableForAutofill| to true.
+      loadTimeData.overrideValues(
+          {fidoAuthenticationAvailableForAutofill: true});
+      addFakePlatformAuthenticator();
       const section =
           createPaymentsSection([], {credit_card_enabled: {value: true}});
 
@@ -459,8 +475,9 @@ cr.define('settings_payments_section', function() {
     });
 
     test('verifyFIDOAuthToggleNotShownIfUserIsNotVerifiable', function() {
-      // Set |userIsFIDOVerifiable| to false.
-      loadTimeData.overrideValues({userIsFIDOVerifiable: false});
+      // Set |fidoAuthenticationAvailableForAutofill| to false.
+      loadTimeData.overrideValues(
+          {fidoAuthenticationAvailableForAutofill: false});
       const section =
           createPaymentsSection([], {credit_card_enabled: {value: true}});
       assertFalse(!!section.$$('#autofillCreditCardFIDOAuthToggle'));
@@ -468,7 +485,9 @@ cr.define('settings_payments_section', function() {
 
     test('verifyFIDOAuthToggleCheckedIfOptedIn', function() {
       // Set FIDO auth pref value to true.
-      loadTimeData.overrideValues({userIsFIDOVerifiable: true});
+      loadTimeData.overrideValues(
+          {fidoAuthenticationAvailableForAutofill: true});
+      addFakePlatformAuthenticator();
       const section = createPaymentsSection([], {
         credit_card_enabled: {value: true},
         credit_card_fido_auth_enabled: {value: true}
@@ -478,7 +497,9 @@ cr.define('settings_payments_section', function() {
 
     test('verifyFIDOAuthToggleUncheckedIfOptedOut', function() {
       // Set FIDO auth pref value to false.
-      loadTimeData.overrideValues({userIsFIDOVerifiable: true});
+      loadTimeData.overrideValues(
+          {fidoAuthenticationAvailableForAutofill: true});
+      addFakePlatformAuthenticator();
       const section = createPaymentsSection([], {
         credit_card_enabled: {value: true},
         credit_card_fido_auth_enabled: {value: false}

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

@@ -30,8 +30,6 @@ namespace autofill {
 namespace {
 // Default timeout for user to respond to WebAuthn prompt.
 constexpr int kWebAuthnTimeoutMs = 3 * 60 * 1000;  // 3 minutes
-// Timeout to wait for synchronous version of IsUserVerifiable().
-constexpr int kIsUserVerifiableTimeoutMs = 1000;
 constexpr char kGooglePaymentsRpid[] = "google.com";
 constexpr char kGooglePaymentsRpName[] = "Google Payments";
 
@@ -138,21 +136,6 @@ void CreditCardFIDOAuthenticator::IsUserVerifiable(
   }
 }
 
-bool CreditCardFIDOAuthenticator::IsUserVerifiable() {
-  if (user_is_verifiable_.has_value())
-    return user_is_verifiable_.value();
-
-  IsUserVerifiable(
-      base::BindOnce(&CreditCardFIDOAuthenticator::SetUserIsVerifiable,
-                     weak_ptr_factory_.GetWeakPtr()));
-
-  user_is_verifiable_callback_received_.declare_only_used_while_idle();
-  user_is_verifiable_callback_received_.TimedWait(
-      base::TimeDelta::FromMilliseconds(kIsUserVerifiableTimeoutMs));
-
-  return user_is_verifiable_.value_or(false);
-}
-
 bool CreditCardFIDOAuthenticator::IsUserOptedIn() {
   return base::FeatureList::IsEnabled(
              features::kAutofillCreditCardAuthentication) &&
@@ -602,9 +585,4 @@ bool CreditCardFIDOAuthenticator::IsValidCreationOptions(
          creation_options.FindStringKey("challenge");
 }
 
-void CreditCardFIDOAuthenticator::SetUserIsVerifiable(bool user_is_verifiable) {
-  user_is_verifiable_ = user_is_verifiable;
-  user_is_verifiable_callback_received_.Signal();
-}
-
 }  // namespace autofill

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

@@ -101,9 +101,6 @@ class CreditCardFIDOAuthenticator
   // and enabled. Otherwise invokes callback with false.
   virtual void IsUserVerifiable(base::OnceCallback<void(bool)> callback);
 
-  // The synchronous version of IsUserVerifiable. Used on settings page load.
-  bool IsUserVerifiable();
-
   // Returns true only if the user has opted-in to use WebAuthn for autofill.
   virtual bool IsUserOptedIn();
 
@@ -202,9 +199,6 @@ class CreditCardFIDOAuthenticator
   // Returns true if |request_options| contains a challenge.
   bool IsValidCreationOptions(const base::Value& creation_options);
 
-  // Sets the value for |user_is_verifiable_|.
-  void SetUserIsVerifiable(bool user_is_verifiable);
-
   // Card being unmasked.
   const CreditCard* card_;
 
@@ -242,9 +236,6 @@ class CreditCardFIDOAuthenticator
   std::unique_ptr<FidoAuthenticationStrikeDatabase>
       fido_authentication_strike_database_;
 
-  // Set when callback for IsUserVerifiable() is invoked with passed value.
-  base::Optional<bool> user_is_verifiable_ = base::nullopt;
-
   // Signaled when callback for IsUserVerifiable() is invoked.
   base::WaitableEvent user_is_verifiable_callback_received_;
 

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

@@ -304,10 +304,6 @@ TEST_F(CreditCardFIDOAuthenticatorTest, IsUserVerifiable_False) {
   EXPECT_FALSE(requester_->is_user_verifiable().value());
 }
 
-TEST_F(CreditCardFIDOAuthenticatorTest, Sync_IsUserVerifiable_False) {
-  EXPECT_FALSE(fido_authenticator_->IsUserVerifiable());
-}
-
 TEST_F(CreditCardFIDOAuthenticatorTest, ParseRequestOptions) {
   base::Value request_options_json = GetTestRequestOptions(
       kTestChallenge, kTestRelyingPartyId, kTestCredentialId);