Add reauth when switching BiometricAuthForFilling toggle in settings

Additionally extracted common authentication logic used in PasswordsPrivateDelegateImpl to a new method to make requesting auth in settings easier. Bug: 1322079 Change-Id: Idd0b851a86b6ef08d947a3e9c9c8cbbb7038bcab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3904644 Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Viktor Semeniuk <vsemeniuk@google.com> Commit-Queue: Karol Sygiet <sygiet@google.com> Cr-Commit-Position: refs/heads/main@{#1052909}
chromium · Sep 29, 2022 · aef8729 · aef8729
1 parent 43f5551
commit aef8729
Show file tree

Hide file tree

Showing 24 changed files with 304 additions and 34 deletions.
diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_api.cc b/chrome/browser/extensions/api/passwords_private/passwords_private_api.cc
@@ -489,4 +489,13 @@ ResponseAction PasswordsPrivateExtendAuthValidityFunction::Run() {
   GetDelegate(browser_context())->ExtendAuthValidity();
   return RespondNow(NoArguments());
 }
+
+// PasswordsPrivateSwitchBiometricAuthBeforeFillingStateFunction
+ResponseAction
+PasswordsPrivateSwitchBiometricAuthBeforeFillingStateFunction::Run() {
+  GetDelegate(browser_context())
+      ->SwitchBiometricAuthBeforeFillingState(GetSenderWebContents());
+  return RespondNow(NoArguments());
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_api.h b/chrome/browser/extensions/api/passwords_private/passwords_private_api.h
@@ -414,6 +414,21 @@ class PasswordsPrivateExtendAuthValidityFunction : public ExtensionFunction {
   ResponseAction Run() override;
 };
 
+class PasswordsPrivateSwitchBiometricAuthBeforeFillingStateFunction
+    : public ExtensionFunction {
+ public:
+  DECLARE_EXTENSION_FUNCTION(
+      "passwordsPrivate.switchBiometricAuthBeforeFillingState",
+      PASSWORDSPRIVATE_SWITCHBIOMETRICAUTHBEFOREFILLINGSTATE)
+
+ protected:
+  ~PasswordsPrivateSwitchBiometricAuthBeforeFillingStateFunction() override =
+      default;
+
+  // ExtensionFunction overrides.
+  ResponseAction Run() override;
+};
+
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_PASSWORDS_PRIVATE_PASSWORDS_PRIVATE_API_H_
diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc b/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc
@@ -354,4 +354,15 @@ IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, ExtendAuthValidity) {
   EXPECT_TRUE(RunPasswordsSubtest("extendAuthValidity")) << message_;
   EXPECT_TRUE(get_authenticator_interaction_status());
 }
+
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest,
+                       SwitchBiometricAuthBeforeFillingState) {
+  EXPECT_FALSE(get_authenticator_interaction_status());
+  EXPECT_TRUE(RunPasswordsSubtest("switchBiometricAuthBeforeFillingState"))
+      << message_;
+  EXPECT_TRUE(get_authenticator_interaction_status());
+}
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h b/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h
@@ -234,6 +234,11 @@ class PasswordsPrivateDelegate : public KeyedService {
 
   // Restarts the authentication timer if it is running.
   virtual void ExtendAuthValidity() = 0;
+
+  // Switches Biometric authentication before filling state after
+  // successful authentication.
+  virtual void SwitchBiometricAuthBeforeFillingState(
+      content::WebContents* web_contents) = 0;
 };
 
 }  // namespace extensions

diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc b/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc
@@ -51,6 +51,7 @@
 
 #if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
 #include "chrome/browser/device_reauth/chrome_biometric_authenticator_factory.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
 #endif
 
 #if BUILDFLAG(IS_WIN)
@@ -205,6 +206,43 @@ extensions::api::passwords_private::ImportResults ConvertImportResults(
   return private_results;
 }
 
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+
+using password_manager::prefs::kBiometricAuthenticationBeforeFilling;
+
+scoped_refptr<device_reauth::BiometricAuthenticator> GetBiometricAuthenticator(
+    content::WebContents* web_contents) {
+  auto* client = ChromePasswordManagerClient::FromWebContents(web_contents);
+  DCHECK(client);
+  return client->GetBiometricAuthenticator();
+}
+
+void ChangeBiometricAuthenticationBeforeFillingSetting(PrefService* prefs,
+                                                       bool success) {
+  if (success) {
+    prefs->SetBoolean(
+        kBiometricAuthenticationBeforeFilling,
+        !prefs->GetBoolean(kBiometricAuthenticationBeforeFilling));
+  }
+}
+
+std::u16string GetMessageForBiometricAuthenticationBeforeFillingSetting(
+    PrefService* prefs) {
+  const bool pref_enabled =
+      prefs->GetBoolean(kBiometricAuthenticationBeforeFilling);
+#if BUILDFLAG(IS_MAC)
+  return l10n_util::GetStringUTF16(
+      pref_enabled ? IDS_PASSWORD_MANAGER_TURN_OFF_FILLING_REAUTH_MAC
+                   : IDS_PASSWORD_MANAGER_TURN_ON_FILLING_REAUTH_MAC);
+#elif BUILDFLAG(IS_WIN)
+  return l10n_util::GetStringUTF16(
+      pref_enabled ? IDS_PASSWORD_MANAGER_TURN_OFF_FILLING_REAUTH_WIN
+                   : IDS_PASSWORD_MANAGER_TURN_ON_FILLING_REAUTH_WIN);
+#endif
+}
+
+#endif
+
 }  // namespace
 
 namespace extensions {
@@ -438,44 +476,20 @@ void PasswordsPrivateDelegateImpl::OsReauthCall(
     password_manager::PasswordAccessAuthenticator::AuthResultCallback
         callback) {
 #if BUILDFLAG(IS_WIN)
-  scoped_refptr<device_reauth::BiometricAuthenticator> biometric_authenticator =
-      ChromeBiometricAuthenticatorFactory::GetInstance()
-          ->GetOrCreateBiometricAuthenticator();
-  base::OnceCallback<void()> on_reauth_completed =
-      base::BindOnce(&PasswordsPrivateDelegateImpl::OnReauthCompleted,
-                     weak_ptr_factory_.GetWeakPtr());
-
-  biometric_authenticator->AuthenticateWithMessage(
-      device_reauth::BiometricAuthRequester::kPasswordsInSettings,
+  AuthenticateWithBiometrics(
       password_manager_util_win::GetMessageForLoginPrompt(purpose),
-      std::move(callback).Then(std::move(on_reauth_completed)));
-
-  biometric_authenticator_ = std::move(biometric_authenticator);
+      std::move(callback));
 #elif BUILDFLAG(IS_MAC)
   scoped_refptr<device_reauth::BiometricAuthenticator> biometric_authenticator =
-      ChromeBiometricAuthenticatorFactory::GetInstance()
-          ->GetOrCreateBiometricAuthenticator();
+      GetBiometricAuthenticator(web_contents_);
   // TODO(crbug.com/1358442): Remove this check.
   if (biometric_authenticator->CanAuthenticate(
           device_reauth::BiometricAuthRequester::kPasswordsInSettings) &&
       base::FeatureList::IsEnabled(
           password_manager::features::kBiometricAuthenticationInSettings)) {
-    base::OnceCallback<void()> on_reauth_completed =
-        base::BindOnce(&PasswordsPrivateDelegateImpl::OnReauthCompleted,
-                       weak_ptr_factory_.GetWeakPtr());
-
-    biometric_authenticator->AuthenticateWithMessage(
-        device_reauth::BiometricAuthRequester::kPasswordsInSettings,
+    AuthenticateWithBiometrics(
         password_manager_util_mac::GetMessageForBiometricLoginPrompt(purpose),
-        std::move(callback).Then(std::move(on_reauth_completed)));
-
-    // If AuthenticateWithMessage is called again(UI on Mac isn't blocked so
-    // user might click multiple times on the button), it invalidates the old
-    // request which triggers PasswordsPrivateDelegateImpl::OnReauthCompleted
-    // which resets biometric_authenticator_. Having a local variable solves
-    // that problem as there's a second scoped_refptr for the authenticator
-    // object.
-    biometric_authenticator_ = std::move(biometric_authenticator);
+        std::move(callback));
   } else {
     bool result = password_manager_util_mac::AuthenticateUser(purpose);
     std::move(callback).Run(result);
@@ -722,6 +736,24 @@ void PasswordsPrivateDelegateImpl::StartAutomatedPasswordChange(
                     /*skip_login=*/false, std::move(callback));
 }
 
+void PasswordsPrivateDelegateImpl::SwitchBiometricAuthBeforeFillingState(
+    content::WebContents* web_contents) {
+#if !BUILDFLAG(IS_MAC) && !BUILDFLAG(IS_WIN)
+  NOTIMPLEMENTED();
+#else
+  DCHECK(base::FeatureList::IsEnabled(
+      password_manager::features::kBiometricAuthenticationForFilling));
+  password_manager::PasswordAccessAuthenticator::AuthResultCallback callback =
+      base::BindOnce(&ChangeBiometricAuthenticationBeforeFillingSetting,
+                     profile_->GetPrefs());
+  web_contents_ = web_contents;
+  AuthenticateWithBiometrics(
+      GetMessageForBiometricAuthenticationBeforeFillingSetting(
+          profile_->GetPrefs()),
+      std::move(callback));
+#endif
+}
+
 password_manager::InsecureCredentialsManager*
 PasswordsPrivateDelegateImpl::GetInsecureCredentialsManager() {
   return password_check_delegate_.GetInsecureCredentialsManager();
@@ -873,4 +905,32 @@ void PasswordsPrivateDelegateImpl::EmitHistogramsForCredentialAccess(
       password_manager::metrics_util::ACCESS_PASSWORD_COUNT);
 }
 
+void PasswordsPrivateDelegateImpl::AuthenticateWithBiometrics(
+    const std::u16string& message,
+    password_manager::PasswordAccessAuthenticator::AuthResultCallback
+        callback) {
+#if !BUILDFLAG(IS_MAC) && !BUILDFLAG(IS_WIN)
+  NOTIMPLEMENTED();
+#else
+  scoped_refptr<device_reauth::BiometricAuthenticator> biometric_authenticator =
+      GetBiometricAuthenticator(web_contents_);
+
+  base::OnceClosure on_reauth_completed =
+      base::BindOnce(&PasswordsPrivateDelegateImpl::OnReauthCompleted,
+                     weak_ptr_factory_.GetWeakPtr());
+
+  biometric_authenticator->AuthenticateWithMessage(
+      device_reauth::BiometricAuthRequester::kPasswordsInSettings, message,
+      std::move(callback).Then(std::move(on_reauth_completed)));
+
+  // If AuthenticateWithMessage is called again(UI on Mac isn't blocked so
+  // user might click multiple times on the button), it invalidates the old
+  // request which triggers PasswordsPrivateDelegateImpl::OnReauthCompleted
+  // which resets biometric_authenticator_. Having a local variable solves
+  // that problem as there's a second scoped_refptr for the authenticator
+  // object.
+  biometric_authenticator_ = std::move(biometric_authenticator);
+#endif
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h b/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h
@@ -115,6 +115,8 @@ class PasswordsPrivateDelegateImpl
   password_manager::InsecureCredentialsManager* GetInsecureCredentialsManager()
       override;
   void ExtendAuthValidity() override;
+  void SwitchBiometricAuthBeforeFillingState(
+      content::WebContents* web_contents) override;
 
   // KeyedService overrides:
   void Shutdown() override;
@@ -207,6 +209,11 @@ class PasswordsPrivateDelegateImpl
   // Invokes PasswordsPrivateEventRouter::OnPasswordManagerAuthTimeout().
   void OsReauthTimeoutCall();
 
+  void AuthenticateWithBiometrics(
+      const std::u16string& message,
+      password_manager::PasswordAccessAuthenticator::AuthResultCallback
+          callback);
+
   // Not owned by this class.
   raw_ptr<Profile> profile_;
 

diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc b/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
@@ -29,6 +29,7 @@
 #include "chrome/browser/ui/passwords/settings/password_manager_porter_interface.h"
 #include "chrome/common/extensions/api/passwords_private.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/device_reauth/mock_biometric_authenticator.h"
 #include "components/password_manager/content/browser/password_manager_log_router_factory.h"
 #include "components/password_manager/core/browser/insecure_credentials_table.h"
 #include "components/password_manager/core/browser/mock_password_feature_manager.h"
@@ -49,6 +50,14 @@
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/base/clipboard/test/test_clipboard.h"
 
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+#include "base/test/scoped_feature_list.h"
+#include "components/password_manager/core/common/password_manager_features.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#endif
+
 using MockReauthCallback = base::MockCallback<
     password_manager::PasswordAccessAuthenticator::ReauthCallback>;
 using password_manager::ReauthPurpose;
@@ -110,11 +119,24 @@ class MockPasswordManagerClient : public ChromePasswordManagerClient {
     return &mock_password_feature_manager_;
   }
 
+  scoped_refptr<device_reauth::BiometricAuthenticator>
+  GetBiometricAuthenticator() override {
+    return biometric_authenticator_;
+  }
+
+  void SetBiometricAuthenticator(
+      scoped_refptr<device_reauth::MockBiometricAuthenticator>
+          biometric_authenticator) {
+    biometric_authenticator_ = std::move(biometric_authenticator);
+  }
+
  private:
   explicit MockPasswordManagerClient(content::WebContents* web_contents)
       : ChromePasswordManagerClient(web_contents, nullptr) {}
 
   password_manager::MockPasswordFeatureManager mock_password_feature_manager_;
+  scoped_refptr<device_reauth::MockBiometricAuthenticator>
+      biometric_authenticator_ = nullptr;
 };
 
 // static
@@ -252,6 +274,9 @@ class PasswordsPrivateDelegateImplTest : public testing::Test {
       CreateAndUseTestAccountPasswordStore(&profile_);
   raw_ptr<ui::TestClipboard> test_clipboard_ =
       ui::TestClipboard::CreateForCurrentThread();
+  scoped_refptr<device_reauth::MockBiometricAuthenticator>
+      biometric_authenticator_ =
+          base::MakeRefCounted<device_reauth::MockBiometricAuthenticator>();
 
  private:
   base::HistogramTester histogram_tester_;
@@ -1095,4 +1120,34 @@ TEST_F(PasswordsPrivateDelegateImplTest, VerifyCastingOfImportResultsStatus) {
       "");
 }
 
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+// Checks if authentication is triggered.
+TEST_F(PasswordsPrivateDelegateImplTest,
+       SwitchBiometricAuthBeforeFillingState) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      password_manager::features::kBiometricAuthenticationForFilling);
+  // This enables uses of TestWebContents.
+  content::RenderViewHostTestEnabler test_render_host_factories;
+  std::unique_ptr<content::WebContents> web_contents =
+      content::WebContentsTester::CreateTestWebContents(&profile_, nullptr);
+  auto* client =
+      MockPasswordManagerClient::CreateForWebContentsAndGet(web_contents.get());
+  client->SetBiometricAuthenticator(biometric_authenticator_);
+  profile_.GetPrefs()->SetBoolean(
+      password_manager::prefs::kBiometricAuthenticationBeforeFilling, false);
+  EXPECT_CALL(
+      *biometric_authenticator_.get(),
+      AuthenticateWithMessage(
+          device_reauth::BiometricAuthRequester::kPasswordsInSettings, _, _))
+      .WillOnce(testing::WithArgs<2>(
+          [](auto callback) { std::move(callback).Run(/*successful=*/true); }));
+  PasswordsPrivateDelegateImpl delegate(&profile_);
+  delegate.SwitchBiometricAuthBeforeFillingState(web_contents.get());
+  // Expects that the switch value will change.
+  EXPECT_TRUE(profile_.GetPrefs()->GetBoolean(
+      password_manager::prefs::kBiometricAuthenticationBeforeFilling));
+}
+#endif
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.cc b/chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.cc
@@ -353,4 +353,9 @@ bool TestPasswordsPrivateDelegate::IsCredentialPresentInInsecureCredentialsList(
                         &api::passwords_private::PasswordUiEntry::id);
 }
 
+void TestPasswordsPrivateDelegate::SwitchBiometricAuthBeforeFillingState(
+    content::WebContents* web_contents) {
+  authenticator_interacted_ = true;
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.h b/chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.h
@@ -98,6 +98,8 @@ class TestPasswordsPrivateDelegate : public PasswordsPrivateDelegate {
   password_manager::InsecureCredentialsManager* GetInsecureCredentialsManager()
       override;
   void ExtendAuthValidity() override;
+  void SwitchBiometricAuthBeforeFillingState(
+      content::WebContents* web_contents) override;
 
   void SetProfile(Profile* profile);
   void SetOptedInForAccountStorage(bool opted_in);

diff --git a/chrome/browser/resources/settings/autofill_page/password_manager_proxy.ts b/chrome/browser/resources/settings/autofill_page/password_manager_proxy.ts
@@ -310,6 +310,12 @@ export interface PasswordManagerProxy {
    * Records the referrer of a given navigation to the Password Check page.
    */
   recordPasswordCheckReferrer(referrer: PasswordCheckReferrer): void;
+
+  /**
+   * Switches Biometric authentication before filling state after
+   * successful authentication.
+   */
+  switchBiometricAuthBeforeFillingState(): void;
 }
 
 /**
@@ -631,6 +637,10 @@ export class PasswordManagerImpl implements PasswordManagerProxy {
         PasswordCheckInteraction.COUNT);
   }
 
+  switchBiometricAuthBeforeFillingState() {
+    chrome.passwordsPrivate.switchBiometricAuthBeforeFillingState();
+  }
+
   /** override */
   recordPasswordCheckReferrer(referrer: PasswordCheckReferrer) {
     chrome.metricsPrivate.recordEnumerationValue(

diff --git a/chrome/browser/resources/settings/autofill_page/passwords_section.html b/chrome/browser/resources/settings/autofill_page/passwords_section.html
@@ -118,7 +118,10 @@
         <settings-toggle-button id="biometricAuthenticationForFillingToggle"
             class="hr"
             label="$i18n{biometricAuthenticaionForFillingLabel}"
-            pref="{{prefs.password_manager.biometric_authentication_filling}}">
+            pref="{{prefs.password_manager.biometric_authentication_filling}}"
+            on-settings-boolean-control-change=
+              "switchBiometricAuthBeforeFillingState_"
+            no-set-pref>
         </settings-toggle-button>
       </template>
     </if>