diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-01-23 17:21:03 +0100 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-01-23 16:25:15 +0000 |
commit | c551f43206405019121bd2b2c93714319a0a3300 (patch) | |
tree | 1f48c30631c421fd4bbb3c36da20183c8a2ed7d7 /chromium/components/signin | |
parent | 7961cea6d1041e3e454dae6a1da660b453efd238 (diff) | |
download | qtwebengine-chromium-c551f43206405019121bd2b2c93714319a0a3300.tar.gz |
BASELINE: Update Chromium to 79.0.3945.139
Change-Id: I336b7182fab9bca80b709682489c07db112eaca5
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/components/signin')
116 files changed, 2092 insertions, 1025 deletions
diff --git a/chromium/components/signin/DEPS b/chromium/components/signin/DEPS index 8001065552e..2bd7b72d440 100644 --- a/chromium/components/signin/DEPS +++ b/chromium/components/signin/DEPS @@ -10,7 +10,6 @@ include_rules = [ # Subdirectories of //components/signin must explicitly allow deps on each # other based on the conceptual deps structure. "-components/signin", - "+components/signin/public", "+components/user_manager", "+components/webdata/common", "+crypto", diff --git a/chromium/components/signin/core/browser/BUILD.gn b/chromium/components/signin/core/browser/BUILD.gn index df01214e98c..1d9e1931121 100644 --- a/chromium/components/signin/core/browser/BUILD.gn +++ b/chromium/components/signin/core/browser/BUILD.gn @@ -23,6 +23,8 @@ static_library("browser") { "chrome_connected_header_helper.h", "consistency_cookie_manager_base.cc", "consistency_cookie_manager_base.h", + "cookie_reminter.cc", + "cookie_reminter.h", "cookie_settings_util.cc", "cookie_settings_util.h", "dice_account_reconcilor_delegate.cc", @@ -82,6 +84,11 @@ static_library("browser") { ] if (is_chromeos) { + sources += [ + "active_directory_account_reconcilor_delegate.cc", + "active_directory_account_reconcilor_delegate.h", + ] + deps += [ "//chromeos/tpm:tpm" ] sources -= [ "signin_status_metrics_provider.cc", "signin_status_metrics_provider_delegate.cc", @@ -110,7 +117,6 @@ source_set("unit_tests") { testonly = true sources = [ "account_investigator_unittest.cc", - "account_reconcilor_delegate_unittest.cc", "account_reconcilor_unittest.cc", "dice_account_reconcilor_delegate_unittest.cc", "mice_account_reconcilor_delegate_unittest.cc", @@ -143,6 +149,7 @@ source_set("unit_tests") { ] if (is_chromeos) { + deps += [ "//chromeos/tpm:test_support" ] sources -= [ "account_investigator_unittest.cc", "signin_status_metrics_provider_unittest.cc", diff --git a/chromium/components/signin/core/browser/DEPS b/chromium/components/signin/core/browser/DEPS index d9ef433896d..080a1d60ee4 100644 --- a/chromium/components/signin/core/browser/DEPS +++ b/chromium/components/signin/core/browser/DEPS @@ -1,3 +1,15 @@ include_rules = [ "+components/metrics", + "+components/signin/public/base", + "+components/signin/public/identity_manager", ] + +specific_include_rules = { + "active_directory_account_reconcilor_delegate.cc": [ + "+chromeos/tpm/install_attributes.h" + ], + "account_reconcilor_unittest.cc": [ + "+chromeos/tpm/install_attributes.h", + "+chromeos/tpm/stub_install_attributes.h" + ] +} diff --git a/chromium/components/signin/core/browser/about_signin_internals.cc b/chromium/components/signin/core/browser/about_signin_internals.cc index 7291d86bcb4..20dcc821e70 100644 --- a/chromium/components/signin/core/browser/about_signin_internals.cc +++ b/chromium/components/signin/core/browser/about_signin_internals.cc @@ -606,8 +606,6 @@ AboutSigninInternals::SigninStatus::ToValue( // A summary of signin related info first. base::ListValue* basic_info = AddSection(signin_info.get(), "Basic Information"); - AddSectionEntry(basic_info, "Chrome Version", - signin_client->GetProductVersion()); AddSectionEntry(basic_info, "Account Consistency", GetAccountConsistencyDescription(account_consistency)); AddSectionEntry( diff --git a/chromium/components/signin/core/browser/account_investigator_unittest.cc b/chromium/components/signin/core/browser/account_investigator_unittest.cc index c1aa8668e8b..89ed251ec0b 100644 --- a/chromium/components/signin/core/browser/account_investigator_unittest.cc +++ b/chromium/components/signin/core/browser/account_investigator_unittest.cc @@ -133,7 +133,7 @@ class AccountInvestigatorTest : public testing::Test { private: // Timer needs a message loop. - base::test::TaskEnvironment task_environment_; + base::test::SingleThreadTaskEnvironment task_environment_; sync_preferences::TestingPrefServiceSyncable prefs_; network::TestURLLoaderFactory test_url_loader_factory_; signin::IdentityTestEnvironment identity_test_env_; diff --git a/chromium/components/signin/core/browser/account_reconcilor.cc b/chromium/components/signin/core/browser/account_reconcilor.cc index d1e171063f7..adcb08eed6d 100644 --- a/chromium/components/signin/core/browser/account_reconcilor.cc +++ b/chromium/components/signin/core/browser/account_reconcilor.cc @@ -22,6 +22,7 @@ #include "build/build_config.h" #include "components/signin/core/browser/account_reconcilor_delegate.h" #include "components/signin/core/browser/consistency_cookie_manager_base.h" +#include "components/signin/core/browser/cookie_reminter.h" #include "components/signin/public/base/account_consistency_method.h" #include "components/signin/public/base/signin_client.h" #include "components/signin/public/base/signin_metrics.h" @@ -296,12 +297,6 @@ void AccountReconcilor::SetConsistencyCookieManager( consistency_cookie_manager_ = std::move(consistency_cookie_manager); } -#if defined(OS_IOS) -void AccountReconcilor::SetIsWKHTTPSystemCookieStoreEnabled(bool is_enabled) { - is_wkhttp_system_cookie_store_enabled_ = is_enabled; -} -#endif // defined(OS_IOS) - void AccountReconcilor::EnableReconcile() { SetState(AccountReconcilorState::ACCOUNT_RECONCILOR_SCHEDULED); RegisterWithAllDependencies(); @@ -353,6 +348,7 @@ void AccountReconcilor::RegisterWithIdentityManager() { if (registered_with_identity_manager_) return; + cookie_reminter_ = std::make_unique<CookieReminter>(identity_manager_); identity_manager_->AddObserver(this); registered_with_identity_manager_ = true; } @@ -362,6 +358,7 @@ void AccountReconcilor::UnregisterWithIdentityManager() { if (!registered_with_identity_manager_) return; + cookie_reminter_.reset(); identity_manager_->RemoveObserver(this); registered_with_identity_manager_ = false; } @@ -375,6 +372,11 @@ AccountReconcilor::GetScopedSyncDataDeletion() { return base::WrapUnique(new ScopedSyncedDataDeletion(this)); } +void AccountReconcilor::ForceCookieRemintingOnNextTokenUpdate( + const CoreAccountInfo& account_info) { + cookie_reminter_->ForceCookieRemintingOnNextTokenUpdate(account_info); +} + void AccountReconcilor::AddObserver(Observer* observer) { observer_list_.AddObserver(observer); } @@ -549,6 +551,13 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint( DCHECK(!set_accounts_in_progress_); DCHECK_EQ(AccountReconcilorState::ACCOUNT_RECONCILOR_RUNNING, state_); +#if defined(OS_CHROMEOS) + // Cookie may need to be reminted on Chrome OS. See https://crbug.com/1012649 + // for details. + if (cookie_reminter_->RemintCookieIfRequired()) + gaia_accounts.clear(); +#endif + bool primary_has_error = identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState( primary_account); @@ -719,14 +728,14 @@ AccountReconcilor::LoadValidAccountsFromTokenService() const { std::vector<CoreAccountId> chrome_account_ids; - // Remove any accounts that have an error. There is no point in trying to - // reconcile them, since it won't work anyway. If the list ends up being + // Remove any accounts that have an error. There is no point in trying to + // reconcile them, since it won't work anyway. If the list ends up being // empty then don't reconcile any accounts. for (const auto& chrome_account_with_refresh_tokens : chrome_accounts_with_refresh_tokens) { if (identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState( chrome_account_with_refresh_tokens.account_id)) { - VLOG(1) << "AccountReconcilor::ValidateAccountsFromTokenService: " + VLOG(1) << "AccountReconcilor::LoadValidAccountsFromTokenService: " << chrome_account_with_refresh_tokens.account_id << " has error, don't reconcile"; continue; @@ -734,7 +743,7 @@ AccountReconcilor::LoadValidAccountsFromTokenService() const { chrome_account_ids.push_back(chrome_account_with_refresh_tokens.account_id); } - VLOG(1) << "AccountReconcilor::ValidateAccountsFromTokenService: " + VLOG(1) << "AccountReconcilor::LoadValidAccountsFromTokenService: " << "Chrome " << chrome_account_ids.size() << " accounts"; return chrome_account_ids; @@ -778,6 +787,14 @@ void AccountReconcilor::FinishReconcile( (number_gaia_accounts > 0) && (first_account != gaia_accounts[0].id); bool rebuild_cookie = first_account_mismatch || (removed_from_cookie > 0); + +#if defined(OS_CHROMEOS) + // Cookie may need to be reminted on Chrome OS. See https://crbug.com/1012649 + // for details. + if (cookie_reminter_->RemintCookieIfRequired()) + rebuild_cookie = true; +#endif + std::vector<gaia::ListedAccount> original_gaia_accounts = gaia_accounts; if (rebuild_cookie) { VLOG(1) << "AccountReconcilor::FinishReconcile: rebuild cookie"; @@ -790,9 +807,12 @@ void AccountReconcilor::FinishReconcile( if (first_account.empty()) { DCHECK(!delegate_->ShouldAbortReconcileIfPrimaryHasError()); + auto revoke_option = + delegate_->ShouldRevokeTokensIfNoPrimaryAccount() + ? AccountReconcilorDelegate::RevokeTokenOption::kRevoke + : AccountReconcilorDelegate::RevokeTokenOption::kDoNotRevoke; reconcile_is_noop_ = !RevokeAllSecondaryTokens( - identity_manager_, - AccountReconcilorDelegate::RevokeTokenOption::kRevoke, primary_account, + identity_manager_, revoke_option, primary_account, delegate_->IsAccountConsistencyEnforced(), signin_metrics::SourceForRefreshTokenOperation:: kAccountReconcilor_Reconcile); @@ -1030,14 +1050,6 @@ void AccountReconcilor::HandleReconcileTimeout() { } bool AccountReconcilor::IsMultiloginEndpointEnabled() const { -#if defined(OS_IOS) - // kUseMultiloginEndpoint feature should not be used if - // kWKHTTPSystemCookieStore feature is disabbled. - // See http://crbug.com/902584. - if (!is_wkhttp_system_cookie_store_enabled_) - return false; -#endif // defined(OS_IOS) - #if defined(OS_ANDROID) if (base::FeatureList::IsEnabled(signin::kMiceFeature)) return true; // Mice is only implemented with multilogin. diff --git a/chromium/components/signin/core/browser/account_reconcilor.h b/chromium/components/signin/core/browser/account_reconcilor.h index f617f00ec6c..3abde7d8ac9 100644 --- a/chromium/components/signin/core/browser/account_reconcilor.h +++ b/chromium/components/signin/core/browser/account_reconcilor.h @@ -38,6 +38,7 @@ class ConsistencyCookieManagerBase; enum class SetAccountsInCookieResult; } +class CookieReminter; class SigninClient; class AccountReconcilor : public KeyedService, @@ -107,11 +108,6 @@ class AccountReconcilor : public KeyedService, std::unique_ptr<signin::ConsistencyCookieManagerBase> consistency_cookie_manager); -#if defined(OS_IOS) - // Sets the WKHTTPSystemCookieStore flag value. - void SetIsWKHTTPSystemCookieStoreEnabled(bool is_enabled); -#endif // defined(OS_IOS) - // Enables and disables the reconciliation. void EnableReconcile(); void DisableReconcile(bool logout_all_gaia_accounts); @@ -137,6 +133,11 @@ class AccountReconcilor : public KeyedService, // from being invalidated during the deletion. std::unique_ptr<ScopedSyncedDataDeletion> GetScopedSyncDataDeletion(); + // Forces a cookie reminting if/when the refresh token for |account_info| is + // updated. + void ForceCookieRemintingOnNextTokenUpdate( + const CoreAccountInfo& account_info); + private: friend class AccountReconcilorTest; friend class DiceBrowserTest; @@ -242,6 +243,10 @@ class AccountReconcilor : public KeyedService, FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, MultiloginLogout); FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestForceDiceMigration, TableRowTest); + FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestActiveDirectory, + TableRowTestMergeSession); + FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestActiveDirectory, + TableRowTestMultilogin); void set_timer_for_testing(std::unique_ptr<base::OneShotTimer> timer); @@ -254,8 +259,6 @@ class AccountReconcilor : public KeyedService, void UnregisterWithAllDependencies(); void RegisterWithIdentityManager(); void UnregisterWithIdentityManager(); - void RegisterWithCookieManagerService(); - void UnregisterWithCookieManagerService(); void RegisterWithContentSettings(); void UnregisterWithContentSettings(); @@ -370,6 +373,7 @@ class AccountReconcilor : public KeyedService, std::vector<CoreAccountId> add_to_cookie_; // Progress of AddAccount calls. bool set_accounts_in_progress_; // Progress of SetAccounts calls. bool chrome_accounts_changed_; + std::unique_ptr<CookieReminter> cookie_reminter_; // Used for the Lock. // StartReconcile() is blocked while this is > 0. @@ -396,11 +400,6 @@ class AccountReconcilor : public KeyedService, signin_metrics::AccountReconcilorState state_; -#if defined(OS_IOS) - // Stores the WKHTTPSystemCookieStore flag value. - bool is_wkhttp_system_cookie_store_enabled_ = false; -#endif // defined(OS_IOS) - std::unique_ptr<signin::ConsistencyCookieManagerBase> consistency_cookie_manager_; diff --git a/chromium/components/signin/core/browser/account_reconcilor_delegate.cc b/chromium/components/signin/core/browser/account_reconcilor_delegate.cc index 46d7d0be1df..f5c622066a7 100644 --- a/chromium/components/signin/core/browser/account_reconcilor_delegate.cc +++ b/chromium/components/signin/core/browser/account_reconcilor_delegate.cc @@ -184,6 +184,10 @@ bool AccountReconcilorDelegate::ShouldRevokeTokensOnCookieDeleted() { return false; } +bool AccountReconcilorDelegate::ShouldRevokeTokensIfNoPrimaryAccount() const { + return true; +} + base::TimeDelta AccountReconcilorDelegate::GetReconcileTimeout() const { return base::TimeDelta::Max(); } diff --git a/chromium/components/signin/core/browser/account_reconcilor_delegate.h b/chromium/components/signin/core/browser/account_reconcilor_delegate.h index d9edf545d9d..6a26da3d446 100644 --- a/chromium/components/signin/core/browser/account_reconcilor_delegate.h +++ b/chromium/components/signin/core/browser/account_reconcilor_delegate.h @@ -108,6 +108,9 @@ class AccountReconcilorDelegate { // invalidated unless it has to be kept for critical Sync operations. virtual bool ShouldRevokeTokensOnCookieDeleted(); + // Returns whether tokens should be revoked when the primary account is empty + virtual bool ShouldRevokeTokensIfNoPrimaryAccount() const; + // Called when reconcile is finished. // |OnReconcileFinished| is always called at the end of reconciliation, // even when there is an error (except in cases where reconciliation times diff --git a/chromium/components/signin/core/browser/account_reconcilor_unittest.cc b/chromium/components/signin/core/browser/account_reconcilor_unittest.cc index 85c57f8ff6a..ba438643b89 100644 --- a/chromium/components/signin/core/browser/account_reconcilor_unittest.cc +++ b/chromium/components/signin/core/browser/account_reconcilor_unittest.cc @@ -38,7 +38,6 @@ #include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/google_service_auth_error.h" -#include "net/url_request/test_url_fetcher_factory.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -46,6 +45,11 @@ #include "components/signin/core/browser/dice_account_reconcilor_delegate.h" #endif +#if defined(OS_CHROMEOS) +#include "chromeos/tpm/stub_install_attributes.h" +#include "components/signin/core/browser/active_directory_account_reconcilor_delegate.h" +#endif + using signin::RevokeTokenAction; using signin_metrics::AccountReconcilorState; @@ -117,9 +121,6 @@ class DummyAccountReconcilorWithDelegate : public AccountReconcilor { identity_manager, account_consistency, dice_migration_completed)) { -#if defined(OS_IOS) - SetIsWKHTTPSystemCookieStoreEnabled(true); -#endif // defined(OS_IOS) Initialize(false /* start_reconcile_if_tokens_available */); } @@ -133,9 +134,6 @@ class DummyAccountReconcilorWithDelegate : public AccountReconcilor { identity_manager, client, std::unique_ptr<signin::AccountReconcilorDelegate>(delegate)) { -#if defined(OS_IOS) - SetIsWKHTTPSystemCookieStoreEnabled(true); -#endif // defined(OS_IOS) Initialize(false /* start_reconcile_if_tokens_available */); } @@ -244,7 +242,9 @@ class AccountReconcilorTest : public ::testing::Test { return &identity_test_env_; } - base::test::TaskEnvironment* task_environment() { return &task_environment_; } + base::test::SingleThreadTaskEnvironment* task_environment() { + return &task_environment_; + } TestSigninClient* test_signin_client() { return &test_signin_client_; } base::HistogramTester* histogram_tester() { return &histogram_tester_; } @@ -282,7 +282,7 @@ class AccountReconcilorTest : public ::testing::Test { network::TestURLLoaderFactory test_url_loader_factory_; private: - base::test::TaskEnvironment task_environment_; + base::test::SingleThreadTaskEnvironment task_environment_; signin::AccountConsistencyMethod account_consistency_; bool dice_migration_completed_ = false; sync_preferences::TestingPrefServiceSyncable pref_service_; @@ -349,7 +349,8 @@ INSTANTIATE_TEST_SUITE_P(Dice_Mirror, signin::AccountConsistencyMethod::kMirror)); AccountReconcilorTest::AccountReconcilorTest() - : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME), + : task_environment_( + base::test::SingleThreadTaskEnvironment::TimeSource::MOCK_TIME), account_consistency_(signin::AccountConsistencyMethod::kDisabled), test_signin_client_(&pref_service_, &test_url_loader_factory_), identity_test_env_(/*test_url_loader_factory=*/nullptr, @@ -1997,6 +1998,194 @@ INSTANTIATE_TEST_SUITE_P( AccountReconcilorTestMirrorMultilogin, ::testing::ValuesIn(GenerateTestCasesFromParams(kMirrorParams))); +#if defined(OS_CHROMEOS) +class AccountReconcilorTestActiveDirectory : public AccountReconcilorTestTable { + public: + AccountReconcilorTestActiveDirectory() = default; + + void SetUp() override { + SetAccountConsistency(signin::AccountConsistencyMethod::kMirror); + } + + private: + chromeos::ScopedStubInstallAttributes install_attributes_{ + chromeos::StubInstallAttributes::CreateActiveDirectoryManaged( + "realm.com", + "device_id")}; + + DISALLOW_COPY_AND_ASSIGN(AccountReconcilorTestActiveDirectory); +}; + +// clang-format off +const std::vector<AccountReconcilorTestTableParam> kActiveDirectoryParams = { +// This table encodes the initial state and expectations of a reconcile. +// See kDiceParams for documentation of the syntax. +// ------------------------------------------------------------------------- +// Tokens |Cookies |First Run |Gaia calls|Tokens aft.|Cookies aft.|M.calls| +// ------------------------------------------------------------------------- +{ "ABC", "ABC", IsFirstReconcile::kBoth, "", "ABC", "ABC", "" }, +{ "ABC", "", IsFirstReconcile::kBoth, "", "ABC", "ABC", "U"}, +{ "", "ABC", IsFirstReconcile::kBoth, "X", "", "", "X"}, +// Order of Gaia accounts can be different from chrome accounts. +{ "ABC", "CBA", IsFirstReconcile::kBoth, "", "ABC", "CBA", "" }, +{ "ABC", "CB", IsFirstReconcile::kBoth, "", "ABC", "CBA", "U"}, +// Gaia accounts which are not present in chrome accounts should be removed. In +// this case Gaia accounts are going to be in the same order as chrome accounts. +{ "A", "AB", IsFirstReconcile::kBoth, "X", "A", "A", "U"}, +{ "AB", "CBA", IsFirstReconcile::kBoth, "X", "AB", "AB", "U"}, +{ "AB", "C", IsFirstReconcile::kBoth, "X", "AB", "AB", "U"}, +// Cookies can be refreshed in pace, without logout. +{ "AB", "xAxB", IsFirstReconcile::kBoth, "", "AB", "AB", "U"}, +// Token error on the account - remove it from cookies +{ "AxB", "AB", IsFirstReconcile::kBoth, "X", "AxB", "A", "U"}, +{ "xAxB", "AB", IsFirstReconcile::kBoth, "X", "xAxB", "", "X"}, +}; +// clang-format on + +TEST_P(AccountReconcilorTestActiveDirectory, TableRowTestMergeSession) { + // Setup tokens. + std::vector<Token> tokens = ParseTokenString(GetParam().tokens); + SetupTokens(GetParam().tokens); + + // Setup cookies. + std::vector<Cookie> cookies = ParseCookieString(GetParam().cookies); + ConfigureCookieManagerService(cookies); + + // Call list accounts now so that the next call completes synchronously. + identity_test_env()->identity_manager()->GetAccountsInCookieJar(); + base::RunLoop().RunUntilIdle(); + + testing::InSequence mock_sequence; + MockAccountReconcilor* reconcilor = GetMockReconcilor( + std::make_unique<signin::ActiveDirectoryAccountReconcilorDelegate>()); + + // Setup expectations. + bool logout_all_accounts = false; + for (int i = 0; GetParam().gaia_api_calls[i] != '\0'; ++i) { + if (GetParam().gaia_api_calls[i] == 'X') { + logout_all_accounts = true; + break; + } + } + if (logout_all_accounts) + EXPECT_CALL(*reconcilor, PerformLogoutAllAccountsAction); + + for (auto& account : tokens) { + if (account.has_error) + continue; + Cookie account_cookie{account.gaia_id, true /*is_valid*/}; + if (logout_all_accounts || !base::Contains(cookies, account_cookie)) { + EXPECT_CALL(*reconcilor, + PerformMergeAction(CoreAccountId(account.email))); + } + } + + // Reconcile. + ASSERT_TRUE(reconcilor); + ASSERT_TRUE(reconcilor->first_execution_); + reconcilor->first_execution_ = + GetParam().is_first_reconcile == IsFirstReconcile::kFirst ? true : false; + reconcilor->StartReconcile(); + + SimulateSetAccountsInCookieCompleted( + reconcilor, signin::SetAccountsInCookieResult::kSuccess); + + ASSERT_FALSE(reconcilor->is_reconcile_started_); + ASSERT_EQ(signin_metrics::ACCOUNT_RECONCILOR_OK, reconcilor->GetState()); + VerifyCurrentTokens(ParseTokenString(GetParam().tokens_after_reconcile)); + + testing::Mock::VerifyAndClearExpectations(reconcilor); + + // Another reconcile is sometimes triggered if Chrome accounts have + // changed. Allow it to finish. + EXPECT_CALL(*reconcilor, PerformSetCookiesAction(testing::_)) + .WillRepeatedly(testing::Return()); + EXPECT_CALL(*reconcilor, PerformLogoutAllAccountsAction()) + .WillRepeatedly(testing::Return()); + ConfigureCookieManagerService({}); + base::RunLoop().RunUntilIdle(); +} + +TEST_P(AccountReconcilorTestActiveDirectory, TableRowTestMultilogin) { + base::test::ScopedFeatureList scoped_feature_list; + scoped_feature_list.InitAndEnableFeature(kUseMultiloginEndpoint); + + // Setup tokens. + std::vector<Token> tokens = ParseTokenString(GetParam().tokens); + SetupTokens(GetParam().tokens); + + // Setup cookies. + std::vector<Cookie> cookies = ParseCookieString(GetParam().cookies); + ConfigureCookieManagerService(cookies); + + // Call list accounts now so that the next call completes synchronously. + identity_test_env()->identity_manager()->GetAccountsInCookieJar(); + base::RunLoop().RunUntilIdle(); + + testing::InSequence mock_sequence; + MockAccountReconcilor* reconcilor = GetMockReconcilor( + std::make_unique<signin::ActiveDirectoryAccountReconcilorDelegate>()); + + // Setup expectations. + bool logout_action = false; + for (int i = 0; GetParam().gaia_api_calls_multilogin[i] != '\0'; ++i) { + if (GetParam().gaia_api_calls_multilogin[i] == 'X') { + logout_action = true; + EXPECT_CALL(*reconcilor, PerformLogoutAllAccountsAction()).Times(1); + cookies.clear(); + continue; + } + if (GetParam().gaia_api_calls_multilogin[i] == 'U') { + std::vector<CoreAccountId> accounts_to_send; + for (int i = 0; GetParam().cookies_after_reconcile[i] != '\0'; ++i) { + char cookie = GetParam().cookies_after_reconcile[i]; + std::string account_to_send = GaiaIdForAccountKey(cookie); + accounts_to_send.push_back(PickAccountIdForAccount( + account_to_send, + accounts_[GetParam().cookies_after_reconcile[i]].email)); + } + const signin::MultiloginParameters params( + gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER, + accounts_to_send); + EXPECT_CALL(*reconcilor, PerformSetCookiesAction(params)).Times(1); + } + } + if (!logout_action) { + EXPECT_CALL(*reconcilor, PerformLogoutAllAccountsAction()).Times(0); + } + + // Reconcile. + ASSERT_TRUE(reconcilor); + ASSERT_TRUE(reconcilor->first_execution_); + reconcilor->first_execution_ = + GetParam().is_first_reconcile == IsFirstReconcile::kFirst ? true : false; + reconcilor->StartReconcile(); + + SimulateSetAccountsInCookieCompleted( + reconcilor, signin::SetAccountsInCookieResult::kSuccess); + + ASSERT_FALSE(reconcilor->is_reconcile_started_); + ASSERT_EQ(signin_metrics::ACCOUNT_RECONCILOR_OK, reconcilor->GetState()); + VerifyCurrentTokens(ParseTokenString(GetParam().tokens_after_reconcile)); + + testing::Mock::VerifyAndClearExpectations(reconcilor); + + // Another reconcile is sometimes triggered if Chrome accounts have + // changed. Allow it to finish. + EXPECT_CALL(*reconcilor, PerformSetCookiesAction(testing::_)) + .WillRepeatedly(testing::Return()); + EXPECT_CALL(*reconcilor, PerformLogoutAllAccountsAction()) + .WillRepeatedly(testing::Return()); + ConfigureCookieManagerService({}); + base::RunLoop().RunUntilIdle(); +} + +INSTANTIATE_TEST_SUITE_P( + ActiveDirectoryTable, + AccountReconcilorTestActiveDirectory, + ::testing::ValuesIn(GenerateTestCasesFromParams(kActiveDirectoryParams))); +#endif // defined(OS_CHROMEOS) + #if defined(OS_ANDROID) // clang-format off const std::vector<AccountReconcilorTestTableParam> kMiceParams = { diff --git a/chromium/components/signin/core/browser/active_directory_account_reconcilor_delegate.cc b/chromium/components/signin/core/browser/active_directory_account_reconcilor_delegate.cc new file mode 100644 index 00000000000..a848799f9c6 --- /dev/null +++ b/chromium/components/signin/core/browser/active_directory_account_reconcilor_delegate.cc @@ -0,0 +1,88 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/signin/core/browser/active_directory_account_reconcilor_delegate.h" + +#include "chromeos/tpm/install_attributes.h" +#include "google_apis/gaia/core_account_id.h" + +namespace signin { + +ActiveDirectoryAccountReconcilorDelegate:: + ActiveDirectoryAccountReconcilorDelegate() { + DCHECK(chromeos::InstallAttributes::Get()->IsActiveDirectoryManaged()); +} + +ActiveDirectoryAccountReconcilorDelegate:: + ~ActiveDirectoryAccountReconcilorDelegate() = default; + +bool ActiveDirectoryAccountReconcilorDelegate::IsAccountConsistencyEnforced() + const { + return true; +} + +gaia::GaiaSource ActiveDirectoryAccountReconcilorDelegate::GetGaiaApiSource() + const { + return gaia::GaiaSource::kAccountReconcilorMirror; +} + +bool ActiveDirectoryAccountReconcilorDelegate::IsReconcileEnabled() const { + return true; +} + +CoreAccountId +ActiveDirectoryAccountReconcilorDelegate::GetFirstGaiaAccountForReconcile( + const std::vector<CoreAccountId>& chrome_accounts, + const std::vector<gaia::ListedAccount>& gaia_accounts, + const CoreAccountId& primary_account, + bool first_execution, + bool will_logout) const { + return GetFirstAccount(chrome_accounts, gaia_accounts); +} + +std::vector<CoreAccountId> +ActiveDirectoryAccountReconcilorDelegate::GetChromeAccountsForReconcile( + const std::vector<CoreAccountId>& chrome_accounts, + const CoreAccountId& primary_account, + const std::vector<gaia::ListedAccount>& gaia_accounts, + const gaia::MultiloginMode mode) const { + DCHECK_EQ(mode, + gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER); + if (chrome_accounts.empty()) + return chrome_accounts; + return ReorderChromeAccountsForReconcile( + chrome_accounts, GetFirstAccount(chrome_accounts, gaia_accounts), + gaia_accounts); +} + +bool ActiveDirectoryAccountReconcilorDelegate:: + ShouldAbortReconcileIfPrimaryHasError() const { + return false; +} + +bool ActiveDirectoryAccountReconcilorDelegate:: + ShouldRevokeTokensIfNoPrimaryAccount() const { + return false; +} + +CoreAccountId ActiveDirectoryAccountReconcilorDelegate::GetFirstAccount( + const std::vector<CoreAccountId>& chrome_accounts, + const std::vector<gaia::ListedAccount>& gaia_accounts) const { + if (chrome_accounts.empty()) + return CoreAccountId(); + // Return first gaia account to preserve the account order in the cookie jar. + // (Gaia accounts which are NOT in chrome_accounts will be removed.) In case + // of first account mismatch, the cookie will be rebuilt and order of accounts + // will be changed. + if (!gaia_accounts.empty() && + base::Contains(chrome_accounts, gaia_accounts[0].id)) { + return gaia_accounts[0].id; + } + // The cookie jar is empty or first Gaia account in the cookie jar is + // not present in Account Manager. Fall back to choosing the first account + // present in Account Manager as the first account for reconciliation. + return chrome_accounts[0]; +} + +} // namespace signin diff --git a/chromium/components/signin/core/browser/active_directory_account_reconcilor_delegate.h b/chromium/components/signin/core/browser/active_directory_account_reconcilor_delegate.h new file mode 100644 index 00000000000..dd1d215fa89 --- /dev/null +++ b/chromium/components/signin/core/browser/active_directory_account_reconcilor_delegate.h @@ -0,0 +1,53 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_SIGNIN_CORE_BROWSER_ACTIVE_DIRECTORY_ACCOUNT_RECONCILOR_DELEGATE_H_ +#define COMPONENTS_SIGNIN_CORE_BROWSER_ACTIVE_DIRECTORY_ACCOUNT_RECONCILOR_DELEGATE_H_ + +#include <vector> + +#include "base/macros.h" +#include "components/signin/core/browser/account_reconcilor_delegate.h" + +namespace signin { + +// AccountReconcilorDelegate specialized for Active Directory accounts. +class ActiveDirectoryAccountReconcilorDelegate + : public AccountReconcilorDelegate { + public: + ActiveDirectoryAccountReconcilorDelegate(); + ~ActiveDirectoryAccountReconcilorDelegate() override; + + // AccountReconcilorDelegate: + bool IsAccountConsistencyEnforced() const override; + gaia::GaiaSource GetGaiaApiSource() const override; + bool IsReconcileEnabled() const override; + CoreAccountId GetFirstGaiaAccountForReconcile( + const std::vector<CoreAccountId>& chrome_accounts, + const std::vector<gaia::ListedAccount>& gaia_accounts, + const CoreAccountId& primary_account, + bool first_execution, + bool will_logout) const override; + + std::vector<CoreAccountId> GetChromeAccountsForReconcile( + const std::vector<CoreAccountId>& chrome_accounts, + const CoreAccountId& primary_account, + const std::vector<gaia::ListedAccount>& gaia_accounts, + const gaia::MultiloginMode mode) const override; + bool ShouldAbortReconcileIfPrimaryHasError() const override; + bool ShouldRevokeTokensIfNoPrimaryAccount() const override; + + private: + // Returns first gaia account if it is present in |chrome_accounts|. + // Otherwise, the first chrome account is returned. + CoreAccountId GetFirstAccount( + const std::vector<CoreAccountId>& chrome_accounts, + const std::vector<gaia::ListedAccount>& gaia_accounts) const; + + DISALLOW_COPY_AND_ASSIGN(ActiveDirectoryAccountReconcilorDelegate); +}; + +} // namespace signin + +#endif // COMPONENTS_SIGNIN_CORE_BROWSER_ACTIVE_DIRECTORY_ACCOUNT_RECONCILOR_DELEGATE_H_ diff --git a/chromium/components/signin/core/browser/android/BUILD.gn b/chromium/components/signin/core/browser/android/BUILD.gn index 488f87090cb..fc0079f78a7 100644 --- a/chromium/components/signin/core/browser/android/BUILD.gn +++ b/chromium/components/signin/core/browser/android/BUILD.gn @@ -6,6 +6,7 @@ import("//build/config/android/rules.gni") generate_jni("jni_headers") { sources = [ + "java/src/org/chromium/components/signin/AccountManagerFacade.java", "java/src/org/chromium/components/signin/AccountTrackerService.java", "java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java", "java/src/org/chromium/components/signin/ConsistencyCookieManager.java", @@ -22,7 +23,7 @@ android_library("java") { "//base:jni_java", "//net/android:net_java", "//third_party/android_deps:android_support_v4_java", - "//third_party/android_deps:com_android_support_support_annotations_java", + "//third_party/android_deps:androidx_annotation_annotation_java", "//ui/android:ui_java", ] @@ -65,7 +66,7 @@ junit_binary("components_signin_junit_tests") { "//base:base_java", "//base:base_java_test_support", "//base:base_junit_test_support", - "//third_party/android_deps:com_android_support_support_annotations_java", + "//third_party/android_deps:androidx_annotation_annotation_java", "//third_party/junit", ] } @@ -77,7 +78,7 @@ android_library("signin_javatests") { ":signin_java_test_support", "//base:base_java", "//base:base_java_test_support", - "//third_party/android_deps:com_android_support_support_annotations_java", + "//third_party/android_deps:androidx_annotation_annotation_java", "//third_party/android_support_test_runner:rules_java", "//third_party/android_support_test_runner:runner_java", "//third_party/jsr-305:jsr_305_javalib", @@ -93,7 +94,7 @@ android_library("signin_java_test_support") { ":java", "//base:base_java", "//base:base_java_test_support", - "//third_party/android_deps:com_android_support_support_annotations_java", + "//third_party/android_deps:androidx_annotation_annotation_java", "//third_party/jsr-305:jsr_305_javalib", "//third_party/junit", ] diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerDelegate.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerDelegate.java index a35a24a4234..279e5e5967a 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerDelegate.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerDelegate.java @@ -9,10 +9,11 @@ import android.accounts.AccountManager; import android.accounts.AuthenticatorDescription; import android.app.Activity; import android.content.Intent; -import android.support.annotation.AnyThread; -import android.support.annotation.MainThread; -import android.support.annotation.Nullable; -import android.support.annotation.WorkerThread; + +import androidx.annotation.AnyThread; +import androidx.annotation.MainThread; +import androidx.annotation.Nullable; +import androidx.annotation.WorkerThread; import org.chromium.base.Callback; diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerFacade.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerFacade.java index be552d5aba4..22e0f30f986 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerFacade.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerFacade.java @@ -17,10 +17,11 @@ import android.os.Build; import android.os.Bundle; import android.os.SystemClock; import android.os.UserManager; -import android.support.annotation.AnyThread; -import android.support.annotation.MainThread; -import android.support.annotation.Nullable; -import android.support.annotation.WorkerThread; + +import androidx.annotation.AnyThread; +import androidx.annotation.MainThread; +import androidx.annotation.Nullable; +import androidx.annotation.WorkerThread; import org.chromium.base.Callback; import org.chromium.base.ContextUtils; @@ -28,6 +29,7 @@ import org.chromium.base.Log; import org.chromium.base.ObserverList; import org.chromium.base.ThreadUtils; import org.chromium.base.VisibleForTesting; +import org.chromium.base.annotations.CalledByNative; import org.chromium.base.metrics.CachedMetrics; import org.chromium.base.task.AsyncTask; import org.chromium.components.signin.util.PatternMatcher; @@ -164,6 +166,7 @@ public class AccountManagerFacade { * @return a singleton instance */ @AnyThread + @CalledByNative public static AccountManagerFacade get() { AccountManagerFacade instance = sAtomicInstance.get(); assert instance != null : "AccountManagerFacade is not initialized!"; diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerResult.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerResult.java index eec772e018d..6ab113ac1f1 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerResult.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerResult.java @@ -4,7 +4,7 @@ package org.chromium.components.signin; -import android.support.annotation.Nullable; +import androidx.annotation.Nullable; /** * AccountManagerResult encapsulates result of {@link AccountManagerFacade} method call. It is a diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountTrackerService.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountTrackerService.java index 204c3ab081d..5b39befeb2b 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountTrackerService.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountTrackerService.java @@ -5,7 +5,8 @@ package org.chromium.components.signin; import android.os.SystemClock; -import android.support.annotation.IntDef; + +import androidx.annotation.IntDef; import org.chromium.base.Log; import org.chromium.base.ObserverList; diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountsChangeObserver.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountsChangeObserver.java index 5fa611c4525..601ddb1772f 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountsChangeObserver.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountsChangeObserver.java @@ -4,7 +4,7 @@ package org.chromium.components.signin; -import android.support.annotation.MainThread; +import androidx.annotation.MainThread; /** * Observer that receives account change notifications. Use {@link AccountManagerFacade#addObserver} diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java index 37490a067c2..d9382f34d5e 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountInfoFetcher.java @@ -14,6 +14,7 @@ import org.chromium.base.ContextUtils; import org.chromium.base.Log; import org.chromium.base.ThreadUtils; import org.chromium.base.annotations.CalledByNative; +import org.chromium.base.annotations.NativeMethods; /** * ChildAccountInfoFetcher for the Android platform. @@ -81,7 +82,8 @@ public final class ChildAccountInfoFetcher { private void setIsChildAccount(boolean isChildAccount) { Log.d(TAG, "Setting child account status for %s to %s", mAccount.name, Boolean.toString(isChildAccount)); - nativeSetIsChildAccount(mNativeAccountFetcherService, mAccountId, isChildAccount); + ChildAccountInfoFetcherJni.get().setIsChildAccount( + mNativeAccountFetcherService, mAccountId, isChildAccount); } @CalledByNative @@ -90,6 +92,9 @@ public final class ChildAccountInfoFetcher { AccountManagerFacade.overrideAccountManagerFacadeForTests(delegate); } - private static native void nativeSetIsChildAccount( - long accountFetcherServicePtr, String accountId, boolean isChildAccount); + @NativeMethods + interface Natives { + void setIsChildAccount( + long accountFetcherServicePtr, String accountId, boolean isChildAccount); + } } diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountStatus.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountStatus.java index 867206ff97d..bc91770d9b3 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountStatus.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ChildAccountStatus.java @@ -4,7 +4,7 @@ package org.chromium.components.signin; -import android.support.annotation.IntDef; +import androidx.annotation.IntDef; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ConsistencyCookieManager.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ConsistencyCookieManager.java index 99eaac54810..44fa6beb23a 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ConsistencyCookieManager.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ConsistencyCookieManager.java @@ -4,7 +4,7 @@ package org.chromium.components.signin; -import android.support.annotation.MainThread; +import androidx.annotation.MainThread; import org.chromium.base.ThreadUtils; import org.chromium.base.annotations.CalledByNative; diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/MutableObservableValue.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/MutableObservableValue.java index 91b8bcd41d3..62ed539e6c2 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/MutableObservableValue.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/MutableObservableValue.java @@ -4,7 +4,7 @@ package org.chromium.components.signin; -import android.support.annotation.MainThread; +import androidx.annotation.MainThread; /** * {@link ObservableValue} subclass that allow value modification using {@link #set} method. Should diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/OAuth2TokenService.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/OAuth2TokenService.java index 375737023a8..a85f51b130e 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/OAuth2TokenService.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/OAuth2TokenService.java @@ -5,16 +5,18 @@ package org.chromium.components.signin; import android.accounts.Account; -import android.support.annotation.MainThread; -import android.support.annotation.Nullable; import android.text.TextUtils; +import androidx.annotation.MainThread; +import androidx.annotation.Nullable; + import org.chromium.base.ContextUtils; import org.chromium.base.Log; import org.chromium.base.StrictModeContext; import org.chromium.base.ThreadUtils; import org.chromium.base.VisibleForTesting; import org.chromium.base.annotations.CalledByNative; +import org.chromium.base.annotations.NativeMethods; import org.chromium.base.task.AsyncTask; import org.chromium.net.NetworkChangeNotifier; @@ -63,32 +65,40 @@ public final class OAuth2TokenService private final long mNativeOAuth2TokenServiceDelegate; private final AccountTrackerService mAccountTrackerService; + private final AccountManagerFacade mAccountManagerFacade; private boolean mPendingUpdate; - private OAuth2TokenService( - long nativeOAuth2TokenServiceDelegate, AccountTrackerService accountTrackerService) { + @VisibleForTesting + public OAuth2TokenService(long nativeOAuth2TokenServiceDelegate, + AccountTrackerService accountTrackerService, + AccountManagerFacade accountManagerFacade) { mNativeOAuth2TokenServiceDelegate = nativeOAuth2TokenServiceDelegate; mAccountTrackerService = accountTrackerService; + mAccountManagerFacade = accountManagerFacade; - mAccountTrackerService.addSystemAccountsSeededListener(this); + // AccountTrackerService might be null in tests. + if (mAccountTrackerService != null) { + mAccountTrackerService.addSystemAccountsSeededListener(this); + } } @CalledByNative - private static OAuth2TokenService create( - long nativeOAuth2TokenServiceDelegate, AccountTrackerService accountTrackerService) { - ThreadUtils.assertOnUiThread(); - return new OAuth2TokenService(nativeOAuth2TokenServiceDelegate, accountTrackerService); + private static OAuth2TokenService create(long nativeOAuth2TokenServiceDelegate, + AccountTrackerService accountTrackerService, + AccountManagerFacade accountManagerFacade) { + assert nativeOAuth2TokenServiceDelegate != 0; + return new OAuth2TokenService( + nativeOAuth2TokenServiceDelegate, accountTrackerService, accountManagerFacade); } - private static Account getAccountOrNullFromUsername(String username) { + private Account getAccountOrNullFromUsername(String username) { if (username == null) { Log.e(TAG, "Username is null"); return null; } - AccountManagerFacade accountManagerFacade = AccountManagerFacade.get(); - Account account = accountManagerFacade.getAccountFromName(username); + Account account = mAccountManagerFacade.getAccountFromName(username); if (account == null) { Log.e(TAG, "Account not found for provided username."); return null; @@ -101,11 +111,11 @@ public final class OAuth2TokenService */ @VisibleForTesting @CalledByNative - public static String[] getSystemAccountNames() { + public String[] getSystemAccountNames() { // TODO(https://crbug.com/768366): Remove this after adding cache to account manager facade. // This function is called by native code on UI thread. try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) { - List<String> accountNames = AccountManagerFacade.get().tryGetGoogleAccountNames(); + List<String> accountNames = mAccountManagerFacade.tryGetGoogleAccountNames(); return accountNames.toArray(new String[accountNames.size()]); } } @@ -116,6 +126,7 @@ public final class OAuth2TokenService * from the OS. updateAccountList should be called to keep these two * in sync. */ + @VisibleForTesting @CalledByNative public static String[] getAccounts() { return getStoredAccounts(); @@ -129,23 +140,26 @@ public final class OAuth2TokenService */ @MainThread @CalledByNative - private static void getAccessTokenFromNative( + private void getAccessTokenFromNative( String username, String scope, final long nativeCallback) { Account account = getAccountOrNullFromUsername(username); if (account == null) { - ThreadUtils.postOnUiThread(() -> nativeOAuth2TokenFetched(null, false, nativeCallback)); + ThreadUtils.postOnUiThread(() -> { + OAuth2TokenServiceJni.get().onOAuth2TokenFetched(null, false, nativeCallback); + }); return; } String oauth2Scope = OAUTH2_SCOPE_PREFIX + scope; getAccessToken(account, oauth2Scope, new GetAccessTokenCallback() { @Override public void onGetTokenSuccess(String token) { - nativeOAuth2TokenFetched(token, false, nativeCallback); + OAuth2TokenServiceJni.get().onOAuth2TokenFetched(token, false, nativeCallback); } @Override public void onGetTokenFailure(boolean isTransientError) { - nativeOAuth2TokenFetched(null, isTransientError, nativeCallback); + OAuth2TokenServiceJni.get().onOAuth2TokenFetched( + null, isTransientError, nativeCallback); } }); } @@ -158,12 +172,30 @@ public final class OAuth2TokenService * @param callback called on successful and unsuccessful fetching of auth token. */ @MainThread - public static void getAccessToken( + public void getAccessToken(Account account, String scope, GetAccessTokenCallback callback) { + getAccessTokenWithFacade(mAccountManagerFacade, account, scope, callback); + } + + /** + * Call this method to retrieve an OAuth2 access token for the given account and scope. Please + * note that this method expects a scope with 'oauth2:' prefix. + * + * @deprecated Use getAccessToken instead. crbug.com/1014098: This method is available as a + * workaround for a callsite where native is not initialized yet. + * + * @param accountManagerFacade AccountManagerFacade to request the access token from. + * @param account the account to get the access token for. + * @param scope The scope to get an auth token for (with Android-style 'oauth2:' prefix). + * @param callback called on successful and unsuccessful fetching of auth token. + */ + @MainThread + @Deprecated + public static void getAccessTokenWithFacade(AccountManagerFacade accountManagerFacade, Account account, String scope, GetAccessTokenCallback callback) { ConnectionRetry.runAuthTask(new AuthTask<String>() { @Override public String run() throws AuthException { - return AccountManagerFacade.get().getAccessToken(account, scope); + return accountManagerFacade.getAccessToken(account, scope); } @Override public void onSuccess(String token) { @@ -182,14 +214,14 @@ public final class OAuth2TokenService */ @MainThread @CalledByNative - public static void invalidateAccessToken(String accessToken) { + public void invalidateAccessToken(String accessToken) { if (TextUtils.isEmpty(accessToken)) { return; } ConnectionRetry.runAuthTask(new AuthTask<Boolean>() { @Override public Boolean run() throws AuthException { - AccountManagerFacade.get().invalidateAccessToken(accessToken); + mAccountManagerFacade.invalidateAccessToken(accessToken); return true; } @Override @@ -233,8 +265,8 @@ public final class OAuth2TokenService * Called by native to check whether the account has an OAuth2 refresh token. */ @CalledByNative - public static boolean hasOAuth2RefreshToken(String accountName) { - if (!AccountManagerFacade.get().isCachePopulated()) { + private boolean hasOAuth2RefreshToken(String accountName) { + if (!mAccountManagerFacade.isCachePopulated()) { return false; } @@ -281,7 +313,8 @@ public final class OAuth2TokenService // change (re-signin or sign out signed-in account). currentlySignedInAccount = null; } - nativeUpdateAccountList(mNativeOAuth2TokenServiceDelegate, currentlySignedInAccount); + OAuth2TokenServiceJni.get().updateAccountList(mNativeOAuth2TokenServiceDelegate, + OAuth2TokenService.this, currentlySignedInAccount); } private boolean isSignedInAccountChanged(String signedInAccountName) { @@ -298,12 +331,12 @@ public final class OAuth2TokenService return accounts == null ? new String[] {} : accounts.toArray(new String[0]); } - @CalledByNative /** * Called by native to save the account IDs that have associated OAuth2 refresh tokens. * This is called during updateAccountList to sync with getSystemAccountNames. * @param accounts IDs to save. */ + @CalledByNative private static void setAccounts(String[] accounts) { Set<String> set = new HashSet<>(Arrays.asList(accounts)); ContextUtils.getAppSharedPreferences() @@ -390,8 +423,11 @@ public final class OAuth2TokenService } } - private static native void nativeOAuth2TokenFetched( - String authToken, boolean isTransientError, long nativeCallback); - private native void nativeUpdateAccountList( - long nativeOAuth2TokenServiceDelegateAndroid, String currentlySignedInAccount); + @NativeMethods + interface Natives { + void onOAuth2TokenFetched( + String authToken, boolean isTransientError, long nativeCallback); + void updateAccountList(long nativeOAuth2TokenServiceDelegateAndroid, + OAuth2TokenService caller, String currentlySignedInAccount); + } } diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ObservableValue.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ObservableValue.java index 8ec6ba9d2a4..94114a89eee 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ObservableValue.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ObservableValue.java @@ -4,9 +4,10 @@ package org.chromium.components.signin; -import android.support.annotation.MainThread; import android.support.v4.util.ObjectsCompat; +import androidx.annotation.MainThread; + import org.chromium.base.ObserverList; import org.chromium.base.ThreadUtils; diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ProfileDataSource.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ProfileDataSource.java index 43ec44a7786..72822f839ea 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ProfileDataSource.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/ProfileDataSource.java @@ -5,7 +5,8 @@ package org.chromium.components.signin; import android.graphics.Bitmap; -import android.support.annotation.Nullable; + +import androidx.annotation.Nullable; import java.util.Map; diff --git a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/SigninActivityMonitor.java b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/SigninActivityMonitor.java index 761b3f2bb16..7fe5018d0b4 100644 --- a/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/SigninActivityMonitor.java +++ b/chromium/components/signin/core/browser/android/java/src/org/chromium/components/signin/SigninActivityMonitor.java @@ -5,7 +5,8 @@ package org.chromium.components.signin; import android.annotation.SuppressLint; -import android.support.annotation.MainThread; + +import androidx.annotation.MainThread; import org.chromium.base.ThreadUtils; diff --git a/chromium/components/signin/core/browser/chrome_connected_header_helper.cc b/chromium/components/signin/core/browser/chrome_connected_header_helper.cc index 6909815553f..ec98e9741ac 100644 --- a/chromium/components/signin/core/browser/chrome_connected_header_helper.cc +++ b/chromium/components/signin/core/browser/chrome_connected_header_helper.cc @@ -58,7 +58,7 @@ ChromeConnectedHeaderHelper::ChromeConnectedHeaderHelper( // static std::string ChromeConnectedHeaderHelper::BuildRequestCookieIfPossible( const GURL& url, - const std::string& account_id, + const std::string& gaia_id, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, int profile_mode_mask) { @@ -66,7 +66,7 @@ std::string ChromeConnectedHeaderHelper::BuildRequestCookieIfPossible( if (!chrome_connected_helper.ShouldBuildRequestHeader(url, cookie_settings)) return ""; return chrome_connected_helper.BuildRequestHeader( - false /* is_header_request */, url, account_id, profile_mode_mask); + false /* is_header_request */, url, gaia_id, profile_mode_mask); } // static @@ -167,7 +167,7 @@ bool ChromeConnectedHeaderHelper::IsUrlEligibleForRequestHeader( std::string ChromeConnectedHeaderHelper::BuildRequestHeader( bool is_header_request, const GURL& url, - const std::string& account_id, + const std::string& gaia_id, int profile_mode_mask) { #if defined(OS_ANDROID) bool is_mice_enabled = base::FeatureList::IsEnabled(kMiceFeature); @@ -183,16 +183,16 @@ std::string ChromeConnectedHeaderHelper::BuildRequestHeader( // filtered upstream and we want to enforce account consistency in Public // Sessions and Active Directory logins. #if !defined(OS_CHROMEOS) - if (account_id.empty() && !is_mice_enabled) + if (gaia_id.empty() && !is_mice_enabled) return std::string(); #endif // !defined(OS_CHROMEOS) std::vector<std::string> parts; - if (!account_id.empty() && + if (!gaia_id.empty() && IsUrlEligibleToIncludeGaiaId(url, is_header_request)) { // Only set the Gaia ID on domains that actually require it. parts.push_back( - base::StringPrintf("%s=%s", kGaiaIdAttrName, account_id.c_str())); + base::StringPrintf("%s=%s", kGaiaIdAttrName, gaia_id.c_str())); } parts.push_back( base::StringPrintf("%s=%s", kProfileModeAttrName, diff --git a/chromium/components/signin/core/browser/chrome_connected_header_helper.h b/chromium/components/signin/core/browser/chrome_connected_header_helper.h index 81831b2b392..2e42fbdb412 100644 --- a/chromium/components/signin/core/browser/chrome_connected_header_helper.h +++ b/chromium/components/signin/core/browser/chrome_connected_header_helper.h @@ -25,7 +25,7 @@ class ChromeConnectedHeaderHelper : public SigninHeaderHelper { // added to the request to |url|. static std::string BuildRequestCookieIfPossible( const GURL& url, - const std::string& account_id, + const std::string& gaia_id, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, int profile_mode_mask); @@ -39,7 +39,7 @@ class ChromeConnectedHeaderHelper : public SigninHeaderHelper { // empty string, in this case the header must not be added. std::string BuildRequestHeader(bool is_header_request, const GURL& url, - const std::string& account_id, + const std::string& gaia_id, int profile_mode_mask); // SigninHeaderHelper implementation: diff --git a/chromium/components/signin/core/browser/cookie_reminter.cc b/chromium/components/signin/core/browser/cookie_reminter.cc new file mode 100644 index 00000000000..71fa4b7aa84 --- /dev/null +++ b/chromium/components/signin/core/browser/cookie_reminter.cc @@ -0,0 +1,63 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/signin/core/browser/cookie_reminter.h" +#include "components/signin/public/identity_manager/accounts_cookie_mutator.h" + +namespace { + +bool DoesAccountRequireCookieReminting( + const std::vector<CoreAccountInfo>& accounts_requiring_cookie_remint, + const CoreAccountInfo& account_info) { + for (const CoreAccountInfo& account_requiring_remint : + accounts_requiring_cookie_remint) { + if (account_info.gaia == account_requiring_remint.gaia) { + return true; + } + } + + return false; +} + +} // namespace + +CookieReminter::CookieReminter(signin::IdentityManager* identity_manager) + : identity_manager_(identity_manager) { + identity_manager_->AddObserver(this); +} + +CookieReminter::~CookieReminter() { + identity_manager_->RemoveObserver(this); +} + +void CookieReminter::ForceCookieRemintingOnNextTokenUpdate( + const CoreAccountInfo& account_info) { + if (DoesAccountRequireCookieReminting(accounts_requiring_cookie_remint_, + account_info)) { + return; + } + + accounts_requiring_cookie_remint_.emplace_back(account_info); +} + +bool CookieReminter::RemintCookieIfRequired() { + if (!is_forced_cookie_reminting_required_) + return false; + + identity_manager_->GetAccountsCookieMutator()->LogOutAllAccounts( + gaia::GaiaSource::kChromeOS); + accounts_requiring_cookie_remint_.clear(); + is_forced_cookie_reminting_required_ = false; + return true; +} + +void CookieReminter::OnRefreshTokenUpdatedForAccount( + const CoreAccountInfo& account_info) { + if (DoesAccountRequireCookieReminting(accounts_requiring_cookie_remint_, + account_info)) { + // Cookies are going to be reminted for all accounts. + accounts_requiring_cookie_remint_.clear(); + is_forced_cookie_reminting_required_ = true; + } +} diff --git a/chromium/components/signin/core/browser/cookie_reminter.h b/chromium/components/signin/core/browser/cookie_reminter.h new file mode 100644 index 00000000000..5875b6b05ce --- /dev/null +++ b/chromium/components/signin/core/browser/cookie_reminter.h @@ -0,0 +1,44 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#ifndef COMPONENTS_SIGNIN_CORE_BROWSER_COOKIE_REMINTER_H_ +#define COMPONENTS_SIGNIN_CORE_BROWSER_COOKIE_REMINTER_H_ + +#include <vector> + +#include "components/signin/public/identity_manager/identity_manager.h" + +// Stores accounts with invalid cookies, which cannot be detected by +// /ListAccounts, so we can force cookie reminting when account is +// reauthenticated. +// +// Subscribed to |OnRefreshTokenUpdatedForAccount| of IdentityManager, calls +// |AccountsCookieMutator::LogOutAllAccounts| after refresh token update of +// any of the accounts that have been added to +// |ForceCookieRemintingOnNextTokenUpdate|. +class CookieReminter : public signin::IdentityManager::Observer { + public: + explicit CookieReminter(signin::IdentityManager* identity_manager); + ~CookieReminter() override; + + // Forces a cookie reminting if/when the refresh token for |account_info| is + // updated. + void ForceCookieRemintingOnNextTokenUpdate( + const CoreAccountInfo& account_info); + + // If there are accounts that require cookie reminting, calls + // |AccountsCookieMutator::LogOutAllAccounts| and returns true. Otherwise + // returns false. + bool RemintCookieIfRequired(); + + // Overridden from signin::IdentityManager::Observer. + void OnRefreshTokenUpdatedForAccount( + const CoreAccountInfo& account_info) override; + + private: + bool is_forced_cookie_reminting_required_ = false; + signin::IdentityManager* identity_manager_; + std::vector<CoreAccountInfo> accounts_requiring_cookie_remint_; +}; + +#endif // COMPONENTS_SIGNIN_CORE_BROWSER_COOKIE_REMINTER_H_ diff --git a/chromium/components/signin/core/browser/dice_header_helper.cc b/chromium/components/signin/core/browser/dice_header_helper.cc index 0c9880ed4c7..4d1bc249174 100644 --- a/chromium/components/signin/core/browser/dice_header_helper.cc +++ b/chromium/components/signin/core/browser/dice_header_helper.cc @@ -194,7 +194,7 @@ bool DiceHeaderHelper::IsUrlEligibleForRequestHeader(const GURL& url) { } std::string DiceHeaderHelper::BuildRequestHeader( - const std::string& sync_account_id, + const std::string& sync_gaia_id, const std::string& device_id) { std::vector<std::string> parts; parts.push_back(base::StringPrintf("version=%s", kDiceProtocolVersion)); @@ -202,8 +202,8 @@ std::string DiceHeaderHelper::BuildRequestHeader( GaiaUrls::GetInstance()->oauth2_chrome_client_id()); if (!device_id.empty()) parts.push_back("device_id=" + device_id); - if (!sync_account_id.empty()) - parts.push_back("sync_account_id=" + sync_account_id); + if (!sync_gaia_id.empty()) + parts.push_back("sync_account_id=" + sync_gaia_id); // Restrict Signin to Sync account only when fixing auth errors. std::string signin_mode = kRequestSigninAll; diff --git a/chromium/components/signin/core/browser/dice_header_helper.h b/chromium/components/signin/core/browser/dice_header_helper.h index d4711431582..4d076f6bfba 100644 --- a/chromium/components/signin/core/browser/dice_header_helper.h +++ b/chromium/components/signin/core/browser/dice_header_helper.h @@ -10,6 +10,7 @@ #include "base/macros.h" #include "components/signin/core/browser/signin_header_helper.h" #include "components/signin/public/base/account_consistency_method.h" +#include "google_apis/gaia/core_account_id.h" class GURL; @@ -36,11 +37,11 @@ class DiceHeaderHelper : public SigninHeaderHelper { // Returns the header value for Dice requests. Returns the empty string when // the header must not be added. - // |sync_account_id| is not empty if Sync is currently enabled for this + // |sync_gaia_id| is not empty if Sync is currently enabled for this // account. // |show_signout_confirmation| is true if Gaia must display the signout // confirmation dialog. - std::string BuildRequestHeader(const std::string& sync_account_id, + std::string BuildRequestHeader(const std::string& sync_gaia_id, const std::string& device_id); // SigninHeaderHelper implementation: diff --git a/chromium/components/signin/core/browser/signin_header_helper.cc b/chromium/components/signin/core/browser/signin_header_helper.cc index cf0a3877b3e..72bb55c9a02 100644 --- a/chromium/components/signin/core/browser/signin_header_helper.cc +++ b/chromium/components/signin/core/browser/signin_header_helper.cc @@ -88,12 +88,12 @@ void RequestAdapter::SetExtraHeaderByName(const std::string& name, std::string BuildMirrorRequestCookieIfPossible( const GURL& url, - const std::string& account_id, + const std::string& gaia_id, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, int profile_mode_mask) { return ChromeConnectedHeaderHelper::BuildRequestCookieIfPossible( - url, account_id, account_consistency, cookie_settings, profile_mode_mask); + url, gaia_id, account_consistency, cookie_settings, profile_mode_mask); } SigninHeaderHelper::SigninHeaderHelper() = default; @@ -147,7 +147,7 @@ SigninHeaderHelper::ParseAccountConsistencyResponseHeader( void AppendOrRemoveMirrorRequestHeader( RequestAdapter* request, const GURL& redirect_url, - const std::string& account_id, + const std::string& gaia_id, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, int profile_mode_mask) { @@ -156,7 +156,7 @@ void AppendOrRemoveMirrorRequestHeader( std::string chrome_connected_header_value; if (chrome_connected_helper.ShouldBuildRequestHeader(url, cookie_settings)) { chrome_connected_header_value = chrome_connected_helper.BuildRequestHeader( - true /* is_header_request */, url, account_id, profile_mode_mask); + true /* is_header_request */, url, gaia_id, profile_mode_mask); } chrome_connected_helper.AppendOrRemoveRequestHeader( request, redirect_url, kChromeConnectedHeader, @@ -166,7 +166,7 @@ void AppendOrRemoveMirrorRequestHeader( bool AppendOrRemoveDiceRequestHeader( RequestAdapter* request, const GURL& redirect_url, - const std::string& account_id, + const std::string& gaia_id, bool sync_enabled, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, @@ -177,7 +177,7 @@ bool AppendOrRemoveDiceRequestHeader( std::string dice_header_value; if (dice_helper.ShouldBuildRequestHeader(url, cookie_settings)) { dice_header_value = dice_helper.BuildRequestHeader( - sync_enabled ? account_id : std::string(), device_id); + sync_enabled ? gaia_id : std::string(), device_id); } return dice_helper.AppendOrRemoveRequestHeader( request, redirect_url, kDiceRequestHeader, dice_header_value); diff --git a/chromium/components/signin/core/browser/signin_header_helper.h b/chromium/components/signin/core/browser/signin_header_helper.h index ac718de9f54..20c4e4dcb8e 100644 --- a/chromium/components/signin/core/browser/signin_header_helper.h +++ b/chromium/components/signin/core/browser/signin_header_helper.h @@ -12,6 +12,7 @@ #include "components/prefs/pref_member.h" #include "components/signin/public/base/account_consistency_method.h" #include "components/signin/public/base/signin_buildflags.h" +#include "google_apis/gaia/core_account_id.h" #include "url/gurl.h" namespace content_settings { @@ -204,7 +205,7 @@ class SigninHeaderHelper { // added to the request to |url|. std::string BuildMirrorRequestCookieIfPossible( const GURL& url, - const std::string& account_id, + const std::string& gaia_id, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, int profile_mode_mask); @@ -215,7 +216,7 @@ std::string BuildMirrorRequestCookieIfPossible( void AppendOrRemoveMirrorRequestHeader( RequestAdapter* request, const GURL& redirect_url, - const std::string& account_id, + const std::string& gaia_id, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, int profile_mode_mask); @@ -227,7 +228,7 @@ void AppendOrRemoveMirrorRequestHeader( bool AppendOrRemoveDiceRequestHeader( RequestAdapter* request, const GURL& redirect_url, - const std::string& account_id, + const std::string& gaia_id, bool sync_enabled, AccountConsistencyMethod account_consistency, const content_settings::CookieSettings* cookie_settings, diff --git a/chromium/components/signin/core/browser/signin_header_helper_unittest.cc b/chromium/components/signin/core/browser/signin_header_helper_unittest.cc index b6f8773773c..91eb1bf22d7 100644 --- a/chromium/components/signin/core/browser/signin_header_helper_unittest.cc +++ b/chromium/components/signin/core/browser/signin_header_helper_unittest.cc @@ -50,10 +50,10 @@ class SigninHeaderHelperTest : public testing::Test { void TearDown() override { settings_map_->ShutdownOnUIThread(); } void CheckMirrorCookieRequest(const GURL& url, - const std::string& account_id, + const std::string& gaia_id, const std::string& expected_request) { EXPECT_EQ(BuildMirrorRequestCookieIfPossible( - url, account_id, account_consistency_, cookie_settings_.get(), + url, gaia_id, account_consistency_, cookie_settings_.get(), PROFILE_MODE_DEFAULT), expected_request); } @@ -112,7 +112,7 @@ class SigninHeaderHelperTest : public testing::Test { } #endif - base::test::TaskEnvironment task_environment_; + base::test::SingleThreadTaskEnvironment task_environment_; bool sync_enabled_ = false; std::string device_id_ = kTestDeviceId; diff --git a/chromium/components/signin/core/browser/signin_internals_util.cc b/chromium/components/signin/core/browser/signin_internals_util.cc index 68884202990..68dc2ee8968 100644 --- a/chromium/components/signin/core/browser/signin_internals_util.cc +++ b/chromium/components/signin/core/browser/signin_internals_util.cc @@ -46,27 +46,4 @@ std::string SigninStatusFieldToString(TimedSigninStatusField field) { return std::string(); } -std::string TokenPrefPath(const std::string& token_name) { - return std::string(kTokenPrefPrefix) + token_name; -} - -// Gets the first few hex characters of the SHA256 hash of the passed in string. -// These are enough to perform equality checks across a single users tokens, -// while preventing outsiders from reverse-engineering the actual token from -// the displayed value. -// Note that for readability (in about:signin-internals), an empty string -// is not hashed, but simply returned as an empty string. -std::string GetTruncatedHash(const std::string& str) { - if (str.empty()) - return str; - - // Since each character in the hash string generates two hex charaters - // we only need half as many charaters in |hash_val| as hex characters - // returned. - const int kTruncateSize = kTruncateTokenStringLength / 2; - char hash_val[kTruncateSize]; - crypto::SHA256HashString(str, &hash_val[0], kTruncateSize); - return base::ToLowerASCII(base::HexEncode(&hash_val[0], kTruncateSize)); -} - } // namespace signin_internals_util diff --git a/chromium/components/signin/core/browser/signin_internals_util.h b/chromium/components/signin/core/browser/signin_internals_util.h index 576710b70c2..dc6a0e240ea 100644 --- a/chromium/components/signin/core/browser/signin_internals_util.h +++ b/chromium/components/signin/core/browser/signin_internals_util.h @@ -19,9 +19,6 @@ namespace signin_internals_util { extern const char kSigninPrefPrefix[]; extern const char kTokenPrefPrefix[]; -// The length of strings returned by GetTruncatedHash() below. -const size_t kTruncateTokenStringLength = 6; - // Helper enums to access fields from SigninStatus (declared below). enum { SIGNIN_FIELDS_BEGIN = 0, @@ -54,23 +51,10 @@ enum { SIGNIN_FIELDS_COUNT = SIGNIN_FIELDS_END - SIGNIN_FIELDS_BEGIN }; -// Returns the root preference path for the service. The path should be -// qualified with one of .value, .status or .time to get the respective -// full preference path names. -std::string TokenPrefPath(const std::string& service_name); - // Returns the name of a SigninStatus field. std::string SigninStatusFieldToString(UntimedSigninStatusField field); std::string SigninStatusFieldToString(TimedSigninStatusField field); -// Gets the first 6 hex characters of the SHA256 hash of the passed in string. -// These are enough to perform equality checks across a single users tokens, -// while preventing outsiders from reverse-engineering the actual token from -// the displayed value. -// Note that for readability (in about:signin-internals), an empty string -// is not hashed, but simply returned as an empty string. -std::string GetTruncatedHash(const std::string& str); - } // namespace signin_internals_util #endif // COMPONENTS_SIGNIN_CORE_BROWSER_SIGNIN_INTERNALS_UTIL_H_ diff --git a/chromium/components/signin/internal/base/BUILD.gn b/chromium/components/signin/internal/base/BUILD.gn new file mode 100644 index 00000000000..e1ff00f2bcf --- /dev/null +++ b/chromium/components/signin/internal/base/BUILD.gn @@ -0,0 +1,18 @@ +# Copyright 2019 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +# TODO(crbug.com/986435) Remove this target when IdentityServicesProvider +# will provide AccountManagerFacade. +source_set("base") { + if (is_android) { + sources = [ + "account_manager_facade_android.cc", + "account_manager_facade_android.h", + ] + deps = [ + "//base", + "//components/signin/core/browser/android:jni_headers", + ] + } +} diff --git a/chromium/components/signin/internal/base/DEPS b/chromium/components/signin/internal/base/DEPS new file mode 100644 index 00000000000..c079a1bff2c --- /dev/null +++ b/chromium/components/signin/internal/base/DEPS @@ -0,0 +1,8 @@ +include_rules = [ +] + +specific_include_rules = { + "account_manager_facade_android.cc": [ + "+components/signin/core/browser/android/jni_headers/AccountManagerFacade_jni.h", + ], +} diff --git a/chromium/components/signin/internal/base/README.md b/chromium/components/signin/internal/base/README.md new file mode 100644 index 00000000000..e69de29bb2d --- /dev/null +++ b/chromium/components/signin/internal/base/README.md diff --git a/chromium/components/signin/internal/base/account_manager_facade_android.cc b/chromium/components/signin/internal/base/account_manager_facade_android.cc new file mode 100644 index 00000000000..83ca3fc562e --- /dev/null +++ b/chromium/components/signin/internal/base/account_manager_facade_android.cc @@ -0,0 +1,12 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/signin/internal/base/account_manager_facade_android.h" + +#include "components/signin/core/browser/android/jni_headers/AccountManagerFacade_jni.h" + +base::android::ScopedJavaLocalRef<jobject> +AccountManagerFacadeAndroid::GetJavaObject() { + return Java_AccountManagerFacade_get(base::android::AttachCurrentThread()); +} diff --git a/chromium/components/signin/internal/base/account_manager_facade_android.h b/chromium/components/signin/internal/base/account_manager_facade_android.h new file mode 100644 index 00000000000..9508a6d11cf --- /dev/null +++ b/chromium/components/signin/internal/base/account_manager_facade_android.h @@ -0,0 +1,19 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_SIGNIN_INTERNAL_BASE_ACCOUNT_MANAGER_FACADE_ANDROID_H_ +#define COMPONENTS_SIGNIN_INTERNAL_BASE_ACCOUNT_MANAGER_FACADE_ANDROID_H_ + +#include "base/android/scoped_java_ref.h" + +// Simple accessor to java's AccountManagerFacade +class AccountManagerFacadeAndroid { + public: + // Returns a reference to the corresponding Java AccountManagerFacade object. + // TODO(crbug.com/986435) Remove direct access to AccountManagerFacade.get + // from C++ when IdentityServicesProvider will handle its instance management. + static base::android::ScopedJavaLocalRef<jobject> GetJavaObject(); +}; + +#endif // COMPONENTS_SIGNIN_INTERNAL_BASE_ACCOUNT_MANAGER_FACADE_ANDROID_H_ diff --git a/chromium/components/signin/internal/identity_manager/BUILD.gn b/chromium/components/signin/internal/identity_manager/BUILD.gn index a6a8aa9037a..9cfafc06a25 100644 --- a/chromium/components/signin/internal/identity_manager/BUILD.gn +++ b/chromium/components/signin/internal/identity_manager/BUILD.gn @@ -22,8 +22,6 @@ source_set("identity_manager") { "diagnostics_provider_impl.h", "gaia_cookie_manager_service.cc", "gaia_cookie_manager_service.h", - "mutable_profile_oauth2_token_service_delegate.cc", - "mutable_profile_oauth2_token_service_delegate.h", "oauth2_token_service_delegate_android.cc", "oauth2_token_service_delegate_android.h", "oauth_multilogin_helper.cc", @@ -72,7 +70,15 @@ source_set("identity_manager") { ] if (is_android) { - deps += [ "//components/signin/core/browser/android:jni_headers" ] + deps += [ + "//components/signin/core/browser/android:jni_headers", + "//components/signin/internal/base", + ] + } else { + sources += [ + "mutable_profile_oauth2_token_service_delegate.cc", + "mutable_profile_oauth2_token_service_delegate.h", + ] } if (is_chromeos) { @@ -101,12 +107,14 @@ source_set("identity_manager") { if (is_ios) { configs += [ "//build/config/compiler:enable_arc" ] + deps += [ "//components/signin/public/identity_manager/ios" ] + } + + if (is_ios || is_android) { sources += [ "device_accounts_synchronizer_impl.cc", "device_accounts_synchronizer_impl.h", ] - - deps += [ "//components/signin/public/identity_manager/ios" ] } } @@ -117,7 +125,6 @@ source_set("unit_tests") { "account_info_util_unittest.cc", "account_tracker_service_unittest.cc", "gaia_cookie_manager_service_unittest.cc", - "mutable_profile_oauth2_token_service_delegate_unittest.cc", "oauth2_token_service_delegate_android_unittest.cc", "oauth_multilogin_helper_unittest.cc", "oauth_multilogin_token_fetcher_unittest.cc", @@ -167,6 +174,10 @@ source_set("unit_tests") { deps += [ "//components/signin/public/identity_manager/ios:test_support" ] } + + if (!is_android) { + sources += [ "mutable_profile_oauth2_token_service_delegate_unittest.cc" ] + } } # This target contains test support that backs the test support for diff --git a/chromium/components/signin/internal/identity_manager/DEPS b/chromium/components/signin/internal/identity_manager/DEPS index 5898a127076..053a11edcce 100644 --- a/chromium/components/signin/internal/identity_manager/DEPS +++ b/chromium/components/signin/internal/identity_manager/DEPS @@ -1,5 +1,9 @@ include_rules = [ "+chromeos/constants/chromeos_features.h", + "+components/signin/internal/base", + "+components/signin/public/base", + "+components/signin/public/identity_manager", + "+components/signin/public/webdata", "+mojo/public", ] diff --git a/chromium/components/signin/internal/identity_manager/account_fetcher_service.cc b/chromium/components/signin/internal/identity_manager/account_fetcher_service.cc index 97889b99ee8..f4e66791cee 100644 --- a/chromium/components/signin/internal/identity_manager/account_fetcher_service.cc +++ b/chromium/components/signin/internal/identity_manager/account_fetcher_service.cc @@ -50,7 +50,7 @@ const char kImageFetcherUmaClient[] = "AccountFetcherService"; const char AccountFetcherService::kLastUpdatePref[] = "account_tracker_service_last_update"; -const int AccountFetcherService::kAccountImageDownloadSize = 64; +const int AccountFetcherService::kAccountImageDownloadSize = 256; // AccountFetcherService implementation AccountFetcherService::AccountFetcherService() = default; diff --git a/chromium/components/signin/internal/identity_manager/account_info_fetcher.h b/chromium/components/signin/internal/identity_manager/account_info_fetcher.h index c43ba6dbfb2..c981f3986bd 100644 --- a/chromium/components/signin/internal/identity_manager/account_info_fetcher.h +++ b/chromium/components/signin/internal/identity_manager/account_info_fetcher.h @@ -33,8 +33,6 @@ class AccountInfoFetcher : public OAuth2AccessTokenManager::Consumer, const CoreAccountId& account_id); ~AccountInfoFetcher() override; - const CoreAccountId& account_id() { return account_id_; } - // Start fetching the account information. void Start(); diff --git a/chromium/components/signin/internal/identity_manager/account_tracker_service_unittest.cc b/chromium/components/signin/internal/identity_manager/account_tracker_service_unittest.cc index 18ca47453d9..00288b8dae7 100644 --- a/chromium/components/signin/internal/identity_manager/account_tracker_service_unittest.cc +++ b/chromium/components/signin/internal/identity_manager/account_tracker_service_unittest.cc @@ -379,7 +379,7 @@ void AccountTrackerServiceTest::ReturnFetchResults( while (GetTestURLLoaderFactory()->IsPending(url.spec())) { GetTestURLLoaderFactory()->SimulateResponseForPendingRequest( url, network::URLLoaderCompletionStatus(net::OK), - network::CreateResourceResponseHead(response_code), response_string, + network::CreateURLResponseHead(response_code), response_string, network::TestURLLoaderFactory::kMostRecentMatch); } } diff --git a/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.cc b/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.cc index d2b158e4d04..f4f8449838f 100644 --- a/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.cc +++ b/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.cc @@ -6,6 +6,7 @@ #include <utility> +#include "build/build_config.h" #include "components/signin/internal/identity_manager/account_tracker_service.h" #include "components/signin/internal/identity_manager/gaia_cookie_manager_service.h" #include "google_apis/gaia/core_account_id.h" @@ -59,6 +60,12 @@ void AccountsCookieMutatorImpl::TriggerCookieJarUpdate() { gaia_cookie_manager_service_->TriggerListAccounts(); } +#if defined(OS_IOS) +void AccountsCookieMutatorImpl::ForceTriggerOnCookieChange() { + gaia_cookie_manager_service_->ForceOnCookieChangeProcessing(); +} +#endif + void AccountsCookieMutatorImpl::LogOutAllAccounts(gaia::GaiaSource source) { gaia_cookie_manager_service_->LogOutAllAccounts(source); } diff --git a/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.h b/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.h index a083bbb1915..dcbcdcf92eb 100644 --- a/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.h +++ b/chromium/components/signin/internal/identity_manager/accounts_cookie_mutator_impl.h @@ -9,6 +9,7 @@ #include <vector> #include "base/macros.h" +#include "build/build_config.h" #include "components/signin/public/identity_manager/accounts_cookie_mutator.h" class AccountTrackerService; @@ -47,6 +48,10 @@ class AccountsCookieMutatorImpl : public AccountsCookieMutator { void TriggerCookieJarUpdate() override; +#if defined(OS_IOS) + void ForceTriggerOnCookieChange() override; +#endif + void LogOutAllAccounts(gaia::GaiaSource source) override; private: diff --git a/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.cc b/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.cc index bff6a3de620..6fa9e606e6d 100644 --- a/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.cc +++ b/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.cc @@ -80,7 +80,7 @@ void AccountsMutatorImpl::RemoveAllAccounts( void AccountsMutatorImpl::InvalidateRefreshTokenForPrimaryAccount( signin_metrics::SourceForRefreshTokenOperation source) { DCHECK(primary_account_manager_->IsAuthenticated()); - AccountInfo primary_account_info = + CoreAccountInfo primary_account_info = primary_account_manager_->GetAuthenticatedAccountInfo(); AddOrUpdateAccount(primary_account_info.gaia, primary_account_info.email, GaiaConstants::kInvalidRefreshToken, @@ -106,11 +106,4 @@ void AccountsMutatorImpl::MoveAccount(AccountsMutator* target, } #endif -void AccountsMutatorImpl::LegacySetRefreshTokenForSupervisedUser( - const std::string& refresh_token) { - token_service_->UpdateCredentials( - CoreAccountId("managed_user@localhost"), refresh_token, - signin_metrics::SourceForRefreshTokenOperation::kSupervisedUser_InitSync); -} - } // namespace signin diff --git a/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.h b/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.h index 0dc2ca05dc2..70dcb00fc29 100644 --- a/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.h +++ b/chromium/components/signin/internal/identity_manager/accounts_mutator_impl.h @@ -56,9 +56,6 @@ class AccountsMutatorImpl : public AccountsMutator { const CoreAccountId& account_id) override; #endif - void LegacySetRefreshTokenForSupervisedUser( - const std::string& refresh_token) override; - private: ProfileOAuth2TokenService* token_service_; AccountTrackerService* account_tracker_service_; diff --git a/chromium/components/signin/internal/identity_manager/android/BUILD.gn b/chromium/components/signin/internal/identity_manager/android/BUILD.gn index d9b022f516c..87a9e21dcff 100644 --- a/chromium/components/signin/internal/identity_manager/android/BUILD.gn +++ b/chromium/components/signin/internal/identity_manager/android/BUILD.gn @@ -3,7 +3,9 @@ import("//build/config/android/rules.gni") generate_jni("jni_headers") { namespace = "signin" sources = [ + "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java", "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java", "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java", + "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java", ] } diff --git a/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.cc b/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.cc index d2a4f923394..1507e18f353 100644 --- a/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.cc +++ b/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.cc @@ -17,6 +17,16 @@ DeviceAccountsSynchronizerImpl::DeviceAccountsSynchronizerImpl( DeviceAccountsSynchronizerImpl::~DeviceAccountsSynchronizerImpl() = default; +#if defined(OS_ANDROID) +void DeviceAccountsSynchronizerImpl:: + ReloadAllAccountsFromSystemWithPrimaryAccount( + const CoreAccountId& primary_account_id) { + token_service_delegate_->ReloadAllAccountsFromSystemWithPrimaryAccount( + primary_account_id); +} +#endif + +#if defined(OS_IOS) void DeviceAccountsSynchronizerImpl::ReloadAllAccountsFromSystem() { token_service_delegate_->ReloadAllAccountsFromSystem(); } @@ -25,5 +35,6 @@ void DeviceAccountsSynchronizerImpl::ReloadAccountFromSystem( const CoreAccountId& account_id) { token_service_delegate_->ReloadAccountFromSystem(account_id); } +#endif } // namespace signin diff --git a/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.h b/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.h index 74679a0a9a1..f043b2aef57 100644 --- a/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.h +++ b/chromium/components/signin/internal/identity_manager/device_accounts_synchronizer_impl.h @@ -5,6 +5,7 @@ #ifndef COMPONENTS_SIGNIN_INTERNAL_IDENTITY_MANAGER_DEVICE_ACCOUNTS_SYNCHRONIZER_IMPL_H_ #define COMPONENTS_SIGNIN_INTERNAL_IDENTITY_MANAGER_DEVICE_ACCOUNTS_SYNCHRONIZER_IMPL_H_ +#include "build/build_config.h" #include "components/signin/public/identity_manager/device_accounts_synchronizer.h" class ProfileOAuth2TokenServiceDelegate; @@ -19,8 +20,15 @@ class DeviceAccountsSynchronizerImpl : public DeviceAccountsSynchronizer { ~DeviceAccountsSynchronizerImpl() override; // DeviceAccountsSynchronizer implementation. +#if defined(OS_ANDROID) + void ReloadAllAccountsFromSystemWithPrimaryAccount( + const CoreAccountId& primary_account_id) override; +#endif + +#if defined(OS_IOS) void ReloadAllAccountsFromSystem() override; void ReloadAccountFromSystem(const CoreAccountId& account_id) override; +#endif private: ProfileOAuth2TokenServiceDelegate* token_service_delegate_ = nullptr; diff --git a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.cc b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.cc index 2ae836ad7b5..f3aa3e0a97c 100644 --- a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.cc +++ b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.cc @@ -10,14 +10,9 @@ FakeProfileOAuth2TokenService::FakeProfileOAuth2TokenService( PrefService* user_prefs) - : FakeProfileOAuth2TokenService( + : ProfileOAuth2TokenService( user_prefs, - std::make_unique<FakeProfileOAuth2TokenServiceDelegate>()) {} - -FakeProfileOAuth2TokenService::FakeProfileOAuth2TokenService( - PrefService* user_prefs, - std::unique_ptr<ProfileOAuth2TokenServiceDelegate> delegate) - : ProfileOAuth2TokenService(user_prefs, std::move(delegate)) { + std::make_unique<FakeProfileOAuth2TokenServiceDelegate>()) { OverrideAccessTokenManagerForTesting( std::make_unique<FakeOAuth2AccessTokenManager>( this /* OAuth2AccessTokenManager::Delegate* */)); @@ -99,3 +94,8 @@ FakeOAuth2AccessTokenManager* FakeProfileOAuth2TokenService::GetFakeAccessTokenManager() { return static_cast<FakeOAuth2AccessTokenManager*>(GetAccessTokenManager()); } + +bool FakeProfileOAuth2TokenService::IsFakeProfileOAuth2TokenServiceForTesting() + const { + return true; +} diff --git a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.h b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.h index 8245090ec7b..dc8f4ba475f 100644 --- a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.h +++ b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service.h @@ -36,9 +36,6 @@ class FakeProfileOAuth2TokenService : public ProfileOAuth2TokenService { public: explicit FakeProfileOAuth2TokenService(PrefService* user_prefs); - FakeProfileOAuth2TokenService( - PrefService* user_prefs, - std::unique_ptr<ProfileOAuth2TokenServiceDelegate> delegate); ~FakeProfileOAuth2TokenService() override; // Gets a list of active requests (can be used by tests to validate that the @@ -81,6 +78,8 @@ class FakeProfileOAuth2TokenService : public ProfileOAuth2TokenService { void set_auto_post_fetch_response_on_message_loop(bool auto_post_response); + bool IsFakeProfileOAuth2TokenServiceForTesting() const override; + private: FakeOAuth2AccessTokenManager* GetFakeAccessTokenManager(); diff --git a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.cc b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.cc index bef96b8b886..65bbb978ab5 100644 --- a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.cc +++ b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.cc @@ -168,3 +168,11 @@ void FakeProfileOAuth2TokenServiceDelegate::UpdateAuthError( it->second->error = error; FireAuthErrorChanged(account_id, error); } + +#if defined(OS_ANDROID) +base::android::ScopedJavaLocalRef<jobject> +FakeProfileOAuth2TokenServiceDelegate::GetJavaObject() { + NOTREACHED(); + return base::android::ScopedJavaLocalRef<jobject>(); +} +#endif diff --git a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.h b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.h index f47aa8ff5f2..d2087528156 100644 --- a/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.h +++ b/chromium/components/signin/internal/identity_manager/fake_profile_oauth2_token_service_delegate.h @@ -9,6 +9,7 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "build/build_config.h" #include "components/signin/internal/identity_manager/profile_oauth2_token_service_delegate.h" #include "google_apis/gaia/google_service_auth_error.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" @@ -73,6 +74,10 @@ class FakeProfileOAuth2TokenServiceDelegate void IssueRefreshTokenForUser(const CoreAccountId& account_id, const std::string& token); +#if defined(OS_ANDROID) + base::android::ScopedJavaLocalRef<jobject> GetJavaObject() override; +#endif + // Maps account ids to info. std::map<CoreAccountId, std::unique_ptr<AccountInfo>> refresh_tokens_; diff --git a/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.cc b/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.cc index bfda70d5b00..29953e42216 100644 --- a/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.cc +++ b/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.cc @@ -32,6 +32,7 @@ #include "net/base/load_flags.h" #include "net/base/net_errors.h" #include "net/cookies/cookie_change_dispatcher.h" +#include "net/cookies/cookie_constants.h" #include "net/http/http_status_code.h" #include "net/traffic_annotation/network_traffic_annotation.h" #include "services/network/public/cpp/resource_request.h" @@ -74,7 +75,7 @@ const net::BackoffEntry::Policy kBackoffPolicy = { // Name of the GAIA cookie that is being observed to detect when available // accounts have changed in the content-area. -const char* const kGaiaCookieName = "APISID"; +const char* const kGaiaCookieName = "SAPISID"; // State of requests to Gaia logout endpoint. Used as entry for histogram // |Signin.GaiaCookieManager.Logout|. @@ -458,7 +459,7 @@ void GaiaCookieManagerService::InitCookieListener() { // testing contexts. if (cookie_manager) { cookie_manager->AddCookieChangeListener( - GaiaUrls::GetInstance()->google_url(), kGaiaCookieName, + GaiaUrls::GetInstance()->secure_google_url(), kGaiaCookieName, cookie_listener_receiver_.BindNewPipeAndPassRemote()); cookie_listener_receiver_.set_disconnect_handler(base::BindOnce( &GaiaCookieManagerService::OnCookieListenerConnectionError, @@ -573,13 +574,16 @@ void GaiaCookieManagerService::TriggerListAccounts() { } void GaiaCookieManagerService::ForceOnCookieChangeProcessing() { - GURL google_url = GaiaUrls::GetInstance()->google_url(); + GURL google_url = GaiaUrls::GetInstance()->secure_google_url(); std::unique_ptr<net::CanonicalCookie> cookie( std::make_unique<net::CanonicalCookie>( kGaiaCookieName, std::string(), "." + google_url.host(), "/", - base::Time(), base::Time(), base::Time(), false, false, - net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_DEFAULT)); - OnCookieChange(*cookie, network::mojom::CookieChangeCause::UNKNOWN_DELETION); + base::Time(), base::Time(), base::Time(), true /* secure */, + false /* httponly */, net::CookieSameSite::NO_RESTRICTION, + net::COOKIE_PRIORITY_DEFAULT)); + OnCookieChange( + net::CookieChangeInfo(*cookie, net::CookieAccessSemantics::UNKNOWN, + net::CookieChangeCause::UNKNOWN_DELETION)); } void GaiaCookieManagerService::LogOutAllAccounts(gaia::GaiaSource source) { @@ -657,13 +661,13 @@ void GaiaCookieManagerService::MarkListAccountsStale() { } void GaiaCookieManagerService::OnCookieChange( - const net::CanonicalCookie& cookie, - network::mojom::CookieChangeCause cause) { - DCHECK_EQ(kGaiaCookieName, cookie.Name()); - DCHECK(cookie.IsDomainMatch(GaiaUrls::GetInstance()->google_url().host())); + const net::CookieChangeInfo& change) { + DCHECK_EQ(kGaiaCookieName, change.cookie.Name()); + DCHECK(change.cookie.IsDomainMatch( + GaiaUrls::GetInstance()->google_url().host())); list_accounts_stale_ = true; - if (cause == network::mojom::CookieChangeCause::EXPLICIT) { + if (change.cause == net::CookieChangeCause::EXPLICIT) { DCHECK(net::CookieChangeCauseIsDeletion(net::CookieChangeCause::EXPLICIT)); if (gaia_cookie_deleted_by_user_action_callback_) { gaia_cookie_deleted_by_user_action_callback_.Run(); diff --git a/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.h b/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.h index 845f1f5e11a..730b18ce2cc 100644 --- a/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.h +++ b/chromium/components/signin/internal/identity_manager/gaia_cookie_manager_service.h @@ -26,6 +26,7 @@ #include "google_apis/gaia/gaia_auth_util.h" #include "mojo/public/cpp/bindings/receiver.h" #include "net/base/backoff_entry.h" +#include "net/cookies/cookie_change_dispatcher.h" #include "services/network/public/mojom/cookie_manager.mojom.h" class GaiaAuthFetcher; @@ -240,7 +241,7 @@ class GaiaCookieManagerService : public GaiaAuthConsumer, void TriggerListAccounts(); // Forces the processing of OnCookieChange. This is public so that callers - // that know the GAIA APISID cookie might have changed can inform the + // that know the GAIA SAPISID cookie might have changed can inform the // service. Virtual for testing. virtual void ForceOnCookieChangeProcessing(); @@ -267,10 +268,10 @@ class GaiaCookieManagerService : public GaiaAuthConsumer, // If set, this callback will be invoked whenever the // GaiaCookieManagerService's list of GAIA accounts is updated. The GCMS - // monitors the APISID cookie and triggers a /ListAccounts call on change. + // monitors the SAPISID cookie and triggers a /ListAccounts call on change. // The GCMS will also call ListAccounts upon the first call to // ListAccounts(). The GCMS will delay calling ListAccounts if other - // requests are in queue that would modify the APISID cookie. + // requests are in queue that would modify the SAPISID cookie. // If the ListAccounts call fails and the GCMS cannot recover, the reason // is passed in |error|. // This method can only be called once. @@ -312,8 +313,7 @@ class GaiaCookieManagerService : public GaiaAuthConsumer, // Overridden from network::mojom::CookieChangeListner. If the cookie relates // to a GAIA APISID cookie, then we call ListAccounts and fire // OnGaiaAccountsInCookieUpdated. - void OnCookieChange(const net::CanonicalCookie& cookie, - network::mojom::CookieChangeCause cause) override; + void OnCookieChange(const net::CookieChangeInfo& change) override; void OnCookieListenerConnectionError(); // Overridden from GaiaAuthConsumer. diff --git a/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.cc b/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.cc index 6b8be38232c..359ad731551 100644 --- a/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.cc +++ b/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.cc @@ -242,7 +242,6 @@ MutableProfileOAuth2TokenServiceDelegate:: scoped_refptr<TokenWebData> token_web_data, signin::AccountConsistencyMethod account_consistency, bool revoke_all_tokens_on_load, - bool can_revoke_credentials, FixRequestErrorCallback fix_request_error_callback) : web_data_service_request_(0), backoff_entry_(&backoff_policy_), @@ -253,7 +252,6 @@ MutableProfileOAuth2TokenServiceDelegate:: token_web_data_(token_web_data), account_consistency_(account_consistency), revoke_all_tokens_on_load_(revoke_all_tokens_on_load), - can_revoke_credentials_(can_revoke_credentials), fix_request_error_callback_(fix_request_error_callback) { VLOG(1) << "MutablePO2TS::MutablePO2TS"; DCHECK(client); @@ -735,8 +733,6 @@ void MutableProfileOAuth2TokenServiceDelegate::PersistCredentials( } void MutableProfileOAuth2TokenServiceDelegate::RevokeAllCredentials() { - if (!can_revoke_credentials_) - return; DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); VLOG(1) << "MutablePO2TS::RevokeAllCredentials"; diff --git a/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h b/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h index 41b71cd2575..67796d20d38 100644 --- a/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h +++ b/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h @@ -5,7 +5,9 @@ #ifndef COMPONENTS_SIGNIN_INTERNAL_IDENTITY_MANAGER_MUTABLE_PROFILE_OAUTH2_TOKEN_SERVICE_DELEGATE_H_ #define COMPONENTS_SIGNIN_INTERNAL_IDENTITY_MANAGER_MUTABLE_PROFILE_OAUTH2_TOKEN_SERVICE_DELEGATE_H_ +#include <map> #include <memory> +#include <string> #include <vector> #include "base/callback.h" @@ -39,7 +41,6 @@ class MutableProfileOAuth2TokenServiceDelegate scoped_refptr<TokenWebData> token_web_data, signin::AccountConsistencyMethod account_consistency, bool revoke_all_tokens_on_load, - bool can_revoke_credentials, FixRequestErrorCallback fix_request_error_callback); ~MutableProfileOAuth2TokenServiceDelegate() override; @@ -229,11 +230,6 @@ class MutableProfileOAuth2TokenServiceDelegate // error state. const bool revoke_all_tokens_on_load_; - // Supervised users cannot revoke credentials. - // TODO(droger): remove this when supervised users are no longer supported on - // any platform. - const bool can_revoke_credentials_; - // Callback function that attempts to correct request errors. Best effort // only. Returns true if the error was fixed and retry should be reattempted. FixRequestErrorCallback fix_request_error_callback_; diff --git a/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate_unittest.cc b/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate_unittest.cc index d0f61c6373b..ce3d3cca6ba 100644 --- a/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate_unittest.cc +++ b/chromium/components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate_unittest.cc @@ -147,7 +147,6 @@ class MutableProfileOAuth2TokenServiceDelegateTest client_.get(), &account_tracker_service_, network::TestNetworkConnectionTracker::GetInstance(), token_web_data_, account_consistency, revoke_all_tokens_on_load_, - true /* can_revoke_credantials */, MutableProfileOAuth2TokenServiceDelegate::FixRequestErrorCallback()); } @@ -1599,11 +1598,12 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, { base::HistogramTester h_tester; token_service.UpdateCredentials("account_id", "refresh_token", - Source::kSupervisedUser_InitSync); - EXPECT_EQ("SupervisedUser::InitSync", source_for_refresh_token_available_); + Source::kInlineLoginHandler_Signin); + EXPECT_EQ("InlineLoginHandler::Signin", + source_for_refresh_token_available_); h_tester.ExpectUniqueSample( "Signin.RefreshTokenUpdated.ToValidToken.Source", - Source::kSupervisedUser_InitSync, 1); + Source::kInlineLoginHandler_Signin, 1); token_service.RevokeCredentials( "account_id", Source::kAccountReconcilor_GaiaCookiesUpdated); diff --git a/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.cc b/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.cc index d5aad257269..5131d95dacf 100644 --- a/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.cc +++ b/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.cc @@ -41,8 +41,10 @@ typedef base::Callback< class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher { public: - AndroidAccessTokenFetcher(OAuth2AccessTokenConsumer* consumer, - const std::string& account_id); + AndroidAccessTokenFetcher( + OAuth2TokenServiceDelegateAndroid* oauth2_token_service_delegate, + OAuth2AccessTokenConsumer* consumer, + const std::string& account_id); ~AndroidAccessTokenFetcher() override; // Overrides from OAuth2AccessTokenFetcher: @@ -59,6 +61,7 @@ class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher { private: std::string CombineScopes(const std::vector<std::string>& scopes); + OAuth2TokenServiceDelegateAndroid* oauth2_token_service_delegate_; std::string account_id_; bool request_was_cancelled_; base::WeakPtrFactory<AndroidAccessTokenFetcher> weak_factory_; @@ -67,9 +70,11 @@ class AndroidAccessTokenFetcher : public OAuth2AccessTokenFetcher { }; AndroidAccessTokenFetcher::AndroidAccessTokenFetcher( + OAuth2TokenServiceDelegateAndroid* oauth2_token_service_delegate, OAuth2AccessTokenConsumer* consumer, const std::string& account_id) : OAuth2AccessTokenFetcher(consumer), + oauth2_token_service_delegate_(oauth2_token_service_delegate), account_id_(account_id), request_was_cancelled_(false), weak_factory_(this) {} @@ -91,7 +96,7 @@ void AndroidAccessTokenFetcher::Start(const std::string& client_id, // Call into Java to get a new token. Java_OAuth2TokenService_getAccessTokenFromNative( - env, j_username, j_scope, + env, oauth2_token_service_delegate_->GetJavaObject(), j_username, j_scope, reinterpret_cast<intptr_t>(heap_callback.release())); } @@ -132,19 +137,24 @@ std::string AndroidAccessTokenFetcher::CombineScopes( } // namespace +// TODO(crbug.com/1009957) Remove disable_interation_with_system_accounts_ +// from OAuth2TokenServiceDelegateAndroid bool OAuth2TokenServiceDelegateAndroid:: disable_interaction_with_system_accounts_ = false; OAuth2TokenServiceDelegateAndroid::OAuth2TokenServiceDelegateAndroid( - AccountTrackerService* account_tracker_service) + AccountTrackerService* account_tracker_service, + const base::android::JavaRef<jobject>& account_manager_facade) : account_tracker_service_(account_tracker_service), fire_refresh_token_loaded_(RT_LOAD_NOT_START) { DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::ctor"; DCHECK(account_tracker_service_); + JNIEnv* env = AttachCurrentThread(); base::android::ScopedJavaLocalRef<jobject> local_java_ref = Java_OAuth2TokenService_create(env, reinterpret_cast<intptr_t>(this), - account_tracker_service_->GetJavaObject()); + account_tracker_service_->GetJavaObject(), + account_manager_facade); java_ref_.Reset(env, local_java_ref.obj()); if (account_tracker_service_->GetMigrationState() == @@ -173,6 +183,10 @@ ScopedJavaLocalRef<jobject> OAuth2TokenServiceDelegateAndroid::GetJavaObject() { bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable( const CoreAccountId& account_id) const { + DCHECK(!disable_interaction_with_system_accounts_) + << __FUNCTION__ + << " needs to interact with system accounts and cannot be used with " + "disable_interaction_with_system_accounts_"; DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable" << " account= " << account_id; std::string account_name = MapAccountIdToAccountName(account_id); @@ -187,7 +201,8 @@ bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable( ScopedJavaLocalRef<jstring> j_account_id = ConvertUTF8ToJavaString(env, account_name); jboolean refresh_token_is_available = - Java_OAuth2TokenService_hasOAuth2RefreshToken(env, j_account_id); + Java_OAuth2TokenService_hasOAuth2RefreshToken(env, java_ref_, + j_account_id); return refresh_token_is_available == JNI_TRUE; } @@ -226,6 +241,7 @@ std::vector<CoreAccountId> OAuth2TokenServiceDelegateAndroid::GetAccounts() JNIEnv* env = AttachCurrentThread(); ScopedJavaLocalRef<jobjectArray> j_accounts = Java_OAuth2TokenService_getAccounts(env); + ; // TODO(fgorski): We may decide to filter out some of the accounts. base::android::AppendJavaStringArrayToStringVector(env, j_accounts, &accounts); @@ -234,10 +250,14 @@ std::vector<CoreAccountId> OAuth2TokenServiceDelegateAndroid::GetAccounts() std::vector<std::string> OAuth2TokenServiceDelegateAndroid::GetSystemAccountNames() { + DCHECK(!disable_interaction_with_system_accounts_) + << __FUNCTION__ + << " needs to interact with system accounts and cannot be used with " + "disable_interaction_with_system_accounts_"; std::vector<std::string> account_names; JNIEnv* env = AttachCurrentThread(); ScopedJavaLocalRef<jobjectArray> j_accounts = - Java_OAuth2TokenService_getSystemAccountNames(env); + Java_OAuth2TokenService_getSystemAccountNames(env, java_ref_); base::android::AppendJavaStringArrayToStringVector(env, j_accounts, &account_names); return account_names; @@ -284,7 +304,8 @@ OAuth2TokenServiceDelegateAndroid::CreateAccessTokenFetcher( std::string account_name = MapAccountIdToAccountName(account_id); DCHECK(!account_name.empty()) << "Cannot find account name for account id " << account_id; - return std::make_unique<AndroidAccessTokenFetcher>(consumer, account_name); + return std::make_unique<AndroidAccessTokenFetcher>(this, consumer, + account_name); } void OAuth2TokenServiceDelegateAndroid::OnAccessTokenInvalidated( @@ -292,11 +313,15 @@ void OAuth2TokenServiceDelegateAndroid::OnAccessTokenInvalidated( const std::string& client_id, const OAuth2AccessTokenManager::ScopeSet& scopes, const std::string& access_token) { + DCHECK(!disable_interaction_with_system_accounts_) + << __FUNCTION__ + << " needs to interact with system accounts and cannot be used with " + "disable_interaction_with_system_accounts_"; ValidateAccountId(account_id); JNIEnv* env = AttachCurrentThread(); ScopedJavaLocalRef<jstring> j_access_token = ConvertUTF8ToJavaString(env, access_token); - Java_OAuth2TokenService_invalidateAccessToken(env, j_access_token); + Java_OAuth2TokenService_invalidateAccessToken(env, java_ref_, j_access_token); } void OAuth2TokenServiceDelegateAndroid::UpdateAccountList( @@ -463,8 +488,9 @@ void OAuth2TokenServiceDelegateAndroid::LoadCredentials( } } -void OAuth2TokenServiceDelegateAndroid::ReloadAccountsFromSystem( - const CoreAccountId& primary_account_id) { +void OAuth2TokenServiceDelegateAndroid:: + ReloadAllAccountsFromSystemWithPrimaryAccount( + const CoreAccountId& primary_account_id) { // UpdateAccountList() effectively synchronizes the accounts in the Token // Service with those present at the system level. UpdateAccountList(primary_account_id, GetValidAccounts(), @@ -487,7 +513,7 @@ CoreAccountId OAuth2TokenServiceDelegateAndroid::MapAccountNameToAccountId( // Called from Java when fetching of an OAuth2 token is finished. The // |authToken| param is only valid when |result| is true. -void JNI_OAuth2TokenService_OAuth2TokenFetched( +void JNI_OAuth2TokenService_OnOAuth2TokenFetched( JNIEnv* env, const JavaParamRef<jstring>& authToken, jboolean isTransientError, diff --git a/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h b/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h index b55902b86cd..5e71e037657 100644 --- a/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h +++ b/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h @@ -31,22 +31,29 @@ class OAuth2TokenServiceDelegateAndroid : public ProfileOAuth2TokenServiceDelegate { public: OAuth2TokenServiceDelegateAndroid( - AccountTrackerService* account_tracker_service); + AccountTrackerService* account_tracker_service, + const base::android::JavaRef<jobject>& account_manager_facade); ~OAuth2TokenServiceDelegateAndroid() override; // Creates a new instance of the OAuth2TokenServiceDelegateAndroid. static OAuth2TokenServiceDelegateAndroid* Create(); // Returns a reference to the corresponding Java OAuth2TokenService object. - base::android::ScopedJavaLocalRef<jobject> GetJavaObject(); + base::android::ScopedJavaLocalRef<jobject> GetJavaObject() override; // Called by the TestingProfile class to disable account validation in - // tests. This prevents the token service from trying to look up system - // accounts which requires special permission. + // tests. This prevents the token service from building the java objects + // which require prior initialization (AccountManagerFacade) + // TODO(crbug.com/1009957) Remove disable_interation_with_system_accounts_ + // from OAuth2TokenServiceDelegateAndroid static void set_disable_interaction_with_system_accounts() { disable_interaction_with_system_accounts_ = true; } + static bool get_disable_interaction_with_system_accounts() { + return disable_interaction_with_system_accounts_; + } + // ProfileOAuth2TokenServiceDelegate overrides: bool RefreshTokenIsAvailable(const CoreAccountId& account_id) const override; GoogleServiceAuthError GetAuthError( @@ -74,7 +81,7 @@ class OAuth2TokenServiceDelegateAndroid void LoadCredentials(const CoreAccountId& primary_account_id) override; - void ReloadAccountsFromSystem( + void ReloadAllAccountsFromSystemWithPrimaryAccount( const CoreAccountId& primary_account_id) override; protected: @@ -132,6 +139,10 @@ class OAuth2TokenServiceDelegateAndroid RefreshTokenLoadStatus fire_refresh_token_loaded_; base::Time last_update_accounts_time_; + // For testing, disables the creation of the java counterpart, see + // set_disable_interaction_with_system_accounts(). + // TODO(crbug.com/1009957) Remove disable_interation_with_system_accounts_ + // from OAuth2TokenServiceDelegateAndroid static bool disable_interaction_with_system_accounts_; DISALLOW_COPY_AND_ASSIGN(OAuth2TokenServiceDelegateAndroid); diff --git a/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android_unittest.cc b/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android_unittest.cc index 6f3272048aa..072a21bfa59 100644 --- a/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android_unittest.cc +++ b/chromium/components/signin/internal/identity_manager/oauth2_token_service_delegate_android_unittest.cc @@ -23,7 +23,7 @@ class OAuth2TokenServiceDelegateAndroidForTest public: OAuth2TokenServiceDelegateAndroidForTest( AccountTrackerService* account_tracker_service) - : OAuth2TokenServiceDelegateAndroid(account_tracker_service) {} + : OAuth2TokenServiceDelegateAndroid(account_tracker_service, nullptr) {} MOCK_METHOD1(SetAccounts, void(const std::vector<CoreAccountId>&)); }; @@ -45,8 +45,6 @@ class OAuth2TokenServiceDelegateAndroidTest : public testing::Test { testing::Test::SetUp(); AccountTrackerService::RegisterPrefs(pref_service_.registry()); account_tracker_service_.Initialize(&pref_service_, base::FilePath()); - OAuth2TokenServiceDelegateAndroid:: - set_disable_interaction_with_system_accounts(); delegate_ = std::make_unique<OAuth2TokenServiceDelegateAndroidForTest>( &account_tracker_service_); delegate_->AddObserver(&observer_); diff --git a/chromium/components/signin/internal/identity_manager/primary_account_manager.cc b/chromium/components/signin/internal/identity_manager/primary_account_manager.cc index 91105525fcc..b96c0018542 100644 --- a/chromium/components/signin/internal/identity_manager/primary_account_manager.cc +++ b/chromium/components/signin/internal/identity_manager/primary_account_manager.cc @@ -21,7 +21,6 @@ #include "components/signin/public/base/signin_metrics.h" #include "components/signin/public/base/signin_pref_names.h" #include "components/signin/public/base/signin_switches.h" -#include "components/signin/public/identity_manager/account_info.h" PrimaryAccountManager::PrimaryAccountManager( SigninClient* client, @@ -54,15 +53,11 @@ void PrimaryAccountManager::RegisterProfilePrefs(PrefRegistrySimple* registry) { registry->RegisterStringPref(prefs::kGoogleServicesLastUsername, std::string()); registry->RegisterStringPref(prefs::kGoogleServicesAccountId, std::string()); - registry->RegisterStringPref(prefs::kGoogleServicesUserAccountId, - std::string()); + registry->RegisterBooleanPref(prefs::kGoogleServicesConsentedToSync, false); registry->RegisterBooleanPref(prefs::kAutologinEnabled, true); registry->RegisterListPref(prefs::kReverseAutologinRejectedEmailList); registry->RegisterBooleanPref(prefs::kSigninAllowed, true); registry->RegisterBooleanPref(prefs::kSignedInWithCredentialProvider, false); - - // Deprecated prefs: will be removed in a future release. - registry->RegisterStringPref(prefs::kGoogleServicesUsername, std::string()); } // static @@ -80,79 +75,50 @@ void PrimaryAccountManager::Initialize(PrefService* local_state) { // clear their login info also (not valid to be logged in without any // tokens). base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess(); - if (cmd_line->HasSwitch(switches::kClearTokenService)) { - client_->GetPrefs()->ClearPref(prefs::kGoogleServicesAccountId); - client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUsername); - client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUserAccountId); - } + if (cmd_line->HasSwitch(switches::kClearTokenService)) + SetPrimaryAccountInternal(CoreAccountInfo(), false); std::string pref_account_id = client_->GetPrefs()->GetString(prefs::kGoogleServicesAccountId); - // Handle backward compatibility: if kGoogleServicesAccountId is empty, but - // kGoogleServicesUsername is not, then this is an old profile that needs to - // be updated. kGoogleServicesUserAccountId should not be empty, and contains - // the gaia_id. Use both properties to prime the account tracker before - // proceeding. - if (pref_account_id.empty()) { - std::string pref_account_username = - client_->GetPrefs()->GetString(prefs::kGoogleServicesUsername); - if (!pref_account_username.empty()) { - // This is an old profile connected to a google account. Migrate from - // kGoogleServicesUsername to kGoogleServicesAccountId. - std::string pref_gaia_id = - client_->GetPrefs()->GetString(prefs::kGoogleServicesUserAccountId); - - // If kGoogleServicesUserAccountId is empty, then this is either a cros - // machine or a really old profile on one of the other platforms. However - // in this case the account tracker should have the gaia_id so fetch it - // from there. - if (pref_gaia_id.empty()) { - AccountInfo info = account_tracker_service_->FindAccountInfoByEmail( - pref_account_username); - pref_gaia_id = info.gaia; - } - - // If |pref_gaia_id| is still empty, this means the profile has been in - // an auth error state for some time (since M39). It could also mean - // a profile that has not been used since M33. Before migration to gaia - // id is complete, the returned value will be the normalized email, which - // is correct. After the migration, the returned value will be empty, - // which means the user is essentially signed out. - // TODO(rogerta): may want to show a toast or something. - pref_account_id = - account_tracker_service_ - ->SeedAccountInfo(pref_gaia_id, pref_account_username) - .id; - - // Set account id before removing obsolete user name in case crash in the - // middle. - client_->GetPrefs()->SetString(prefs::kGoogleServicesAccountId, - pref_account_id); - - // Now remove obsolete preferences. - client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUsername); - } - - // TODO(rogerta): once migration to gaia id is complete, remove - // kGoogleServicesUserAccountId and change all uses of that pref to - // kGoogleServicesAccountId. + // Initial value for the kGoogleServicesConsentedToSync preference if it is + // missing. + const PrefService::Preference* consented_pref = + client_->GetPrefs()->FindPreference( + prefs::kGoogleServicesConsentedToSync); + if (consented_pref->IsDefaultValue()) { + client_->GetPrefs()->SetBoolean(prefs::kGoogleServicesConsentedToSync, + !pref_account_id.empty()); } if (!pref_account_id.empty()) { if (account_tracker_service_->GetMigrationState() == AccountTrackerService::MIGRATION_IN_PROGRESS) { - AccountInfo account_info = + CoreAccountInfo account_info = account_tracker_service_->FindAccountInfoByEmail(pref_account_id); // |account_info.gaia| could be empty if |account_id| is already gaia id. if (!account_info.gaia.empty()) { pref_account_id = account_info.gaia; client_->GetPrefs()->SetString(prefs::kGoogleServicesAccountId, - pref_account_id); + account_info.gaia); } } - SetAuthenticatedAccountId(CoreAccountId(pref_account_id)); } + + bool consented = + client_->GetPrefs()->GetBoolean(prefs::kGoogleServicesConsentedToSync); + CoreAccountInfo account_info = + account_tracker_service_->GetAccountInfo(CoreAccountId(pref_account_id)); + if (consented) { + DCHECK(!account_info.account_id.empty()); + // First reset the state, because SetAuthenticatedAccountInfo can only be + // called if the user is not already signed in. + SetPrimaryAccountInternal(CoreAccountInfo(), /*consented=*/false); + SetAuthenticatedAccountInfo(account_info); + } else { + SetPrimaryAccountInternal(account_info, consented); + } + if (policy_manager_) { policy_manager_->InitializePolicy(local_state, this); } @@ -166,124 +132,136 @@ bool PrimaryAccountManager::IsInitialized() const { return initialized_; } -AccountInfo PrimaryAccountManager::GetAuthenticatedAccountInfo() const { - return account_tracker_service_->GetAccountInfo(GetAuthenticatedAccountId()); +CoreAccountInfo PrimaryAccountManager::GetAuthenticatedAccountInfo() const { + if (!IsAuthenticated()) + return CoreAccountInfo(); + return primary_account_info(); } -const CoreAccountId& PrimaryAccountManager::GetAuthenticatedAccountId() const { - return authenticated_account_id_; +CoreAccountId PrimaryAccountManager::GetAuthenticatedAccountId() const { + return GetAuthenticatedAccountInfo().account_id; } -void PrimaryAccountManager::SetAuthenticatedAccountInfo( - const std::string& gaia_id, - const std::string& email) { - DCHECK(!gaia_id.empty()); - DCHECK(!email.empty()); - - CoreAccountId account_id = - account_tracker_service_->SeedAccountInfo(gaia_id, email); - SetAuthenticatedAccountId(account_id); +CoreAccountInfo PrimaryAccountManager::GetUnconsentedPrimaryAccountInfo() + const { + return primary_account_info(); +} + +bool PrimaryAccountManager::HasUnconsentedPrimaryAccount() const { + return !primary_account_info().account_id.empty(); } -void PrimaryAccountManager::SetAuthenticatedAccountId( - const CoreAccountId& account_id) { - DCHECK(!account_id.empty()); - if (!authenticated_account_id_.empty()) { - DCHECK_EQ(account_id, authenticated_account_id_) - << "Changing the authenticated account while authenticated is not " - "allowed."; +void PrimaryAccountManager::SetUnconsentedPrimaryAccountInfo( + CoreAccountInfo account_info) { + if (IsAuthenticated()) { + DCHECK_EQ(account_info, GetAuthenticatedAccountInfo()); return; } - std::string pref_account_id = - client_->GetPrefs()->GetString(prefs::kGoogleServicesAccountId); + bool account_changed = account_info != primary_account_info(); + SetPrimaryAccountInternal(account_info, /*consented_to_sync=*/false); - DCHECK(pref_account_id.empty() || pref_account_id == account_id.id) - << "account_id=" << account_id << " pref_account_id=" << pref_account_id; - authenticated_account_id_ = account_id; - client_->GetPrefs()->SetString(prefs::kGoogleServicesAccountId, - account_id.id); - - // This preference is set so that code on I/O thread has access to the - // Gaia id of the signed in user. - AccountInfo info = account_tracker_service_->GetAccountInfo(account_id); - - // When this function is called from Initialize(), it's possible for - // |info.gaia| to be empty when migrating from a really old profile. - if (!info.gaia.empty()) { - client_->GetPrefs()->SetString(prefs::kGoogleServicesUserAccountId, - info.gaia); + if (account_changed) { + for (Observer& observer : observers_) + observer.UnconsentedPrimaryAccountChanged(primary_account_info()); } +} + +void PrimaryAccountManager::SetAuthenticatedAccountInfo( + const CoreAccountInfo& account_info) { + DCHECK(!account_info.account_id.empty()); + DCHECK(!IsAuthenticated()); + +#if DCHECK_IS_ON() + { + std::string pref_account_id = + client_->GetPrefs()->GetString(prefs::kGoogleServicesAccountId); + bool consented_to_sync = + client_->GetPrefs()->GetBoolean(prefs::kGoogleServicesConsentedToSync); + + DCHECK(pref_account_id.empty() || !consented_to_sync || + pref_account_id == account_info.account_id.id) + << "account_id=" << account_info.account_id + << " pref_account_id=" << pref_account_id; + } +#endif // DCHECK_IS_ON() + + SetPrimaryAccountInternal(account_info, /*consented_to_sync=*/true); // Go ahead and update the last signed in account info here as well. Once a // user is signed in the corresponding preferences should match. Doing it here // as opposed to on signin allows us to catch the upgrade scenario. client_->GetPrefs()->SetString(prefs::kGoogleServicesLastAccountId, - account_id.id); + account_info.account_id.id); client_->GetPrefs()->SetString(prefs::kGoogleServicesLastUsername, - info.email); + account_info.email); // Commit authenticated account info immediately so that it does not get lost // if Chrome crashes before the next commit interval. client_->GetPrefs()->CommitPendingWrite(); - - if (on_authenticated_account_set_callback_) { - on_authenticated_account_set_callback_.Run(info); - } } -void PrimaryAccountManager::ClearAuthenticatedAccountId() { - authenticated_account_id_ = CoreAccountId(); - if (on_authenticated_account_cleared_callback_) { - on_authenticated_account_cleared_callback_.Run(); +void PrimaryAccountManager::SetPrimaryAccountInternal( + const CoreAccountInfo& account_info, + bool consented_to_sync) { + primary_account_info_ = account_info; + + PrefService* prefs = client_->GetPrefs(); + const std::string& account_id = primary_account_info_.account_id.id; + if (account_id.empty()) { + DCHECK(!consented_to_sync); + prefs->ClearPref(prefs::kGoogleServicesAccountId); + prefs->ClearPref(prefs::kGoogleServicesConsentedToSync); + } else { + prefs->SetString(prefs::kGoogleServicesAccountId, account_id); + prefs->SetBoolean(prefs::kGoogleServicesConsentedToSync, consented_to_sync); } } bool PrimaryAccountManager::IsAuthenticated() const { - return !authenticated_account_id_.empty(); + bool consented_pref = + client_->GetPrefs()->GetBoolean(prefs::kGoogleServicesConsentedToSync); + DCHECK(!consented_pref || !primary_account_info().account_id.empty()); + return consented_pref; } -void PrimaryAccountManager::SetGoogleSigninSucceededCallback( - AccountSigninCallback callback) { - DCHECK(!on_google_signin_succeeded_callback_) - << "GoogleSigninSucceededCallback shouldn't be set multiple times."; - on_google_signin_succeeded_callback_ = callback; -} +void PrimaryAccountManager::SignIn(const std::string& username) { + CoreAccountInfo info = + account_tracker_service_->FindAccountInfoByEmail(username); + DCHECK(!info.gaia.empty()); + DCHECK(!info.email.empty()); + DCHECK(!info.account_id.empty()); + if (IsAuthenticated()) { + DCHECK_EQ(info.account_id, GetAuthenticatedAccountId()) + << "Changing the authenticated account while it is not allowed."; + return; + } -#if !defined(OS_CHROMEOS) -void PrimaryAccountManager::SetGoogleSignedOutCallback( - AccountSigninCallback callback) { - DCHECK(!on_google_signed_out_callback_) - << "GoogleSignedOutCallback shouldn't be set multiple times."; - on_google_signed_out_callback_ = callback; -} -#endif // !defined(OS_CHROMEOS) + bool account_changed = info != primary_account_info(); + SetAuthenticatedAccountInfo(info); -void PrimaryAccountManager::SetAuthenticatedAccountSetCallback( - AccountSigninCallback callback) { - DCHECK(!on_authenticated_account_set_callback_) - << "AuthenticatedAccountSetCallback shouldn't be set multiple times."; - on_authenticated_account_set_callback_ = callback; + for (Observer& observer : observers_) { + if (account_changed) + observer.UnconsentedPrimaryAccountChanged(info); + observer.GoogleSigninSucceeded(info); + } } -void PrimaryAccountManager::SetAuthenticatedAccountClearedCallback( - AccountClearedCallback callback) { - DCHECK(!on_authenticated_account_cleared_callback_) - << "AuthenticatedAccountClearedCallback shouldn't be set multiple times."; - on_authenticated_account_cleared_callback_ = callback; +void PrimaryAccountManager::UpdateAuthenticatedAccountInfo() { + DCHECK(!primary_account_info().account_id.empty()); + DCHECK(IsAuthenticated()); + const CoreAccountInfo info = account_tracker_service_->GetAccountInfo( + primary_account_info().account_id); + DCHECK_EQ(info.account_id, primary_account_info().account_id); + SetPrimaryAccountInternal(info, /*consented_to_sync=*/true); } -void PrimaryAccountManager::SignIn(const std::string& username) { - AccountInfo info = account_tracker_service_->FindAccountInfoByEmail(username); - DCHECK(!info.gaia.empty()); - DCHECK(!info.email.empty()); - - bool reauth_in_progress = IsAuthenticated(); - - SetAuthenticatedAccountInfo(info.gaia, info.email); +void PrimaryAccountManager::AddObserver(Observer* observer) { + observers_.AddObserver(observer); +} - if (!reauth_in_progress && on_google_signin_succeeded_callback_) - on_google_signin_succeeded_callback_.Run(GetAuthenticatedAccountInfo()); +void PrimaryAccountManager::RemoveObserver(Observer* observer) { + observers_.RemoveObserver(observer); } #if !defined(OS_CHROMEOS) @@ -346,13 +324,9 @@ void PrimaryAccountManager::OnSignoutDecisionReached( return; } - AccountInfo account_info = GetAuthenticatedAccountInfo(); - const CoreAccountId account_id = GetAuthenticatedAccountId(); - const std::string username = account_info.email; - ClearAuthenticatedAccountId(); + const CoreAccountInfo account_info = GetAuthenticatedAccountInfo(); client_->GetPrefs()->ClearPref(prefs::kGoogleServicesHostedDomain); - client_->GetPrefs()->ClearPref(prefs::kGoogleServicesAccountId); - client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUserAccountId); + SetPrimaryAccountInternal(CoreAccountInfo(), /*consented_to_sync=*/false); // Revoke all tokens before sending signed_out notification, because there // may be components that don't listen for token service events when the @@ -365,23 +339,20 @@ void PrimaryAccountManager::OnSignoutDecisionReached( kPrimaryAccountManager_ClearAccount); break; case RemoveAccountsOption::kRemoveAuthenticatedAccountIfInError: - if (token_service_->RefreshTokenHasError(account_id)) + if (token_service_->RefreshTokenHasError(account_info.account_id)) token_service_->RevokeCredentials( - account_id, signin_metrics::SourceForRefreshTokenOperation:: - kPrimaryAccountManager_ClearAccount); + account_info.account_id, + signin_metrics::SourceForRefreshTokenOperation:: + kPrimaryAccountManager_ClearAccount); break; case RemoveAccountsOption::kKeepAllAccounts: // Do nothing. break; } - FireGoogleSignedOut(account_info); -} - -void PrimaryAccountManager::FireGoogleSignedOut( - const AccountInfo& account_info) { - if (on_google_signed_out_callback_) { - on_google_signed_out_callback_.Run(account_info); + for (Observer& observer : observers_) { + observer.GoogleSignedOut(account_info); + observer.UnconsentedPrimaryAccountChanged(primary_account_info()); } } @@ -397,8 +368,9 @@ void PrimaryAccountManager::OnRefreshTokensLoaded() { if (token_service_->HasLoadCredentialsFinishedWithNoErrors()) { std::vector<AccountInfo> accounts_in_tracker_service = account_tracker_service_->GetAccounts(); + const CoreAccountId authenticated_account_id = GetAuthenticatedAccountId(); for (const auto& account : accounts_in_tracker_service) { - if (GetAuthenticatedAccountId() != account.account_id && + if (authenticated_account_id != account.account_id && !token_service_->RefreshTokenIsAvailable(account.account_id)) { VLOG(0) << "Removed account from account tracker service: " << account.account_id; diff --git a/chromium/components/signin/internal/identity_manager/primary_account_manager.h b/chromium/components/signin/internal/identity_manager/primary_account_manager.h index 1c6b8915347..6945d780379 100644 --- a/chromium/components/signin/internal/identity_manager/primary_account_manager.h +++ b/chromium/components/signin/internal/identity_manager/primary_account_manager.h @@ -21,14 +21,15 @@ #include <memory> #include <string> -#include "base/callback.h" -#include "base/callback_list.h" #include "base/macros.h" +#include "base/observer_list.h" +#include "base/observer_list_types.h" +#include "base/optional.h" #include "components/signin/internal/identity_manager/profile_oauth2_token_service_observer.h" #include "components/signin/public/base/account_consistency_method.h" #include "components/signin/public/base/signin_client.h" +#include "components/signin/public/identity_manager/account_info.h" -struct AccountInfo; class AccountTrackerService; class PrefRegistrySimple; class PrefService; @@ -42,9 +43,22 @@ enum class SignoutDelete; class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver { public: - typedef base::RepeatingCallback<void(const AccountInfo&)> - AccountSigninCallback; - typedef base::RepeatingCallback<void()> AccountClearedCallback; + class Observer : public base::CheckedObserver { + public: + // Called whenever a user signs into Google services such as sync. + // Not called during a reauth. + virtual void GoogleSigninSucceeded(const CoreAccountInfo& info) {} + + // Called whenever the unconsented primary account changes. This includes + // the changes for the consented primary account as well. + virtual void UnconsentedPrimaryAccountChanged(const CoreAccountInfo& info) { + } + +#if !defined(OS_CHROMEOS) + // Called whenever the currently signed-in user has been signed out. + virtual void GoogleSignedOut(const CoreAccountInfo& info) {} +#endif + }; #if !defined(OS_CHROMEOS) // Used to remove accounts from the token service and the account tracker. @@ -78,51 +92,30 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver { // If a user has previously signed in (and has not signed out), this returns // the know information of the account. Otherwise, it returns an empty struct. - AccountInfo GetAuthenticatedAccountInfo() const; + CoreAccountInfo GetAuthenticatedAccountInfo() const; // If a user has previously signed in (and has not signed out), this returns - // the account id. Otherwise, it returns an empty string. This id is the - // G+/Focus obfuscated gaia id of the user. It can be used to uniquely + // the account id. Otherwise, it returns an empty CoreAccountId. This id is + // the G+/Focus obfuscated gaia id of the user. It can be used to uniquely // identify an account, so for example as a key to map accounts to data. For // code that needs a unique id to represent the connected account, call this // method. Example: the AccountStatusMap type in // MutableProfileOAuth2TokenService. For code that needs to know the // normalized email address of the connected account, use - // GetAuthenticatedAccountInfo().email. Example: to show the string "Signed - // in as XXX" in the hotdog menu. - const CoreAccountId& GetAuthenticatedAccountId() const; - - // Sets the authenticated user's Gaia ID and display email. Internally, - // this will seed the account information in AccountTrackerService and pick - // the right account_id for this account. - void SetAuthenticatedAccountInfo(const std::string& gaia_id, - const std::string& email); + // GetAuthenticatedAccountInfo().email. Example: to show the string + // "Signed in as XXX" in the hotdog menu. + CoreAccountId GetAuthenticatedAccountId() const; // Returns true if there is an authenticated user. bool IsAuthenticated() const; - // If set, this callback will be invoked whenever a user signs into Google - // services such as sync. This callback is not called during a reauth. - void SetGoogleSigninSucceededCallback(AccountSigninCallback callback); - -#if !defined(OS_CHROMEOS) - // If set, this callback will be invoked whenever the currently signed-in user - // for a user has been signed out. - void SetGoogleSignedOutCallback(AccountSigninCallback callback); -#endif - - // If set, this callback will be invoked during the signin as soon as - // PrimaryAccountManager::authenticated_account_id_ is set. - void SetAuthenticatedAccountSetCallback(AccountSigninCallback callback); - - // If set, this callback will be invoked during the signout as soon as - // PrimaryAccountManager::authenticated_account_id_ is cleared. - void SetAuthenticatedAccountClearedCallback(AccountClearedCallback callback); - // Signs a user in. PrimaryAccountManager assumes that |username| can be used // to look up the corresponding account_id and gaia_id for this email. void SignIn(const std::string& username); + // Updates the authenticated account information from AccountTrackerService. + void UpdateAuthenticatedAccountInfo(); + // Signout API surfaces (not supported on ChromeOS, where signout is not // permitted). #if !defined(OS_CHROMEOS) @@ -151,21 +144,36 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver { signin_metrics::SignoutDelete signout_delete_metric); #endif + // Adds and removes observers. + void AddObserver(Observer* observer); + void RemoveObserver(Observer* observer); + + // Provides access to the core information of the user's unconsented primary + // account. Returns an empty info, if there is no such account. + CoreAccountInfo GetUnconsentedPrimaryAccountInfo() const; + + // Returns whether the user's unconsented primary account is available. + bool HasUnconsentedPrimaryAccount() const; + + // Sets the unconsented primary account. The unconsented primary account can + // only be changed if the user is not authenticated. If the user is + // authenticated, use Signout() instead. + void SetUnconsentedPrimaryAccountInfo(CoreAccountInfo account_info); + private: - // Sets the authenticated user's account id. + // Sets the authenticated user's account id, when the user has consented to + // sync. // If the user is already authenticated with the same account id, then this // method is a no-op. // It is forbidden to call this method if the user is already authenticated // with a different account (this method will DCHECK in that case). // |account_id| must not be empty. To log the user out, use // ClearAuthenticatedAccountId() instead. - void SetAuthenticatedAccountId(const CoreAccountId& account_id); + void SetAuthenticatedAccountInfo(const CoreAccountInfo& account_info); - // Clears the authenticated user's account id. - // This method is not public because PrimaryAccountManager does not allow - // signing out by default. Subclasses implementing a sign-out functionality - // need to call this. - void ClearAuthenticatedAccountId(); + // Sets |primary_account_info_| and updates the associated preferences. + void SetPrimaryAccountInternal(const CoreAccountInfo& account_info, + bool consented_to_sync); #if !defined(OS_CHROMEOS) // Starts the sign out process. @@ -180,42 +188,35 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver { RemoveAccountsOption remove_option, SigninClient::SignoutDecision signout_decision); - // Send all observers |GoogleSignedOut| notifications. - void FireGoogleSignedOut(const AccountInfo& account_info); - // ProfileOAuth2TokenServiceObserver: void OnRefreshTokensLoaded() override; #endif + const CoreAccountInfo& primary_account_info() const { + return primary_account_info_; + } + SigninClient* client_; // The ProfileOAuth2TokenService instance associated with this object. Must // outlive this object. - ProfileOAuth2TokenService* token_service_; - - AccountTrackerService* account_tracker_service_; - - bool initialized_; + ProfileOAuth2TokenService* token_service_ = nullptr; + AccountTrackerService* account_tracker_service_ = nullptr; - // Account id after successful authentication. - CoreAccountId authenticated_account_id_; - - // Callbacks which will be invoked, if set, for signin-related events. - AccountSigninCallback on_google_signin_succeeded_callback_; -#if !defined(OS_CHROMEOS) - AccountSigninCallback on_google_signed_out_callback_; -#endif - AccountSigninCallback on_authenticated_account_set_callback_; - AccountClearedCallback on_authenticated_account_cleared_callback_; + bool initialized_ = false; - // The list of callbacks notified on shutdown. - base::CallbackList<void()> on_shutdown_callback_list_; + // Account id after successful authentication. The account may or may not be + // consented to Sync. + // Must be kept in sync with prefs. Use SetPrimaryAccountInternal() to change + // this field. + CoreAccountInfo primary_account_info_; #if !defined(OS_CHROMEOS) signin::AccountConsistencyMethod account_consistency_; #endif std::unique_ptr<PrimaryAccountPolicyManager> policy_manager_; + base::ObserverList<Observer> observers_; DISALLOW_COPY_AND_ASSIGN(PrimaryAccountManager); }; diff --git a/chromium/components/signin/internal/identity_manager/primary_account_manager_unittest.cc b/chromium/components/signin/internal/identity_manager/primary_account_manager_unittest.cc index 55deafe0061..b6b3a4ddb5a 100644 --- a/chromium/components/signin/internal/identity_manager/primary_account_manager_unittest.cc +++ b/chromium/components/signin/internal/identity_manager/primary_account_manager_unittest.cc @@ -31,7 +31,8 @@ #include "components/signin/internal/identity_manager/primary_account_policy_manager_impl.h" #endif -class PrimaryAccountManagerTest : public testing::Test { +class PrimaryAccountManagerTest : public testing::Test, + public PrimaryAccountManager::Observer { public: PrimaryAccountManagerTest() : test_signin_client_(&user_prefs_), @@ -40,7 +41,7 @@ class PrimaryAccountManagerTest : public testing::Test { std::make_unique<FakeProfileOAuth2TokenServiceDelegate>()), account_consistency_(signin::AccountConsistencyMethod::kDisabled), num_successful_signins_(0), - num_signouts_(0) { + num_unconsented_account_changed_(0) { AccountFetcherService::RegisterPrefs(user_prefs_.registry()); AccountTrackerService::RegisterPrefs(user_prefs_.registry()); ProfileOAuth2TokenService::RegisterProfilePrefs(user_prefs_.registry()); @@ -94,21 +95,13 @@ class PrimaryAccountManagerTest : public testing::Test { &test_signin_client_, &token_service_, &account_tracker_, account_consistency_, std::move(policy_manager)); manager_->Initialize(&local_state_); - - // PrimaryAccountManagerTest will outlive the PrimaryAccountManager, so - // base::Unretained is safe. - manager_->SetGoogleSigninSucceededCallback( - base::BindRepeating(&PrimaryAccountManagerTest::GoogleSigninSucceeded, - base::Unretained(this))); -#if !defined(OS_CHROMEOS) - manager_->SetGoogleSignedOutCallback(base::BindRepeating( - &PrimaryAccountManagerTest::GoogleSignedOut, base::Unretained(this))); -#endif + manager_->AddObserver(this); } // Shuts down |manager_|. void ShutDownManager() { DCHECK(manager_); + manager_->RemoveObserver(this); manager_.reset(); } @@ -124,11 +117,13 @@ class PrimaryAccountManagerTest : public testing::Test { EXPECT_EQ(1, num_successful_signins_); } - void GoogleSigninSucceeded(const AccountInfo& account_info) { + void GoogleSigninSucceeded(const CoreAccountInfo& account_info) override { num_successful_signins_++; } - void GoogleSignedOut(const AccountInfo& account_info) { num_signouts_++; } + void UnconsentedPrimaryAccountChanged(const CoreAccountInfo& info) override { + num_unconsented_account_changed_++; + } base::test::TaskEnvironment task_environment_; sync_preferences::TestingPrefServiceSyncable user_prefs_; @@ -145,7 +140,7 @@ class PrimaryAccountManagerTest : public testing::Test { std::vector<std::string> cookies_; signin::AccountConsistencyMethod account_consistency_; int num_successful_signins_; - int num_signouts_; + int num_unconsented_account_changed_; }; #if !defined(OS_CHROMEOS) @@ -165,6 +160,7 @@ TEST_F(PrimaryAccountManagerTest, SignOut) { EXPECT_FALSE(manager_->IsAuthenticated()); EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty()); EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty()); + EXPECT_EQ(CoreAccountInfo(), manager_->GetUnconsentedPrimaryAccountInfo()); } TEST_F(PrimaryAccountManagerTest, SignOutRevoke) { @@ -208,6 +204,8 @@ TEST_F(PrimaryAccountManagerTest, SignOutDiceNoRevoke) { std::vector<CoreAccountId> expected_tokens = {main_account_id, other_account_id}; EXPECT_EQ(expected_tokens, token_service_.GetAccounts()); + EXPECT_EQ(manager_->GetUnconsentedPrimaryAccountInfo(), + manager_->GetAuthenticatedAccountInfo()); } TEST_F(PrimaryAccountManagerTest, SignOutDiceWithError) { @@ -246,7 +244,8 @@ TEST_F(PrimaryAccountManagerTest, SignOutWhileProhibited) { EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty()); EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty()); - manager_->SetAuthenticatedAccountInfo("gaia_id", "user@gmail.com"); + AddToAccountTracker("gaia_id", "user@gmail.com"); + manager_->SignIn("user@gmail.com"); signin_client()->set_is_signout_allowed(false); manager_->SignOut(signin_metrics::SIGNOUT_TEST, signin_metrics::SignoutDelete::IGNORE_METRIC); @@ -282,17 +281,21 @@ TEST_F(PrimaryAccountManagerTest, ProhibitedAfterStartup) { } #endif -TEST_F(PrimaryAccountManagerTest, ExternalSignIn) { +TEST_F(PrimaryAccountManagerTest, SignIn) { CreatePrimaryAccountManager(); EXPECT_EQ("", manager_->GetAuthenticatedAccountInfo().email); EXPECT_EQ("", manager_->GetAuthenticatedAccountId()); EXPECT_EQ(0, num_successful_signins_); + EXPECT_EQ(0, num_unconsented_account_changed_); std::string account_id = AddToAccountTracker("gaia_id", "user@gmail.com"); manager_->SignIn("user@gmail.com"); EXPECT_EQ(1, num_successful_signins_); + EXPECT_EQ(1, num_unconsented_account_changed_); EXPECT_EQ("user@gmail.com", manager_->GetAuthenticatedAccountInfo().email); EXPECT_EQ(account_id, manager_->GetAuthenticatedAccountId()); + EXPECT_EQ(manager_->GetUnconsentedPrimaryAccountInfo(), + manager_->GetAuthenticatedAccountInfo()); } TEST_F(PrimaryAccountManagerTest, @@ -301,15 +304,18 @@ TEST_F(PrimaryAccountManagerTest, EXPECT_EQ("", manager_->GetAuthenticatedAccountInfo().email); EXPECT_EQ("", manager_->GetAuthenticatedAccountId()); EXPECT_EQ(0, num_successful_signins_); + EXPECT_EQ(0, num_unconsented_account_changed_); std::string account_id = AddToAccountTracker("gaia_id", "user@gmail.com"); manager_->SignIn("user@gmail.com"); EXPECT_EQ(1, num_successful_signins_); + EXPECT_EQ(1, num_unconsented_account_changed_); EXPECT_EQ("user@gmail.com", manager_->GetAuthenticatedAccountInfo().email); EXPECT_EQ(account_id, manager_->GetAuthenticatedAccountId()); manager_->SignIn("user@gmail.com"); EXPECT_EQ(1, num_successful_signins_); + EXPECT_EQ(1, num_unconsented_account_changed_); EXPECT_EQ("user@gmail.com", manager_->GetAuthenticatedAccountInfo().email); EXPECT_EQ(account_id, manager_->GetAuthenticatedAccountId()); } @@ -327,58 +333,6 @@ TEST_F(PrimaryAccountManagerTest, SigninNotAllowed) { } #endif -TEST_F(PrimaryAccountManagerTest, UpgradeToNewPrefs) { - user_prefs_.SetString(prefs::kGoogleServicesUsername, "user@gmail.com"); - user_prefs_.SetString(prefs::kGoogleServicesUserAccountId, "account_id"); - CreatePrimaryAccountManager(); - EXPECT_EQ("user@gmail.com", manager_->GetAuthenticatedAccountInfo().email); - - if (account_tracker()->GetMigrationState() == - AccountTrackerService::MIGRATION_NOT_STARTED) { - // TODO(rogerta): until the migration to gaia id, the account id will remain - // the old username. - EXPECT_EQ("user@gmail.com", manager_->GetAuthenticatedAccountId()); - EXPECT_EQ("user@gmail.com", - user_prefs_.GetString(prefs::kGoogleServicesAccountId)); - } else { - EXPECT_EQ("account_id", manager_->GetAuthenticatedAccountId()); - EXPECT_EQ("account_id", - user_prefs_.GetString(prefs::kGoogleServicesAccountId)); - } - EXPECT_EQ("", user_prefs_.GetString(prefs::kGoogleServicesUsername)); - - // Make sure account tracker was updated. - AccountInfo info = - account_tracker()->GetAccountInfo(manager_->GetAuthenticatedAccountId()); - EXPECT_EQ("user@gmail.com", info.email); - EXPECT_EQ("account_id", info.gaia); -} - -TEST_F(PrimaryAccountManagerTest, CanonicalizesPrefs) { - // This unit test is not needed after migrating to gaia id. - if (account_tracker()->GetMigrationState() == - AccountTrackerService::MIGRATION_NOT_STARTED) { - user_prefs_.SetString(prefs::kGoogleServicesUsername, "user.C@gmail.com"); - - CreatePrimaryAccountManager(); - EXPECT_EQ("user.C@gmail.com", - manager_->GetAuthenticatedAccountInfo().email); - - // TODO(rogerta): until the migration to gaia id, the account id will remain - // the old username. - EXPECT_EQ("userc@gmail.com", manager_->GetAuthenticatedAccountId()); - EXPECT_EQ("userc@gmail.com", - user_prefs_.GetString(prefs::kGoogleServicesAccountId)); - EXPECT_EQ("", user_prefs_.GetString(prefs::kGoogleServicesUsername)); - - // Make sure account tracker has a canonicalized username. - AccountInfo info = account_tracker()->GetAccountInfo( - manager_->GetAuthenticatedAccountId()); - EXPECT_EQ("user.C@gmail.com", info.email); - EXPECT_EQ("userc@gmail.com", info.account_id); - } -} - TEST_F(PrimaryAccountManagerTest, GaiaIdMigration) { if (account_tracker()->GetMigrationState() != AccountTrackerService::MIGRATION_NOT_STARTED) { @@ -408,7 +362,7 @@ TEST_F(PrimaryAccountManagerTest, GaiaIdMigration) { } } -TEST_F(PrimaryAccountManagerTest, VeryOldProfileGaiaIdMigration) { +TEST_F(PrimaryAccountManagerTest, GaiaIdMigrationCrashInTheMiddle) { if (account_tracker()->GetMigrationState() != AccountTrackerService::MIGRATION_NOT_STARTED) { std::string email = "user@gmail.com"; @@ -428,43 +382,97 @@ TEST_F(PrimaryAccountManagerTest, VeryOldProfileGaiaIdMigration) { account_tracker()->Shutdown(); account_tracker()->Initialize(prefs(), base::FilePath()); - client_prefs->ClearPref(prefs::kGoogleServicesAccountId); - client_prefs->SetString(prefs::kGoogleServicesUsername, email); + client_prefs->SetString(prefs::kGoogleServicesAccountId, gaia_id); CreatePrimaryAccountManager(); EXPECT_EQ(gaia_id, manager_->GetAuthenticatedAccountId()); EXPECT_EQ(gaia_id, user_prefs_.GetString(prefs::kGoogleServicesAccountId)); + + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(AccountTrackerService::MIGRATION_DONE, + account_tracker()->GetMigrationState()); } } -TEST_F(PrimaryAccountManagerTest, GaiaIdMigrationCrashInTheMiddle) { - if (account_tracker()->GetMigrationState() != - AccountTrackerService::MIGRATION_NOT_STARTED) { - std::string email = "user@gmail.com"; - std::string gaia_id = "account_gaia_id"; +TEST_F(PrimaryAccountManagerTest, RestoreFromPrefsConsented) { + CoreAccountId account_id = AddToAccountTracker("gaia_id", "user@gmail.com"); + user_prefs_.SetString(prefs::kGoogleServicesAccountId, account_id); + user_prefs_.SetBoolean(prefs::kGoogleServicesConsentedToSync, true); + CreatePrimaryAccountManager(); + EXPECT_EQ("user@gmail.com", manager_->GetAuthenticatedAccountInfo().email); + EXPECT_EQ(account_id.id, manager_->GetAuthenticatedAccountId()); + EXPECT_EQ(manager_->GetUnconsentedPrimaryAccountInfo(), + manager_->GetAuthenticatedAccountInfo()); +} - PrefService* client_prefs = signin_client()->GetPrefs(); - client_prefs->SetInteger(prefs::kAccountIdMigrationState, - AccountTrackerService::MIGRATION_NOT_STARTED); - ListPrefUpdate update(client_prefs, prefs::kAccountInfo); - update->Clear(); - auto dict = std::make_unique<base::DictionaryValue>(); - dict->SetString("account_id", email); - dict->SetString("email", email); - dict->SetString("gaia", gaia_id); - update->Append(std::move(dict)); +TEST_F(PrimaryAccountManagerTest, RestoreFromPrefsUnconsented) { + CoreAccountId account_id = AddToAccountTracker("gaia_id", "user@gmail.com"); + user_prefs_.SetString(prefs::kGoogleServicesAccountId, account_id); + user_prefs_.SetBoolean(prefs::kGoogleServicesConsentedToSync, false); + CreatePrimaryAccountManager(); + EXPECT_EQ("user@gmail.com", + manager_->GetUnconsentedPrimaryAccountInfo().email); + EXPECT_EQ(account_id.id, + manager_->GetUnconsentedPrimaryAccountInfo().account_id); + EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().IsEmpty()); +} - account_tracker()->Shutdown(); - account_tracker()->Initialize(prefs(), base::FilePath()); +// If kGoogleServicesConsentedToSync is missing, the account is fully +// authenticated. +TEST_F(PrimaryAccountManagerTest, RestoreFromPrefsMissingConsentPref) { + CoreAccountId account_id = AddToAccountTracker("gaia_id", "user@gmail.com"); + user_prefs_.SetString(prefs::kGoogleServicesAccountId, account_id); - client_prefs->SetString(prefs::kGoogleServicesAccountId, gaia_id); + const PrefService::Preference* consented_pref = + user_prefs_.FindPreference(prefs::kGoogleServicesConsentedToSync); + ASSERT_TRUE(consented_pref); // Pref is registered. + ASSERT_TRUE(consented_pref->IsDefaultValue()); // Pref is not set. - CreatePrimaryAccountManager(); - EXPECT_EQ(gaia_id, manager_->GetAuthenticatedAccountId()); - EXPECT_EQ(gaia_id, user_prefs_.GetString(prefs::kGoogleServicesAccountId)); + CreatePrimaryAccountManager(); + EXPECT_TRUE(user_prefs_.GetBoolean(prefs::kGoogleServicesConsentedToSync)); + EXPECT_EQ("user@gmail.com", manager_->GetAuthenticatedAccountInfo().email); + EXPECT_EQ(account_id.id, manager_->GetAuthenticatedAccountId()); + EXPECT_EQ(manager_->GetUnconsentedPrimaryAccountInfo(), + manager_->GetAuthenticatedAccountInfo()); +} - base::RunLoop().RunUntilIdle(); - EXPECT_EQ(AccountTrackerService::MIGRATION_DONE, - account_tracker()->GetMigrationState()); - } +TEST_F(PrimaryAccountManagerTest, SetUnconsentedPrimaryAccountInfo) { + CreatePrimaryAccountManager(); + EXPECT_EQ(CoreAccountInfo(), manager_->GetUnconsentedPrimaryAccountInfo()); + EXPECT_EQ(0, num_unconsented_account_changed_); + EXPECT_EQ(0, num_successful_signins_); + + // Set the unconsented primary account. + CoreAccountInfo account_info; + account_info.account_id = CoreAccountId("gaia_id"); + account_info.gaia = "gaia_id"; + account_info.email = "user@gmail.com"; + manager_->SetUnconsentedPrimaryAccountInfo(account_info); + EXPECT_EQ(0, num_successful_signins_); + EXPECT_EQ(1, num_unconsented_account_changed_); + EXPECT_EQ(account_info, manager_->GetUnconsentedPrimaryAccountInfo()); + EXPECT_EQ(CoreAccountInfo(), manager_->GetAuthenticatedAccountInfo()); + + // Set the same account again. + manager_->SetUnconsentedPrimaryAccountInfo(account_info); + EXPECT_EQ(0, num_successful_signins_); + EXPECT_EQ(1, num_unconsented_account_changed_); + EXPECT_EQ(account_info, manager_->GetUnconsentedPrimaryAccountInfo()); + EXPECT_EQ(CoreAccountInfo(), manager_->GetAuthenticatedAccountInfo()); + + // Change the email to another equivalent email. The account is updated but + // observers are not notified. + account_info.email = "us.er@gmail.com"; + manager_->SetUnconsentedPrimaryAccountInfo(account_info); + EXPECT_EQ(0, num_successful_signins_); + EXPECT_EQ(1, num_unconsented_account_changed_); + EXPECT_EQ(account_info, manager_->GetUnconsentedPrimaryAccountInfo()); + EXPECT_EQ(CoreAccountInfo(), manager_->GetAuthenticatedAccountInfo()); + + // Clear it. + manager_->SetUnconsentedPrimaryAccountInfo(CoreAccountInfo()); + EXPECT_EQ(0, num_successful_signins_); + EXPECT_EQ(2, num_unconsented_account_changed_); + EXPECT_EQ(CoreAccountInfo(), manager_->GetUnconsentedPrimaryAccountInfo()); + EXPECT_EQ(CoreAccountInfo(), manager_->GetAuthenticatedAccountInfo()); } diff --git a/chromium/components/signin/internal/identity_manager/primary_account_policy_manager_impl.cc b/chromium/components/signin/internal/identity_manager/primary_account_policy_manager_impl.cc index 426aa6b4daf..f87e3b90655 100644 --- a/chromium/components/signin/internal/identity_manager/primary_account_policy_manager_impl.cc +++ b/chromium/components/signin/internal/identity_manager/primary_account_policy_manager_impl.cc @@ -39,7 +39,7 @@ void PrimaryAccountPolicyManagerImpl::InitializePolicy( base::Bind(&PrimaryAccountPolicyManagerImpl::OnSigninAllowedPrefChanged, base::Unretained(this), primary_account_manager)); - AccountInfo account_info = + CoreAccountInfo account_info = primary_account_manager->GetAuthenticatedAccountInfo(); if (!account_info.account_id.empty() && (!IsAllowedUsername(account_info.email) || !IsSigninAllowed())) { diff --git a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.cc b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.cc index 5308623e4e5..e245f5953a0 100644 --- a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.cc +++ b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.cc @@ -28,8 +28,8 @@ std::string SourceToString(SourceForRefreshTokenOperation source) { return "Unknown"; case SourceForRefreshTokenOperation::kTokenService_LoadCredentials: return "TokenService::LoadCredentials"; - case SourceForRefreshTokenOperation::kSupervisedUser_InitSync: - return "SupervisedUser::InitSync"; + case SourceForRefreshTokenOperation::kDeprecatedSupervisedUser_InitSync: + return "DeprecatedSupervisedUser::InitSync"; case SourceForRefreshTokenOperation::kInlineLoginHandler_Signin: return "InlineLoginHandler::Signin"; case SourceForRefreshTokenOperation::kPrimaryAccountManager_ClearAccount: @@ -355,6 +355,11 @@ void ProfileOAuth2TokenService::OverrideAccessTokenManagerForTesting( token_manager_ = std::move(token_manager); } +bool ProfileOAuth2TokenService::IsFakeProfileOAuth2TokenServiceForTesting() + const { + return false; +} + OAuth2AccessTokenManager* ProfileOAuth2TokenService::GetAccessTokenManager() { return token_manager_.get(); } diff --git a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.h b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.h index 8ddf142a4db..a7f8c3873b7 100644 --- a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.h +++ b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service.h @@ -257,6 +257,8 @@ class ProfileOAuth2TokenService : public OAuth2AccessTokenManager::Delegate, void OverrideAccessTokenManagerForTesting( std::unique_ptr<OAuth2AccessTokenManager> token_manager); + virtual bool IsFakeProfileOAuth2TokenServiceForTesting() const; + protected: OAuth2AccessTokenManager* GetAccessTokenManager(); diff --git a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_builder.cc b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_builder.cc index 8544d3acb71..9613765f84e 100644 --- a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_builder.cc +++ b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_builder.cc @@ -14,6 +14,7 @@ #include "components/signin/public/base/signin_client.h" #if defined(OS_ANDROID) +#include "components/signin/internal/base/account_manager_facade_android.h" #include "components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h" #else #include "components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h" @@ -22,7 +23,6 @@ #if defined(OS_CHROMEOS) #include "chromeos/components/account_manager/account_manager.h" -#include "chromeos/constants/chromeos_features.h" #include "components/signin/internal/identity_manager/profile_oauth2_token_service_delegate_chromeos.h" #include "components/user_manager/user_manager.h" #endif // defined(OS_CHROMEOS) @@ -35,10 +35,17 @@ namespace { #if defined(OS_ANDROID) +// TODO(crbug.com/986435) Provide AccountManagerFacade as a parameter once +// IdentityServicesProvider owns its instance management. std::unique_ptr<OAuth2TokenServiceDelegateAndroid> CreateAndroidOAuthDelegate( AccountTrackerService* account_tracker_service) { + auto account_manager_facade = + OAuth2TokenServiceDelegateAndroid:: + get_disable_interaction_with_system_accounts() + ? nullptr + : AccountManagerFacadeAndroid::GetJavaObject(); return std::make_unique<OAuth2TokenServiceDelegateAndroid>( - account_tracker_service); + account_tracker_service, account_manager_facade); } #elif defined(OS_IOS) std::unique_ptr<ProfileOAuth2TokenServiceIOSDelegate> CreateIOSOAuthDelegate( @@ -49,8 +56,7 @@ std::unique_ptr<ProfileOAuth2TokenServiceIOSDelegate> CreateIOSOAuthDelegate( signin_client, std::move(device_accounts_provider), account_tracker_service); } -#else // !defined(OS_ANDROID) && !defined(OS_IOS) -#if defined(OS_CHROMEOS) +#elif defined(OS_CHROMEOS) std::unique_ptr<signin::ProfileOAuth2TokenServiceDelegateChromeOS> CreateCrOsOAuthDelegate( AccountTrackerService* account_tracker_service, @@ -62,25 +68,7 @@ CreateCrOsOAuthDelegate( account_tracker_service, network_connection_tracker, account_manager, is_regular_profile); } -#endif // defined(OS_CHROMEOS) - -// Supervised users cannot revoke credentials. -bool CanRevokeCredentials() { -#if defined(OS_CHROMEOS) - // UserManager may not exist in unit_tests. - if (user_manager::UserManager::IsInitialized() && - user_manager::UserManager::Get()->IsLoggedInAsSupervisedUser()) { - // Don't allow revoking credentials for Chrome OS supervised users. - // See http://crbug.com/332032 - LOG(ERROR) << "Attempt to revoke supervised user refresh " - << "token detected, ignoring."; - return false; - } -#endif - - return true; -} - +#else std::unique_ptr<MutableProfileOAuth2TokenServiceDelegate> CreateMutableProfileOAuthDelegate( AccountTrackerService* account_tracker_service, @@ -102,7 +90,6 @@ CreateMutableProfileOAuthDelegate( return std::make_unique<MutableProfileOAuth2TokenServiceDelegate>( signin_client, account_tracker_service, network_connection_tracker, token_web_data, account_consistency, revoke_all_tokens_on_load, - CanRevokeCredentials(), #if defined(OS_WIN) reauth_callback #else @@ -139,24 +126,19 @@ CreateOAuth2TokenServiceDelegate( return CreateIOSOAuthDelegate(signin_client, std::move(device_accounts_provider), account_tracker_service); -#else // !defined(OS_ANDROID) && !defined(OS_IOS) -#if defined(OS_CHROMEOS) - if (chromeos::features::IsAccountManagerEnabled()) { - return CreateCrOsOAuthDelegate(account_tracker_service, - network_connection_tracker, account_manager, - is_regular_profile); - } -#endif // defined(OS_CHROMEOS) - // Fall back to |MutableProfileOAuth2TokenServiceDelegate|: - // 1. On all platforms other than Android and Chrome OS. - // 2. On Chrome OS, if Account Manager has not been switched on yet - // (chromeos::features::IsAccountManagerEnabled). +#elif defined(OS_CHROMEOS) + return CreateCrOsOAuthDelegate(account_tracker_service, + network_connection_tracker, account_manager, + is_regular_profile); +#else + // Fall back to |MutableProfileOAuth2TokenServiceDelegate| on all platforms + // other than Android, iOS, and Chrome OS. return CreateMutableProfileOAuthDelegate( account_tracker_service, account_consistency, delete_signin_cookies_on_exit, token_web_data, signin_client, #if defined(OS_WIN) reauth_callback, -#endif +#endif // defined(OS_WIN) network_connection_tracker); #endif // defined(OS_ANDROID) diff --git a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_delegate.h b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_delegate.h index d22b7ea6327..219ec69fa46 100644 --- a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_delegate.h +++ b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_delegate.h @@ -20,6 +20,10 @@ #include "google_apis/gaia/oauth2_access_token_manager.h" #include "net/base/backoff_entry.h" +#if defined(OS_ANDROID) +#include "base/android/jni_android.h" +#endif + namespace network { class SharedURLLoaderFactory; } @@ -133,9 +137,12 @@ class ProfileOAuth2TokenServiceDelegate { #endif #if defined(OS_ANDROID) + // Returns a reference to the corresponding Java object. + virtual base::android::ScopedJavaLocalRef<jobject> GetJavaObject() = 0; + // Triggers platform specific implementation for Android to reload accounts // from system. - virtual void ReloadAccountsFromSystem( + virtual void ReloadAllAccountsFromSystemWithPrimaryAccount( const CoreAccountId& primary_account_id) {} #endif diff --git a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_unittest.cc b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_unittest.cc index 6c04923187a..50dfc691e38 100644 --- a/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_unittest.cc +++ b/chromium/components/signin/internal/identity_manager/profile_oauth2_token_service_unittest.cc @@ -21,9 +21,6 @@ #include "google_apis/gaia/oauth2_access_token_manager.h" #include "google_apis/gaia/oauth2_access_token_manager_test_util.h" #include "net/http/http_status_code.h" -#include "net/url_request/test_url_fetcher_factory.h" -#include "net/url_request/url_fetcher_delegate.h" -#include "net/url_request/url_request_test_util.h" #include "services/network/test/test_utils.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -116,9 +113,10 @@ class ProfileOAuth2TokenServiceTest : public testing::Test { } protected: - base::test::TaskEnvironment task_environment_{ - base::test::TaskEnvironment::MainThreadType::IO}; // net:: stuff needs IO - // message loop. + base::test::SingleThreadTaskEnvironment task_environment_{ + base::test::SingleThreadTaskEnvironment::MainThreadType:: + IO}; // net:: stuff needs IO + // message loop. network::TestURLLoaderFactory* test_url_loader_factory_ = nullptr; FakeProfileOAuth2TokenServiceDelegate* delegate_ptr_ = nullptr; // Not owned. std::unique_ptr<ProfileOAuth2TokenService> oauth2_service_; @@ -422,11 +420,10 @@ TEST_F(ProfileOAuth2TokenServiceTest, base::RunLoop().RunUntilIdle(); network::URLLoaderCompletionStatus ok_status(net::OK); - network::ResourceResponseHead response_head = - network::CreateResourceResponseHead(net::HTTP_OK); + auto response_head = network::CreateURLResponseHead(net::HTTP_OK); EXPECT_TRUE(test_url_loader_factory_->SimulateResponseForPendingRequest( - GaiaUrls::GetInstance()->oauth2_token_url(), ok_status, response_head, - GetValidTokenResponse("second token", 3600), + GaiaUrls::GetInstance()->oauth2_token_url(), ok_status, + std::move(response_head), GetValidTokenResponse("second token", 3600), network::TestURLLoaderFactory::kMostRecentMatch)); EXPECT_EQ(1, consumer2.number_of_successful_tokens_); EXPECT_EQ(0, consumer2.number_of_errors_); @@ -497,11 +494,10 @@ TEST_F(ProfileOAuth2TokenServiceTest, StartRequestForMultiloginMobile) { base::RunLoop().RunUntilIdle(); network::URLLoaderCompletionStatus ok_status(net::OK); - network::ResourceResponseHead response_head = - network::CreateResourceResponseHead(net::HTTP_OK); + auto response_head = network::CreateURLResponseHead(net::HTTP_OK); EXPECT_TRUE(test_url_loader_factory_->SimulateResponseForPendingRequest( - GaiaUrls::GetInstance()->oauth2_token_url(), ok_status, response_head, - GetValidTokenResponse("second token", 3600), + GaiaUrls::GetInstance()->oauth2_token_url(), ok_status, + std::move(response_head), GetValidTokenResponse("second token", 3600), network::TestURLLoaderFactory::kMostRecentMatch)); EXPECT_EQ(1, consumer_.number_of_successful_tokens_); EXPECT_EQ(0, consumer_.number_of_errors_); @@ -733,7 +729,7 @@ TEST_F(ProfileOAuth2TokenServiceTest, FixRequestErrorIfPossible) { max_reties >= 0 && consumer_.number_of_successful_tokens_ != 1; --max_reties) { base::RunLoop().RunUntilIdle(); - base::PlatformThread::Sleep(TimeDelta::FromSeconds(1)); + base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(1)); } EXPECT_EQ(1, consumer_.number_of_successful_tokens_); diff --git a/chromium/components/signin/ios/DEPS b/chromium/components/signin/ios/DEPS deleted file mode 100644 index 228a84fcbbe..00000000000 --- a/chromium/components/signin/ios/DEPS +++ /dev/null @@ -1,5 +0,0 @@ -include_rules = [ - "+ios/web/common", - "+ios/web/public", - "+third_party/ocmock", -] diff --git a/chromium/components/signin/ios/browser/BUILD.gn b/chromium/components/signin/ios/browser/BUILD.gn index 095aa04f21e..dd87e0b6531 100644 --- a/chromium/components/signin/ios/browser/BUILD.gn +++ b/chromium/components/signin/ios/browser/BUILD.gn @@ -7,6 +7,8 @@ source_set("browser") { sources = [ "account_consistency_service.h", "account_consistency_service.mm", + "features.cc", + "features.h", "manage_accounts_delegate.h", "wait_for_network_callback_helper.cc", "wait_for_network_callback_helper.h", diff --git a/chromium/components/signin/ios/browser/DEPS b/chromium/components/signin/ios/browser/DEPS index 1cbae6d899d..4f8bc28b730 100644 --- a/chromium/components/signin/ios/browser/DEPS +++ b/chromium/components/signin/ios/browser/DEPS @@ -1,3 +1,11 @@ +include_rules = [ + "+components/signin/public/base", + "+components/signin/public/identity_manager", + "+ios/web/common", + "+ios/web/public", + "+third_party/ocmock", +] + specific_include_rules = { "account_consistency_service.mm": [ "+components/signin/core/browser/account_reconcilor.h", diff --git a/chromium/components/signin/ios/browser/account_consistency_service.mm b/chromium/components/signin/ios/browser/account_consistency_service.mm index 0f8da0cafce..1febb6d9902 100644 --- a/chromium/components/signin/ios/browser/account_consistency_service.mm +++ b/chromium/components/signin/ios/browser/account_consistency_service.mm @@ -19,6 +19,7 @@ #include "components/signin/core/browser/account_reconcilor.h" #include "components/signin/core/browser/signin_header_helper.h" #include "components/signin/public/base/account_consistency_method.h" +#include "components/signin/public/identity_manager/accounts_cookie_mutator.h" #include "ios/web/common/web_view_creation_util.h" #include "ios/web/public/browser_state.h" #import "ios/web/public/navigation/web_state_policy_decider.h" @@ -370,7 +371,7 @@ void AccountConsistencyService::ApplyCookieRequests() { FinishedApplyingCookieRequest(false); return; } - // Create expiration date of Now+2y to roughly follow the APISID cookie. + // Create expiration date of Now+2y to roughly follow the SAPISID cookie. expiration_date = (base::Time::Now() + base::TimeDelta::FromDays(730)).ToJsTime(); break; @@ -470,10 +471,10 @@ void AccountConsistencyService::OnBrowsingDataRemoved() { base::DictionaryValue dict; prefs_->Set(kDomainsWithCookiePref, dict); - // APISID cookie has been removed, notify the GCMS. + // SAPISID cookie has been removed, notify the GCMS. // TODO(https://crbug.com/930582) : Remove the need to expose this method // or move it to the network::CookieManager. - identity_manager_->ForceTriggerOnCookieChange(); + identity_manager_->GetAccountsCookieMutator()->ForceTriggerOnCookieChange(); } void AccountConsistencyService::OnPrimaryAccountSet( diff --git a/chromium/components/signin/ios/browser/account_consistency_service_unittest.mm b/chromium/components/signin/ios/browser/account_consistency_service_unittest.mm index 420def833f0..3fbf1b3cc39 100644 --- a/chromium/components/signin/ios/browser/account_consistency_service_unittest.mm +++ b/chromium/components/signin/ios/browser/account_consistency_service_unittest.mm @@ -441,8 +441,9 @@ TEST_F(AccountConsistencyServiceTest, DomainsClearedOnBrowsingDataRemoved) { base::RunLoop run_loop; identity_test_env_->identity_manager_observer() ->SetOnAccountsInCookieUpdatedCallback(run_loop.QuitClosure()); - // OnBrowsingDataRemoved triggers IdentityManager::ForceTriggerOnCookieChange - // and finally IdentityManager::Observer::OnAccountsInCookieUpdated is called. + // OnBrowsingDataRemoved triggers + // AccountsCookieMutator::ForceTriggerOnCookieChange and finally + // IdentityManager::Observer::OnAccountsInCookieUpdated is called. account_consistency_service_->OnBrowsingDataRemoved(); run_loop.Run(); @@ -468,8 +469,9 @@ TEST_F(AccountConsistencyServiceTest, DomainsClearedOnBrowsingDataRemoved2) { base::RunLoop run_loop; identity_test_env_->identity_manager_observer() ->SetOnAccountsInCookieUpdatedCallback(run_loop.QuitClosure()); - // OnBrowsingDataRemoved triggers IdentityManager::ForceTriggerOnCookieChange - // and finally IdentityManager::Observer::OnAccountsInCookieUpdated is called. + // OnBrowsingDataRemoved triggers + // AccountsCookieMutator::ForceTriggerOnCookieChange and finally + // IdentityManager::Observer::OnAccountsInCookieUpdated is called. account_consistency_service_->OnBrowsingDataRemoved(); run_loop.Run(); EXPECT_TRUE(remove_cookie_callback_called_); diff --git a/chromium/components/signin/ios/browser/features.cc b/chromium/components/signin/ios/browser/features.cc new file mode 100644 index 00000000000..9419512289c --- /dev/null +++ b/chromium/components/signin/ios/browser/features.cc @@ -0,0 +1,16 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/signin/ios/browser/features.h" + +namespace signin { + +const base::Feature kForceStartupSigninPromo{"ForceStartupSigninPromo", + base::FEATURE_DISABLED_BY_DEFAULT}; + +bool ForceStartupSigninPromo() { + return base::FeatureList::IsEnabled(kForceStartupSigninPromo); +} + +} // namespace signin diff --git a/chromium/components/signin/ios/browser/features.h b/chromium/components/signin/ios/browser/features.h new file mode 100644 index 00000000000..5cdf203db84 --- /dev/null +++ b/chromium/components/signin/ios/browser/features.h @@ -0,0 +1,20 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_SIGNIN_IOS_BROWSER_FEATURES_H_ +#define COMPONENTS_SIGNIN_IOS_BROWSER_FEATURES_H_ + +#include "base/feature_list.h" + +namespace signin { + +// Features to trigger the startup sign-in promo at boot. +extern const base::Feature kForceStartupSigninPromo; + +// Returns true if the startup sign-in promo should be displayed at boot. +bool ForceStartupSigninPromo(); + +} // namespace signin + +#endif // COMPONENTS_SIGNIN_IOS_BROWSER_FEATURES_H_ diff --git a/chromium/components/signin/public/DEPS b/chromium/components/signin/public/DEPS new file mode 100644 index 00000000000..2d5ba5e6615 --- /dev/null +++ b/chromium/components/signin/public/DEPS @@ -0,0 +1,5 @@ +include_rules = [ + # Subdirectories of //components/signin/public must explicitly allow deps on + # each other based on the conceptual deps structure. + "-components/signin/public" +] diff --git a/chromium/components/signin/public/base/BUILD.gn b/chromium/components/signin/public/base/BUILD.gn index 688252e9342..b247d5da3cb 100644 --- a/chromium/components/signin/public/base/BUILD.gn +++ b/chromium/components/signin/public/base/BUILD.gn @@ -52,6 +52,14 @@ static_library("base") { } } +if (is_android) { + java_cpp_enum("signin_metrics_enum_javagen") { + sources = [ + "signin_metrics.h", + ] + } +} + static_library("test_support") { testonly = true sources = [ diff --git a/chromium/components/signin/public/base/signin_client.h b/chromium/components/signin/public/base/signin_client.h index 88ecd18dd5d..578d4d5c3ea 100644 --- a/chromium/components/signin/public/base/signin_client.h +++ b/chromium/components/signin/public/base/signin_client.h @@ -56,10 +56,6 @@ class SigninClient : public KeyedService { // Returns the CookieManager for the client. virtual network::mojom::CookieManager* GetCookieManager() = 0; - // Returns a string containing the version info of the product in which the - // Signin component is being used. - virtual std::string GetProductVersion() = 0; - // Called before Google sign-out started. Implementers must run the // |on_signout_decision_reached|, passing a SignoutDecision to allow/disallow // sign-out to continue. When to disallow sign-out is implementation specific. diff --git a/chromium/components/signin/public/base/signin_metrics.cc b/chromium/components/signin/public/base/signin_metrics.cc index 64a04cb7543..14e10526eb2 100644 --- a/chromium/components/signin/public/base/signin_metrics.cc +++ b/chromium/components/signin/public/base/signin_metrics.cc @@ -742,11 +742,6 @@ void LogAuthError(const GoogleServiceAuthError& auth_error) { } } -void LogSigninConfirmHistogramValue(ConfirmationUsage action) { - UMA_HISTOGRAM_ENUMERATION("Signin.OneClickConfirmation", action, - HISTOGRAM_CONFIRM_MAX); -} - void LogAccountReconcilorStateOnGaiaResponse(AccountReconcilorState state) { UMA_HISTOGRAM_ENUMERATION("Signin.AccountReconcilorState.OnGaiaResponse", state, ACCOUNT_RECONCILOR_HISTOGRAM_COUNT); diff --git a/chromium/components/signin/public/base/signin_metrics.h b/chromium/components/signin/public/base/signin_metrics.h index ca12cc21287..cfb512245f8 100644 --- a/chromium/components/signin/public/base/signin_metrics.h +++ b/chromium/components/signin/public/base/signin_metrics.h @@ -26,7 +26,7 @@ enum DifferentPrimaryAccounts { }; // Track all the ways a profile can become signed out as a histogram. -// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.signin +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.signin.metrics // GENERATED_JAVA_CLASS_NAME_OVERRIDE: SignoutReason enum ProfileSignout : int { // The value used within unit tests. @@ -128,7 +128,7 @@ enum Source { // "Signin.SigninStartedAccessPoint" and "Signin.SigninCompletedAccessPoint" // histograms. // A Java counterpart will be generated for this enum. -// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.signin +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.signin.metrics // GENERATED_JAVA_CLASS_NAME_OVERRIDE: SigninAccessPoint enum class AccessPoint : int { ACCESS_POINT_START_PAGE = 0, @@ -183,7 +183,7 @@ enum class PromoAction : int { // Enum values which enumerates all reasons to start sign in process. // A Java counterpart will be generated for this enum. -// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.signin +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.signin.metrics // GENERATED_JAVA_CLASS_NAME_OVERRIDE: SigninReason enum class Reason : int { REASON_SIGNIN_PRIMARY_ACCOUNT = 0, @@ -248,6 +248,7 @@ enum class AccountEquality : int { // When the user is give a choice of deleting their profile or not when signing // out, the |DELETED| or |KEEPING| metric should be used. If the user is not // given any option, then use the |IGNORE_METRIC| value should be used. +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.signin.metrics enum class SignoutDelete : int { DELETED = 0, KEEPING, @@ -294,7 +295,9 @@ enum class AccountRelation : int { enum class SourceForRefreshTokenOperation { kUnknown, kTokenService_LoadCredentials, - kSupervisedUser_InitSync, + // NOTE: This is no longer used but is kept per the comment above about not + // renumbering. + kDeprecatedSupervisedUser_InitSync, kInlineLoginHandler_Signin, kPrimaryAccountManager_ClearAccount, kPrimaryAccountManager_LegacyPreDiceSigninFlow, @@ -375,8 +378,6 @@ void LogExternalCcResultFetches( // Track when the current authentication error changed. void LogAuthError(const GoogleServiceAuthError& auth_error); -void LogSigninConfirmHistogramValue(ConfirmationUsage action); - // Records the AccountReconcilor |state| when GAIA returns a specific response. // If |state| is different than ACCOUNT_RECONCILOR_OK it means the user will // be shown a different set of accounts in the content-area and the settings UI. diff --git a/chromium/components/signin/public/base/signin_pref_names.cc b/chromium/components/signin/public/base/signin_pref_names.cc index 0fe30dad4e8..3d2a442d235 100644 --- a/chromium/components/signin/public/base/signin_pref_names.cc +++ b/chromium/components/signin/public/base/signin_pref_names.cc @@ -41,12 +41,15 @@ const char kGaiaCookieChangedTime[] = "gaia_cookie.changed_time"; // double that should be converted into base::Time. const char kGaiaCookiePeriodicReportTime[] = "gaia_cookie.periodic_report_time"; -// Typically contains an obfuscated gaiaid and will match the value of -// kGoogleServicesUserAccountId. Some platforms and legacy clients may have +// Typically contains an obfuscated gaiaid. Some platforms may have // an email stored in this preference instead. This is transitional and will -// eventually be fixed, allowing the removal of kGoogleServicesUserAccountId. +// eventually be fixed. const char kGoogleServicesAccountId[] = "google.services.account_id"; +// Boolean indicating if the user gave consent for Sync. +const char kGoogleServicesConsentedToSync[] = + "google.services.consented_to_sync"; + // The profile's hosted domain; empty if unset; kNoHostedDomainFound if there // is none. const char kGoogleServicesHostedDomain[] = "google.services.hosted_domain"; @@ -56,12 +59,10 @@ const char kGoogleServicesHostedDomain[] = "google.services.hosted_domain"; const char kGoogleServicesLastAccountId[] = "google.services.last_account_id"; // String the identifies the last user that logged into sync and other -// google services. As opposed to kGoogleServicesUsername, this value is not -// cleared on signout, but while the user is signed in the two values will -// be the same. This pref remains in order to pre-fill the sign in page when -// reconnecting a profile, but programmatic checks to see if a given account -// is the same as the last account should use kGoogleServicesLastAccountId -// instead. +// google services. This value is not cleared on signout. +// This pref remains in order to pre-fill the sign in page when reconnecting a +// profile, but programmatic checks to see if a given account is the same as the +// last account should use kGoogleServicesLastAccountId instead. const char kGoogleServicesLastUsername[] = "google.services.last_username"; // Device id scoped to single signin. This device id will be regenerated if user @@ -70,15 +71,6 @@ const char kGoogleServicesLastUsername[] = "google.services.last_username"; const char kGoogleServicesSigninScopedDeviceId[] = "google.services.signin_scoped_device_id"; -// Obfuscated account ID that identifies the current user logged into sync and -// other google services. -const char kGoogleServicesUserAccountId[] = "google.services.user_account_id"; - -// String that identifies the current user logged into sync and other google -// services. -// DEPRECATED. -const char kGoogleServicesUsername[] = "google.services.username"; - // Local state pref containing a string regex that restricts which accounts // can be used to log in to chrome (e.g. "*@google.com"). If missing or blank, // all accounts are allowed (no restrictions). diff --git a/chromium/components/signin/public/base/signin_pref_names.h b/chromium/components/signin/public/base/signin_pref_names.h index 26f1c460dd9..1db578bdae0 100644 --- a/chromium/components/signin/public/base/signin_pref_names.h +++ b/chromium/components/signin/public/base/signin_pref_names.h @@ -17,12 +17,11 @@ extern const char kGaiaCookieHash[]; extern const char kGaiaCookieChangedTime[]; extern const char kGaiaCookiePeriodicReportTime[]; extern const char kGoogleServicesAccountId[]; +extern const char kGoogleServicesConsentedToSync[]; extern const char kGoogleServicesHostedDomain[]; extern const char kGoogleServicesLastAccountId[]; extern const char kGoogleServicesLastUsername[]; extern const char kGoogleServicesSigninScopedDeviceId[]; -extern const char kGoogleServicesUserAccountId[]; -extern const char kGoogleServicesUsername[]; extern const char kGoogleServicesUsernamePattern[]; extern const char kReverseAutologinRejectedEmailList[]; extern const char kSignedInWithCredentialProvider[]; diff --git a/chromium/components/signin/public/base/test_signin_client.cc b/chromium/components/signin/public/base/test_signin_client.cc index cefecdde86a..d7a5567df22 100644 --- a/chromium/components/signin/public/base/test_signin_client.cc +++ b/chromium/components/signin/public/base/test_signin_client.cc @@ -70,10 +70,6 @@ void TestSigninClient::OverrideTestUrlLoaderFactory( test_url_loader_factory_ = factory; } -std::string TestSigninClient::GetProductVersion() { - return ""; -} - void TestSigninClient::SetNetworkCallsDelayed(bool value) { network_calls_delayed_ = value; diff --git a/chromium/components/signin/public/base/test_signin_client.h b/chromium/components/signin/public/base/test_signin_client.h index 120f28810c8..009adc9e540 100644 --- a/chromium/components/signin/public/base/test_signin_client.h +++ b/chromium/components/signin/public/base/test_signin_client.h @@ -47,9 +47,6 @@ class TestSigninClient : public SigninClient { base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached, signin_metrics::ProfileSignout signout_source_metric) override; - // Returns the empty string. - std::string GetProductVersion() override; - // Wraps the test_url_loader_factory(). scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override; diff --git a/chromium/components/signin/public/identity_manager/BUILD.gn b/chromium/components/signin/public/identity_manager/BUILD.gn index 149b8c49409..537e423ee9e 100644 --- a/chromium/components/signin/public/identity_manager/BUILD.gn +++ b/chromium/components/signin/public/identity_manager/BUILD.gn @@ -2,6 +2,10 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +if (is_android) { + import("//build/config/android/rules.gni") +} + source_set("identity_manager") { sources = [ "access_token_fetcher.cc", @@ -20,6 +24,8 @@ source_set("identity_manager") { "identity_manager.h", "identity_manager_builder.cc", "identity_manager_builder.h", + "identity_mutator.cc", + "identity_mutator.h", "identity_utils.cc", "identity_utils.h", "load_credentials_state.h", @@ -71,6 +77,14 @@ source_set("identity_manager") { ] } +if (is_android) { + java_cpp_enum("identity_manager_enum_javagen") { + sources = [ + "primary_account_mutator.h", + ] + } +} + source_set("unit_tests") { testonly = true diff --git a/chromium/components/signin/public/identity_manager/DEPS b/chromium/components/signin/public/identity_manager/DEPS index eada2e2a7b8..e340bfa7df0 100644 --- a/chromium/components/signin/public/identity_manager/DEPS +++ b/chromium/components/signin/public/identity_manager/DEPS @@ -1,3 +1,4 @@ include_rules = [ "+components/signin/internal", + "+components/signin/public", ] diff --git a/chromium/components/signin/public/identity_manager/README.md b/chromium/components/signin/public/identity_manager/README.md index b1542f2f796..45b45bdc59f 100644 --- a/chromium/components/signin/public/identity_manager/README.md +++ b/chromium/components/signin/public/identity_manager/README.md @@ -115,7 +115,35 @@ and the relevant IdentityManager(::Observer) API surfaces). //components/identity/public/identity_manager and you believe that you have a new use case for one of these API surfaces, you should first contact the OWNERS of //components/signin to discuss this use case and how best to realize - it. + it. With these caveats, here are the details of how various operations are + supported on the various platforms on which Chrome runs: + * Mutating the primary account: Setting the primary account is done via + PrimaryAccountMutator::SetPrimaryAccount(); see the comments on the + declaration of that method for the conditions required for the setting of + the primary account to succeed. On all platforms other than ChromeOS, the + primary account can be cleared via + PrimaryAccountMutator::ClearPrimaryAccount() (on ChromeOS, the primary + account cannot be cleared as there is no user-level flow to sign out of + the browser). + * Updating the state of the accounts in the Gaia cookie: This is supported via + AccountsCookieMutator. + * Mutating the set of accounts with refresh tokens: The ways in which this + occurs are platform-specific due to the differences in underlying platform + account management (or lack thereof). We detail these below: + - Windows/Mac/Linux: Chrome manages the user's OAuth2 refresh tokens + internally. Adding and removing accounts with refresh tokens is done via + AccountsMutator. + - ChromeOS: Chrome is backed by ChromeOS' platform-level AccountManager. + Chrome's view of the accounts with refresh tokens is synchronized with + the platform-level state by observing that platform-level AccountManager + internally. + - Android: Chrome is backed by Android's platform-level AccountManager (in + Java). Chrome's view of the accounts with refresh tokens is synchronized + with the platform-level state via IdentityMutator.java. + - iOS: Chrome is backed by Google's iOS SSO library for supporting shared + identities between Google's various iOS applications. Chrome's view of the + accounts with refresh tokens is synchronized with the platform-level state + via DeviceAccountsSynchronizer. # Mental Mapping from Chromium's Historical API Surfaces for Signin Documentation on the mapping between usage of legacy signin diff --git a/chromium/components/signin/public/identity_manager/account_info.cc b/chromium/components/signin/public/identity_manager/account_info.cc index 9435008860a..0a61b7d74e4 100644 --- a/chromium/components/signin/public/identity_manager/account_info.cc +++ b/chromium/components/signin/public/identity_manager/account_info.cc @@ -7,6 +7,7 @@ #if defined(OS_ANDROID) #include "base/android/jni_string.h" +#include "components/signin/internal/identity_manager/android/jni_headers/CoreAccountId_jni.h" #include "components/signin/internal/identity_manager/android/jni_headers/CoreAccountInfo_jni.h" #endif @@ -127,13 +128,41 @@ std::ostream& operator<<(std::ostream& os, const CoreAccountInfo& account) { #if defined(OS_ANDROID) base::android::ScopedJavaLocalRef<jobject> ConvertToJavaCoreAccountInfo( + JNIEnv* env, const CoreAccountInfo& account_info) { - if (account_info.IsEmpty()) - return base::android::ScopedJavaLocalRef<jobject>(); - JNIEnv* env = base::android::AttachCurrentThread(); return signin::Java_CoreAccountInfo_Constructor( - env, - base::android::ConvertUTF8ToJavaString(env, account_info.account_id.id), - base::android::ConvertUTF8ToJavaString(env, account_info.email)); + env, ConvertToJavaCoreAccountId(env, account_info.account_id), + base::android::ConvertUTF8ToJavaString(env, account_info.email), + base::android::ConvertUTF8ToJavaString(env, account_info.gaia)); +} + +base::android::ScopedJavaLocalRef<jobject> ConvertToJavaCoreAccountId( + JNIEnv* env, + const CoreAccountId& account_id) { + DCHECK(!account_id.empty()); + return signin::Java_CoreAccountId_Constructor( + env, base::android::ConvertUTF8ToJavaString(env, account_id.id)); +} + +CoreAccountInfo ConvertFromJavaCoreAccountInfo( + JNIEnv* env, + const base::android::JavaRef<jobject>& j_core_account_info) { + CoreAccountInfo account; + account.account_id = ConvertFromJavaCoreAccountId( + env, signin::Java_CoreAccountInfo_getId(env, j_core_account_info)); + account.gaia = base::android::ConvertJavaStringToUTF8( + signin::Java_CoreAccountInfo_getGaiaId(env, j_core_account_info)); + account.email = base::android::ConvertJavaStringToUTF8( + signin::Java_CoreAccountInfo_getName(env, j_core_account_info)); + return account; +} + +CoreAccountId ConvertFromJavaCoreAccountId( + JNIEnv* env, + const base::android::JavaRef<jobject>& j_core_account_id) { + CoreAccountId id; + id.id = base::android::ConvertJavaStringToUTF8( + signin::Java_CoreAccountId_getId(env, j_core_account_id)); + return id; } #endif diff --git a/chromium/components/signin/public/identity_manager/account_info.h b/chromium/components/signin/public/identity_manager/account_info.h index c5b0359e032..0c7f93def8e 100644 --- a/chromium/components/signin/public/identity_manager/account_info.h +++ b/chromium/components/signin/public/identity_manager/account_info.h @@ -82,7 +82,23 @@ std::ostream& operator<<(std::ostream& os, const CoreAccountInfo& account); #if defined(OS_ANDROID) // Constructs a Java CoreAccountInfo from the provided C++ CoreAccountInfo base::android::ScopedJavaLocalRef<jobject> ConvertToJavaCoreAccountInfo( + JNIEnv* env, const CoreAccountInfo& account_info); + +// Constructs a Java CoreAccountId from the provided C++ CoreAccountId +base::android::ScopedJavaLocalRef<jobject> ConvertToJavaCoreAccountId( + JNIEnv* env, + const CoreAccountId& account_id); + +// Constructs a C++ CoreAccountInfo from the provided Java CoreAccountInfo +CoreAccountInfo ConvertFromJavaCoreAccountInfo( + JNIEnv* env, + const base::android::JavaRef<jobject>& j_core_account_info); + +// Constructs a C++ CoreAccountId from the provided Java CoreAccountId +CoreAccountId ConvertFromJavaCoreAccountId( + JNIEnv* env, + const base::android::JavaRef<jobject>& j_core_account_id); #endif #endif // COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_ACCOUNT_INFO_H_ diff --git a/chromium/components/signin/public/identity_manager/accounts_cookie_mutator.h b/chromium/components/signin/public/identity_manager/accounts_cookie_mutator.h index af5dd83e6f7..eda6d8727a3 100644 --- a/chromium/components/signin/public/identity_manager/accounts_cookie_mutator.h +++ b/chromium/components/signin/public/identity_manager/accounts_cookie_mutator.h @@ -9,6 +9,7 @@ #include <vector> #include "base/macros.h" +#include "build/build_config.h" #include "google_apis/gaia/gaia_auth_fetcher.h" struct CoreAccountId; @@ -63,6 +64,16 @@ class AccountsCookieMutator { // know that the contents of the Gaia cookie might have changed. virtual void TriggerCookieJarUpdate() = 0; +#if defined(OS_IOS) + // Forces the processing of GaiaCookieManagerService::OnCookieChange. On + // iOS, it's necessary to force-trigger the processing of cookie changes + // from the client as the normal mechanism for internally observing them + // is not wired up. + // TODO(https://crbug.com/930582) : Remove the need to expose this method + // or move it to the network::CookieManager. + virtual void ForceTriggerOnCookieChange() = 0; +#endif + // Remove all accounts from the Gaia cookie. virtual void LogOutAllAccounts(gaia::GaiaSource source) = 0; diff --git a/chromium/components/signin/public/identity_manager/accounts_cookie_mutator_unittest.cc b/chromium/components/signin/public/identity_manager/accounts_cookie_mutator_unittest.cc index 57be5bdc05f..9a1d5f06a71 100644 --- a/chromium/components/signin/public/identity_manager/accounts_cookie_mutator_unittest.cc +++ b/chromium/components/signin/public/identity_manager/accounts_cookie_mutator_unittest.cc @@ -12,6 +12,7 @@ #include "base/test/bind_test_util.h" #include "base/test/gtest_util.h" #include "base/test/task_environment.h" +#include "build/build_config.h" #include "components/signin/public/base/list_accounts_test_utils.h" #include "components/signin/public/base/test_signin_client.h" #include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h" @@ -53,6 +54,7 @@ enum class AccountsCookiesMutatorAction { kSetAccountsInCookie, kTriggerCookieJarUpdateNoAccounts, kTriggerCookieJarUpdateOneAccount, + kTriggerOnCookieChangeNoAccounts, }; } // namespace @@ -119,6 +121,9 @@ class AccountsCookieMutatorTest : public testing::Test { SetListAccountsResponseOneAccount(kTestAccountEmail, kTestAccountGaiaId, GetTestURLLoaderFactory()); break; + case AccountsCookiesMutatorAction::kTriggerOnCookieChangeNoAccounts: + SetListAccountsResponseNoAccounts(GetTestURLLoaderFactory()); + break; } } @@ -401,6 +406,22 @@ TEST_F(AccountsCookieMutatorTest, TriggerCookieJarUpdate_OneListedAccounts) { GoogleServiceAuthError::NONE); } +#if defined(OS_IOS) +TEST_F(AccountsCookieMutatorTest, ForceTriggerOnCookieChange) { + PrepareURLLoaderResponsesForAction( + AccountsCookiesMutatorAction::kTriggerOnCookieChangeNoAccounts); + + base::RunLoop run_loop; + identity_manager_observer()->SetOnAccountsInCookieUpdatedCallback( + run_loop.QuitClosure()); + + // Forces the processing of OnCookieChange and it calls + // OnGaiaAccountsInCookieUpdated. + accounts_cookie_mutator()->ForceTriggerOnCookieChange(); + run_loop.Run(); +} +#endif + // Test that trying to log out all sessions generates the right network request. TEST_F(AccountsCookieMutatorTest, LogOutAllAccounts) { base::RunLoop run_loop; diff --git a/chromium/components/signin/public/identity_manager/accounts_mutator.h b/chromium/components/signin/public/identity_manager/accounts_mutator.h index eb55698778a..b169947c5e9 100644 --- a/chromium/components/signin/public/identity_manager/accounts_mutator.h +++ b/chromium/components/signin/public/identity_manager/accounts_mutator.h @@ -68,11 +68,6 @@ class AccountsMutator { const CoreAccountId& account_id) = 0; #endif - // Updates the refresh token for the supervised user. - // TODO(860492): Remove this once supervised user support is removed. - virtual void LegacySetRefreshTokenForSupervisedUser( - const std::string& refresh_token) = 0; - private: DISALLOW_COPY_AND_ASSIGN(AccountsMutator); }; diff --git a/chromium/components/signin/public/identity_manager/accounts_mutator_unittest.cc b/chromium/components/signin/public/identity_manager/accounts_mutator_unittest.cc index 47769f98beb..fc3b06e46fb 100644 --- a/chromium/components/signin/public/identity_manager/accounts_mutator_unittest.cc +++ b/chromium/components/signin/public/identity_manager/accounts_mutator_unittest.cc @@ -26,7 +26,6 @@ const char kTestEmail[] = "test_user@test.com"; const char kTestEmail2[] = "test_user@test-2.com"; const char kRefreshToken[] = "refresh_token"; const char kRefreshToken2[] = "refresh_token_2"; -const char kSupervisedUserPseudoEmail[] = "managed_user@localhost"; // Class that observes diagnostics updates from signin::IdentityManager. class TestIdentityManagerDiagnosticsObserver @@ -609,38 +608,6 @@ TEST_F(AccountsMutatorTest, MoveAccount) { } #endif // BUILDFLAG(ENABLE_DICE_SUPPORT) -TEST_F(AccountsMutatorTest, LegacySetRefreshTokenForSupervisedUser) { - // Abort the test if the current platform does not support accounts mutation. - if (!accounts_mutator()) - return; - - EXPECT_EQ(identity_manager()->GetAccountsWithRefreshTokens().size(), 0U); - - base::RunLoop run_loop; - identity_manager_observer()->SetOnRefreshTokenUpdatedCallback( - run_loop.QuitClosure()); - - accounts_mutator()->LegacySetRefreshTokenForSupervisedUser(kRefreshToken); - run_loop.Run(); - - // In the context of supervised users, the ProfileOAuth2TokenService is used - // without the AccountTrackerService being used, so we can't use any of the - // IdentityManager::FindExtendedAccountInfoForAccountWithRefreshTokenBy*() - // methods since they won't find any account. Use - // GetAccountsWithRefreshTokens() and HasAccountWithRefreshToken*() instead, - // that only relies in the PO2TS. - std::vector<CoreAccountInfo> accounts = - identity_manager()->GetAccountsWithRefreshTokens(); - EXPECT_EQ(accounts.size(), 1U); - EXPECT_EQ(accounts[0].account_id, kSupervisedUserPseudoEmail); - EXPECT_EQ(accounts[0].email, kSupervisedUserPseudoEmail); - EXPECT_TRUE( - identity_manager()->HasAccountWithRefreshToken(accounts[0].account_id)); - EXPECT_FALSE( - identity_manager()->HasAccountWithRefreshTokenInPersistentErrorState( - accounts[0].account_id)); -} - TEST_F(AccountsMutatorTest, UpdateAccessTokenFromSource) { // Abort the test if the current platform does not support accounts mutation. if (!accounts_mutator()) diff --git a/chromium/components/signin/public/identity_manager/android/BUILD.gn b/chromium/components/signin/public/identity_manager/android/BUILD.gn index 6d96dc4ddf8..3dfa7cf22de 100644 --- a/chromium/components/signin/public/identity_manager/android/BUILD.gn +++ b/chromium/components/signin/public/identity_manager/android/BUILD.gn @@ -6,13 +6,19 @@ android_library("java") { "//base:jni_java", "//components/signin/core/browser/android:java", "//third_party/android_deps:android_support_v4_java", - "//third_party/android_deps:com_android_support_support_annotations_java", + "//third_party/android_deps:androidx_annotation_annotation_java", + ] + + srcjar_deps = [ + "//components/signin/public/base:signin_metrics_enum_javagen", + "//components/signin/public/identity_manager:identity_manager_enum_javagen", ] java_files = [ - "java/src/org/chromium/components/signin/identitymanager/IdentityManager.java", "java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java", "java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java", + "java/src/org/chromium/components/signin/identitymanager/IdentityManager.java", + "java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java", ] annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ] diff --git a/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java index e690e4abca6..bd963cce960 100644 --- a/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java +++ b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java @@ -4,48 +4,46 @@ package org.chromium.components.signin.identitymanager; -import android.support.annotation.NonNull; +import androidx.annotation.NonNull; + +import org.chromium.base.annotations.CalledByNative; /** - * A wrapper around Gaia ID that represents a stable account identifier. - * - * This wrapper helps to make sure that code using accounts doesn't accidentally use account name in - * place of Gaia ID and vice versa. - * + * Represents the id of an account, which can be either a Gaia ID or email depending on the + * migration status within AccountTrackerService. * This class has a native counterpart called CoreAccountId. */ public class CoreAccountId { - private final String mGaiaId; + private final String mId; /** - * Constructs a new CoreAccountId from a String representation of Gaia ID. + * Constructs a new CoreAccountId from a String representation of the account ID. */ - public CoreAccountId(@NonNull String gaiaId) { - assert gaiaId != null; - // Check that a user email is not used by mistake. - assert !gaiaId.contains("@"); - - mGaiaId = gaiaId; + @CalledByNative + public CoreAccountId(@NonNull String id) { + assert id != null; + mId = id; } - public String getGaiaIdAsString() { - return mGaiaId; + @CalledByNative + public String getId() { + return mId; } @Override public String toString() { - return mGaiaId; + return mId; } @Override public int hashCode() { - return mGaiaId.hashCode(); + return mId.hashCode(); } @Override public boolean equals(Object obj) { if (!(obj instanceof CoreAccountId)) return false; CoreAccountId other = (CoreAccountId) obj; - return mGaiaId.equals(other.mGaiaId); + return mId.equals(other.mId); } } diff --git a/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java index cbb28e2f244..d72e0b52aaa 100644 --- a/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java +++ b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java @@ -5,7 +5,8 @@ package org.chromium.components.signin.identitymanager; import android.accounts.Account; -import android.support.annotation.NonNull; + +import androidx.annotation.NonNull; import org.chromium.base.annotations.CalledByNative; import org.chromium.components.signin.AccountManagerFacade; @@ -17,8 +18,6 @@ import org.chromium.components.signin.AccountManagerFacade; * * This class has a native counterpart called CoreAccountInfo. There are several differences between * these two classes: - * - Android class doesn't store Gaia ID as a string because {@link CoreAccountId} already contains - * it. * - Android class additionally exposes {@link android.accounts.Account} object for interactions * with the system. * - Android class has the "account name" whereas the native class has "email". This is the same @@ -27,26 +26,44 @@ import org.chromium.components.signin.AccountManagerFacade; public class CoreAccountInfo { private final CoreAccountId mId; private final Account mAccount; + private final String mGaiaId; - public CoreAccountInfo(@NonNull CoreAccountId id, @NonNull Account account) { + /** + * Constructs a CoreAccountInfo with the provided parameters + * @param id A CoreAccountId associated with the account, equal to either account.name or + * gaiaId. + * @param account Android account. + * @param gaiaId String representation of the Gaia ID. Must not be an email address. + */ + public CoreAccountInfo( + @NonNull CoreAccountId id, @NonNull Account account, @NonNull String gaiaId) { assert id != null; assert account != null; + assert gaiaId != null; + assert !gaiaId.contains("@"); mId = id; mAccount = account; + mGaiaId = gaiaId; } @CalledByNative - private CoreAccountInfo(@NonNull String id, @NonNull String name) { + private CoreAccountInfo( + @NonNull CoreAccountId id, @NonNull String name, @NonNull String gaiaId) { assert id != null; assert name != null; + assert gaiaId != null; + assert !gaiaId.contains("@"); + + mId = id; mAccount = AccountManagerFacade.createAccountFromName(name); - mId = new CoreAccountId(id); + mGaiaId = gaiaId; } /** * Returns a unique identifier of the current account. */ + @CalledByNative public CoreAccountId getId() { return mId; } @@ -54,11 +71,20 @@ public class CoreAccountInfo { /** * Returns a name of the current account. */ + @CalledByNative public String getName() { return mAccount.name; } /** + * Returns the string representation of the Gaia ID + */ + @CalledByNative + public String getGaiaId() { + return mGaiaId; + } + + /** * Returns {@link android.accounts.Account} object holding a name of the current account. */ public Account getAccount() { diff --git a/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java index 29ab16b5284..c9c4e26288a 100644 --- a/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java +++ b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java @@ -4,7 +4,10 @@ package org.chromium.components.signin.identitymanager; +import android.support.annotation.Nullable; + import org.chromium.base.ObserverList; +import org.chromium.base.VisibleForTesting; import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.NativeMethods; @@ -41,12 +44,13 @@ public class IdentityManager { * Called by native to create an instance of IdentityManager. */ @CalledByNative - static private IdentityManager create(long nativeIdentityManager) { + private static IdentityManager create(long nativeIdentityManager) { + assert nativeIdentityManager != 0; return new IdentityManager(nativeIdentityManager); } - private IdentityManager(long nativeIdentityManager) { - assert nativeIdentityManager != 0; + @VisibleForTesting + public IdentityManager(long nativeIdentityManager) { mNativeIdentityManager = nativeIdentityManager; } @@ -86,7 +90,8 @@ public class IdentityManager { * Notifies observers that the primary account was cleared in C++. */ @CalledByNative - private void onPrimaryAccountCleared(CoreAccountInfo account) { + @VisibleForTesting + public void onPrimaryAccountCleared(CoreAccountInfo account) { for (Observer observer : mObservers) { observer.onPrimaryAccountCleared(account); } @@ -95,12 +100,26 @@ public class IdentityManager { /** * Returns whether the user's primary account is available. */ - boolean hasPrimaryAccount() { + public boolean hasPrimaryAccount() { return IdentityManagerJni.get().hasPrimaryAccount(mNativeIdentityManager); } + /** + * Looks up and returns information for account with given |email_address|. If the account + * cannot be found, return a null value. + */ + public @Nullable CoreAccountInfo + findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(String email) { + return IdentityManagerJni.get() + .findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress( + mNativeIdentityManager, email); + } + @NativeMethods interface Natives { public boolean hasPrimaryAccount(long nativeIdentityManager); + public @Nullable CoreAccountInfo + findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress( + long nativeIdentityManager, String email); } } diff --git a/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java new file mode 100644 index 00000000000..311c8a3709d --- /dev/null +++ b/chromium/components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java @@ -0,0 +1,76 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.components.signin.identitymanager; + +import org.chromium.base.annotations.CalledByNative; +import org.chromium.base.annotations.NativeMethods; +import org.chromium.components.signin.metrics.SignoutDelete; +import org.chromium.components.signin.metrics.SignoutReason; + +/** + * IdentityMutator is the write interface of IdentityManager, see identity_mutator.h for more + * information. + */ +public class IdentityMutator { + private static final String TAG = "IdentityMutator"; + + // Pointer to native IdentityMutator, not final because of destroy(). + private long mNativeIdentityMutator; + + @CalledByNative + private IdentityMutator(long nativeIdentityMutator) { + assert nativeIdentityMutator != 0; + mNativeIdentityMutator = nativeIdentityMutator; + } + + /** + * Called by native IdentityManager upon KeyedService's shutdown + */ + @CalledByNative + private void destroy() { + mNativeIdentityMutator = 0; + } + + /** + * Marks the account with |account_id| as the primary account, and returns whether the operation + * succeeded or not. To succeed, this requires that: + * - the account is known by the IdentityManager. + * - setting the primary account is allowed, + * - the account username is allowed by policy, + * - there is not already a primary account set. + */ + public boolean setPrimaryAccount(CoreAccountId accountId) { + return IdentityMutatorJni.get().setPrimaryAccount(mNativeIdentityMutator, accountId); + } + + /** + * Clears the primary account, and returns whether the operation succeeded or not. Depending on + * |action|, the other accounts known to the IdentityManager may be deleted. + */ + public boolean clearPrimaryAccount(@ClearAccountsAction int action, + @SignoutReason int sourceMetric, @SignoutDelete int deleteMetric) { + return IdentityMutatorJni.get().clearPrimaryAccount( + mNativeIdentityMutator, action, sourceMetric, deleteMetric); + } + + /** + * Reloads the accounts in the token service from the system accounts. This API calls + * ProfileOAuth2TokenServiceDelegate::ReloadAllAccountsFromSystemWithPrimaryAccount. + */ + public void reloadAllAccountsFromSystemWithPrimaryAccount(CoreAccountId accountId) { + IdentityMutatorJni.get().reloadAllAccountsFromSystemWithPrimaryAccount( + mNativeIdentityMutator, accountId); + } + + @NativeMethods + interface Natives { + public boolean setPrimaryAccount(long nativeJniIdentityMutator, CoreAccountId accountId); + public boolean clearPrimaryAccount(long nativeJniIdentityMutator, + @ClearAccountsAction int action, @SignoutReason int sourceMetric, + @SignoutDelete int deleteMetric); + public void reloadAllAccountsFromSystemWithPrimaryAccount( + long nativeJniIdentityMutator, CoreAccountId accountId); + } +} diff --git a/chromium/components/signin/public/identity_manager/device_accounts_synchronizer.h b/chromium/components/signin/public/identity_manager/device_accounts_synchronizer.h index bf99d7cb3cd..a5ccc95af27 100644 --- a/chromium/components/signin/public/identity_manager/device_accounts_synchronizer.h +++ b/chromium/components/signin/public/identity_manager/device_accounts_synchronizer.h @@ -5,6 +5,7 @@ #ifndef COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_DEVICE_ACCOUNTS_SYNCHRONIZER_H_ #define COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_DEVICE_ACCOUNTS_SYNCHRONIZER_H_ +#include "build/build_config.h" #include "google_apis/gaia/core_account_id.h" namespace signin { @@ -16,6 +17,15 @@ class DeviceAccountsSynchronizer { DeviceAccountsSynchronizer() = default; virtual ~DeviceAccountsSynchronizer() = default; +#if defined(OS_ANDROID) + // Reloads the information of all device-level accounts. All device-level + // accounts will be visible in IdentityManager::GetAccountsWithRefreshTokens() + // with any persistent errors cleared after this method is called. + virtual void ReloadAllAccountsFromSystemWithPrimaryAccount( + const CoreAccountId& primary_account_id) = 0; +#endif + +#if defined(OS_IOS) // Reloads the information of all device-level accounts. All device-level // accounts will be visible in IdentityManager::GetAccountsWithRefreshTokens() // with any persistent errors cleared after this method is called. @@ -25,6 +35,7 @@ class DeviceAccountsSynchronizer { // account will be visible in IdentityManager::GetAccountsWithRefreshTokens() // with any persistent error cleared after this method is called. virtual void ReloadAccountFromSystem(const CoreAccountId& account_id) = 0; +#endif // Class is non-copyable, non-moveable. DeviceAccountsSynchronizer(const DeviceAccountsSynchronizer&) = delete; diff --git a/chromium/components/signin/public/identity_manager/identity_manager.cc b/chromium/components/signin/public/identity_manager/identity_manager.cc index 79bee2b42b7..a24ed6dca8f 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager.cc +++ b/chromium/components/signin/public/identity_manager/identity_manager.cc @@ -11,8 +11,6 @@ #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" -#include "components/signin/internal/identity_manager/primary_account_manager.h" -#include "components/signin/internal/identity_manager/profile_oauth2_token_service.h" #include "components/signin/internal/identity_manager/ubertoken_fetcher_impl.h" #include "components/signin/public/identity_manager/accounts_cookie_mutator.h" #include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h" @@ -25,30 +23,13 @@ #if defined(OS_ANDROID) #include "base/android/jni_string.h" #include "components/signin/internal/identity_manager/android/jni_headers/IdentityManager_jni.h" -#include "components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h" +#include "components/signin/internal/identity_manager/profile_oauth2_token_service_delegate.h" #elif !defined(OS_IOS) #include "components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h" #endif namespace signin { -namespace { - -// Local copy of the account ID used for supervised users (defined in //chrome -// as supervised_users::kSupervisedUserPseudoEmail). Simply copied to avoid -// plumbing it from //chrome all the way down through the Identity Service just -// to handle the corner cases below. -// TODO(860492): Remove this once supervised user support is removed. -const char kSupervisedUserPseudoEmail[] = "managed_user@localhost"; - -// A made-up Gaia ID to populate the supervised user's AccountInfo with in order -// to maintain the invariant that the AccountInfos passed out by IdentityManager -// always have an account ID, Gaia ID, and email set. -// TODO(860492): Remove this once supervised user support is removed. -const char kSupervisedUserPseudoGaiaID[] = "managed_user_gaia_id"; - -} // namespace - IdentityManager::IdentityManager( std::unique_ptr<AccountTrackerService> account_tracker_service, std::unique_ptr<ProfileOAuth2TokenService> token_service, @@ -65,35 +46,16 @@ IdentityManager::IdentityManager( gaia_cookie_manager_service_(std::move(gaia_cookie_manager_service)), primary_account_manager_(std::move(primary_account_manager)), account_fetcher_service_(std::move(account_fetcher_service)), - primary_account_mutator_(std::move(primary_account_mutator)), - accounts_mutator_(std::move(accounts_mutator)), - accounts_cookie_mutator_(std::move(accounts_cookie_mutator)), - diagnostics_provider_(std::move(diagnostics_provider)), - device_accounts_synchronizer_(std::move(device_accounts_synchronizer)) { + identity_mutator_(std::move(primary_account_mutator), + std::move(accounts_mutator), + std::move(accounts_cookie_mutator), + std::move(device_accounts_synchronizer)), + diagnostics_provider_(std::move(diagnostics_provider)) { DCHECK(account_fetcher_service_); - DCHECK(accounts_cookie_mutator_); DCHECK(diagnostics_provider_); - DCHECK(!accounts_mutator_ || !device_accounts_synchronizer_) - << "Cannot have both an AccountsMutator and a DeviceAccountsSynchronizer"; - - // IdentityManager will outlive the PrimaryAccountManager, so base::Unretained - // is safe. - primary_account_manager_->SetGoogleSigninSucceededCallback( - base::BindRepeating(&IdentityManager::GoogleSigninSucceeded, - base::Unretained(this))); - primary_account_manager_->SetAuthenticatedAccountSetCallback( - base::BindRepeating(&IdentityManager::AuthenticatedAccountSet, - base::Unretained(this))); - primary_account_manager_->SetAuthenticatedAccountClearedCallback( - base::BindRepeating(&IdentityManager::AuthenticatedAccountCleared, - base::Unretained(this))); -#if !defined(OS_CHROMEOS) - primary_account_manager_->SetGoogleSignedOutCallback(base::BindRepeating( - &IdentityManager::GoogleSignedOut, base::Unretained(this))); -#endif - - token_service_->AddObserver(this); + primary_account_manager_observer_.Add(primary_account_manager_.get()); + token_service_observer_.Add(token_service_.get()); token_service_->AddAccessTokenDiagnosticsObserver(this); // IdentityManager owns the ATS, GCMS and PO2TS instances and will outlive @@ -115,15 +77,6 @@ IdentityManager::IdentityManager( base::BindRepeating(&IdentityManager::OnRefreshTokenRevokedFromSource, base::Unretained(this))); - // Seed the primary account with any state that |primary_account_manager_| - // loaded from prefs. - if (primary_account_manager_->IsAuthenticated()) { - CoreAccountInfo account = - primary_account_manager_->GetAuthenticatedAccountInfo(); - DCHECK(!account.account_id.empty()); - SetPrimaryAccountInternal(std::move(account)); - } - #if defined(OS_ANDROID) java_identity_manager_ = Java_IdentityManager_create( base::android::AttachCurrentThread(), reinterpret_cast<intptr_t>(this)); @@ -136,7 +89,6 @@ IdentityManager::~IdentityManager() { token_service_->Shutdown(); account_tracker_service_->Shutdown(); - token_service_->RemoveObserver(this); token_service_->RemoveAccessTokenDiagnosticsObserver(this); #if defined(OS_ANDROID) @@ -156,21 +108,7 @@ void IdentityManager::RemoveObserver(Observer* observer) { // TODO(862619) change return type to base::Optional<CoreAccountInfo> CoreAccountInfo IdentityManager::GetPrimaryAccountInfo() const { - DCHECK_EQ(primary_account_.has_value(), - primary_account_manager_->IsAuthenticated()); - auto result = primary_account_.value_or(CoreAccountInfo()); - DCHECK_EQ(result.account_id, - primary_account_manager_->GetAuthenticatedAccountId()); -#if DCHECK_IS_ON() - CoreAccountInfo primary_account_manager_account = - primary_account_manager_->GetAuthenticatedAccountInfo(); - if (!primary_account_manager_account.account_id.empty()) { - DCHECK_EQ(primary_account_manager_account, result) - << "If primary_account_manager_'s account is set (account has a " - "refresh token), primary_account_ must have the same value."; - } -#endif - return result; + return primary_account_manager_->GetAuthenticatedAccountInfo(); } CoreAccountId IdentityManager::GetPrimaryAccountId() const { @@ -178,9 +116,7 @@ CoreAccountId IdentityManager::GetPrimaryAccountId() const { } bool IdentityManager::HasPrimaryAccount() const { - DCHECK_EQ(primary_account_.has_value(), - primary_account_manager_->IsAuthenticated()); - return primary_account_.has_value(); + return primary_account_manager_->IsAuthenticated(); } CoreAccountId IdentityManager::GetUnconsentedPrimaryAccountId() const { @@ -188,11 +124,11 @@ CoreAccountId IdentityManager::GetUnconsentedPrimaryAccountId() const { } CoreAccountInfo IdentityManager::GetUnconsentedPrimaryAccountInfo() const { - return unconsented_primary_account_.value_or(CoreAccountInfo()); + return primary_account_manager_->GetUnconsentedPrimaryAccountInfo(); } bool IdentityManager::HasUnconsentedPrimaryAccount() const { - return unconsented_primary_account_.has_value(); + return primary_account_manager_->HasUnconsentedPrimaryAccount(); } std::unique_ptr<AccessTokenFetcher> @@ -377,19 +313,19 @@ AccountsInCookieJarInfo IdentityManager::GetAccountsInCookieJar() const { } PrimaryAccountMutator* IdentityManager::GetPrimaryAccountMutator() { - return primary_account_mutator_.get(); + return identity_mutator_.GetPrimaryAccountMutator(); } AccountsMutator* IdentityManager::GetAccountsMutator() { - return accounts_mutator_.get(); + return identity_mutator_.GetAccountsMutator(); } AccountsCookieMutator* IdentityManager::GetAccountsCookieMutator() { - return accounts_cookie_mutator_.get(); + return identity_mutator_.GetAccountsCookieMutator(); } DeviceAccountsSynchronizer* IdentityManager::GetDeviceAccountsSynchronizer() { - return device_accounts_synchronizer_.get(); + return identity_mutator_.GetDeviceAccountsSynchronizer(); } void IdentityManager::AddDiagnosticsObserver(DiagnosticsObserver* observer) { @@ -414,10 +350,7 @@ IdentityManager::GetAccountIdMigrationState() const { CoreAccountId IdentityManager::PickAccountIdForAccount( const std::string& gaia, const std::string& email) const { - // TODO(triploblastic@): Remove explicit conversion once - // primary_account_manager has been fixed to use CoreAccountId. - return CoreAccountId( - account_tracker_service_->PickAccountIdForAccount(gaia, email)); + return account_tracker_service_->PickAccountIdForAccount(gaia, email); } // static @@ -437,29 +370,11 @@ void IdentityManager::RegisterProfilePrefs(PrefRegistrySimple* registry) { #endif } -#if !defined(OS_IOS) && !defined(OS_ANDROID) -void IdentityManager::DeprecatedLoadCredentialsForSupervisedUser( - const CoreAccountId& primary_account_id) { - token_service_->LoadCredentials(primary_account_id); -} -#endif - DiagnosticsProvider* IdentityManager::GetDiagnosticsProvider() { return diagnostics_provider_.get(); } -#if defined(OS_IOS) -void IdentityManager::ForceTriggerOnCookieChange() { - gaia_cookie_manager_service_->ForceOnCookieChangeProcessing(); -} -#endif - #if defined(OS_ANDROID) -void IdentityManager::LegacyReloadAccountsFromSystem() { - token_service_->GetDelegate()->ReloadAccountsFromSystem( - GetPrimaryAccountId()); -} - base::android::ScopedJavaLocalRef<jobject> IdentityManager::LegacyGetAccountTrackerServiceJavaObject() { return account_tracker_service_->GetJavaObject(); @@ -467,10 +382,7 @@ IdentityManager::LegacyGetAccountTrackerServiceJavaObject() { base::android::ScopedJavaLocalRef<jobject> IdentityManager::LegacyGetOAuth2TokenServiceJavaObject() { - OAuth2TokenServiceDelegateAndroid* delegate = - static_cast<OAuth2TokenServiceDelegateAndroid*>( - token_service_->GetDelegate()); - return delegate->GetJavaObject(); + return token_service_->GetDelegate()->GetJavaObject(); } base::android::ScopedJavaLocalRef<jobject> IdentityManager::GetJavaObject() { @@ -478,6 +390,12 @@ base::android::ScopedJavaLocalRef<jobject> IdentityManager::GetJavaObject() { return base::android::ScopedJavaLocalRef<jobject>(java_identity_manager_); } +base::android::ScopedJavaLocalRef<jobject> +IdentityManager::GetIdentityMutatorJavaObject() { + return base::android::ScopedJavaLocalRef<jobject>( + identity_mutator_.GetJavaObject()); +} + void IdentityManager::ForceRefreshOfExtendedAccountInfo( const CoreAccountId& account_id) { DCHECK(HasAccountWithRefreshToken(account_id)); @@ -487,6 +405,18 @@ void IdentityManager::ForceRefreshOfExtendedAccountInfo( bool IdentityManager::HasPrimaryAccount(JNIEnv* env) const { return HasPrimaryAccount(); } + +base::android::ScopedJavaLocalRef<jobject> IdentityManager:: + FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress( + JNIEnv* env, + const base::android::JavaParamRef<jstring>& j_email) const { + auto account_info = + FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress( + base::android::ConvertJavaStringToUTF8(env, j_email)); + if (!account_info.has_value()) + return nullptr; + return ConvertToJavaCoreAccountInfo(env, account_info.value()); +} #endif PrimaryAccountManager* IdentityManager::GetPrimaryAccountManager() { @@ -521,43 +451,16 @@ AccountInfo IdentityManager::GetAccountInfoForAccountWithRefreshToken( AccountInfo account_info = account_tracker_service_->GetAccountInfo(account_id); - - // In the context of supervised users, the ProfileOAuth2TokenService is used - // without the AccountTrackerService being used. This is the only case in - // which the AccountTrackerService will potentially not know about the - // account. In this context, |account_id| is always set to - // kSupervisedUserPseudoEmail. Populate the information manually in this case - // to maintain the invariant that the account ID, gaia ID, and email are - // always set. - // TODO(860492): Remove this special case once supervised user support is - // removed. - DCHECK(!account_info.IsEmpty() || - account_id.id == kSupervisedUserPseudoEmail); - if (account_id.id == kSupervisedUserPseudoEmail && account_info.IsEmpty()) { - account_info.account_id = account_id; - account_info.email = kSupervisedUserPseudoEmail; - account_info.gaia = kSupervisedUserPseudoGaiaID; - } + DCHECK(!account_info.IsEmpty()); return account_info; } -void IdentityManager::SetPrimaryAccountInternal( - base::Optional<CoreAccountInfo> account_info) { - primary_account_ = std::move(account_info); - UpdateUnconsentedPrimaryAccount(); -} - void IdentityManager::UpdateUnconsentedPrimaryAccount() { - base::Optional<CoreAccountInfo> new_unconsented_primary_account = + base::Optional<CoreAccountInfo> account = ComputeUnconsentedPrimaryAccountInfo(); - if (unconsented_primary_account_ != new_unconsented_primary_account) { - unconsented_primary_account_ = std::move(new_unconsented_primary_account); - for (auto& observer : observer_list_) { - observer.OnUnconsentedPrimaryAccountChanged( - unconsented_primary_account_.value_or(CoreAccountInfo())); - } - } + if (account) + primary_account_manager_->SetUnconsentedPrimaryAccountInfo(*account); } base::Optional<CoreAccountInfo> @@ -571,51 +474,83 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const { // request to GAIA that lists cookie accounts. return base::nullopt; #else + AccountsInCookieJarInfo cookie_info = GetAccountsInCookieJar(); + + if (AreRefreshTokensLoaded() && GetAccountsWithRefreshTokens().empty()) + return CoreAccountInfo(); + std::vector<gaia::ListedAccount> cookie_accounts = - GetAccountsInCookieJar().signed_in_accounts; - if (cookie_accounts.empty()) + cookie_info.signed_in_accounts; + if (cookie_info.accounts_are_fresh && cookie_accounts.empty()) + return CoreAccountInfo(); + + if (!AreRefreshTokensLoaded() || !cookie_info.accounts_are_fresh) { + // If cookies or tokens are not loaded, it is not possible to fully compute + // the unconsented primary account. However, if the current unconsented + // primary account is no longer valid, it has to be removed. + CoreAccountId current_account = GetUnconsentedPrimaryAccountId(); + if (!current_account.empty()) { + if (AreRefreshTokensLoaded() && + !HasAccountWithRefreshToken(current_account)) { + return CoreAccountInfo(); + } + if (cookie_info.accounts_are_fresh && + cookie_accounts[0].id != current_account) { + return CoreAccountInfo(); + } + } return base::nullopt; + } + // At this point, cookies and tokens are loaded and neither are empty. const CoreAccountId first_account_id = cookie_accounts[0].id; if (!HasAccountWithRefreshToken(first_account_id)) - return base::nullopt; + return CoreAccountInfo(); return GetAccountInfoForAccountWithRefreshToken(first_account_id); #endif } -void IdentityManager::GoogleSigninSucceeded(const AccountInfo& account_info) { +void IdentityManager::GoogleSigninSucceeded( + const CoreAccountInfo& account_info) { + UpdateUnconsentedPrimaryAccount(); for (auto& observer : observer_list_) { observer.OnPrimaryAccountSet(account_info); } #if defined(OS_ANDROID) - if (java_identity_manager_) + if (java_identity_manager_) { + JNIEnv* env = base::android::AttachCurrentThread(); Java_IdentityManager_onPrimaryAccountSet( - base::android::AttachCurrentThread(), java_identity_manager_, - ConvertToJavaCoreAccountInfo(account_info)); + env, java_identity_manager_, + ConvertToJavaCoreAccountInfo(env, account_info)); + } #endif } -void IdentityManager::GoogleSignedOut(const AccountInfo& account_info) { +void IdentityManager::UnconsentedPrimaryAccountChanged( + const CoreAccountInfo& account_info) { + for (auto& observer : observer_list_) + observer.OnUnconsentedPrimaryAccountChanged(account_info); +} + +#if !defined(OS_CHROMEOS) +void IdentityManager::GoogleSignedOut(const CoreAccountInfo& account_info) { DCHECK(!HasPrimaryAccount()); + DCHECK(!account_info.IsEmpty()); + UpdateUnconsentedPrimaryAccount(); for (auto& observer : observer_list_) { observer.OnPrimaryAccountCleared(account_info); } #if defined(OS_ANDROID) - if (java_identity_manager_) + if (java_identity_manager_) { + JNIEnv* env = base::android::AttachCurrentThread(); Java_IdentityManager_onPrimaryAccountCleared( - base::android::AttachCurrentThread(), java_identity_manager_, - ConvertToJavaCoreAccountInfo(account_info)); + env, java_identity_manager_, + ConvertToJavaCoreAccountInfo(env, account_info)); + } #endif } -void IdentityManager::AuthenticatedAccountSet(const AccountInfo& account_info) { - DCHECK(primary_account_manager_->IsAuthenticated()); - SetPrimaryAccountInternal(account_info); -} -void IdentityManager::AuthenticatedAccountCleared() { - DCHECK(!primary_account_manager_->IsAuthenticated()); - SetPrimaryAccountInternal(base::nullopt); -} +#endif // !defined(OS_CHROMEOS) void IdentityManager::OnRefreshTokenAvailable(const CoreAccountId& account_id) { UpdateUnconsentedPrimaryAccount(); @@ -719,9 +654,14 @@ void IdentityManager::OnRefreshTokenRevokedFromSource( } void IdentityManager::OnAccountUpdated(const AccountInfo& info) { - if (primary_account_ && primary_account_->account_id == info.account_id) { - SetPrimaryAccountInternal(info); + if (HasPrimaryAccount()) { + const CoreAccountId primary_account_id = GetPrimaryAccountId(); + if (primary_account_id == info.account_id) { + primary_account_manager_->UpdateAuthenticatedAccountInfo(); + UpdateUnconsentedPrimaryAccount(); + } } + for (auto& observer : observer_list_) { observer.OnExtendedAccountInfoUpdated(info); } diff --git a/chromium/components/signin/public/identity_manager/identity_manager.h b/chromium/components/signin/public/identity_manager/identity_manager.h index 66aa6cc9574..d3f730c929d 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager.h +++ b/chromium/components/signin/public/identity_manager/identity_manager.h @@ -10,11 +10,15 @@ #include "base/macros.h" #include "base/observer_list.h" +#include "base/scoped_observer.h" #include "build/build_config.h" #include "components/keyed_service/core/keyed_service.h" +#include "components/signin/internal/identity_manager/primary_account_manager.h" +#include "components/signin/internal/identity_manager/profile_oauth2_token_service.h" #include "components/signin/internal/identity_manager/profile_oauth2_token_service_observer.h" #include "components/signin/public/identity_manager/access_token_fetcher.h" #include "components/signin/public/identity_manager/account_info.h" +#include "components/signin/public/identity_manager/identity_mutator.h" #include "components/signin/public/identity_manager/ubertoken_fetcher.h" #include "google_apis/gaia/oauth2_access_token_manager.h" #include "services/identity/public/cpp/scope_set.h" @@ -38,19 +42,13 @@ class PrefRegistrySimple; class AccountFetcherService; class AccountTrackerService; class GaiaCookieManagerService; -class PrimaryAccountManager; -class ProfileOAuth2TokenService; namespace signin { -class AccountsMutator; -class AccountsCookieMutator; struct AccountsInCookieJarInfo; class IdentityManagerTest; class IdentityTestEnvironment; -class DeviceAccountsSynchronizer; class DiagnosticsProvider; -class PrimaryAccountMutator; enum class ClearPrimaryAccountPolicy; struct CookieParamsForTest; @@ -58,6 +56,7 @@ struct CookieParamsForTest; // ./README.md for detailed documentation. class IdentityManager : public KeyedService, public OAuth2AccessTokenManager::DiagnosticsObserver, + public PrimaryAccountManager::Observer, public ProfileOAuth2TokenServiceObserver { public: class Observer { @@ -421,38 +420,11 @@ class IdentityManager : public KeyedService, // Registers per-profile prefs used by this class. static void RegisterProfilePrefs(PrefRegistrySimple* registry); -#if !defined(OS_IOS) && !defined(OS_ANDROID) - // Explicitly triggers the loading of accounts in the context of supervised - // users. - // TODO(https://crbug.com/860492): Remove this method when supervised users - // support is eliminated. - void DeprecatedLoadCredentialsForSupervisedUser( - const CoreAccountId& primary_account_id); -#endif - // Returns pointer to the object used to obtain diagnostics about the internal // state of IdentityManager. DiagnosticsProvider* GetDiagnosticsProvider(); -#if defined(OS_IOS) - // Forces the processing of GaiaCookieManagerService::OnCookieChange. On - // iOS, it's necessary to force-trigger the processing of cookie changes - // from the client as the normal mechanism for internally observing them - // is not wired up. - // TODO(https://crbug.com/930582) : Remove the need to expose this method - // or move it to the network::CookieManager. - void ForceTriggerOnCookieChange(); -#endif - #if defined(OS_ANDROID) - // Reloads the accounts in the token service from the system accounts. This - // API calls ProfileOAuth2TokenServiceDelegate::ReloadAccountsFromSystem and - // it triggers platform specific implementation for Android. NOTE: In normal - // usage, this method SHOULD NOT be called. - // TODO(https://crbug.com/930094): Expose this through - // DeviceAccountsSynchronizer - void LegacyReloadAccountsFromSystem(); - // Returns a pointer to the AccountTrackerService Java instance associated // with this object. // TODO(https://crbug.com/934688): Eliminate this method once @@ -467,10 +439,12 @@ class IdentityManager : public KeyedService, base::android::ScopedJavaLocalRef<jobject> LegacyGetOAuth2TokenServiceJavaObject(); - // Get the reference on the java IdentityManager, InitializeJavaObject must - // be called before hand. + // Get the reference on the java IdentityManager. base::android::ScopedJavaLocalRef<jobject> GetJavaObject(); + // Provide the reference on the java IdentityMutator. + base::android::ScopedJavaLocalRef<jobject> GetIdentityMutatorJavaObject(); + // This method has the contractual assumption that the account is a known // account and has as its semantics that it fetches the account info for the // account, triggering an OnExtendedAccountInfoUpdated() callback if the info @@ -479,6 +453,11 @@ class IdentityManager : public KeyedService, // Overloads for calls from java: bool HasPrimaryAccount(JNIEnv* env) const; + + base::android::ScopedJavaLocalRef<jobject> + FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress( + JNIEnv* env, + const base::android::JavaParamRef<jstring>& j_email) const; #endif private: @@ -617,13 +596,17 @@ class IdentityManager : public KeyedService, // Figures out and returns the current unconsented primary account based on // current cookies. + // Returns nullopt if the account could not be computed, and CoreAccountInfo() + // if there is no account. base::Optional<CoreAccountInfo> ComputeUnconsentedPrimaryAccountInfo() const; - // PrimaryAccountManager callbacks: - void GoogleSigninSucceeded(const AccountInfo& account_info); - void GoogleSignedOut(const AccountInfo& account_info); - void AuthenticatedAccountSet(const AccountInfo& account_info); - void AuthenticatedAccountCleared(); + // PrimaryAccountManager::Observer: + void GoogleSigninSucceeded(const CoreAccountInfo& account_info) override; + void UnconsentedPrimaryAccountChanged( + const CoreAccountInfo& account_info) override; +#if !defined(OS_CHROMEOS) + void GoogleSignedOut(const CoreAccountInfo& account_info) override; +#endif // ProfileOAuth2TokenServiceObserver: void OnRefreshTokenAvailable(const CoreAccountId& account_id) override; @@ -670,23 +653,16 @@ class IdentityManager : public KeyedService, std::unique_ptr<PrimaryAccountManager> primary_account_manager_; std::unique_ptr<AccountFetcherService> account_fetcher_service_; - // PrimaryAccountMutator instance. May be null if mutation of the primary - // account state is not supported on the current platform. - std::unique_ptr<PrimaryAccountMutator> primary_account_mutator_; - - // AccountsMutator instance. May be null if mutation of accounts is not - // supported on the current platform. - std::unique_ptr<AccountsMutator> accounts_mutator_; - - // AccountsCookieMutator instance. Guaranteed to be non-null, as this - // functionality is supported on all platforms. - std::unique_ptr<AccountsCookieMutator> accounts_cookie_mutator_; + IdentityMutator identity_mutator_; // DiagnosticsProvider instance. std::unique_ptr<DiagnosticsProvider> diagnostics_provider_; - // DeviceAccountsSynchronizer instance. - std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer_; + // Scoped observers. + ScopedObserver<PrimaryAccountManager, PrimaryAccountManager::Observer> + primary_account_manager_observer_{this}; + ScopedObserver<ProfileOAuth2TokenService, ProfileOAuth2TokenServiceObserver> + token_service_observer_{this}; // Lists of observers. // Makes sure lists are empty on destruction. @@ -694,10 +670,6 @@ class IdentityManager : public KeyedService, base::ObserverList<DiagnosticsObserver, true>::Unchecked diagnostics_observer_list_; - // If |primary_account_| is set, it must equal |unconsented_primary_account_|. - base::Optional<CoreAccountInfo> primary_account_; - base::Optional<CoreAccountInfo> unconsented_primary_account_; - #if defined(OS_ANDROID) // Java-side IdentityManager object. base::android::ScopedJavaGlobalRef<jobject> java_identity_manager_; 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 7dc60a28104..da113643252 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager_builder.cc +++ b/chromium/components/signin/public/identity_manager/identity_manager_builder.cc @@ -30,10 +30,13 @@ #endif #if defined(OS_IOS) -#include "components/signin/internal/identity_manager/device_accounts_synchronizer_impl.h" #include "components/signin/public/identity_manager/ios/device_accounts_provider.h" #endif +#if defined(OS_ANDROID) || defined(OS_IOS) +#include "components/signin/internal/identity_manager/device_accounts_synchronizer_impl.h" +#endif + #if !defined(OS_ANDROID) && !defined(OS_IOS) #include "components/signin/internal/identity_manager/accounts_mutator_impl.h" #endif @@ -155,7 +158,7 @@ std::unique_ptr<IdentityManager> BuildIdentityManager( std::move(params->image_decoder)); std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer; -#if defined(OS_IOS) +#if defined(OS_IOS) || defined(OS_ANDROID) device_accounts_synchronizer = std::make_unique<DeviceAccountsSynchronizerImpl>( token_service->GetDelegate()); 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 ca1216e21b8..d15f8eb897e 100644 --- a/chromium/components/signin/public/identity_manager/identity_manager_unittest.cc +++ b/chromium/components/signin/public/identity_manager/identity_manager_unittest.cc @@ -41,6 +41,8 @@ #include "components/sync_preferences/testing_pref_service_syncable.h" #include "google_apis/gaia/core_account_id.h" #include "google_apis/gaia/google_service_auth_error.h" +#include "net/cookies/cookie_change_dispatcher.h" +#include "net/cookies/cookie_constants.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/network/test/test_cookie_manager.h" #include "services/network/test/test_url_loader_factory.h" @@ -347,8 +349,8 @@ class IdentityManagerTest : public testing::Test { if (primary_account_manager_setup == PrimaryAccountManagerSetup::kWithAuthenticatedAccout) { - primary_account_manager->SetAuthenticatedAccountInfo(kTestGaiaId, - kTestEmail); + account_tracker_service->SeedAccountInfo(kTestGaiaId, kTestEmail); + primary_account_manager->SignIn(kTestEmail); } auto accounts_cookie_mutator = std::make_unique<AccountsCookieMutatorImpl>( @@ -383,8 +385,9 @@ class IdentityManagerTest : public testing::Test { void SimulateCookieDeletedByUser( network::mojom::CookieChangeListener* listener, const net::CanonicalCookie& cookie) { - listener->OnCookieChange(cookie, - network::mojom::CookieChangeCause::EXPLICIT); + listener->OnCookieChange( + net::CookieChangeInfo(cookie, net::CookieAccessSemantics::UNKNOWN, + net::CookieChangeCause::EXPLICIT)); } void SimulateOAuthMultiloginFinished(GaiaCookieManagerService* manager, @@ -549,6 +552,9 @@ TEST_F(IdentityManagerTest, HasPrimaryAccount) { ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT); EXPECT_FALSE(identity_manager()->HasPrimaryAccount()); EXPECT_FALSE(identity_manager()->HasUnconsentedPrimaryAccount()); + EXPECT_FALSE(identity_manager_observer() + ->PrimaryAccountFromClearedCallback() + .IsEmpty()); #endif } @@ -1080,8 +1086,9 @@ TEST_F(IdentityManagerTest, RemoveAccessTokenFromCache) { std::set<std::string> scopes{"scope"}; std::string access_token = "access_token"; - identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo( - kTestGaiaId, kTestEmail); + identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId, + kTestEmail); + identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail); token_service()->UpdateCredentials(primary_account_id(), "refresh_token"); base::RunLoop run_loop; @@ -1119,8 +1126,9 @@ TEST_F(IdentityManagerTest, identity_manager_diagnostics_observer() ->set_on_access_token_requested_callback(run_loop.QuitClosure()); - identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo( - kTestGaiaId, kTestEmail); + identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId, + kTestEmail); + identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail); token_service()->UpdateCredentials(primary_account_id(), "refresh_token"); std::set<std::string> scopes{"scope"}; @@ -1207,8 +1215,9 @@ TEST_F(IdentityManagerTest, ObserveAccessTokenFetch) { identity_manager_diagnostics_observer() ->set_on_access_token_requested_callback(run_loop.QuitClosure()); - identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo( - kTestGaiaId, kTestEmail); + identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId, + kTestEmail); + identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail); token_service()->UpdateCredentials(primary_account_id(), "refresh_token"); std::set<std::string> scopes{"scope"}; @@ -1260,8 +1269,9 @@ TEST_F(IdentityManagerTest, identity_manager_diagnostics_observer() ->set_on_access_token_request_completed_callback(run_loop.QuitClosure()); - identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo( - kTestGaiaId, kTestEmail); + identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId, + kTestEmail); + identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail); token_service()->UpdateCredentials(primary_account_id(), "refresh_token"); token_service()->set_auto_post_fetch_response_on_message_loop(true); @@ -1327,35 +1337,6 @@ TEST_F(IdentityManagerTest, GetAccountsCookieMutator) { EXPECT_TRUE(mutator); } -#if !defined(OS_IOS) && !defined(OS_ANDROID) -// Tests that requesting a load of accounts results in the notification -// firing that tokens were loaded. -TEST_F(IdentityManagerTest, DeprecatedLoadCredentialsForSupervisedUser) { - base::RunLoop run_loop; - identity_manager_observer()->SetOnRefreshTokensLoadedCallback( - run_loop.QuitClosure()); - - // Load the accounts and ensure that we see the resulting notification that - // they were loaded. - identity_manager()->DeprecatedLoadCredentialsForSupervisedUser(""); - run_loop.Run(); -} -#endif - -#if defined(OS_IOS) -TEST_F(IdentityManagerTest, ForceTriggerOnCookieChange) { - base::RunLoop run_loop; - identity_manager_observer()->SetOnAccountsInCookieUpdatedCallback( - run_loop.QuitClosure()); - - SetListAccountsResponseNoAccounts(test_url_loader_factory()); - // Forces the processing of OnCookieChange and it calls - // OnGaiaAccountsInCookieUpdated. - identity_manager()->ForceTriggerOnCookieChange(); - run_loop.Run(); -} -#endif - #if defined(OS_CHROMEOS) // On ChromeOS, AccountTrackerService first receives the normalized email // address from GAIA and then later has it updated with the user's @@ -1575,6 +1556,58 @@ TEST_F( empty_info); EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), empty_info); } + +TEST_F(IdentityManagerTest, + UnconsentedPrimaryAccountTokenRevokedWithStaleCookies) { + ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT); + + // Add an unconsented primary account, incl. proper cookies. + AccountInfo account_info = MakeAccountAvailableWithCookies( + identity_manager(), test_url_loader_factory(), kTestEmail2, kTestGaiaId2); + EXPECT_EQ(kTestEmail2, account_info.email); + + EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), + account_info); + + // Make the cookies stale and remove the account. + signin::SetFreshnessOfAccountsInGaiaCookie(identity_manager(), false); + RemoveRefreshTokenForAccount(identity_manager(), account_info.account_id); + AccountsInCookieJarInfo cookie_info = + identity_manager()->GetAccountsInCookieJar(); + ASSERT_FALSE(cookie_info.accounts_are_fresh); + // Unconsented account was removed. + EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), + CoreAccountInfo()); +} + +TEST_F(IdentityManagerTest, + UnconsentedPrimaryAccountTokenRevokedWithStaleCookiesMultipleAccounts) { + ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT); + + // Add two accounts with cookies. + AccountInfo main_account_info = + MakeAccountAvailable(identity_manager(), kTestEmail2); + AccountInfo secondary_account_info = + MakeAccountAvailable(identity_manager(), kTestEmail3); + SetCookieAccounts( + identity_manager(), test_url_loader_factory(), + {{main_account_info.email, main_account_info.gaia}, + {secondary_account_info.email, secondary_account_info.gaia}}); + + EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), + main_account_info); + + // Make the cookies stale and remove the main account. + signin::SetFreshnessOfAccountsInGaiaCookie(identity_manager(), false); + RemoveRefreshTokenForAccount(identity_manager(), + main_account_info.account_id); + AccountsInCookieJarInfo cookie_info = + identity_manager()->GetAccountsInCookieJar(); + ASSERT_FALSE(cookie_info.accounts_are_fresh); + // Unconsented account was removed. + EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), + CoreAccountInfo()); +} #endif TEST_F(IdentityManagerTest, CallbackSentOnRefreshTokenRemovalOfUnknownAccount) { @@ -1994,10 +2027,10 @@ TEST_F(IdentityManagerTest, CallbackSentOnAccountsCookieDeletedByUserAction) { base::RunLoop run_loop; identity_manager_observer()->SetOnCookieDeletedByUserCallback( run_loop.QuitClosure()); - net::CanonicalCookie cookie("APISID", std::string(), ".google.com", "/", - base::Time(), base::Time(), base::Time(), false, - false, net::CookieSameSite::NO_RESTRICTION, - net::COOKIE_PRIORITY_DEFAULT); + net::CanonicalCookie cookie( + "SAPISID", std::string(), ".google.com", "/", base::Time(), base::Time(), + base::Time(), /*secure=*/true, false, net::CookieSameSite::NO_RESTRICTION, + net::COOKIE_PRIORITY_DEFAULT); SimulateCookieDeletedByUser(identity_manager()->GetGaiaCookieManagerService(), cookie); run_loop.Run(); @@ -2026,19 +2059,21 @@ TEST_F(IdentityManagerTest, OnNetworkInitialized) { // Note that this call differs from calling SimulateCookieDeletedByUser() // directly in the sense that SimulateCookieDeletedByUser() does not go // through any mojo pipe. - net::CanonicalCookie cookie("APISID", std::string(), ".google.com", "/", - base::Time(), base::Time(), base::Time(), false, - false, net::CookieSameSite::NO_RESTRICTION, - net::COOKIE_PRIORITY_DEFAULT); + net::CanonicalCookie cookie( + "SAPISID", std::string(), ".google.com", "/", base::Time(), base::Time(), + base::Time(), /*secure=*/true, false, net::CookieSameSite::NO_RESTRICTION, + net::COOKIE_PRIORITY_DEFAULT); test_cookie_manager_ptr->DispatchCookieChange( - cookie, network::mojom::CookieChangeCause::EXPLICIT); + net::CookieChangeInfo(cookie, net::CookieAccessSemantics::UNKNOWN, + net::CookieChangeCause::EXPLICIT)); run_loop.Run(); } TEST_F(IdentityManagerTest, BatchChangeObserversAreNotifiedOnCredentialsUpdate) { - identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo( - kTestGaiaId, kTestEmail); + identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId, + kTestEmail); + identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail); token_service()->UpdateCredentials(primary_account_id(), "refresh_token"); EXPECT_EQ(1ul, identity_manager_observer()->BatchChangeRecords().size()); diff --git a/chromium/components/signin/public/identity_manager/identity_mutator.cc b/chromium/components/signin/public/identity_manager/identity_mutator.cc new file mode 100644 index 00000000000..331001672de --- /dev/null +++ b/chromium/components/signin/public/identity_manager/identity_mutator.cc @@ -0,0 +1,109 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/signin/public/identity_manager/identity_mutator.h" + +#include "components/signin/public/identity_manager/accounts_cookie_mutator.h" +#include "components/signin/public/identity_manager/accounts_mutator.h" +#include "components/signin/public/identity_manager/device_accounts_synchronizer.h" +#include "components/signin/public/identity_manager/primary_account_mutator.h" + +#if defined(OS_ANDROID) +#include "base/android/jni_string.h" +#include "components/signin/internal/identity_manager/android/jni_headers/IdentityMutator_jni.h" +#include "components/signin/public/identity_manager/account_info.h" +#endif + +namespace signin { + +#if defined(OS_ANDROID) +JniIdentityMutator::JniIdentityMutator(IdentityMutator* identity_mutator) + : identity_mutator_(identity_mutator) {} + +bool JniIdentityMutator::SetPrimaryAccount( + JNIEnv* env, + const base::android::JavaParamRef<jobject>& primary_account_id) { + PrimaryAccountMutator* primary_account_mutator = + identity_mutator_->GetPrimaryAccountMutator(); + DCHECK(primary_account_mutator); + return primary_account_mutator->SetPrimaryAccount( + ConvertFromJavaCoreAccountId(env, primary_account_id)); +} + +bool JniIdentityMutator::ClearPrimaryAccount(JNIEnv* env, + jint action, + jint source_metric, + jint delete_metric) { + PrimaryAccountMutator* primary_account_mutator = + identity_mutator_->GetPrimaryAccountMutator(); + DCHECK(primary_account_mutator); + return primary_account_mutator->ClearPrimaryAccount( + PrimaryAccountMutator::ClearAccountsAction::kDefault, + static_cast<signin_metrics::ProfileSignout>(source_metric), + static_cast<signin_metrics::SignoutDelete>(delete_metric)); +} + +void JniIdentityMutator::ReloadAllAccountsFromSystemWithPrimaryAccount( + JNIEnv* env, + const base::android::JavaParamRef<jobject>& primary_account_id) { + DeviceAccountsSynchronizer* device_accounts_synchronizer = + identity_mutator_->GetDeviceAccountsSynchronizer(); + DCHECK(device_accounts_synchronizer); + device_accounts_synchronizer->ReloadAllAccountsFromSystemWithPrimaryAccount( + ConvertFromJavaCoreAccountId(env, primary_account_id)); +} +#endif // defined(OS_ANDROID) + +IdentityMutator::IdentityMutator( + std::unique_ptr<PrimaryAccountMutator> primary_account_mutator, + std::unique_ptr<AccountsMutator> accounts_mutator, + std::unique_ptr<AccountsCookieMutator> accounts_cookie_mutator, + std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer) + : primary_account_mutator_(std::move(primary_account_mutator)), + accounts_mutator_(std::move(accounts_mutator)), + accounts_cookie_mutator_(std::move(accounts_cookie_mutator)), + device_accounts_synchronizer_(std::move(device_accounts_synchronizer)) { + DCHECK(accounts_cookie_mutator_); + DCHECK(!accounts_mutator_ || !device_accounts_synchronizer_) + << "Cannot have both an AccountsMutator and a DeviceAccountsSynchronizer"; + +#if defined(OS_ANDROID) + jni_identity_mutator_.reset(new JniIdentityMutator(this)); + java_identity_mutator_ = Java_IdentityMutator_Constructor( + base::android::AttachCurrentThread(), + reinterpret_cast<intptr_t>(jni_identity_mutator_.get())); +#endif +} + +IdentityMutator::~IdentityMutator() { +#if defined(OS_ANDROID) + if (java_identity_mutator_) + Java_IdentityMutator_destroy(base::android::AttachCurrentThread(), + java_identity_mutator_); +#endif +} + +#if defined(OS_ANDROID) +base::android::ScopedJavaLocalRef<jobject> IdentityMutator::GetJavaObject() { + DCHECK(java_identity_mutator_); + return base::android::ScopedJavaLocalRef<jobject>(java_identity_mutator_); +} +#endif + +PrimaryAccountMutator* IdentityMutator::GetPrimaryAccountMutator() { + return primary_account_mutator_.get(); +} + +AccountsMutator* IdentityMutator::GetAccountsMutator() { + return accounts_mutator_.get(); +} + +AccountsCookieMutator* IdentityMutator::GetAccountsCookieMutator() { + return accounts_cookie_mutator_.get(); +} + +DeviceAccountsSynchronizer* IdentityMutator::GetDeviceAccountsSynchronizer() { + return device_accounts_synchronizer_.get(); +} +} // namespace signin diff --git a/chromium/components/signin/public/identity_manager/identity_mutator.h b/chromium/components/signin/public/identity_manager/identity_mutator.h new file mode 100644 index 00000000000..c6316e7adab --- /dev/null +++ b/chromium/components/signin/public/identity_manager/identity_mutator.h @@ -0,0 +1,138 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_IDENTITY_MUTATOR_H_ +#define COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_IDENTITY_MUTATOR_H_ + +#include <memory> + +#include "build/build_config.h" +#if defined(OS_ANDROID) +#include "base/android/jni_android.h" +#endif + +namespace signin { +class AccountsMutator; +class AccountsCookieMutator; +class PrimaryAccountMutator; +class DeviceAccountsSynchronizer; + +#if defined(OS_ANDROID) +class IdentityMutator; + +// This class is the JNI interface accessing IdentityMutator. +// This is created by IdentityMutator and can only be accessed by JNI generated +// code (IdentityMutator_jni.h), i.e. by IdentityMutator.java. +class JniIdentityMutator { + public: + // JniIdentityMutator is non-copyable, non-movable + JniIdentityMutator(IdentityMutator&& other) = delete; + JniIdentityMutator const& operator=(IdentityMutator&& other) = delete; + + JniIdentityMutator(const IdentityMutator& other) = delete; + JniIdentityMutator const& operator=(const IdentityMutator& other) = delete; + + // Called by java to mark the account with |account_id| as the primary + // account, and return whether the operation succeeded or not. To succeed, + // this requires that: + // - the account is known by the IdentityManager. + // - setting the primary account is allowed, + // - the account username is allowed by policy, + // - there is not already a primary account set. + bool SetPrimaryAccount( + JNIEnv* env, + const base::android::JavaParamRef<jobject>& primary_account_id); + + // Called by java to clear the primary account, and return whether the + // operation succeeded or not. Depending on |action|, the other accounts known + // to the IdentityManager may be deleted. + bool ClearPrimaryAccount(JNIEnv* env, + jint action, + jint source_metric, + jint delete_metric); + + // Called by java to reload the accounts in the token service from the system + // accounts. + void ReloadAllAccountsFromSystemWithPrimaryAccount( + JNIEnv* env, + const base::android::JavaParamRef<jobject>& primary_account_id); + + private: + friend IdentityMutator; + + JniIdentityMutator(IdentityMutator* identity_mutator); + + IdentityMutator* identity_mutator_; +}; +#endif + +// IdentityMutator is the mutating interface for IdentityManager. +class IdentityMutator { + public: + IdentityMutator( + std::unique_ptr<PrimaryAccountMutator> primary_account_mutator, + std::unique_ptr<AccountsMutator> accounts_mutator, + std::unique_ptr<AccountsCookieMutator> accounts_cookie_mutator, + std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer); + + virtual ~IdentityMutator(); + + // IdentityMutator is non-copyable, non-moveable. + IdentityMutator(IdentityMutator&& other) = delete; + IdentityMutator const& operator=(IdentityMutator&& other) = delete; + + IdentityMutator(const IdentityMutator& other) = delete; + IdentityMutator const& operator=(const IdentityMutator& other) = delete; + +#if defined(OS_ANDROID) + // Get the reference on the java IdentityManager. + base::android::ScopedJavaLocalRef<jobject> GetJavaObject(); +#endif + + // Returns pointer to the object used to change the signed-in state of the + // primary account, if supported on the current platform. Otherwise, returns + // null. + PrimaryAccountMutator* GetPrimaryAccountMutator(); + + // Returns pointer to the object used to seed accounts and mutate state of + // accounts' refresh tokens, if supported on the current platform. Otherwise, + // returns null. + AccountsMutator* GetAccountsMutator(); + + // Returns pointer to the object used to manipulate the cookies stored and the + // accounts associated with them. Guaranteed to be non-null. + AccountsCookieMutator* GetAccountsCookieMutator(); + + // Returns pointer to the object used to seed accounts information from the + // device-level accounts. May be null if the system has no such notion. + DeviceAccountsSynchronizer* GetDeviceAccountsSynchronizer(); + + private: +#if defined(OS_ANDROID) + // C++ endpoint for identity mutator calls originating from java. + std::unique_ptr<JniIdentityMutator> jni_identity_mutator_; + + // Java-side IdentityMutator object. + base::android::ScopedJavaGlobalRef<jobject> java_identity_mutator_; +#endif + + // PrimaryAccountMutator instance. May be null if mutation of the primary + // account state is not supported on the current platform. + std::unique_ptr<PrimaryAccountMutator> primary_account_mutator_; + + // AccountsMutator instance. May be null if mutation of accounts is not + // supported on the current platform. + std::unique_ptr<AccountsMutator> accounts_mutator_; + + // AccountsCookieMutator instance. Guaranteed to be non-null, as this + // functionality is supported on all platforms. + std::unique_ptr<AccountsCookieMutator> accounts_cookie_mutator_; + + // DeviceAccountsSynchronizer instance. + std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer_; +}; + +} // namespace signin + +#endif // COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_IDENTITY_MUTATOR_H_ 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 69068413873..088157de9d8 100644 --- a/chromium/components/signin/public/identity_manager/identity_test_environment.cc +++ b/chromium/components/signin/public/identity_manager/identity_test_environment.cc @@ -128,6 +128,11 @@ void IdentityTestEnvironment::Initialize() { "If your test has an existing one, move it to be initialized before " "IdentityTestEnvironment. Otherwise, use " "base::test::TaskEnvironment."; + DCHECK(identity_manager() + ->GetTokenService() + ->IsFakeProfileOAuth2TokenServiceForTesting()) + << "IdentityTestEnvironment requires the ProfileOAuth2TokenService used " + "to subclass FakeProfileOAuth2TokenServiceForTesting."; test_identity_manager_observer_ = std::make_unique<TestIdentityManagerObserver>(this->identity_manager()); this->identity_manager()->AddDiagnosticsObserver(this); @@ -273,6 +278,15 @@ AccountInfo IdentityTestEnvironment::MakeAccountAvailable( return signin::MakeAccountAvailable(identity_manager(), email); } +AccountInfo IdentityTestEnvironment::MakeAccountAvailableWithCookies( + const std::string& email, + const std::string& gaia_id) { + return signin::MakeAccountAvailableWithCookies( + identity_manager(), + dependencies_owner_->signin_client()->GetTestURLLoaderFactory(), email, + gaia_id); +} + void IdentityTestEnvironment::SetRefreshTokenForAccount( const CoreAccountId& account_id) { return signin::SetRefreshTokenForAccount(identity_manager(), account_id); diff --git a/chromium/components/signin/public/identity_manager/identity_test_environment.h b/chromium/components/signin/public/identity_manager/identity_test_environment.h index ab6184e8b99..5c8773399f1 100644 --- a/chromium/components/signin/public/identity_manager/identity_test_environment.h +++ b/chromium/components/signin/public/identity_manager/identity_test_environment.h @@ -114,6 +114,14 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver { // Returns the AccountInfo of the newly-available account. AccountInfo MakePrimaryAccountAvailable(const std::string& email); + // Combination of MakeAccountAvailable() and SetCookieAccounts() for a single + // account. It makes an account available for the given email address, and + // GAIA ID, setting the cookies and the refresh token that correspond uniquely + // to that email address. Blocks until the account is available. Returns the + // AccountInfo of the newly-available account. + AccountInfo MakeAccountAvailableWithCookies(const std::string& email, + const std::string& gaia_id); + // Clears the primary account if present, with |policy| used to determine // whether to keep or remove all accounts. On non-ChromeOS, results in the // firing of the IdentityManager and PrimaryAccountManager callbacks for 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 9b014606a23..09a6983c54b 100644 --- a/chromium/components/signin/public/identity_manager/identity_test_utils.h +++ b/chromium/components/signin/public/identity_manager/identity_test_utils.h @@ -164,7 +164,8 @@ void UpdatePersistentErrorOfRefreshTokenForAccount( void DisableAccessTokenFetchRetries(IdentityManager* identity_manager); #if defined(OS_ANDROID) -// Disables interaction with system accounts, which requires special permission. +// Disables interaction with system accounts, which requires special +// initialization of the java subsystems (AccountManagerFacade). void DisableInteractionWithSystemAccounts(); #endif diff --git a/chromium/components/signin/public/identity_manager/primary_account_mutator.h b/chromium/components/signin/public/identity_manager/primary_account_mutator.h index 452335d1148..0b23bde235c 100644 --- a/chromium/components/signin/public/identity_manager/primary_account_mutator.h +++ b/chromium/components/signin/public/identity_manager/primary_account_mutator.h @@ -7,6 +7,8 @@ #include <string> +#include "build/build_config.h" + namespace signin_metrics { enum ProfileSignout : int; enum class SignoutDelete; @@ -19,7 +21,7 @@ namespace signin { // PrimaryAccountMutator is the interface to set and clear the primary account // (see IdentityManager for more information). // -// It is a pure interface that has concrete implementation on platform that +// This interface has concrete implementations on platform that // support changing the signed-in state during the lifetime of the application. // On other platforms, there is no implementation, and no instance will be // available at runtime (thus accessors may return null). @@ -27,6 +29,7 @@ class PrimaryAccountMutator { public: // Represents the options for handling the accounts known to the // IdentityManager upon calling ClearPrimaryAccount(). + // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.signin.identitymanager enum class ClearAccountsAction { kDefault, // Default action based on internal policy. kKeepAll, // Keep all accounts. |