From afd3531ccc130b096ca4f2566a6aa89ddaeb12ec Mon Sep 17 00:00:00 2001 From: Brittany Hartmire Date: Thu, 16 Mar 2023 00:21:03 +0000 Subject: [PATCH] [SmartLock] Delete ProximityAuthLocalStatePrefManager Now that sign in with Smart Lock is deprecated, we no longer need to access any Smart Lock prefs outside of a user profile. This cl removes Smart Lock local state prefs and the ProximityAuthLocalStatePrefManager. TEST=existing unittests pass, manually tested Smart Lock with strongbad and pixel4a. Change-Id: I364801111bc8260fc0f7cb1b3225fcea77781644 Bug: b/227674947 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4335758 Reviewed-by: Tommy Nyquist Reviewed-by: Ryan Hansberry Commit-Queue: Brittany Hartmire Cr-Commit-Position: refs/heads/main@{#1117868} --- .../login/easy_unlock/easy_unlock_service.cc | 5 +- .../login/easy_unlock/easy_unlock_service.h | 1 + .../easy_unlock_service_regular.cc | 2 - chrome/browser/prefs/browser_prefs.cc | 15 ++ chrome/common/pref_names.cc | 6 - chrome/common/pref_names.h | 1 - .../ash/components/proximity_auth/BUILD.gn | 3 - ...proximity_auth_local_state_pref_manager.cc | 177 ------------------ .../proximity_auth_local_state_pref_manager.h | 73 -------- ..._auth_local_state_pref_manager_unittest.cc | 146 --------------- .../proximity_auth_pref_manager.h | 6 + .../proximity_auth_pref_names.cc | 10 +- .../proximity_auth_pref_names.h | 1 - .../proximity_auth_profile_pref_manager.cc | 76 +------- .../proximity_auth_profile_pref_manager.h | 14 -- ...mity_auth_profile_pref_manager_unittest.cc | 36 ---- 16 files changed, 33 insertions(+), 539 deletions(-) delete mode 100644 chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.cc delete mode 100644 chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h delete mode 100644 chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager_unittest.cc diff --git a/chrome/browser/ash/login/easy_unlock/easy_unlock_service.cc b/chrome/browser/ash/login/easy_unlock/easy_unlock_service.cc index e758b121395bb..e9a3dc6c4bf86 100644 --- a/chrome/browser/ash/login/easy_unlock/easy_unlock_service.cc +++ b/chrome/browser/ash/login/easy_unlock/easy_unlock_service.cc @@ -34,7 +34,6 @@ #include "chromeos/ash/components/dbus/dbus_thread_manager.h" #include "chromeos/ash/components/login/auth/public/user_context.h" #include "chromeos/ash/components/multidevice/logging/logging.h" -#include "chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h" #include "chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.h" #include "chromeos/ash/components/proximity_auth/proximity_auth_system.h" #include "chromeos/ash/components/proximity_auth/screenlock_bridge.h" @@ -146,7 +145,6 @@ void EasyUnlockService::RegisterProfilePrefs( // static void EasyUnlockService::RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterDictionaryPref(prefs::kEasyUnlockHardlockState); - proximity_auth::ProximityAuthLocalStatePrefManager::RegisterPrefs(registry); } // static @@ -158,8 +156,7 @@ void EasyUnlockService::ResetLocalStateForUser(const AccountId& account_id) { return; for (const std::string& pref : - std::vector{prefs::kEasyUnlockHardlockState, - prefs::kEasyUnlockLocalStateUserPrefs}) { + std::vector{prefs::kEasyUnlockHardlockState}) { ScopedDictPrefUpdate update(local_state, pref); update->Remove(account_id.GetUserEmail()); } diff --git a/chrome/browser/ash/login/easy_unlock/easy_unlock_service.h b/chrome/browser/ash/login/easy_unlock/easy_unlock_service.h index 17b19768470de..12b9f9f1d8bbf 100644 --- a/chrome/browser/ash/login/easy_unlock/easy_unlock_service.h +++ b/chrome/browser/ash/login/easy_unlock/easy_unlock_service.h @@ -63,6 +63,7 @@ class EasyUnlockService : public KeyedService, static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); // Registers Easy Unlock local state entries. + // TODO(b/227674947): Delete static void RegisterPrefs(PrefRegistrySimple* registry); // Removes the hardlock state for the given user. diff --git a/chrome/browser/ash/login/easy_unlock/easy_unlock_service_regular.cc b/chrome/browser/ash/login/easy_unlock/easy_unlock_service_regular.cc index bfc62c6b9894b..e31ec35d661f1 100644 --- a/chrome/browser/ash/login/easy_unlock/easy_unlock_service_regular.cc +++ b/chrome/browser/ash/login/easy_unlock/easy_unlock_service_regular.cc @@ -186,8 +186,6 @@ void EasyUnlockServiceRegular::InitializeInternal() { pref_manager_ = std::make_unique( profile()->GetPrefs(), multidevice_setup_client_); - pref_manager_->StartSyncingToLocalState(g_browser_process->local_state(), - GetAccountId()); // If `device_sync_client_` is not ready yet, wait for it to call back on // OnReady(). diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc index 2aeb117f0c302..3826c517befda 100644 --- a/chrome/browser/prefs/browser_prefs.cc +++ b/chrome/browser/prefs/browser_prefs.cc @@ -818,6 +818,11 @@ const char kGlanceablesSignoutScreenshotDuration[] = "ash.signout_screenshot.duration"; #endif // BUILDFLAG(IS_CHROMEOS_ASH) +// Deprecated 03/2023. +#if BUILDFLAG(IS_CHROMEOS_ASH) +const char kEasyUnlockLocalStateUserPrefs[] = "easy_unlock.user_prefs"; +#endif // BUILDFLAG(IS_CHROMEOS_ASH) + // Register local state used only for migration (clearing or moving to a new // key). void RegisterLocalStatePrefsForMigration(PrefRegistrySimple* registry) { @@ -910,6 +915,11 @@ void RegisterLocalStatePrefsForMigration(PrefRegistrySimple* registry) { registry->RegisterTimeDeltaPref(kGlanceablesSignoutScreenshotDuration, base::TimeDelta()); #endif // BUILDFLAG(IS_CHROMEOS_ASH) + +// Deprecated 03/2023. +#if BUILDFLAG(IS_CHROMEOS_ASH) + registry->RegisterDictionaryPref(kEasyUnlockLocalStateUserPrefs); +#endif // BUILDFLAG(IS_CHROMEOS_ASH) } // Register prefs used only for migration (clearing or moving to a new key). @@ -1882,6 +1892,11 @@ void MigrateObsoleteLocalStatePrefs(PrefService* local_state) { local_state->ClearPref(kGlanceablesSignoutScreenshotDuration); #endif // BUILDFLAG(IS_CHROMEOS_ASH) +// Added 03/2023. +#if BUILDFLAG(IS_CHROMEOS_ASH) + local_state->ClearPref(kEasyUnlockLocalStateUserPrefs); +#endif // BUILDFLAG(IS_CHROMEOS_ASH) + // Please don't delete the following line. It is used by PRESUBMIT.py. // END_MIGRATE_OBSOLETE_LOCAL_STATE_PREFS diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 405454e27eaf9..e7ebdddb326bb 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -2923,12 +2923,6 @@ const char kCryptAuthInstanceIdToken[] = "cryptauth.instance_id_token"; // A dictionary that maps user id to hardlock state. const char kEasyUnlockHardlockState[] = "easy_unlock.hardlock_state"; -// A dictionary in local state containing each user's Easy Unlock profile -// preferences, so they can be accessed outside of the user's profile. The value -// is a dictionary containing an entry for each user. Each user's entry mirrors -// their profile's Easy Unlock preferences. -const char kEasyUnlockLocalStateUserPrefs[] = "easy_unlock.user_prefs"; - // Boolean that indicates whether elevation is needed to recover Chrome upgrade. const char kRecoveryComponentNeedsElevation[] = "recovery_component.needs_elevation"; diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index fb08a62d9544e..c6807efcad18c 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -985,7 +985,6 @@ extern const char kCryptAuthDeviceId[]; extern const char kCryptAuthInstanceId[]; extern const char kCryptAuthInstanceIdToken[]; extern const char kEasyUnlockHardlockState[]; -extern const char kEasyUnlockLocalStateUserPrefs[]; extern const char kRecoveryComponentNeedsElevation[]; diff --git a/chromeos/ash/components/proximity_auth/BUILD.gn b/chromeos/ash/components/proximity_auth/BUILD.gn index 1e8ede2bebb68..0c57e65be7cec 100644 --- a/chromeos/ash/components/proximity_auth/BUILD.gn +++ b/chromeos/ash/components/proximity_auth/BUILD.gn @@ -16,8 +16,6 @@ static_library("proximity_auth") { "metrics.cc", "metrics.h", "proximity_auth_client.h", - "proximity_auth_local_state_pref_manager.cc", - "proximity_auth_local_state_pref_manager.h", "proximity_auth_pref_manager.h", "proximity_auth_pref_names.cc", "proximity_auth_pref_names.h", @@ -96,7 +94,6 @@ source_set("unit_tests") { testonly = true sources = [ "messenger_impl_unittest.cc", - "proximity_auth_local_state_pref_manager_unittest.cc", "proximity_auth_profile_pref_manager_unittest.cc", "proximity_auth_system_unittest.cc", "proximity_monitor_impl_unittest.cc", diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.cc b/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.cc deleted file mode 100644 index c22a33004c4de..0000000000000 --- a/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.cc +++ /dev/null @@ -1,177 +0,0 @@ -// Copyright 2017 The Chromium Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h" - -#include -#include - -#include "base/logging.h" -#include "base/values.h" -#include "chromeos/ash/components/multidevice/logging/logging.h" -#include "chromeos/ash/components/proximity_auth/proximity_auth_pref_names.h" -#include "chromeos/ash/services/multidevice_setup/public/cpp/prefs.h" -#include "components/prefs/pref_registry_simple.h" -#include "components/prefs/pref_service.h" -#include "components/prefs/scoped_user_pref_update.h" - -namespace proximity_auth { - -ProximityAuthLocalStatePrefManager::ProximityAuthLocalStatePrefManager( - PrefService* local_state) - : local_state_(local_state) {} - -ProximityAuthLocalStatePrefManager::~ProximityAuthLocalStatePrefManager() {} - -// static. -void ProximityAuthLocalStatePrefManager::RegisterPrefs( - PrefRegistrySimple* registry) { - // Prefs for all users are stored in a dictionary under this pref name. - registry->RegisterDictionaryPref(prefs::kEasyUnlockLocalStateUserPrefs); - - // Most Smart Lock prefs are stored in regular user prefs, and then copied out - // to local state for reference. This particular pref, in contrast, needs its - // source of truth to be in the local state, because it needs to be written - // to from the login screen. - registry->RegisterDictionaryPref( - prefs::kProximityAuthHasShownLoginDisabledMessage); -} - -void ProximityAuthLocalStatePrefManager::SetIsEasyUnlockEnabled( - bool is_easy_unlock_enabled) const { - NOTREACHED(); -} - -void ProximityAuthLocalStatePrefManager::SetEasyUnlockEnabledStateSet() const { - NOTREACHED(); -} - -void ProximityAuthLocalStatePrefManager::SetActiveUser( - const AccountId& active_user) { - active_user_ = active_user; -} - -void ProximityAuthLocalStatePrefManager::SetLastPromotionCheckTimestampMs( - int64_t timestamp_ms) { - NOTREACHED(); -} - -int64_t ProximityAuthLocalStatePrefManager::GetLastPromotionCheckTimestampMs() - const { - NOTREACHED(); - return 0; -} - -void ProximityAuthLocalStatePrefManager::SetPromotionShownCount(int count) { - NOTREACHED(); -} - -int ProximityAuthLocalStatePrefManager::GetPromotionShownCount() const { - NOTREACHED(); - return 0; -} - -bool ProximityAuthLocalStatePrefManager::IsEasyUnlockAllowed() const { - const base::Value::Dict* user_prefs = GetActiveUserPrefsDictionary(); - if (user_prefs) { - absl::optional pref_value = - user_prefs->FindBool(ash::multidevice_setup::kSmartLockAllowedPrefName); - if (pref_value.has_value()) { - return pref_value.value(); - } - } - PA_LOG(ERROR) << "Failed to get easyunlock_allowed."; - return true; -} - -bool ProximityAuthLocalStatePrefManager::IsEasyUnlockEnabled() const { - const base::Value::Dict* user_prefs = GetActiveUserPrefsDictionary(); - if (user_prefs) { - absl::optional pref_value = - user_prefs->FindBool(ash::multidevice_setup::kSmartLockEnabledPrefName); - if (pref_value.has_value()) { - return pref_value.value(); - } - } - PA_LOG(ERROR) << "Failed to get easyunlock_enabled."; - return false; -} - -bool ProximityAuthLocalStatePrefManager::IsEasyUnlockEnabledStateSet() const { - NOTREACHED(); - return false; -} - -bool ProximityAuthLocalStatePrefManager::IsChromeOSLoginAllowed() const { - const base::Value::Dict* user_prefs = GetActiveUserPrefsDictionary(); - if (user_prefs) { - absl::optional pref_value = user_prefs->FindBool( - ash::multidevice_setup::kSmartLockSigninAllowedPrefName); - if (pref_value.has_value()) { - return pref_value.value(); - } - } - PA_LOG(VERBOSE) << "Failed to get is_chrome_login_allowed, not disallowing"; - return true; -} - -void ProximityAuthLocalStatePrefManager::SetIsChromeOSLoginEnabled( - bool is_enabled) { - NOTREACHED(); -} - -bool ProximityAuthLocalStatePrefManager::IsChromeOSLoginEnabled() const { - const base::Value::Dict* user_prefs = GetActiveUserPrefsDictionary(); - if (user_prefs) { - absl::optional pref_value = - user_prefs->FindBool(prefs::kProximityAuthIsChromeOSLoginEnabled); - if (pref_value.has_value()) { - return pref_value.value(); - } - } - PA_LOG(ERROR) << "Failed to get is_chrome_login_enabled."; - return false; -} - -void ProximityAuthLocalStatePrefManager::SetHasShownLoginDisabledMessage( - bool has_shown) { - ScopedDictPrefUpdate update(local_state_, - prefs::kEasyUnlockLocalStateUserPrefs); - - // Get or create a dictionary to persist `has_shown` for `active_user_`. - base::Value::Dict* current_user_prefs = - update->EnsureDict(active_user_.GetUserEmail()); - current_user_prefs->Set(prefs::kProximityAuthHasShownLoginDisabledMessage, - has_shown); -} - -bool ProximityAuthLocalStatePrefManager::HasShownLoginDisabledMessage() const { - const base::Value::Dict* user_prefs = GetActiveUserPrefsDictionary(); - if (!user_prefs) - return false; - - return user_prefs->FindBool(prefs::kProximityAuthHasShownLoginDisabledMessage) - .value_or(false); -} - -const base::Value::Dict* -ProximityAuthLocalStatePrefManager::GetActiveUserPrefsDictionary() const { - if (!active_user_.is_valid()) { - PA_LOG(ERROR) << "No active account."; - return nullptr; - } - - const base::Value::Dict& all_user_prefs_dict = - local_state_->GetDict(prefs::kEasyUnlockLocalStateUserPrefs); - - const base::Value::Dict* current_user_prefs = - all_user_prefs_dict.FindDict(active_user_.GetUserEmail()); - if (!current_user_prefs) { - PA_LOG(ERROR) << "Failed to find prefs for current user."; - return nullptr; - } - return current_user_prefs; -} - -} // namespace proximity_auth diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h b/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h deleted file mode 100644 index f9381d8342528..0000000000000 --- a/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright 2017 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_COMPONENTS_PROXIMITY_AUTH_PROXIMITY_AUTH_LOCAL_STATE_PREF_MANAGER_H_ -#define CHROMEOS_ASH_COMPONENTS_PROXIMITY_AUTH_PROXIMITY_AUTH_LOCAL_STATE_PREF_MANAGER_H_ - -#include "base/values.h" -#include "chromeos/ash/components/proximity_auth/proximity_auth_pref_manager.h" -#include "components/account_id/account_id.h" - -class PrefRegistrySimple; -class PrefService; - -namespace proximity_auth { - -// Implementation of ProximityAuthPrefManager based on the device's local state, -// used before the user logs in. After login, ProximityAuthProfilePrefManager is -// used to manage prefs inside the user session. -// Note: All prefs managed by this class are read-only. These prefs are synced -// from each of the user's profile prefs. For privacy reasons, only a subset of -// all prefs are accessible from the local state. -class ProximityAuthLocalStatePrefManager : public ProximityAuthPrefManager { - public: - explicit ProximityAuthLocalStatePrefManager(PrefService* local_state); - - ProximityAuthLocalStatePrefManager( - const ProximityAuthLocalStatePrefManager&) = delete; - ProximityAuthLocalStatePrefManager& operator=( - const ProximityAuthLocalStatePrefManager&) = delete; - - ~ProximityAuthLocalStatePrefManager() override; - - // Registers the prefs used by this class to the given |pref_service|. - static void RegisterPrefs(PrefRegistrySimple* registry); - - // Changes the current user for whom to fetch prefs, i.e. when the focused - // user pod changes. - void SetActiveUser(const AccountId& active_user); - AccountId active_user() { return active_user_; } - - // ProximityAuthPrefManager: - bool IsEasyUnlockAllowed() const override; - bool IsEasyUnlockEnabled() const override; - bool IsEasyUnlockEnabledStateSet() const override; - bool IsChromeOSLoginAllowed() const override; - bool IsChromeOSLoginEnabled() const override; - - private: - // ProximityAuthPrefManager: - void SetIsEasyUnlockEnabled(bool is_easy_unlock_enabled) const override; - void SetEasyUnlockEnabledStateSet() const override; - void SetLastPromotionCheckTimestampMs(int64_t timestamp_ms) override; - int64_t GetLastPromotionCheckTimestampMs() const override; - void SetPromotionShownCount(int count) override; - int GetPromotionShownCount() const override; - void SetIsChromeOSLoginEnabled(bool is_enabled) override; - void SetHasShownLoginDisabledMessage(bool has_shown) override; - bool HasShownLoginDisabledMessage() const override; - - const base::Value::Dict* GetActiveUserPrefsDictionary() const; - - // Contains local state preferences that outlive the lifetime of this object - // and across process restarts. Not owned and must outlive this instance. - PrefService* local_state_; - - // The account id of the active user for which to fetch the prefs. - AccountId active_user_; -}; - -} // namespace proximity_auth - -#endif // CHROMEOS_ASH_COMPONENTS_PROXIMITY_AUTH_PROXIMITY_AUTH_LOCAL_STATE_PREF_MANAGER_H_ diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager_unittest.cc b/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager_unittest.cc deleted file mode 100644 index bb049a2fc4d20..0000000000000 --- a/chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager_unittest.cc +++ /dev/null @@ -1,146 +0,0 @@ -// Copyright 2017 The Chromium Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h" - -#include -#include -#include - -#include "base/json/json_reader.h" -#include "chromeos/ash/components/proximity_auth/proximity_auth_pref_names.h" -#include "chromeos/ash/services/multidevice_setup/public/cpp/prefs.h" -#include "components/prefs/scoped_user_pref_update.h" -#include "components/prefs/testing_pref_service.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace proximity_auth { -namespace { - -const char kUser1[] = "songttim@gmail.com"; -const bool kIsChromeOSLoginEnabled1 = true; -const bool kIsEasyUnlockAllowed1 = true; -const bool kIsEasyUnlockEnabled1 = true; - -const char kUser2[] = "tengs@google.com"; -const bool kIsChromeOSLoginEnabled2 = false; -const bool kIsEasyUnlockAllowed2 = false; -const bool kIsEasyUnlockEnabled2 = false; - -const char kUnknownUser[] = "tengs@chromium.org"; -const bool kIsChromeOSLoginEnabledDefault = false; -const bool kIsEasyUnlockAllowedDefault = true; -const bool kIsEasyUnlockEnabledDefault = false; - -} // namespace - -class ProximityAuthLocalStatePrefManagerTest : public testing::Test { - public: - ProximityAuthLocalStatePrefManagerTest( - const ProximityAuthLocalStatePrefManagerTest&) = delete; - ProximityAuthLocalStatePrefManagerTest& operator=( - const ProximityAuthLocalStatePrefManagerTest&) = delete; - - protected: - ProximityAuthLocalStatePrefManagerTest() - : user1_(AccountId::FromUserEmail(kUser1)), - user2_(AccountId::FromUserEmail(kUser2)), - unknown_user_(AccountId::FromUserEmail(kUnknownUser)) {} - - ~ProximityAuthLocalStatePrefManagerTest() override {} - - void SetUp() override { - ProximityAuthLocalStatePrefManager::RegisterPrefs(local_state_.registry()); - - // Note: in normal circumstances, these prefs are synced to local state in - // ProximityAuthProfilePrefService. - base::Value::Dict user1_prefs; - user1_prefs.Set(proximity_auth::prefs::kProximityAuthIsChromeOSLoginEnabled, - kIsChromeOSLoginEnabled1); - user1_prefs.Set(ash::multidevice_setup::kSmartLockAllowedPrefName, - kIsEasyUnlockAllowed1); - user1_prefs.Set(ash::multidevice_setup::kSmartLockEnabledPrefName, - kIsEasyUnlockEnabled1); - ScopedDictPrefUpdate update1(&local_state_, - prefs::kEasyUnlockLocalStateUserPrefs); - update1->Set(user1_.GetUserEmail(), std::move(user1_prefs)); - - base::Value::Dict user2_prefs; - user2_prefs.Set(proximity_auth::prefs::kProximityAuthIsChromeOSLoginEnabled, - kIsChromeOSLoginEnabled2); - user2_prefs.Set(ash::multidevice_setup::kSmartLockAllowedPrefName, - kIsEasyUnlockAllowed2); - user2_prefs.Set(ash::multidevice_setup::kSmartLockEnabledPrefName, - kIsEasyUnlockEnabled2); - ScopedDictPrefUpdate update2(&local_state_, - prefs::kEasyUnlockLocalStateUserPrefs); - update2->Set(user2_.GetUserEmail(), std::move(user2_prefs)); - } - - AccountId user1_; - AccountId user2_; - AccountId unknown_user_; - TestingPrefServiceSimple local_state_; -}; - -TEST_F(ProximityAuthLocalStatePrefManagerTest, RegisterPrefs) { - EXPECT_TRUE( - local_state_.FindPreference(prefs::kEasyUnlockLocalStateUserPrefs)); -} - -TEST_F(ProximityAuthLocalStatePrefManagerTest, IsEasyUnlockAllowed) { - ProximityAuthLocalStatePrefManager pref_manager(&local_state_); - - // If no active user is set, return the default value. - EXPECT_EQ(kIsEasyUnlockAllowedDefault, pref_manager.IsEasyUnlockAllowed()); - - // Unknown users should return the default value. - pref_manager.SetActiveUser(unknown_user_); - EXPECT_EQ(kIsEasyUnlockAllowedDefault, pref_manager.IsEasyUnlockAllowed()); - - // Test users with set values. - pref_manager.SetActiveUser(user1_); - EXPECT_EQ(kIsEasyUnlockAllowed1, pref_manager.IsEasyUnlockAllowed()); - pref_manager.SetActiveUser(user2_); - EXPECT_EQ(kIsEasyUnlockAllowed2, pref_manager.IsEasyUnlockAllowed()); -} - -TEST_F(ProximityAuthLocalStatePrefManagerTest, IsEasyUnlockEnabled) { - ProximityAuthLocalStatePrefManager pref_manager(&local_state_); - - // If no active user is set, return the default value. - EXPECT_EQ(kIsEasyUnlockEnabledDefault, pref_manager.IsEasyUnlockEnabled()); - - // Unknown users should return the default value. - pref_manager.SetActiveUser(unknown_user_); - EXPECT_EQ(kIsEasyUnlockEnabledDefault, pref_manager.IsEasyUnlockEnabled()); - - // Test users with set values. - pref_manager.SetActiveUser(user1_); - EXPECT_EQ(kIsEasyUnlockEnabled1, pref_manager.IsEasyUnlockEnabled()); - pref_manager.SetActiveUser(user2_); - EXPECT_EQ(kIsEasyUnlockEnabled2, pref_manager.IsEasyUnlockEnabled()); -} - -TEST_F(ProximityAuthLocalStatePrefManagerTest, IsChromeOSLoginEnabled) { - ProximityAuthLocalStatePrefManager pref_manager(&local_state_); - - // If no active user is set, return the default value. - EXPECT_EQ(kIsChromeOSLoginEnabledDefault, - pref_manager.IsChromeOSLoginEnabled()); - - // Unknown users should return the default value. - pref_manager.SetActiveUser(unknown_user_); - EXPECT_EQ(kIsChromeOSLoginEnabledDefault, - pref_manager.IsChromeOSLoginEnabled()); - - // Test users with set values. - pref_manager.SetActiveUser(user1_); - EXPECT_EQ(kIsChromeOSLoginEnabled1, pref_manager.IsChromeOSLoginEnabled()); - pref_manager.SetActiveUser(user2_); - EXPECT_EQ(kIsChromeOSLoginEnabled2, pref_manager.IsChromeOSLoginEnabled()); -} - -} // namespace proximity_auth diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_pref_manager.h b/chromeos/ash/components/proximity_auth/proximity_auth_pref_manager.h index b16655487f488..d050a9919b9e0 100644 --- a/chromeos/ash/components/proximity_auth/proximity_auth_pref_manager.h +++ b/chromeos/ash/components/proximity_auth/proximity_auth_pref_manager.h @@ -12,6 +12,9 @@ namespace proximity_auth { // Interface for setting and getting persistent user preferences. There is an // implementation for a logged in profile and the device local state when the // user is logged out. +// TODO(b/227674947): Now that Sign in with Smart Lock is removed, this class is +// no longer needed before a user logs in. ProximityAuthPrefManager and +// ProximityAuthProfilePrefManager can be combined into one class. class ProximityAuthPrefManager { public: ProximityAuthPrefManager() {} @@ -51,15 +54,18 @@ class ProximityAuthPrefManager { // Getter for whether EasyUnlock is allowed for ChromeOS login (in addition to // screen lock). + // TODO(b/227674947): Delete now that Sign in with Smart Lock is deprecated. virtual bool IsChromeOSLoginAllowed() const = 0; // Setter and getter for whether EasyUnlock is enabled for ChromeOS login (in // addition to screen lock). + // TODO(b/227674947): Delete now that Sign in with Smart Lock is deprecated. virtual void SetIsChromeOSLoginEnabled(bool is_enabled) = 0; virtual bool IsChromeOSLoginEnabled() const = 0; // Setter and getter for whether the "Signin with Smart Lock is disabled" // message on the login screen has been shown. + // TODO(b/227674947): Delete now that Sign in with Smart Lock is deprecated. virtual void SetHasShownLoginDisabledMessage(bool has_shown) = 0; virtual bool HasShownLoginDisabledMessage() const = 0; }; diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.cc b/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.cc index da243f0aeaa97..d2ff1f3182a05 100644 --- a/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.cc +++ b/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.cc @@ -11,12 +11,6 @@ namespace prefs { // explicitly enabled by the user (through setup) or disabled via Settings. const char kEasyUnlockEnabledStateSet[] = "easy_unlock.enabled_state_set"; -// A dictionary in local state containing each user's Easy Unlock profile -// preferences, so they can be accessed outside of the user's profile. The value -// is a dictionary containing an entry for each user. Each user's entry mirrors -// their profile's Easy Unlock preferences. -const char kEasyUnlockLocalStateUserPrefs[] = "easy_unlock.user_prefs"; - // The timestamp of the last promotion check in milliseconds. const char kProximityAuthLastPromotionCheckTimestampMs[] = "proximity_auth.last_promotion_check_timestamp_ms"; @@ -31,10 +25,14 @@ const char kProximityAuthRemoteBleDevices[] = // Whether or not EasyUnlock is enabled on the ChromeOS login screen (in // addition to the lock screen). +// TODO(b/227674947): Delete this pref now that Sign in with Smart Lock is +// deprecated. const char kProximityAuthIsChromeOSLoginEnabled[] = "proximity_auth.is_chromeos_login_enabled"; // The dictionary containing remote BLE devices. +// TODO(b/227674947): Delete this pref now that Sign in with Smart Lock is +// deprecated. const char kProximityAuthHasShownLoginDisabledMessage[] = "proximity_auth.has_shown_login_disabled_message"; diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.h b/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.h index 8242460e796a6..e51c04ad3bcb2 100644 --- a/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.h +++ b/chromeos/ash/components/proximity_auth/proximity_auth_pref_names.h @@ -9,7 +9,6 @@ namespace proximity_auth { namespace prefs { extern const char kEasyUnlockEnabledStateSet[]; -extern const char kEasyUnlockLocalStateUserPrefs[]; extern const char kProximityAuthLastPromotionCheckTimestampMs[]; extern const char kProximityAuthPromotionShownCount[]; extern const char kProximityAuthRemoteBleDevices[]; diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.cc b/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.cc index 72c85da3a9f3c..9eda991cfdbdf 100644 --- a/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.cc +++ b/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.cc @@ -29,8 +29,6 @@ ProximityAuthProfilePrefManager::ProximityAuthProfilePrefManager( } ProximityAuthProfilePrefManager::~ProximityAuthProfilePrefManager() { - registrar_.RemoveAll(); - multidevice_setup_client_->RemoveObserver(this); } @@ -47,60 +45,6 @@ void ProximityAuthProfilePrefManager::RegisterPrefs( user_prefs::PrefRegistrySyncable::SYNCABLE_OS_PREF); } -void ProximityAuthProfilePrefManager::StartSyncingToLocalState( - PrefService* local_state, - const AccountId& account_id) { - local_state_ = local_state; - account_id_ = account_id; - - if (!account_id_.is_valid()) { - PA_LOG(ERROR) << "Invalid account_id."; - return; - } - - auto on_pref_changed_callback = base::BindRepeating( - &ProximityAuthProfilePrefManager::SyncPrefsToLocalState, - weak_ptr_factory_.GetWeakPtr()); - - registrar_.Init(pref_service_); - registrar_.Add(ash::multidevice_setup::kSmartLockAllowedPrefName, - on_pref_changed_callback); - registrar_.Add(ash::multidevice_setup::kSmartLockEnabledDeprecatedPrefName, - on_pref_changed_callback); - registrar_.Add(proximity_auth::prefs::kProximityAuthIsChromeOSLoginEnabled, - on_pref_changed_callback); - registrar_.Add(ash::multidevice_setup::kSmartLockSigninAllowedPrefName, - on_pref_changed_callback); - - SyncPrefsToLocalState(); -} - -void ProximityAuthProfilePrefManager::SyncPrefsToLocalState() { - base::Value::Dict user_prefs_dict; - - user_prefs_dict.Set(ash::multidevice_setup::kSmartLockAllowedPrefName, - IsEasyUnlockAllowed()); - user_prefs_dict.Set(ash::multidevice_setup::kSmartLockEnabledPrefName, - IsEasyUnlockEnabled()); - user_prefs_dict.Set(ash::multidevice_setup::kSmartLockSigninAllowedPrefName, - IsChromeOSLoginAllowed()); - user_prefs_dict.Set(prefs::kProximityAuthIsChromeOSLoginEnabled, - IsChromeOSLoginEnabled()); - - // If Signin with Smart Lock is enabled, then the "has shown Signin with - // Smart Lock is disabled message" flag should be false, to ensure the message - // is displayed if Signin with Smart Lock is disabled. Otherwise, copy the - // old value. - bool has_shown_login_disabled_message = - IsChromeOSLoginEnabled() ? false : HasShownLoginDisabledMessage(); - user_prefs_dict.Set(prefs::kProximityAuthHasShownLoginDisabledMessage, - has_shown_login_disabled_message); - - ScopedDictPrefUpdate update(local_state_, - prefs::kEasyUnlockLocalStateUserPrefs); - update->Set(account_id_.GetUserEmail(), std::move(user_prefs_dict)); -} - bool ProximityAuthProfilePrefManager::IsEasyUnlockAllowed() const { return pref_service_->GetBoolean( ash::multidevice_setup::kSmartLockAllowedPrefName); @@ -176,25 +120,17 @@ void ProximityAuthProfilePrefManager::SetHasShownLoginDisabledMessage( } bool ProximityAuthProfilePrefManager::HasShownLoginDisabledMessage() const { - const base::Value::Dict& all_user_prefs_dict = - local_state_->GetDict(prefs::kEasyUnlockLocalStateUserPrefs); - const base::Value::Dict* current_user_prefs = - all_user_prefs_dict.FindDict(account_id_.GetUserEmail()); - if (!current_user_prefs) { - PA_LOG(ERROR) << "Failed to find local state prefs for current user."; - return false; - } - - return current_user_prefs - ->FindBool(prefs::kProximityAuthHasShownLoginDisabledMessage) - .value_or(false); + // This method previously depended on easy unlock local state prefs, which are + // now deprecated with the removal of sign in with Smart Lock. + // TODO(b/227674947): Delete this method. + return false; } void ProximityAuthProfilePrefManager::OnFeatureStatesChanged( const ash::multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap& feature_states_map) { - if (local_state_ && account_id_.is_valid()) - SyncPrefsToLocalState(); + // TODO(b/227674947): Delete this method. With no more need for local state + // prefs, there's nothing to do here. } } // namespace proximity_auth diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.h b/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.h index 050b70a484359..28d3e3620fabf 100644 --- a/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.h +++ b/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager.h @@ -9,7 +9,6 @@ #include "chromeos/ash/components/proximity_auth/proximity_auth_pref_manager.h" #include "chromeos/ash/services/multidevice_setup/public/cpp/multidevice_setup_client.h" #include "components/account_id/account_id.h" -#include "components/prefs/pref_change_registrar.h" class PrefService; @@ -40,11 +39,6 @@ class ProximityAuthProfilePrefManager ~ProximityAuthProfilePrefManager() override; - // Initializes the manager to listen to pref changes and sync prefs to the - // user's local state. - void StartSyncingToLocalState(PrefService* local_state, - const AccountId& account_id); - // Registers the prefs used by this class to the given |pref_service|. static void RegisterPrefs(user_prefs::PrefRegistrySyncable* registry); @@ -70,18 +64,10 @@ class ProximityAuthProfilePrefManager feature_states_map) override; private: - void SyncPrefsToLocalState(); - // Contains perferences that outlive the lifetime of this object and across // process restarts. Not owned and must outlive this instance. PrefService* pref_service_ = nullptr; - // Listens to pref changes so they can be synced to the local state. - PrefChangeRegistrar registrar_; - - // The local state to which to sync the profile prefs. - PrefService* local_state_ = nullptr; - // The account id of the current profile. AccountId account_id_; diff --git a/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager_unittest.cc b/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager_unittest.cc index bd570a00cb4ae..25634ba87ef93 100644 --- a/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager_unittest.cc +++ b/chromeos/ash/components/proximity_auth/proximity_auth_profile_pref_manager_unittest.cc @@ -9,7 +9,6 @@ #include #include -#include "chromeos/ash/components/proximity_auth/proximity_auth_local_state_pref_manager.h" #include "chromeos/ash/components/proximity_auth/proximity_auth_pref_names.h" #include "chromeos/ash/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h" #include "chromeos/ash/services/multidevice_setup/public/cpp/prefs.h" @@ -21,8 +20,6 @@ namespace proximity_auth { namespace { -const char kUserEmail[] = "testuser@example.com"; - const int64_t kPromotionCheckTimestampMs1 = 1111111111L; const int64_t kPromotionCheckTimestampMs2 = 2222222222L; @@ -92,37 +89,4 @@ TEST_F(ProximityAuthProfilePrefManagerTest, IsChromeOSLoginEnabled) { EXPECT_FALSE(pref_manager_->IsChromeOSLoginEnabled()); } -TEST_F(ProximityAuthProfilePrefManagerTest, SyncsToLocalPrefOnChange) { - // Use a local variable to ensure that the PrefRegistrar adds and removes - // observers on the same thread. - ProximityAuthProfilePrefManager profile_pref_manager( - &pref_service_, fake_multidevice_setup_client_.get()); - - TestingPrefServiceSimple local_state; - AccountId account_id = AccountId::FromUserEmail(kUserEmail); - ProximityAuthLocalStatePrefManager::RegisterPrefs(local_state.registry()); - profile_pref_manager.StartSyncingToLocalState(&local_state, account_id); - - // Use the local state pref manager to verify that prefs are synced correctly. - ProximityAuthLocalStatePrefManager local_pref_manager(&local_state); - local_pref_manager.SetActiveUser(account_id); - - profile_pref_manager.SetIsChromeOSLoginEnabled(true); - profile_pref_manager.SetIsEasyUnlockEnabled(true); - EXPECT_TRUE(local_pref_manager.IsChromeOSLoginEnabled()); - - profile_pref_manager.SetIsChromeOSLoginEnabled(false); - profile_pref_manager.SetIsEasyUnlockEnabled(false); - EXPECT_FALSE(local_pref_manager.IsChromeOSLoginEnabled()); - EXPECT_FALSE(local_pref_manager.IsEasyUnlockEnabled()); - - // Test changing the kEasyUnlockAllowed pref value directly (e.g. through - // enterprise policy). - EXPECT_TRUE(local_pref_manager.IsEasyUnlockAllowed()); - pref_service_.SetBoolean(ash::multidevice_setup::kSmartLockAllowedPrefName, - false); - EXPECT_FALSE(profile_pref_manager.IsEasyUnlockAllowed()); - EXPECT_FALSE(local_pref_manager.IsEasyUnlockAllowed()); -} - } // namespace proximity_auth