quick_unlock: Add TestApi to override quick_unlock checks

Currently, to prevent need of setting up user prefs in every test that
want to interact with quick_unlock, there's an EnabledForTesting API
that sets a global test flag to override all checks. But there are more
than one purposes an auth method can be used for, and also more than one
quick_unlock auth methods. Making the APIs and underlying mechanisms
more complicate can give tests more fine-grained control of what they
expect to be enabled in the test, and might help us capture some bugs in
code that accessed wrong set of purposes for auth methods. Put these
test apis into a TestApi class.

Bug: b:214871750
Test: CQ
Test: all modified unit tests, browser tests
Change-Id: I7d67fd8de77c35ab4bc36feec93b4c03bc48eacf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3412557
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Jon Mann <jonmann@chromium.org>
Commit-Queue: Howard Yang <hcyang@google.com>
Cr-Commit-Position: refs/heads/main@{#973440}

chrome/browser/ash/login/lock/fingerprint_unlock_browsertest.cc

@@ -68,14 +68,13 @@ class FingerprintUnlockTest : public InProcessBrowserTest {
   ~FingerprintUnlockTest() override = default;
 
   void SetUp() override {
-    quick_unlock::EnabledForTesting(true);
+    test_api_ = std::make_unique<quick_unlock::TestApi>(
+        /*override_quick_unlock=*/true);
+    test_api_->EnableFingerprintByPolicy(quick_unlock::Purpose::kUnlock);
     InProcessBrowserTest::SetUp();
   }
 
-  void TearDown() override {
-    quick_unlock::EnabledForTesting(false);
-    InProcessBrowserTest::TearDown();
-  }
+  void TearDown() override { InProcessBrowserTest::TearDown(); }
 
   void SetUpInProcessBrowserTestFixture() override {
     zero_duration_mode_ =
@@ -235,6 +234,7 @@ class FingerprintUnlockTest : public InProcessBrowserTest {
   QuickUnlockStorage* quick_unlock_storage_;
 
   std::unique_ptr<ui::ScopedAnimationDurationScaleMode> zero_duration_mode_;
+  std::unique_ptr<quick_unlock::TestApi> test_api_;
 };
 
 // Provides test clocks, quick unlock and an enrolled fingerprint to the tests.

chrome/browser/ash/login/lock/screen_locker_browsertest.cc

@@ -56,10 +56,10 @@ class ScreenLockerTest : public InProcessBrowserTest {
             ui::ScopedAnimationDurationScaleMode::ZERO_DURATION);
   }
 
-  void TearDown() override { quick_unlock::EnabledForTesting(false); }
-
   void EnrollFingerprint() {
-    quick_unlock::EnabledForTesting(true);
+    test_api_ = std::make_unique<quick_unlock::TestApi>(
+        /*override_quick_unlock=*/true);
+    test_api_->EnableFingerprintByPolicy(quick_unlock::Purpose::kUnlock);
 
     FakeBiodClient::Get()->StartEnrollSession(
         "test-user", std::string(),
@@ -87,6 +87,7 @@ class ScreenLockerTest : public InProcessBrowserTest {
   void OnStartSession(const dbus::ObjectPath& path) {}
 
   std::unique_ptr<ui::ScopedAnimationDurationScaleMode> zero_duration_mode_;
+  std::unique_ptr<quick_unlock::TestApi> test_api_;
 };
 
 IN_PROC_BROWSER_TEST_F(ScreenLockerTest, TestBadThenGoodPassword) {

chrome/browser/ash/login/oobe_interactive_ui_test.cc

@@ -562,8 +562,12 @@ class OobeEndToEndTestSetupMixin : public InProcessBrowserTestMixin {
   }
 
   void SetUpInProcessBrowserTestFixture() override {
-    if (params_.is_quick_unlock_enabled)
-      quick_unlock::EnabledForTesting(true);
+    if (params_.is_quick_unlock_enabled) {
+      test_api_ = std::make_unique<quick_unlock::TestApi>(
+          /*override_quick_unlock=*/true);
+      test_api_->EnableFingerprintByPolicy(quick_unlock::Purpose::kAny);
+      test_api_->EnablePinByPolicy(quick_unlock::Purpose::kAny);
+    }
 
     if (params_.arc_state != ArcState::kNotAvailable) {
       recommend_apps_fetcher_factory_ =
@@ -594,8 +598,6 @@ class OobeEndToEndTestSetupMixin : public InProcessBrowserTestMixin {
     recommend_apps_fetcher_factory_.reset();
   }
 
-  void TearDown() override { quick_unlock::EnabledForTesting(false); }
-
   std::unique_ptr<HttpResponse> HandleRequest(const HttpRequest& request) {
     auto response = std::make_unique<BasicHttpResponse>();
     if (request.relative_url != "/arc-tos/about/play-terms.html") {
@@ -627,6 +629,7 @@ class OobeEndToEndTestSetupMixin : public InProcessBrowserTestMixin {
   std::unique_ptr<ScopedTestRecommendAppsFetcherFactory>
       recommend_apps_fetcher_factory_;
   net::EmbeddedTestServer* arc_tos_server_;
+  std::unique_ptr<quick_unlock::TestApi> test_api_;
 };
 
 }  // namespace

chrome/browser/ash/login/quick_unlock/fingerprint_storage_unittest.cc

@@ -37,9 +37,10 @@ class FingerprintStorageUnitTest : public testing::Test {
   ~FingerprintStorageUnitTest() override {}
 
   // testing::Test:
-  void SetUp() override { EnabledForTesting(true); }
-
-  void TearDown() override { EnabledForTesting(false); }
+  void SetUp() override {
+    test_api_ = std::make_unique<TestApi>(/*override_quick_unlock=*/true);
+    test_api_->EnableFingerprintByPolicy(Purpose::kAny);
+  }
 
   void SetRecords(int records_number) {
     profile_->GetPrefs()->SetInteger(prefs::kQuickUnlockFingerprintRecord,
@@ -48,6 +49,7 @@ class FingerprintStorageUnitTest : public testing::Test {
 
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<TestingProfile> profile_;
+  std::unique_ptr<TestApi> test_api_;
 };
 
 }  // namespace

chrome/browser/ash/login/quick_unlock/pin_storage_cryptohome.cc

@@ -130,8 +130,9 @@ void CheckForCryptohomedService(int attempt,
 
 bool IsCryptohomePinDisabledByPolicy(const AccountId& account_id,
                                      Purpose purpose) {
-  if (quick_unlock::IsEnabledForTesting()) {
-    return false;
+  auto* test_api = quick_unlock::TestApi::Get();
+  if (test_api && test_api->IsQuickUnlockOverridden()) {
+    return !test_api->IsPinEnabledByPolicy(purpose);
   }
   PrefService* pref_service = nullptr;
   Profile* profile = ProfileHelper::Get()->GetProfileByAccountId(account_id);

chrome/browser/ash/login/quick_unlock/pin_storage_cryptohome_unittest.cc

@@ -36,7 +36,8 @@ class PinStorageCryptohomeUnitTest : public testing::Test {
 
   // testing::Test:
   void SetUp() override {
-    EnabledForTesting(true);
+    test_api_ = std::make_unique<TestApi>(/*override_quick_unlock=*/true);
+    test_api_->EnablePinByPolicy(Purpose::kAny);
     SystemSaltGetter::Initialize();
     CryptohomeMiscClient::InitializeFake();
     UserDataAuthClient::InitializeFake();
@@ -48,7 +49,6 @@ class PinStorageCryptohomeUnitTest : public testing::Test {
     UserDataAuthClient::Shutdown();
     CryptohomeMiscClient::Shutdown();
     SystemSaltGetter::Shutdown();
-    EnabledForTesting(false);
   }
 
   bool IsPinSet() const {
@@ -178,6 +178,7 @@ class PinStorageCryptohomeUnitTest : public testing::Test {
   std::unique_ptr<PinStorageCryptohome> storage_;
   AccountId test_account_id_{
       AccountId::FromUserEmailGaiaId("user@example.com", "11111")};
+  std::unique_ptr<TestApi> test_api_;
 };
 
 }  // namespace

chrome/browser/ash/login/quick_unlock/pin_storage_prefs_unittest.cc

@@ -28,9 +28,10 @@ class PinStoragePrefsUnitTest : public testing::Test {
   ~PinStoragePrefsUnitTest() override = default;
 
   // testing::Test:
-  void SetUp() override { EnabledForTesting(true); }
-
-  void TearDown() override { EnabledForTesting(false); }
+  void SetUp() override {
+    test_api_ = std::make_unique<TestApi>(/*override_quick_unlock=*/true);
+    test_api_->EnablePinByPolicy(Purpose::kAny);
+  }
 
   PinStoragePrefs* PinStoragePrefs() const {
     return QuickUnlockFactory::GetForProfile(profile_.get())
@@ -39,6 +40,7 @@ class PinStoragePrefsUnitTest : public testing::Test {
 
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<TestingProfile> profile_;
+  std::unique_ptr<TestApi> test_api_;
 };
 
 }  // namespace

chrome/browser/ash/login/quick_unlock/quick_unlock_storage_unittest.cc

@@ -48,8 +48,8 @@ class QuickUnlockStorageUnitTest : public testing::Test {
   ~QuickUnlockStorageUnitTest() override {}
 
   // testing::Test:
-  void SetUp() override { EnabledForTesting(true); }
-  void TearDown() override { EnabledForTesting(false); }
+  void SetUp() override {}
+  void TearDown() override {}
 
   void ExpireAuthToken() {
     QuickUnlockFactory::GetForProfile(profile_.get())->auth_token_->Reset();

chrome/browser/ash/login/quick_unlock/quick_unlock_utils.cc

@@ -30,9 +30,7 @@ namespace ash {
 namespace quick_unlock {
 namespace {
 
-// Quick unlock is enabled regardless of flags.
-bool enabled_for_testing_ = false;
-bool disable_pin_by_policy_for_testing_ = false;
+TestApi* g_instance = nullptr;
 
 // Options for the quick unlock allowlist.
 const char kFactorsOptionAll[] = "all";
@@ -83,8 +81,59 @@ bool IsPinDisabledByPolicySinglePurpose(const PrefService* pref_service,
 
 }  // namespace
 
+TestApi::TestApi(bool override_quick_unlock)
+    : overridden_(override_quick_unlock) {
+  old_instance_ = g_instance;
+  g_instance = this;
+  std::fill(pin_purposes_enabled_by_policy_,
+            pin_purposes_enabled_by_policy_ + kNumOfPurposes, false);
+  std::fill(fingerprint_purposes_enabled_by_policy_,
+            fingerprint_purposes_enabled_by_policy_ + kNumOfPurposes, false);
+}
+
+TestApi::~TestApi() {
+  CHECK_EQ(g_instance, this);
+  g_instance = old_instance_;
+}
+
+TestApi* TestApi::Get() {
+  return g_instance;
+}
+
+bool TestApi::IsQuickUnlockOverridden() {
+  return overridden_;
+}
+
+void TestApi::EnablePinByPolicy(Purpose purpose) {
+  if (purpose != Purpose::kAny) {
+    pin_purposes_enabled_by_policy_[static_cast<int>(Purpose::kAny)] = true;
+  }
+  pin_purposes_enabled_by_policy_[static_cast<int>(purpose)] = true;
+}
+
+void TestApi::EnableFingerprintByPolicy(Purpose purpose) {
+  if (purpose != Purpose::kAny) {
+    fingerprint_purposes_enabled_by_policy_[static_cast<int>(Purpose::kAny)] =
+        true;
+  }
+  fingerprint_purposes_enabled_by_policy_[static_cast<int>(purpose)] = true;
+}
+
+bool TestApi::IsPinEnabledByPolicy(Purpose purpose) {
+  return pin_purposes_enabled_by_policy_[static_cast<int>(purpose)];
+}
+
+bool TestApi::IsFingerprintEnabledByPolicy(Purpose purpose) {
+  return fingerprint_purposes_enabled_by_policy_[static_cast<int>(purpose)];
+}
+
 bool IsFingerprintDisabledByPolicy(const PrefService* pref_service,
                                    Purpose purpose) {
+  auto* test_api = TestApi::Get();
+  if (test_api && test_api->IsQuickUnlockOverridden()) {
+    return !test_api->IsFingerprintEnabledByPolicy(purpose);
+  }
+
   if (purpose == Purpose::kAny) {
     return IsFingerprintDisabledByPolicySinglePurpose(pref_service,
                                                       Purpose::kUnlock) &&
@@ -137,11 +186,10 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry) {
 }
 
 bool IsPinDisabledByPolicy(PrefService* pref_service, Purpose purpose) {
-  if (disable_pin_by_policy_for_testing_)
-    return true;
-
-  if (enabled_for_testing_)
-    return false;
+  auto* test_api = TestApi::Get();
+  if (test_api && test_api->IsQuickUnlockOverridden()) {
+    return !test_api->IsPinEnabledByPolicy(purpose);
+  }
 
   if (purpose == Purpose::kAny) {
     return IsPinDisabledByPolicySinglePurpose(pref_service, Purpose::kUnlock) &&
@@ -181,7 +229,8 @@ FingerprintLocation GetFingerprintLocation() {
 }
 
 bool IsFingerprintSupported() {
-  if (enabled_for_testing_)
+  auto* test_api = TestApi::Get();
+  if (test_api && test_api->IsQuickUnlockOverridden())
     return true;
 
   const base::CommandLine* command_line =
@@ -191,15 +240,17 @@ bool IsFingerprintSupported() {
 }
 
 bool IsFingerprintEnabled(Profile* profile, Purpose purpose) {
-  if (enabled_for_testing_)
-    return true;
+  // Don't need to check these when using flags to control fingerprint behavior
+  // in tests.
+  auto* test_api = TestApi::Get();
+  if (!test_api || !test_api->IsQuickUnlockOverridden()) {
+    if (!IsFingerprintSupported())
+      return false;
 
-  if (!IsFingerprintSupported())
-    return false;
-
-  // Disable fingerprint if the profile does not belong to the primary user.
-  if (profile != ProfileManager::GetPrimaryUserProfile())
-    return false;
+    // Disable fingerprint if the profile does not belong to the primary user.
+    if (profile != ProfileManager::GetPrimaryUserProfile())
+      return false;
+  }
 
   // Disable fingerprint if disallowed by policy.
   if (IsFingerprintDisabledByPolicy(profile->GetPrefs(), purpose))
@@ -253,17 +304,5 @@ void AddFingerprintResources(content::WebUIDataSource* html_source) {
                           is_lottie_animation);
 }
 
-void EnabledForTesting(bool state) {
-  enabled_for_testing_ = state;
-}
-
-bool IsEnabledForTesting() {
-  return enabled_for_testing_;
-}
-
-void DisablePinByPolicyForTesting(bool disable) {
-  disable_pin_by_policy_for_testing_ = disable;
-}
-
 }  // namespace quick_unlock
 }  // namespace ash

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

@@ -26,6 +26,8 @@ enum class Purpose {
   kAny,
   kUnlock,
   kWebAuthn,
+  // Total number of available purposes.
+  kNumOfPurposes,
 };
 
 // Enumeration specifying the possible intervals before a strong auth
@@ -50,6 +52,50 @@ enum class FingerprintLocation {
   UNKNOWN = 6,
 };
 
+// Override quick unlock checks for testing.
+class TestApi {
+ public:
+  // Setting state to true has an effect that instead of checking user prefs,
+  // quick_unlock will use test flags to determine whether PIN/fingerprint is
+  // enabled by policy. Setting to false resets the effect and quick_unlock will
+  // resume to checking prefs, and also clears the previously set test flags.
+  // Typical usage is setting state to true and enabling the purposes a test
+  // needs by calling Enable*ByPolicyForTesting in SetUp, and setting state to
+  // false in TearDown.
+  explicit TestApi(bool override_quick_unlock);
+
+  ~TestApi();
+
+  static TestApi* Get();
+
+  // Returns the current state of OverrideQuickUnlock.
+  bool IsQuickUnlockOverridden();
+
+  // Enable the specified purpose for PIN using a test flag. All purposes that
+  // are not enabled will be treated as disabled when
+  // EnablePinByPolicyUsingFlagsForTesting's state is true. When called, this
+  // will automatically set OverrideQuickUnlock's state to true.
+  void EnablePinByPolicy(Purpose purpose);
+
+  // Enable the specified purpose for fingerprint using a test flag. All
+  // purposes that are not enabled will be treated as disabled when
+  // EnableFingerprintByPolicyUsingFlagsForTesting's state is true. When called,
+  // this will automatically set OverrideQuickUnlock's state to true.
+  void EnableFingerprintByPolicy(Purpose purpose);
+
+  bool IsPinEnabledByPolicy(Purpose purpose);
+  bool IsFingerprintEnabledByPolicy(Purpose purpose);
+
+ private:
+  static constexpr int kNumOfPurposes =
+      static_cast<int>(Purpose::kNumOfPurposes);
+
+  TestApi* old_instance_;
+  bool overridden_;
+  bool pin_purposes_enabled_by_policy_[kNumOfPurposes];
+  bool fingerprint_purposes_enabled_by_policy_[kNumOfPurposes];
+};
+
 base::TimeDelta PasswordConfirmationFrequencyToTimeDelta(
     PasswordConfirmationFrequency frequency);
 
@@ -81,15 +127,6 @@ bool IsFingerprintDisabledByPolicy(const PrefService* pref_service,
 // Is used to display correct UI assets. Returns TABLET_POWER_BUTTON by default.
 FingerprintLocation GetFingerprintLocation();
 
-// Enable or Disable quick-unlock modes for testing
-void EnabledForTesting(bool state);
-
-// Returns true if EnableForTesting() was previously called.
-bool IsEnabledForTesting();
-
-// Forcibly disable PIN for testing purposes.
-void DisablePinByPolicyForTesting(bool disable);
-
 // Add fingerprint animations and illustrations. Used for the Fingerprint setup
 // screen and the settings.
 void AddFingerprintResources(content::WebUIDataSource* html_source);
@@ -102,14 +139,13 @@ void AddFingerprintResources(content::WebUIDataSource* html_source);
 namespace chromeos {
 namespace quick_unlock {
 using ::ash::quick_unlock::AddFingerprintResources;
-using ::ash::quick_unlock::DisablePinByPolicyForTesting;
-using ::ash::quick_unlock::EnabledForTesting;
 using ::ash::quick_unlock::FingerprintLocation;
 using ::ash::quick_unlock::GetFingerprintLocation;
 using ::ash::quick_unlock::IsFingerprintEnabled;
 using ::ash::quick_unlock::IsPinDisabledByPolicy;
 using ::ash::quick_unlock::IsPinEnabled;
 using ::ash::quick_unlock::Purpose;
+using ::ash::quick_unlock::TestApi;
 }  // namespace quick_unlock
 }  // namespace chromeos
 

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

@@ -55,7 +55,9 @@ class FingerprintSetupTest : public OobeBaseTest {
 
   void SetUpOnMainThread() override {
     // Enable fingerprint for testing.
-    quick_unlock::EnabledForTesting(true);
+    test_api_ = std::make_unique<quick_unlock::TestApi>(
+        /*override_quick_unlock=*/true);
+    test_api_->EnableFingerprintByPolicy(quick_unlock::Purpose::kUnlock);
 
     // Override the screen exit callback with our own method.
     FingerprintSetupScreen* fingerprint_screen =
@@ -143,6 +145,7 @@ class FingerprintSetupTest : public OobeBaseTest {
   FingerprintSetupScreen::ScreenExitCallback original_callback_;
   base::RepeatingClosure screen_exit_callback_;
   LoginManagerMixin login_manager_{&mixin_host_};
+  std::unique_ptr<quick_unlock::TestApi> test_api_;
 };
 
 IN_PROC_BROWSER_TEST_F(FingerprintSetupTest, FingerprintEnrollHalf) {
@@ -204,8 +207,9 @@ IN_PROC_BROWSER_TEST_F(FingerprintSetupTest, FingerprintEnrollLimit) {
 IN_PROC_BROWSER_TEST_F(FingerprintSetupTest, FingerprintDisabled) {
   PerformLogin();
 
-  // Disable fingerprint
-  quick_unlock::EnabledForTesting(false);
+  // Disable fingerprint (resetting flags).
+  auto test_api = std::make_unique<quick_unlock::TestApi>(
+      /*override_quick_unlock=*/true);
 
   WizardController::default_controller()->AdvanceToScreen(
       FingerprintSetupScreenView::kScreenId);

chrome/browser/extensions/api/quick_unlock_private/quick_unlock_private_api_unittest.cc

@@ -196,7 +196,9 @@ class QuickUnlockPrivateUnitTest
         {1, 2, 3, 4, 5, 6, 7, 8});
 
     // Rebuild quick unlock state.
-    ash::quick_unlock::EnabledForTesting(true);
+    test_api_ = std::make_unique<ash::quick_unlock::TestApi>(
+        /*override_quick_unlock=*/true);
+    test_api_->EnablePinByPolicy(ash::quick_unlock::Purpose::kAny);
     ash::quick_unlock::PinBackend::ResetForTesting();
 
     base::RunLoop().RunUntilIdle();
@@ -237,15 +239,13 @@ class QuickUnlockPrivateUnitTest
   }
 
   void TearDown() override {
-    ash::quick_unlock::EnabledForTesting(false);
-    ash::quick_unlock::DisablePinByPolicyForTesting(false);
-
     base::RunLoop().RunUntilIdle();
 
     ExtensionApiUnittest::TearDown();
 
     fake_user_manager_ = nullptr;
     scoped_user_manager_.reset();
+    test_api_.reset();
 
     ash::SystemSaltGetter::Shutdown();
     ash::UserDataAuthClient::Shutdown();
@@ -616,6 +616,12 @@ class QuickUnlockPrivateUnitTest
 
   bool IsAutosubmitFeatureEnabled() { return std::get<1>(GetParam()); }
 
+  void DisablePinByPolicy() {
+    test_api_.reset();
+    test_api_ = std::make_unique<ash::quick_unlock::TestApi>(
+        /*override_quick_unlock=*/true);
+  }
+
   base::test::ScopedFeatureList feature_list_;
   sync_preferences::TestingPrefServiceSyncable* test_pref_service_;
 
@@ -662,6 +668,7 @@ class QuickUnlockPrivateUnitTest
   bool expect_modes_changed_ = false;
   ash::UserContext auth_token_user_context_;
   std::string token_;
+  std::unique_ptr<ash::quick_unlock::TestApi> test_api_;
 };
 
 // Verifies that GetAuthTokenValid succeeds when a valid password is provided.
@@ -716,14 +723,16 @@ TEST_P(QuickUnlockPrivateUnitTest, GetAvailableModes) {
   EXPECT_EQ(GetAvailableModes(),
             QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN});
 
-  ash::quick_unlock::DisablePinByPolicyForTesting(true);
+  // Reset the flags set in Setup.
+  DisablePinByPolicy();
   EXPECT_TRUE(GetAvailableModes().empty());
 }
 
 // Verfies that trying to set modes with a valid PIN failes when PIN is blocked
 // by policy.
 TEST_P(QuickUnlockPrivateUnitTest, SetModesForPinFailsWhenPinDisabledByPolicy) {
-  ash::quick_unlock::DisablePinByPolicyForTesting(true);
+  // Reset the flags set in Setup.
+  DisablePinByPolicy();
   EXPECT_FALSE(SetModesWithError("[\"valid\", [\"PIN\"], [\"111\"]]").empty());
 }
 

docs/login/fingerprint.md

@@ -21,8 +21,8 @@ dbus service. Tests can use `FakeBiodClient` to create an enrollment session and
 simulate fingerprint enrollment scan progress.
 
 Fingerprint unlock policy is dependent on device policy and hardware specs.
-Fingerprint can be manually enabled for testing by calling:
-ash::quick_unlock::EnabledForTesting(true) to force enable fingerprint features.
+Fingerprint can be manually enabled for testing using ash::quick_unlock::TestApi
+to force enable fingerprint features.
 
 Fingerprint unlock is not considered strong authentication. This means that,
 under certain policies, fingerprint cannot be the only method of authentication