diff options
Diffstat (limited to 'chromium/components/signin/public/identity_manager')
19 files changed, 269 insertions, 72 deletions
diff --git a/chromium/components/signin/public/identity_manager/BUILD.gn b/chromium/components/signin/public/identity_manager/BUILD.gn index e726761f763..fac2ebe4be2 100644 --- a/chromium/components/signin/public/identity_manager/BUILD.gn +++ b/chromium/components/signin/public/identity_manager/BUILD.gn @@ -174,6 +174,10 @@ source_set("test_support") { "//testing/gtest", ] + if (is_chromeos_ash) { + deps += [ "//ash/components/account_manager" ] + } + if (is_chromeos_lacros) { deps += [ "//components/account_manager_core" ] } diff --git a/chromium/components/signin/public/identity_manager/account_capabilities.cc b/chromium/components/signin/public/identity_manager/account_capabilities.cc index 9a224dff02c..b6d109499d1 100644 --- a/chromium/components/signin/public/identity_manager/account_capabilities.cc +++ b/chromium/components/signin/public/identity_manager/account_capabilities.cc @@ -77,6 +77,10 @@ signin::Tribool AccountCapabilities::is_subject_to_parental_controls() const { return GetCapabilityByName(kIsSubjectToParentalControlsCapabilityName); } +signin::Tribool AccountCapabilities::can_toggle_auto_updates() const { + return GetCapabilityByName(kCanToggleAutoUpdatesName); +} + bool AccountCapabilities::UpdateWith(const AccountCapabilities& other) { bool modified = false; diff --git a/chromium/components/signin/public/identity_manager/account_capabilities.h b/chromium/components/signin/public/identity_manager/account_capabilities.h index 9bc27f627dd..3e0b7dd3cc3 100644 --- a/chromium/components/signin/public/identity_manager/account_capabilities.h +++ b/chromium/components/signin/public/identity_manager/account_capabilities.h @@ -55,6 +55,9 @@ class AccountCapabilities { // Chrome applies parental controls to accounts with this capability. signin::Tribool is_subject_to_parental_controls() const; + // Chrome can toggle auto updates with this capability. + signin::Tribool can_toggle_auto_updates() const; + // Whether none of the capabilities has `signin::Tribool::kUnknown`. bool AreAllCapabilitiesKnown() const; diff --git a/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.cc b/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.cc index 40a4a7db791..956131f604d 100644 --- a/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.cc +++ b/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.cc @@ -42,6 +42,10 @@ void AccountCapabilitiesTestMutator::set_is_subject_to_parental_controls( value; } +void AccountCapabilitiesTestMutator::set_can_toggle_auto_updates(bool value) { + capabilities_->capabilities_map_[kCanToggleAutoUpdatesName] = value; +} + void AccountCapabilitiesTestMutator::SetAllSupportedCapabilities(bool value) { for (const std::string& name : AccountCapabilities::GetSupportedAccountCapabilityNames()) { diff --git a/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.h b/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.h index 2c40cd011a9..08a2ac41a35 100644 --- a/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.h +++ b/chromium/components/signin/public/identity_manager/account_capabilities_test_mutator.h @@ -5,6 +5,7 @@ #ifndef COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_ACCOUNT_CAPABILITIES_TEST_MUTATOR_H_ #define COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_ACCOUNT_CAPABILITIES_TEST_MUTATOR_H_ +#include "base/memory/raw_ptr.h" #include "components/signin/public/identity_manager/account_capabilities.h" // Support class that allows callers to modify internal capability state @@ -21,12 +22,13 @@ class AccountCapabilitiesTestMutator { void set_can_run_chrome_privacy_sandbox_trials(bool value); void set_can_stop_parental_supervision(bool value); void set_is_subject_to_parental_controls(bool value); + void set_can_toggle_auto_updates(bool value); // Modifies all supported capabilities at once. void SetAllSupportedCapabilities(bool value); private: - AccountCapabilities* capabilities_; + raw_ptr<AccountCapabilities> capabilities_; }; #endif // COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_ACCOUNT_CAPABILITIES_TEST_MUTATOR_H_ diff --git a/chromium/components/signin/public/identity_manager/account_capabilities_unittest.cc b/chromium/components/signin/public/identity_manager/account_capabilities_unittest.cc index bfde01b5409..31698927e83 100644 --- a/chromium/components/signin/public/identity_manager/account_capabilities_unittest.cc +++ b/chromium/components/signin/public/identity_manager/account_capabilities_unittest.cc @@ -73,6 +73,18 @@ TEST_F(AccountCapabilitiesTest, IsSubjectToParentalControls) { signin::Tribool::kFalse); } +TEST_F(AccountCapabilitiesTest, CanToggleAutoUpdates) { + AccountCapabilities capabilities; + EXPECT_EQ(capabilities.can_toggle_auto_updates(), signin::Tribool::kUnknown); + + AccountCapabilitiesTestMutator mutator(&capabilities); + mutator.set_can_toggle_auto_updates(true); + EXPECT_EQ(capabilities.can_toggle_auto_updates(), signin::Tribool::kTrue); + + mutator.set_can_toggle_auto_updates(false); + EXPECT_EQ(capabilities.can_toggle_auto_updates(), signin::Tribool::kFalse); +} + TEST_F(AccountCapabilitiesTest, AreAllCapabilitiesKnown_Empty) { AccountCapabilities capabilities; EXPECT_FALSE(capabilities.AreAllCapabilitiesKnown()); diff --git a/chromium/components/signin/public/identity_manager/identity_manager.cc b/chromium/components/signin/public/identity_manager/identity_manager.cc index 2d8d85cd4da..223b74bf7dd 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager.cc +++ b/chromium/components/signin/public/identity_manager/identity_manager.cc @@ -121,7 +121,7 @@ IdentityManager::IdentityManager(IdentityManager::InitParameters&& parameters) #if BUILDFLAG(IS_CHROMEOS_LACROS) signin_client_(parameters.signin_client), #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager_facade_(parameters.account_manager_facade), #endif identity_mutator_(std::move(parameters.primary_account_mutator), @@ -424,11 +424,13 @@ void IdentityManager::OnNetworkInitialized() { account_fetcher_service_->OnNetworkInitialized(); } +#if BUILDFLAG(IS_CHROMEOS_ASH) IdentityManager::AccountIdMigrationState IdentityManager::GetAccountIdMigrationState() const { return static_cast<IdentityManager::AccountIdMigrationState>( account_tracker_service_->GetMigrationState()); } +#endif CoreAccountId IdentityManager::PickAccountIdForAccount( const std::string& gaia, @@ -448,9 +450,6 @@ void IdentityManager::RegisterProfilePrefs(PrefRegistrySimple* registry) { AccountFetcherService::RegisterPrefs(registry); AccountTrackerService::RegisterPrefs(registry); GaiaCookieManagerService::RegisterPrefs(registry); -#if BUILDFLAG(ENABLE_DICE_SUPPORT) - MutableProfileOAuth2TokenServiceDelegate::RegisterProfilePrefs(registry); -#endif } DiagnosticsProvider* IdentityManager::GetDiagnosticsProvider() { @@ -559,7 +558,7 @@ GaiaCookieManagerService* IdentityManager::GetGaiaCookieManagerService() const { return gaia_cookie_manager_service_.get(); } -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager::AccountManagerFacade* IdentityManager::GetAccountManagerFacade() const { return account_manager_facade_; diff --git a/chromium/components/signin/public/identity_manager/identity_manager.h b/chromium/components/signin/public/identity_manager/identity_manager.h index 4cce79d3bf3..912c2b7c453 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager.h +++ b/chromium/components/signin/public/identity_manager/identity_manager.h @@ -33,7 +33,7 @@ #include "base/time/time.h" #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) namespace account_manager { class AccountManagerFacade; } @@ -371,7 +371,7 @@ class IdentityManager : public KeyedService, #if BUILDFLAG(IS_CHROMEOS_LACROS) SigninClient* signin_client = nullptr; #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager::AccountManagerFacade* account_manager_facade = nullptr; #endif @@ -397,6 +397,7 @@ class IdentityManager : public KeyedService, // initialized. void OnNetworkInitialized(); +#if BUILDFLAG(IS_CHROMEOS_ASH) // Methods related to migration of account IDs from email to Gaia ID. // TODO(https://crbug.com/883272): Remove these once all platforms have // migrated to the new account_id based on gaia (currently, only ChromeOS @@ -413,6 +414,7 @@ class IdentityManager : public KeyedService, // Returns the currently saved state of the migration of account IDs. AccountIdMigrationState GetAccountIdMigrationState() const; +#endif // Picks the correct account_id for the specified account depending on the // migration state. @@ -550,7 +552,7 @@ class IdentityManager : public KeyedService, const std::string& locale, const std::string& picture_url); -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) friend account_manager::AccountManagerFacade* GetAccountManagerFacade( IdentityManager* identity_manager); #endif // BUILDFLAG(IS_CHROMEOS_ASH) @@ -626,7 +628,7 @@ class IdentityManager : public KeyedService, AccountTrackerService* GetAccountTrackerService() const; AccountFetcherService* GetAccountFetcherService() const; GaiaCookieManagerService* GetGaiaCookieManagerService() const; -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager::AccountManagerFacade* GetAccountManagerFacade() const; #endif @@ -686,7 +688,7 @@ class IdentityManager : public KeyedService, #if BUILDFLAG(IS_CHROMEOS_LACROS) SigninClient* const signin_client_; #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager::AccountManagerFacade* const account_manager_facade_; #endif diff --git a/chromium/components/signin/public/identity_manager/identity_manager_builder.cc b/chromium/components/signin/public/identity_manager/identity_manager_builder.cc index 904912c9ff1..330c7c3e7ff 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager_builder.cc +++ b/chromium/components/signin/public/identity_manager/identity_manager_builder.cc @@ -112,12 +112,15 @@ IdentityManager::InitParameters BuildIdentityManagerInitParameters( BuildProfileOAuth2TokenService( params->pref_service, account_tracker_service.get(), params->network_connection_tracker, params->account_consistency, -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) params->account_manager_facade, params->is_regular_profile, -#endif // BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#endif // BUILDFLAG(IS_CHROMEOS) +#if BUILDFLAG(ENABLE_DICE_SUPPORT) || BUILDFLAG(IS_CHROMEOS_LACROS) + params->delete_signin_cookies_on_exit, +#endif // BUILDFLAG(ENABLE_DICE_SUPPORT) || BUILDFLAG(IS_CHROMEOS_LACROS) #if BUILDFLAG(ENABLE_DICE_SUPPORT) - params->delete_signin_cookies_on_exit, params->token_web_data, -#endif + params->token_web_data, +#endif // BUILDFLAG(ENABLE_DICE_SUPPORT) #if BUILDFLAG(IS_IOS) std::move(params->device_accounts_provider), #endif @@ -127,7 +130,8 @@ IdentityManager::InitParameters BuildIdentityManagerInitParameters( params->signin_client); auto gaia_cookie_manager_service = std::make_unique<GaiaCookieManagerService>( - token_service.get(), params->signin_client); + account_tracker_service.get(), token_service.get(), + params->signin_client); std::unique_ptr<PrimaryAccountManager> primary_account_manager = BuildPrimaryAccountManager(params->signin_client, @@ -185,7 +189,7 @@ IdentityManager::InitParameters BuildIdentityManagerInitParameters( #if BUILDFLAG(IS_CHROMEOS_LACROS) init_params.signin_client = params->signin_client; #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) init_params.account_manager_facade = params->account_manager_facade; #endif diff --git a/chromium/components/signin/public/identity_manager/identity_manager_builder.h b/chromium/components/signin/public/identity_manager/identity_manager_builder.h index 3f76111603d..0cdab6f70ff 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager_builder.h +++ b/chromium/components/signin/public/identity_manager/identity_manager_builder.h @@ -10,7 +10,6 @@ #include "base/files/file_path.h" #include "base/memory/raw_ptr.h" #include "build/build_config.h" -#include "build/chromeos_buildflags.h" #include "components/signin/public/base/signin_buildflags.h" #include "components/signin/public/identity_manager/identity_manager.h" @@ -41,7 +40,7 @@ namespace network { class NetworkConnectionTracker; } -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) namespace account_manager { class AccountManagerFacade; } @@ -63,12 +62,15 @@ struct IdentityManagerBuildParams { base::FilePath profile_path; raw_ptr<SigninClient> signin_client = nullptr; -#if BUILDFLAG(ENABLE_DICE_SUPPORT) +#if BUILDFLAG(ENABLE_DICE_SUPPORT) || BUILDFLAG(IS_CHROMEOS_LACROS) bool delete_signin_cookies_on_exit = false; +#endif + +#if BUILDFLAG(ENABLE_DICE_SUPPORT) scoped_refptr<TokenWebData> token_web_data; #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager::AccountManagerFacade* account_manager_facade = nullptr; bool is_regular_profile = false; #endif diff --git a/chromium/components/signin/public/identity_manager/identity_manager_builder_unittest.cc b/chromium/components/signin/public/identity_manager/identity_manager_builder_unittest.cc index 8d57ea54e6e..bf5b607cf0a 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager_builder_unittest.cc +++ b/chromium/components/signin/public/identity_manager/identity_manager_builder_unittest.cc @@ -9,7 +9,6 @@ #include "base/files/scoped_temp_dir.h" #include "base/test/task_environment.h" #include "build/build_config.h" -#include "build/chromeos_buildflags.h" #include "components/image_fetcher/core/fake_image_decoder.h" #include "components/signin/internal/identity_manager/account_fetcher_service.h" #include "components/signin/public/base/test_signin_client.h" @@ -23,7 +22,7 @@ #include "components/signin/public/identity_manager/identity_test_utils.h" #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) #include "components/account_manager_core/mock_account_manager_facade.h" #endif @@ -86,7 +85,7 @@ TEST_F(IdentityManagerBuilderTest, BuildIdentityManagerInitParameters) { std::make_unique<FakeDeviceAccountsProvider>(); #endif -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) auto account_manager_facade = std::make_unique<account_manager::MockAccountManagerFacade>(); params.account_manager_facade = account_manager_facade.get(); @@ -111,9 +110,8 @@ TEST_F(IdentityManagerBuilderTest, BuildIdentityManagerInitParameters) { EXPECT_EQ(init_params.device_accounts_synchronizer, nullptr); EXPECT_NE(init_params.accounts_mutator, nullptr); #endif -#if defined(IS_CHROMEOS) - EXPECT_NE(init_params.ash_account_manager_facade, nullptr); - EXPECT_TRUE(init_params.is_regular_profile); +#if BUILDFLAG(IS_CHROMEOS) + EXPECT_NE(init_params.account_manager_facade, nullptr); #endif } diff --git a/chromium/components/signin/public/identity_manager/identity_manager_unittest.cc b/chromium/components/signin/public/identity_manager/identity_manager_unittest.cc index a71f5a10746..a7599181e47 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager_unittest.cc +++ b/chromium/components/signin/public/identity_manager/identity_manager_unittest.cc @@ -391,8 +391,9 @@ class IdentityManagerTest : public testing::Test { #endif auto gaia_cookie_manager_service = - std::make_unique<GaiaCookieManagerService>(token_service.get(), - &signin_client_); + std::make_unique<GaiaCookieManagerService>( + account_tracker_service.get(), token_service.get(), + &signin_client_); auto account_fetcher_service = std::make_unique<AccountFetcherService>(); account_fetcher_service->Initialize( @@ -1725,7 +1726,7 @@ TEST_F(IdentityManagerTest, IdentityManagerGetsTokensLoadedEvent) { // we fake the credentials loaded state and force another load in // order to be able to capture the TokensLoaded event. token_service()->set_all_credentials_loaded_for_testing(false); - token_service()->LoadCredentials(CoreAccountId()); + token_service()->LoadCredentials(CoreAccountId(), /*is_syncing=*/false); run_loop.Run(); } @@ -2118,8 +2119,8 @@ TEST_F(IdentityManagerTest, CallbackSentOnAccountsCookieDeletedByUserAction) { run_loop.QuitClosure()); auto cookie = net::CanonicalCookie::CreateUnsafeCookieForTesting( "SAPISID", std::string(), ".google.com", "/", base::Time(), base::Time(), - base::Time(), /*secure=*/true, false, net::CookieSameSite::NO_RESTRICTION, - net::COOKIE_PRIORITY_DEFAULT, false); + base::Time(), base::Time(), /*secure=*/true, false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_DEFAULT, false); SimulateCookieDeletedByUser(identity_manager()->GetGaiaCookieManagerService(), *cookie); run_loop.Run(); @@ -2150,8 +2151,8 @@ TEST_F(IdentityManagerTest, OnNetworkInitialized) { // through any mojo pipe. auto cookie = net::CanonicalCookie::CreateUnsafeCookieForTesting( "SAPISID", std::string(), ".google.com", "/", base::Time(), base::Time(), - base::Time(), /*secure=*/true, false, net::CookieSameSite::NO_RESTRICTION, - net::COOKIE_PRIORITY_DEFAULT, false); + base::Time(), base::Time(), /*secure=*/true, false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_DEFAULT, false); test_cookie_manager_ptr->DispatchCookieChange(net::CookieChangeInfo( *cookie, net::CookieAccessResult(), net::CookieChangeCause::EXPLICIT)); run_loop.Run(); @@ -2352,7 +2353,7 @@ TEST_F(IdentityManagerTest, AreRefreshTokensLoaded) { // order to test AreRefreshTokensLoaded. token_service()->set_all_credentials_loaded_for_testing(false); EXPECT_FALSE(identity_manager()->AreRefreshTokensLoaded()); - token_service()->LoadCredentials(CoreAccountId()); + token_service()->LoadCredentials(CoreAccountId(), /*is_syncing=*/false); run_loop.Run(); EXPECT_TRUE(identity_manager()->AreRefreshTokensLoaded()); } @@ -2405,10 +2406,12 @@ TEST_F(IdentityManagerTest, SetPrimaryAccountClearsExistingPrimaryAccount) { } #endif +#if BUILDFLAG(IS_CHROMEOS_ASH) TEST_F(IdentityManagerTest, AccountIdMigration_DoneOnInitialization) { EXPECT_EQ(IdentityManager::AccountIdMigrationState::MIGRATION_DONE, identity_manager()->GetAccountIdMigrationState()); } +#endif // Checks that IdentityManager::Observer gets OnAccountUpdated when account info // is updated. @@ -2455,6 +2458,7 @@ TEST_F(IdentityManagerTest, TestOnAccountRemovedWithInfoCallback) { TEST_F(IdentityManagerTest, TestPickAccountIdForAccount) { const CoreAccountId account_id = identity_manager()->PickAccountIdForAccount(kTestGaiaId, kTestEmail); +#if BUILDFLAG(IS_CHROMEOS_ASH) const bool account_id_migration_done = identity_manager()->GetAccountIdMigrationState() == IdentityManager::AccountIdMigrationState::MIGRATION_DONE; @@ -2463,6 +2467,9 @@ TEST_F(IdentityManagerTest, TestPickAccountIdForAccount) { } else { EXPECT_TRUE(gaia::AreEmailsSame(kTestEmail, account_id.ToString())); } +#else + EXPECT_TRUE(gaia::AreEmailsSame(kTestGaiaId, account_id.ToString())); +#endif } #if BUILDFLAG(IS_ANDROID) diff --git a/chromium/components/signin/public/identity_manager/identity_test_environment.cc b/chromium/components/signin/public/identity_manager/identity_test_environment.cc index 314ac25ed12..b0947a422a2 100644 --- a/chromium/components/signin/public/identity_manager/identity_test_environment.cc +++ b/chromium/components/signin/public/identity_manager/identity_test_environment.cc @@ -344,8 +344,8 @@ IdentityTestEnvironment::FinishBuildIdentityManagerForTests( primary_account_manager->Initialize(pref_service); std::unique_ptr<GaiaCookieManagerService> gaia_cookie_manager_service = - std::make_unique<GaiaCookieManagerService>(token_service.get(), - signin_client); + std::make_unique<GaiaCookieManagerService>( + account_tracker_service.get(), token_service.get(), signin_client); IdentityManager::InitParameters init_params; init_params.primary_account_mutator = std::make_unique<PrimaryAccountMutatorImpl>( @@ -672,7 +672,7 @@ void IdentityTestEnvironment::ResetToAccountsNotYetLoadedFromDiskState() { } void IdentityTestEnvironment::ReloadAccountsFromDisk() { - fake_token_service()->LoadCredentials(CoreAccountId()); + fake_token_service()->LoadCredentials(CoreAccountId(), /*is_syncing=*/false); } bool IdentityTestEnvironment::IsAccessTokenRequestPending() { diff --git a/chromium/components/signin/public/identity_manager/identity_test_utils.cc b/chromium/components/signin/public/identity_manager/identity_test_utils.cc index a583e60bbe1..fdaddfbcbb4 100644 --- a/chromium/components/signin/public/identity_manager/identity_test_utils.cc +++ b/chromium/components/signin/public/identity_manager/identity_test_utils.cc @@ -9,6 +9,7 @@ #include "base/guid.h" #include "base/run_loop.h" #include "build/build_config.h" +#include "build/chromeos_buildflags.h" #include "components/signin/internal/identity_manager/account_fetcher_service.h" #include "components/signin/internal/identity_manager/account_tracker_service.h" #include "components/signin/internal/identity_manager/gaia_cookie_manager_service.h" @@ -22,7 +23,7 @@ #include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/gaia_constants.h" -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) #include "components/account_manager_core/account.h" #include "components/account_manager_core/account_manager_facade.h" #endif @@ -83,7 +84,7 @@ void UpdateRefreshTokenForAccount( run_loop.QuitClosure()); // TODO(crbug.com/1226041): simplify this when all Lacros Profiles use Mirror. -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) if (ShouldUseAccountManagerFacade(identity_manager)) { const AccountInfo& account_info = account_tracker_service->GetAccountInfo(account_id); @@ -94,7 +95,7 @@ void UpdateRefreshTokenForAccount( GetAccountManagerFacade(identity_manager) ->UpsertAccountForTesting(account, new_token); } else -#endif // BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#endif // BUILDFLAG(IS_CHROMEOS) { token_service->UpdateCredentials(account_id, new_token); } @@ -273,6 +274,25 @@ void ClearPrimaryAccount(IdentityManager* identity_manager) { #endif } +void WaitForPrimaryAccount(IdentityManager* identity_manager, + ConsentLevel consent_level, + const CoreAccountId& account_id) { + if (identity_manager->GetPrimaryAccountId(consent_level) == account_id) + return; + + base::RunLoop run_loop; + TestIdentityManagerObserver primary_account_observer(identity_manager); + primary_account_observer.SetOnPrimaryAccountChangedCallback(base::BindOnce( + [](IdentityManager* identity_manager, ConsentLevel consent_level, + const CoreAccountId& account_id, base::RunLoop* run_loop, + PrimaryAccountChangeEvent event) { + if (identity_manager->GetPrimaryAccountId(consent_level) == account_id) + run_loop->Quit(); + }, + identity_manager, consent_level, account_id, &run_loop)); + run_loop.Run(); +} + AccountInfo MakeAccountAvailable(IdentityManager* identity_manager, const std::string& email) { AccountTrackerService* account_tracker_service = @@ -358,7 +378,7 @@ void RemoveRefreshTokenForAccount(IdentityManager* identity_manager, run_loop.QuitClosure()); // TODO(crbug.com/1226041): simplify this when all Lacros Profiles use Mirror. -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) if (ShouldUseAccountManagerFacade(identity_manager)) { const AccountInfo& account_info = identity_manager->GetAccountTrackerService()->GetAccountInfo( @@ -367,7 +387,7 @@ void RemoveRefreshTokenForAccount(IdentityManager* identity_manager, ->RemoveAccountForTesting(account_manager::AccountKey{ account_info.gaia, account_manager::AccountType::kGaia}); } else -#endif // BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#endif // BUILDFLAG(IS_CHROMEOS) { identity_manager->GetTokenService()->RevokeCredentials(account_id); } @@ -514,7 +534,7 @@ void SimulateSuccessfulFetchOfAccountInfo(IdentityManager* identity_manager, account_tracker_service->SetAccountInfoFromUserInfo(account_id, &user_info); } -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager::AccountManagerFacade* GetAccountManagerFacade( IdentityManager* identity_manager) { return identity_manager->GetAccountManagerFacade(); diff --git a/chromium/components/signin/public/identity_manager/identity_test_utils.h b/chromium/components/signin/public/identity_manager/identity_test_utils.h index 499bb6ed890..fb972ec3f3a 100644 --- a/chromium/components/signin/public/identity_manager/identity_test_utils.h +++ b/chromium/components/signin/public/identity_manager/identity_test_utils.h @@ -9,7 +9,6 @@ #include "base/bind.h" #include "build/build_config.h" -#include "build/chromeos_buildflags.h" #include "components/signin/public/base/consent_level.h" #include "components/signin/public/identity_manager/account_info.h" @@ -17,7 +16,7 @@ namespace network { class TestURLLoaderFactory; } -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) namespace account_manager { class AccountManagerFacade; } @@ -96,6 +95,16 @@ void RevokeSyncConsent(IdentityManager* identity_manager); // NOTE: See disclaimer at top of file re: direct usage. void ClearPrimaryAccount(IdentityManager* identity_manager); +// Waits until the primary account id at consent_level to be equal to +// |account_id|. +// +// Note: Passing an empty |account_id| will make this function wait until +// the primary account id is cleared at the |consent_level| (calling +// identity_manager->HasPrimaryAccount(consent_level) will return false) +void WaitForPrimaryAccount(IdentityManager* identity_manager, + ConsentLevel consent_level, + const CoreAccountId& account_id); + // Makes an account available for the given email address, generating a GAIA ID // and refresh token that correspond uniquely to that email address. Blocks // until the account is available. Returns the AccountInfo of the @@ -206,7 +215,7 @@ void SimulateSuccessfulFetchOfAccountInfo(IdentityManager* identity_manager, const std::string& locale, const std::string& picture_url); -#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(IS_CHROMEOS) account_manager::AccountManagerFacade* GetAccountManagerFacade( IdentityManager* identity_manager); #endif diff --git a/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.cc b/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.cc index d0fcf8a380f..b5deee8a055 100644 --- a/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.cc +++ b/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.cc @@ -18,16 +18,38 @@ PrimaryAccountAccessTokenFetcher::PrimaryAccountAccessTokenFetcher( const std::string& oauth_consumer_name, IdentityManager* identity_manager, const ScopeSet& scopes, - AccessTokenFetcher::TokenCallback callback, Mode mode, ConsentLevel consent) : oauth_consumer_name_(oauth_consumer_name), identity_manager_(identity_manager), scopes_(scopes), - callback_(std::move(callback)), mode_(mode), consent_(consent) { identity_manager_observation_.Observe(identity_manager_.get()); +} + +PrimaryAccountAccessTokenFetcher::PrimaryAccountAccessTokenFetcher( + const std::string& oauth_consumer_name, + IdentityManager* identity_manager, + const ScopeSet& scopes, + AccessTokenFetcher::TokenCallback callback, + Mode mode, + ConsentLevel consent) + : PrimaryAccountAccessTokenFetcher(oauth_consumer_name, + identity_manager, + scopes, + mode, + consent) { + Start(std::move(callback)); +} + +PrimaryAccountAccessTokenFetcher::~PrimaryAccountAccessTokenFetcher() = default; + +void PrimaryAccountAccessTokenFetcher::Start( + AccessTokenFetcher::TokenCallback callback) { + DCHECK(callback); + DCHECK(!callback_); + callback_ = std::move(callback); if (mode_ == Mode::kImmediate || AreCredentialsAvailable()) { StartAccessTokenRequest(); return; @@ -35,8 +57,6 @@ PrimaryAccountAccessTokenFetcher::PrimaryAccountAccessTokenFetcher( waiting_for_account_available_ = true; } -PrimaryAccountAccessTokenFetcher::~PrimaryAccountAccessTokenFetcher() = default; - CoreAccountId PrimaryAccountAccessTokenFetcher::GetAccountId() const { return identity_manager_->GetPrimaryAccountId(consent_); } diff --git a/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.h b/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.h index d4a4802b8e6..5d4f8b4d49c 100644 --- a/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.h +++ b/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher.h @@ -67,28 +67,29 @@ struct AccessTokenInfo; // // create access token requests could and should just create // // PrimaryAccountAccessTokenFetchers directly themselves rather than // // introducing wrapper API surfaces. +// // MyClass::StartAccessTokenRequestForPrimaryAccount() { // // Choose scopes to obtain for the access token. // ScopeSet scopes; // scopes.insert(GaiaConstants::kMyFirstScope); // scopes.insert(GaiaConstants::kMySecondScope); - +// // // Choose the mode in which to fetch the access token: // // see AccessTokenFetcher::Mode below for definitions. // auto mode = // PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable; - +// // // Create the fetcher. // access_token_fetcher_ = // std::make_unique<PrimaryAccountAccessTokenFetcher>( // /*consumer_name=*/"MyClass", // identity_manager_, // scopes, +// mode, // base::BindOnce(&MyClass::OnAccessTokenRequestCompleted, // // It is safe to use base::Unretained as // // |this| owns |access_token_fetcher_|. -// base::Unretained(this)), -// mode); +// base::Unretained(this))); // // } // void MyClass::OnAccessTokenRequestCompleted( @@ -111,7 +112,7 @@ struct AccessTokenInfo; // } // } // -// Concrete test example: +// Concrete test example: // TEST(MyClassTest, SuccessfulAccessTokenFetchForPrimaryAccount) { // IdentityTestEnvironment identity_test_env; // @@ -120,7 +121,8 @@ struct AccessTokenInfo; // // // Make the primary account available, which should result in an access // // token fetch being made on behalf of |my_class|. -// identity_test_env.MakePrimaryAccountAvailable("test_email"); +// identity_test_env. +// MakePrimaryAccountAvailable("test_email", ConsentLevel::kSync); // // identity_test_env. // WaitForAccessTokenRequestIfNecessaryAndRespondWithToken( @@ -131,6 +133,38 @@ struct AccessTokenInfo; // // the test can now perform any desired validation of expected actions // // |MyClass| took in response. // } +// +// It is also possible to create a PrimaryAccountAccessTokenFetcher that does +// not immediately start its access token request, as follows: +// +// auto access_token_fetcher = +// std::make_unique<PrimaryAccountAccessTokenFetcher>( +// /*consumer_name=*/"MyClass", +// identity_manager_, +// scopes, +// mode); +// +// This allows an ownership model wherein MyClass does not store a +// std::unique_ptr<PrimaryAccountAccessTokenFetcher> instance variable; instead, +// the fetcher is bound to the |TokenCallback| itself and destroyed +// automatically when the callback is called or destroyed. Continuing the +// example from above: +// +// auto* access_token_fetcher_ptr = access_token_fetcher.get(); +// access_token_fetcher_ptr.Start( +// base::BindOnce(&MyClass::OnAccessTokenRequestCompleted, +// // |this| no longer owns |access_token_fetcher|, so +// // a |WeakPtr| is required. +// my_class_weak_factory_.GetWeakPtr()), +// std::move(access_token_fetcher)); +// +// void MyClass::OnAccessTokenRequestCompleted( +// std::unique_ptr<PrimaryAccountAccessTokenFetcher> fetcher, +// GoogleServiceAuthError error, +// AccessTokenInfo access_token_info) { +// ... +// // |fetcher| is destroyed automatically when this callback finishes. +// } class PrimaryAccountAccessTokenFetcher : public IdentityManager::Observer { public: // Specifies how this instance should behave: @@ -146,13 +180,17 @@ class PrimaryAccountAccessTokenFetcher : public IdentityManager::Observer { // if the user is not signed in and doesn't sign in. enum class Mode { kImmediate, kWaitUntilAvailable }; - // Instantiates a fetcher and immediately starts the process of obtaining an - // OAuth2 access token for the given |scopes|. The |callback| is called once - // the request completes (successful or not). If the - // PrimaryAccountAccessTokenFetcher is destroyed before the process completes, - // the callback is not called. - // |consent| defaults to kSync because historically having an "authenticated" - // account was tied to browser sync. See ./README.md. + // Instantiates an OAuth2 access token fetcher for the given |scopes|. Once + // the fetcher is created, the caller initiates its token request using + // |Start()|. |consent| defaults to |kSync| because historically having an + // "authenticated" account was tied to browser sync. See ./README.md. + PrimaryAccountAccessTokenFetcher(const std::string& oauth_consumer_name, + IdentityManager* identity_manager, + const ScopeSet& scopes, + Mode mode, + ConsentLevel consent = ConsentLevel::kSync); + + // Convenience constructor that immediately issues the access token request. PrimaryAccountAccessTokenFetcher(const std::string& oauth_consumer_name, IdentityManager* identity_manager, const ScopeSet& scopes, @@ -167,6 +205,12 @@ class PrimaryAccountAccessTokenFetcher : public IdentityManager::Observer { ~PrimaryAccountAccessTokenFetcher() override; + // Starts the process of obtaining an OAuth2 access token for the fetcher's + // |scopes_|. |callback| is called once the request completes (successfully + // or not). If the fetcher is destroyed before the process completes, the + // callback is not called. + void Start(AccessTokenFetcher::TokenCallback callback); + // Exposed for tests. bool access_token_request_retried() { return access_token_retried_; } diff --git a/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher_unittest.cc b/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher_unittest.cc index a4d347a6789..ce483752431 100644 --- a/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher_unittest.cc +++ b/chromium/components/signin/public/identity_manager/primary_account_access_token_fetcher_unittest.cc @@ -75,6 +75,17 @@ class PrimaryAccountAccessTokenFetcherTest return CreateFetcher(std::move(callback), mode, consent); } + std::unique_ptr<PrimaryAccountAccessTokenFetcher> CreateDelayedStartFetcher( + PrimaryAccountAccessTokenFetcher::Mode mode) { + // API scope that does not require consent. + std::set<std::string> scopes = { + GaiaConstants::kChromeSafeBrowsingOAuth2Scope}; + ConsentLevel consent = GetParam(); + return std::make_unique<PrimaryAccountAccessTokenFetcher>( + "test_consumer", identity_test_env_->identity_manager(), scopes, mode, + consent); + } + IdentityTestEnvironment* identity_test_env() { return identity_test_env_.get(); } @@ -121,6 +132,27 @@ TEST_P(PrimaryAccountAccessTokenFetcherTest, OneShotShouldReturnAccessToken) { } TEST_P(PrimaryAccountAccessTokenFetcherTest, + DelayedOneShotShouldReturnAccessToken) { + TestTokenCallback callback; + + CoreAccountId account_id = SignIn(); + + // Signed in and refresh token already exists, so this should result in a + // request for an access token. + auto fetcher = CreateDelayedStartFetcher( + PrimaryAccountAccessTokenFetcher::Mode::kImmediate); + + // Once the access token request is fulfilled, we should get called back with + // the access token. + EXPECT_CALL(callback, Run(GoogleServiceAuthError::AuthErrorNone(), + access_token_info())); + fetcher->Start(callback.Get()); + identity_test_env()->WaitForAccessTokenRequestIfNecessaryAndRespondWithToken( + access_token_info().token, access_token_info().expiration_time, + access_token_info().id_token); +} + +TEST_P(PrimaryAccountAccessTokenFetcherTest, WaitAndRetryShouldReturnAccessToken) { TestTokenCallback callback; @@ -141,6 +173,27 @@ TEST_P(PrimaryAccountAccessTokenFetcherTest, access_token_info().id_token); } +TEST_P(PrimaryAccountAccessTokenFetcherTest, + DelayedWaitAndRetryShouldReturnAccessToken) { + TestTokenCallback callback; + + CoreAccountId account_id = SignIn(); + + // Signed in and refresh token already exists, so this should result in a + // request for an access token. + auto fetcher = CreateDelayedStartFetcher( + PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable); + + // Once the access token request is fulfilled, we should get called back with + // the access token. + EXPECT_CALL(callback, Run(GoogleServiceAuthError::AuthErrorNone(), + access_token_info())); + fetcher->Start(callback.Get()); + identity_test_env()->WaitForAccessTokenRequestIfNecessaryAndRespondWithToken( + access_token_info().token, access_token_info().expiration_time, + access_token_info().id_token); +} + TEST_P(PrimaryAccountAccessTokenFetcherTest, ShouldNotReplyIfDestroyed) { TestTokenCallback callback; @@ -151,6 +204,8 @@ TEST_P(PrimaryAccountAccessTokenFetcherTest, ShouldNotReplyIfDestroyed) { auto fetcher = CreateFetcher( callback.Get(), PrimaryAccountAccessTokenFetcher::Mode::kImmediate); + EXPECT_CALL(callback, Run).Times(0); + // Destroy the fetcher before the access token request is fulfilled. fetcher.reset(); @@ -160,6 +215,20 @@ TEST_P(PrimaryAccountAccessTokenFetcherTest, ShouldNotReplyIfDestroyed) { access_token_info().id_token); } +TEST_P(PrimaryAccountAccessTokenFetcherTest, ShouldNotReplyIfNotStarted) { + TestTokenCallback callback; + + CoreAccountId account_id = SignIn(); + + // Signed in and refresh token already exists, so this would result in a + // request for an access token if the fetcher were started. + auto fetcher = CreateDelayedStartFetcher( + PrimaryAccountAccessTokenFetcher::Mode::kImmediate); + + // No token request generated because the fetcher has not been started. + EXPECT_FALSE(identity_test_env()->IsAccessTokenRequestPending()); +} + TEST_P(PrimaryAccountAccessTokenFetcherTest, OneShotCallsBackWhenSignedOut) { base::RunLoop run_loop; @@ -201,6 +270,8 @@ TEST_P(PrimaryAccountAccessTokenFetcherTest, auto fetcher = CreateFetcher( callback.Get(), PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable); + + EXPECT_CALL(callback, Run).Times(0); } TEST_P(PrimaryAccountAccessTokenFetcherTest, ShouldWaitForSignIn) { diff --git a/chromium/components/signin/public/identity_manager/primary_account_mutator_unittest.cc b/chromium/components/signin/public/identity_manager/primary_account_mutator_unittest.cc index aa0962935e2..25c7303fec4 100644 --- a/chromium/components/signin/public/identity_manager/primary_account_mutator_unittest.cc +++ b/chromium/components/signin/public/identity_manager/primary_account_mutator_unittest.cc @@ -518,14 +518,6 @@ TEST_F(PrimaryAccountMutatorTest, RevokeSyncConsent_DiceConsistency) { RemoveAccountExpectation::kKeepAll); } -// Test that revoking the sync consent when DICE account consistency is -// enabled clears the primary account if it uin auth error state. -TEST_F(PrimaryAccountMutatorTest, RevokeSyncConsent_DiceConsistency_AuthError) { - RunRevokeSyncConsentTest(signin::AccountConsistencyMethod::kDice, - RemoveAccountExpectation::kRemoveAll, - AuthExpectation::kAuthError); -} - #else //! BUILDFLAG(IS_CHROMEOS_ASH) TEST_F(PrimaryAccountMutatorTest, CROS_ASH_RevokeSyncConsent) { |