Expose auth-factor based pin methods to os-settings webui

This commit adds support for PINs to the AuthFactorEditor mojo service.
This service will eventually supercede the quickUnlockPrivate extension
API. The implementation of the PIN editing interface calls into the
internals of the corresponding quickUnlockPrivate extension API. Because
//chromeos (where the mojo service lives) cannot directly depend on
//chrome (where quickUnlockPrivate lives) this made it necessary to
introduce an interface for dependency injection.

This commit add the implementation of the service only; the webui client
will be migrated in a follow-up commit. The few changes of WebUIs code
are only to adapt to the move of the mojo `ConfigureResult` enum out of
the interface into the module namespace, and the renaming of the
`kClientError` option to kFatalError.

Bug: b:258481055
Change-Id: Id417e7b543b282ea4ce9e7c315319cb1f8019c12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3918259
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Martin Bidlingmaier <mbid@google.com>
Cr-Commit-Position: refs/heads/main@{#1096884}

chrome/browser/ash/login/quick_unlock/pin_backend.h

@@ -10,6 +10,7 @@
 #include "base/functional/callback.h"
 #include "chromeos/ash/components/login/auth/public/auth_callbacks.h"
 #include "chromeos/ash/components/login/auth/public/key.h"
+#include "chromeos/ash/services/auth_factor_config/chrome_browser_delegates.h"
 #include "components/prefs/pref_service.h"
 
 class AccountId;
@@ -24,7 +25,7 @@ enum class Purpose;
 
 // Provides high-level access to the user's PIN. The underlying storage can be
 // either cryptohome or prefs.
-class PinBackend {
+class PinBackend : public ash::auth::PinBackendDelegate {
  public:
   using BoolCallback = base::OnceCallback<void(bool)>;
 
@@ -45,7 +46,7 @@ class PinBackend {
   PinBackend(const PinBackend&) = delete;
   PinBackend& operator=(const PinBackend&) = delete;
 
-  ~PinBackend();
+  ~PinBackend() override;
 
   // Check to see if the PinBackend supports login. This is true when the
   // cryptohome backend is available.
@@ -61,7 +62,7 @@ class PinBackend {
   void Set(const AccountId& account_id,
            const std::string& auth_token,
            const std::string& pin,
-           BoolCallback did_set);
+           BoolCallback did_set) override;
 
   // Set the state of PIN auto submit for the given user. Called when enabling
   // auto submit through the confirmation dialog in Settings.
@@ -73,7 +74,7 @@ class PinBackend {
   // Remove the given user's PIN.
   void Remove(const AccountId& account_id,
               const std::string& auth_token,
-              BoolCallback did_remove);
+              BoolCallback did_remove) override;
 
   // Is PIN authentication available for the given account? Even if PIN is set,
   // it may not be available for authentication due to some additional

chrome/browser/ash/login/quick_unlock/quick_unlock_factory.h

@@ -7,7 +7,7 @@
 
 #include "base/memory/singleton.h"
 #include "chrome/browser/profiles/profile_keyed_service_factory.h"
-#include "chromeos/ash/services/auth_factor_config/quick_unlock_storage_delegate.h"
+#include "chromeos/ash/services/auth_factor_config/chrome_browser_delegates.h"
 #include "components/account_id/account_id.h"
 
 class Profile;

chrome/browser/ash/login/screens/cryptohome_recovery_setup_screen.cc

@@ -88,13 +88,13 @@ void CryptohomeRecoverySetupScreen::ExitScreen(
 }
 
 void CryptohomeRecoverySetupScreen::OnRecoveryConfigured(
-    auth::RecoveryFactorEditor::ConfigureResult result) {
+    auth::mojom::ConfigureResult result) {
   switch (result) {
-    case auth::RecoveryFactorEditor::ConfigureResult::kSuccess:
+    case auth::mojom::ConfigureResult::kSuccess:
       ExitScreen(*context(), Result::DONE);
       break;
-    case auth::RecoveryFactorEditor::ConfigureResult::kInvalidTokenError:
-    case auth::RecoveryFactorEditor::ConfigureResult::kClientError:
+    case auth::mojom::ConfigureResult::kInvalidTokenError:
+    case auth::mojom::ConfigureResult::kFatalError:
       LOG(ERROR) << "Failed to setup recovery factor, result "
                  << static_cast<int>(result);
       ExitScreen(*context(), Result::SKIPPED);

chrome/browser/ash/login/screens/cryptohome_recovery_setup_screen.h

@@ -47,7 +47,7 @@ class CryptohomeRecoverySetupScreen : public BaseScreen {
 
  private:
   void ExitScreen(WizardContext& wizard_context, Result result);
-  void OnRecoveryConfigured(auth::RecoveryFactorEditor::ConfigureResult result);
+  void OnRecoveryConfigured(auth::mojom::ConfigureResult result);
   base::WeakPtr<CryptohomeRecoverySetupScreenView> view_ = nullptr;
   ScreenExitCallback exit_callback_;
   base::WeakPtrFactory<CryptohomeRecoverySetupScreen> weak_ptr_factory_{this};

chrome/browser/chrome_browser_interface_binders.cc

@@ -1122,6 +1122,9 @@ void PopulateChromeWebUIFrameBinders(
   RegisterWebUIControllerInterfaceBinder<ash::auth::mojom::RecoveryFactorEditor,
                                          ash::settings::OSSettingsUI>(map);
 
+  RegisterWebUIControllerInterfaceBinder<ash::auth::mojom::PinFactorEditor,
+                                         ash::settings::OSSettingsUI>(map);
+
   RegisterWebUIControllerInterfaceBinder<
       ash::cellular_setup::mojom::ESimManager, ash::settings::OSSettingsUI,
       ash::NetworkUI, ash::OobeUI>(map);

chrome/browser/resources/settings/chromeos/os_people_page/local_data_recovery_dialog.js

@@ -16,7 +16,7 @@ import 'chrome://resources/polymer/v3_0/iron-icon/iron-icon.js';
 import '../../settings_shared.css.js';
 
 import {I18nBehavior, I18nBehaviorInterface} from 'chrome://resources/ash/common/i18n_behavior.js';
-import {AuthFactor, FactorObserverInterface, FactorObserverReceiver, ManagementType, RecoveryFactorEditor_ConfigureResult} from 'chrome://resources/mojo/chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-webui.js';
+import {AuthFactor, ConfigureResult, FactorObserverInterface, FactorObserverReceiver, ManagementType} from 'chrome://resources/mojo/chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-webui.js';
 import {mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {getTemplate} from './local_data_recovery_dialog.html.js';
@@ -94,7 +94,7 @@ class LocalDataRecoveryDialogElement extends
 
       const {result} = await this.recoveryFactorEditor.configure(
           this.authToken.token, false);
-      if (result !== RecoveryFactorEditor_ConfigureResult.kSuccess) {
+      if (result !== ConfigureResult.kSuccess) {
         console.error('RecoveryFactorEditor::Configure failed:', result);
       }
     } finally {

chrome/browser/resources/settings/chromeos/os_people_page/lock_screen.js

@@ -34,7 +34,7 @@ import {I18nBehavior, I18nBehaviorInterface} from 'chrome://resources/ash/common
 import {LockScreenProgress, recordLockScreenProgress} from 'chrome://resources/ash/common/quick_unlock/lock_screen_constants.js';
 import {WebUIListenerBehavior, WebUIListenerBehaviorInterface} from 'chrome://resources/ash/common/web_ui_listener_behavior.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
-import {AuthFactor, FactorObserverInterface, FactorObserverReceiver, ManagementType, RecoveryFactorEditor_ConfigureResult} from 'chrome://resources/mojo/chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-webui.js';
+import {AuthFactor, ConfigureResult, FactorObserverInterface, FactorObserverReceiver, ManagementType} from 'chrome://resources/mojo/chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-webui.js';
 import {afterNextRender, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {Setting} from '../../mojom-webui/setting.mojom-webui.js';
@@ -642,13 +642,13 @@ class SettingsLockScreenElement extends SettingsLockScreenElementBase {
       const {result} = await this.recoveryFactorEditor.configure(
           this.authToken.token, shouldEnable);
       switch (result) {
-        case RecoveryFactorEditor_ConfigureResult.kSuccess:
+        case ConfigureResult.kSuccess:
           break;
-        case RecoveryFactorEditor_ConfigureResult.kInvalidTokenError:
+        case ConfigureResult.kInvalidTokenError:
           // This will open the password prompt.
           this.dispatchAuthTokenInvalidEvent_();
           return;
-        case RecoveryFactorEditor_ConfigureResult.kClientError:
+        case ConfigureResult.kFatalError:
           console.error('Error configuring recovery');
           return;
       }

chrome/browser/resources/settings/chromeos/os_people_page/lock_state_behavior.js

@@ -4,7 +4,7 @@
 
 import {I18nBehavior, I18nBehaviorInterface} from 'chrome://resources/ash/common/i18n_behavior.js';
 import {WebUIListenerBehavior, WebUIListenerBehaviorInterface} from 'chrome://resources/ash/common/web_ui_listener_behavior.js';
-import {AuthFactorConfig, AuthFactorConfigInterface, RecoveryFactorEditor, RecoveryFactorEditorInterface} from 'chrome://resources/mojo/chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-webui.js';
+import {AuthFactorConfig, AuthFactorConfigInterface, PinFactorEditor, PinFactorEditorInterface, RecoveryFactorEditor, RecoveryFactorEditorInterface} from 'chrome://resources/mojo/chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-webui.js';
 
 /**
  * @fileoverview
@@ -78,6 +78,13 @@ export const LockStateBehaviorImpl = {
      */
     recoveryFactorEditor:
         {type: Object, value: RecoveryFactorEditor.getRemote()},
+
+    /**
+     * Interface for calls to the ash PinFactorEditor service.  May be
+     * overridden by tests.
+     * @type {PinFactorEditorInterface}
+     */
+    pinFactorEditor: {type: Object, value: PinFactorEditor.getRemote()},
   },
 
   /** @override */
@@ -222,6 +229,13 @@ export class LockStateBehaviorInterface {
      * @type {RecoveryFactorEditorInterface}
      */
     this.recoveryFactorEditor;
+
+    /**
+     * Interface for calls to the ash PinFactorEditor service.  May be
+     * overridden by tests.
+     * @type {PinFactorEditorInterface}
+     */
+    this.pinFactorEditor;
   }
 
   /**

chrome/browser/ui/webui/settings/ash/os_settings_ui.cc

@@ -15,6 +15,7 @@
 #include "ash/webui/personalization_app/search/search.mojom.h"
 #include "ash/webui/personalization_app/search/search_handler.h"
 #include "base/metrics/histogram_functions.h"
+#include "chrome/browser/ash/login/quick_unlock/pin_backend.h"
 #include "chrome/browser/ash/login/quick_unlock/quick_unlock_factory.h"
 #include "chrome/browser/ash/web_applications/personalization_app/personalization_app_manager.h"
 #include "chrome/browser/ash/web_applications/personalization_app/personalization_app_manager_factory.h"
@@ -250,6 +251,15 @@ void OSSettingsUI::BindInterface(
       web_ui()->GetWebContents(), std::move(receiver));
 }
 
+void OSSettingsUI::BindInterface(
+    mojo::PendingReceiver<auth::mojom::PinFactorEditor> receiver) {
+  auto* pin_backend = quick_unlock::PinBackend::GetInstance();
+  CHECK(pin_backend);
+  auth::BindToPinFactorEditor(std::move(receiver),
+                              quick_unlock::QuickUnlockFactory::GetDelegate(),
+                              *pin_backend);
+}
+
 WEB_UI_CONTROLLER_TYPE_IMPL(OSSettingsUI)
 
 }  // namespace ash::settings

chrome/browser/ui/webui/settings/ash/os_settings_ui.h

@@ -149,6 +149,8 @@ class OSSettingsUI : public ui::MojoWebUIController {
       mojo::PendingReceiver<auth::mojom::AuthFactorConfig> receiver);
   void BindInterface(
       mojo::PendingReceiver<auth::mojom::RecoveryFactorEditor> receiver);
+  void BindInterface(
+      mojo::PendingReceiver<auth::mojom::PinFactorEditor> receiver);
 
   // Binds to the Jelly dynamic color Mojo
   void BindInterface(

chromeos/ash/services/auth_factor_config/BUILD.gn

@@ -10,9 +10,11 @@ static_library("auth_factor_config") {
   sources = [
     "auth_factor_config.cc",
     "auth_factor_config.h",
+    "chrome_browser_delegates.h",
     "in_process_instances.cc",
     "in_process_instances.h",
-    "quick_unlock_storage_delegate.h",
+    "pin_factor_editor.cc",
+    "pin_factor_editor.h",
     "recovery_factor_editor.cc",
     "recovery_factor_editor.h",
   ]

chromeos/ash/services/auth_factor_config/auth_factor_config.cc

@@ -88,7 +88,21 @@ void AuthFactorConfig::GetManagementType(
               : mojom::ManagementType::kNone;
 
       std::move(callback).Run(result);
-      break;
+      return;
+    }
+    case mojom::AuthFactor::kPin: {
+      const auto* user = ::user_manager::UserManager::Get()->GetPrimaryUser();
+      CHECK(user);
+      const PrefService* prefs = quick_unlock_storage_->GetPrefService(*user);
+      CHECK(prefs);
+
+      if (prefs->IsManagedPreference(prefs::kQuickUnlockModeAllowlist) ||
+          prefs->IsManagedPreference(prefs::kWebAuthnFactors)) {
+        std::move(callback).Run(mojom::ManagementType::kUser);
+      } else {
+        std::move(callback).Run(mojom::ManagementType::kNone);
+      }
+      return;
     }
   }
 }
@@ -106,7 +120,37 @@ void AuthFactorConfig::IsEditable(const std::string& auth_token,
 
       std::move(callback).Run(
           prefs->IsUserModifiablePreference(prefs::kRecoveryFactorBehavior));
-      break;
+      return;
+    }
+    case mojom::AuthFactor::kPin: {
+      const auto* user = ::user_manager::UserManager::Get()->GetPrimaryUser();
+      CHECK(user);
+      const PrefService* prefs = quick_unlock_storage_->GetPrefService(*user);
+      CHECK(prefs);
+
+      // Lists of factors that are allowed for some purpose.
+      const base::Value::List* pref_lists[] = {
+          &prefs->GetList(prefs::kQuickUnlockModeAllowlist),
+          &prefs->GetList(prefs::kWebAuthnFactors),
+      };
+
+      // Values in factor lists that match PINs.
+      const base::Value pref_list_values[] = {
+          base::Value("all"),
+          base::Value("PIN"),
+      };
+
+      for (const auto* pref_list : pref_lists) {
+        for (const auto& pref_list_value : pref_list_values) {
+          if (base::Contains(*pref_list, pref_list_value)) {
+            std::move(callback).Run(true);
+            return;
+          }
+        }
+      }
+
+      std::move(callback).Run(false);
+      return;
     }
   }
 }

chromeos/ash/services/auth_factor_config/auth_factor_config.h

@@ -5,8 +5,8 @@
 #ifndef CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_AUTH_FACTOR_CONFIG_H_
 #define CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_AUTH_FACTOR_CONFIG_H_
 
+#include "chromeos/ash/services/auth_factor_config/chrome_browser_delegates.h"
 #include "chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom.h"
-#include "chromeos/ash/services/auth_factor_config/quick_unlock_storage_delegate.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote_set.h"
 

chromeos/ash/services/auth_factor_config/quick_unlock_storage_delegate.h → chromeos/ash/services/auth_factor_config/chrome_browser_delegates.h

@@ -1,12 +1,13 @@
-// Copyright 2022 The Chromium Authors
+// Copyright 2023 The Chromium Authors
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_QUICK_UNLOCK_STORAGE_DELEGATE_H_
-#define CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_QUICK_UNLOCK_STORAGE_DELEGATE_H_
+#ifndef CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_CHROME_BROWSER_DELEGATES_H_
+#define CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_CHROME_BROWSER_DELEGATES_H_
 
 #include <memory>
 #include <string>
+#include "base/functional/callback.h"
 #include "chromeos/ash/components/login/auth/public/user_context.h"
 #include "components/prefs/pref_service.h"
 #include "components/user_manager/user.h"
@@ -29,6 +30,25 @@ class QuickUnlockStorageDelegate {
   virtual PrefService* GetPrefService(const ::user_manager::User& user) = 0;
 };
 
+class PinBackendDelegate {
+ public:
+  using BoolCallback = base::OnceCallback<void(bool)>;
+
+  PinBackendDelegate() = default;
+  PinBackendDelegate(const PinBackendDelegate&) = delete;
+  PinBackendDelegate& operator=(const PinBackendDelegate&) = delete;
+  virtual ~PinBackendDelegate() = default;
+
+  virtual void Set(const AccountId& account_id,
+                   const std::string& auth_token,
+                   const std::string& pin,
+                   BoolCallback did_set) = 0;
+
+  virtual void Remove(const AccountId& account_id,
+                      const std::string& auth_token,
+                      BoolCallback did_remove) = 0;
+};
+
 }  // namespace ash::auth
 
-#endif  // CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_QUICK_UNLOCK_STORAGE_DELEGATE_H_
+#endif  // CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_CHROME_BROWSER_DELEGATES_H_

chromeos/ash/services/auth_factor_config/in_process_instances.cc

@@ -8,6 +8,7 @@
 
 #include "base/no_destructor.h"
 #include "chromeos/ash/services/auth_factor_config/auth_factor_config.h"
+#include "chromeos/ash/services/auth_factor_config/pin_factor_editor.h"
 #include "chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-test-utils.h"
 #include "chromeos/ash/services/auth_factor_config/recovery_factor_editor.h"
 
@@ -28,6 +29,13 @@ RecoveryFactorEditor& GetRecoveryFactorEditorImpl(
   return *recovery_factor_editor;
 }
 
+PinFactorEditor& GetPinFactorEditorImpl(QuickUnlockStorageDelegate& storage,
+                                        PinBackendDelegate& pin_backend) {
+  static base::NoDestructor<PinFactorEditor> pin_factor_editor(
+      &GetAuthFactorConfigImpl(storage), &pin_backend, &storage);
+  return *pin_factor_editor;
+}
+
 }  // namespace
 
 void BindToAuthFactorConfig(
@@ -52,4 +60,12 @@ mojom::RecoveryFactorEditor& GetRecoveryFactorEditor(
   return GetRecoveryFactorEditorImpl(delegate);
 }
 
+void BindToPinFactorEditor(
+    mojo::PendingReceiver<mojom::PinFactorEditor> receiver,
+    QuickUnlockStorageDelegate& storage,
+    PinBackendDelegate& pin_backend) {
+  GetPinFactorEditorImpl(storage, pin_backend)
+      .BindReceiver(std::move(receiver));
+}
+
 }  // namespace ash::auth

chromeos/ash/services/auth_factor_config/in_process_instances.h

@@ -5,8 +5,8 @@
 #ifndef CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_IN_PROCESS_INSTANCES_H_
 #define CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_IN_PROCESS_INSTANCES_H_
 
+#include "chromeos/ash/services/auth_factor_config/chrome_browser_delegates.h"
 #include "chromeos/ash/services/auth_factor_config/public/mojom/auth_factor_config.mojom-forward.h"
-#include "chromeos/ash/services/auth_factor_config/quick_unlock_storage_delegate.h"
 #include "chromeos/ash/services/auth_factor_config/recovery_factor_editor.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 
@@ -32,6 +32,11 @@ void BindToRecoveryFactorEditor(
 mojom::RecoveryFactorEditor& GetRecoveryFactorEditor(
     QuickUnlockStorageDelegate&);
 
+void BindToPinFactorEditor(
+    mojo::PendingReceiver<mojom::PinFactorEditor> receiver,
+    QuickUnlockStorageDelegate&,
+    PinBackendDelegate&);
+
 }  // namespace ash::auth
 
 #endif  // CHROMEOS_ASH_SERVICES_AUTH_FACTOR_CONFIG_IN_PROCESS_INSTANCES_H_