diff --git a/components/sync/base/pref_names.cc b/components/sync/base/pref_names.cc index 9a77d5ac9a00bf..d7b03624e4d6d3 100644 --- a/components/sync/base/pref_names.cc +++ b/components/sync/base/pref_names.cc @@ -77,6 +77,7 @@ const char kSyncEncryptionBootstrapToken[] = "sync.encryption_bootstrap_token"; const char kSyncKeystoreEncryptionBootstrapToken[] = "sync.keystore_encryption_bootstrap_token"; +const char kSyncGaiaId[] = "sync.gaia_id"; const char kSyncCacheGuid[] = "sync.cache_guid"; const char kSyncBirthday[] = "sync.birthday"; const char kSyncBagOfChips[] = "sync.bag_of_chips"; diff --git a/components/sync/base/pref_names.h b/components/sync/base/pref_names.h index 4e40b21f97f401..56d178a81236f5 100644 --- a/components/sync/base/pref_names.h +++ b/components/sync/base/pref_names.h @@ -42,6 +42,7 @@ extern const char kSyncRequested[]; extern const char kSyncEncryptionBootstrapToken[]; extern const char kSyncKeystoreEncryptionBootstrapToken[]; +extern const char kSyncGaiaId[]; extern const char kSyncCacheGuid[]; extern const char kSyncBirthday[]; extern const char kSyncBagOfChips[]; diff --git a/components/sync/base/sync_prefs.cc b/components/sync/base/sync_prefs.cc index 204b720a1f446c..f71b4e0d075187 100644 --- a/components/sync/base/sync_prefs.cc +++ b/components/sync/base/sync_prefs.cc @@ -298,6 +298,7 @@ void SyncPrefs::RegisterProfilePrefs( std::string()); // Internal or bookkeeping prefs. + registry->RegisterStringPref(prefs::kSyncGaiaId, std::string()); registry->RegisterStringPref(prefs::kSyncCacheGuid, std::string()); registry->RegisterStringPref(prefs::kSyncBirthday, std::string()); registry->RegisterStringPref(prefs::kSyncBagOfChips, std::string()); @@ -366,6 +367,7 @@ void SyncPrefs::ClearLocalSyncTransportData() { pref_service_->ClearPref(prefs::kSyncPassphrasePrompted); pref_service_->ClearPref(prefs::kSyncInvalidationVersions); pref_service_->ClearPref(prefs::kSyncLastRunVersion); + pref_service_->ClearPref(prefs::kSyncGaiaId); pref_service_->ClearPref(prefs::kSyncCacheGuid); pref_service_->ClearPref(prefs::kSyncBirthday); pref_service_->ClearPref(prefs::kSyncBagOfChips); @@ -628,6 +630,15 @@ void SyncPrefs::RegisterTypeSelectedPref( registry->RegisterBooleanPref(pref_name, false); } +void SyncPrefs::SetGaiaId(const std::string& gaia_id) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + pref_service_->SetString(prefs::kSyncGaiaId, gaia_id); +} + +std::string SyncPrefs::GetGaiaId() const { + return pref_service_->GetString(prefs::kSyncGaiaId); +} + void SyncPrefs::SetCacheGuid(const std::string& cache_guid) { pref_service_->SetString(prefs::kSyncCacheGuid, cache_guid); } diff --git a/components/sync/base/sync_prefs.h b/components/sync/base/sync_prefs.h index 75b71e7a0c6f26..3268aa2f4b754d 100644 --- a/components/sync/base/sync_prefs.h +++ b/components/sync/base/sync_prefs.h @@ -152,6 +152,8 @@ class SyncPrefs : public CryptoSyncPrefs, // Maps |type| to its corresponding preference name. static const char* GetPrefNameForType(UserSelectableType type); + void SetGaiaId(const std::string& gaia_id); + std::string GetGaiaId() const; void SetCacheGuid(const std::string& cache_guid); std::string GetCacheGuid() const; void SetBirthday(const std::string& birthday); diff --git a/components/sync/driver/profile_sync_service.cc b/components/sync/driver/profile_sync_service.cc index f155f9c6269d3c..b4c538da8acaad 100644 --- a/components/sync/driver/profile_sync_service.cc +++ b/components/sync/driver/profile_sync_service.cc @@ -160,6 +160,38 @@ std::string GenerateCacheGUID() { return guid; } +bool IsLocalSyncTransportDataValid(const SyncPrefs& sync_prefs, + const CoreAccountInfo& core_account_info) { + // If the cache GUID is empty, it most probably is because local sync data + // has been fully cleared via ClearLocalSyncTransportData() due to + // ShutdownReason::DISABLE_SYNC. Let's return false here anyway to make sure + // all prefs are cleared and a new random cache GUID generated. + if (sync_prefs.GetCacheGuid().empty()) { + return false; + } + + // If cache GUID is initialized but the birthday isn't, it means the first + // sync cycle never completed (OnEngineInitialized()). This should be a rare + // case and theoretically harmless to resume, but as safety precaution, its + // simpler to regenerate the cache GUID and start from scratch, to avoid + // protocol violations (fetching updates requires that the request either has + // a birthday, or there should be no progress marker). + if (sync_prefs.GetBirthday().empty()) { + return false; + } + + // Make sure the cached account information (gaia ID) is equal to the current + // one (otherwise the data may be corrupt). Note that, for local sync + // (IsLocalSyncEnabled()), the authenticated account is always empty. + if (sync_prefs.GetGaiaId() != core_account_info.gaia) { + DLOG(WARNING) << "Found mismatching gaia ID in sync preferences"; + return false; + } + + // All good: local sync data looks initialized and valid. + return true; +} + } // namespace ProfileSyncService::InitParams::InitParams() = default; @@ -502,6 +534,22 @@ void ProfileSyncService::InitializeBackendTaskRunnerIfNeeded() { void ProfileSyncService::StartUpSlowEngineComponents() { DCHECK(IsEngineAllowedToStart()); + const CoreAccountInfo authenticated_account_info = + GetAuthenticatedAccountInfo(); + + if (IsLocalSyncEnabled()) { + // With local sync (roaming profiles) there is no identity manager and hence + // |authenticated_account_info| is empty. This is required for + // IsLocalSyncTransportDataValid() to work properly. + DCHECK(authenticated_account_info.gaia.empty()); + DCHECK(authenticated_account_info.account_id.empty()); + } else { + // Except for local sync (roaming profiles), the user must be signed in for + // sync to start. + DCHECK(!authenticated_account_info.gaia.empty()); + DCHECK(!authenticated_account_info.account_id.empty()); + } + engine_ = sync_client_->GetSyncApiComponentFactory()->CreateSyncEngine( debug_identifier_, sync_client_->GetInvalidationService(), sync_prefs_.AsWeakPtr()); @@ -511,15 +559,22 @@ void ProfileSyncService::StartUpSlowEngineComponents() { last_actionable_error_ = SyncProtocolError(); } - // If either the cache GUID or the birthday are uninitialized, it means we - // haven't completed a first sync cycle (OnEngineInitialized()). Theoretically - // both or neither should be empty, but just in case, we regenerate the cache - // GUID if either of them is missing, to avoid protocol violations (fetching - // updates requires that the request either has a birthday, or there should be - // no progress marker). - if (sync_prefs_.GetCacheGuid().empty() || sync_prefs_.GetBirthday().empty()) { + // The gaia ID in SyncPrefs was introduced with M81, so having an empty value + // is legitimate and should be populated as a one-off migration. + // TODO(mastiz): Clean up this migration code after a grace period (e.g. 1 + // year). + if (sync_prefs_.GetGaiaId().empty()) { + sync_prefs_.SetGaiaId(authenticated_account_info.gaia); + } + + if (!IsLocalSyncTransportDataValid(sync_prefs_, authenticated_account_info)) { + // Either the local data is uninitialized or corrupt, so let's throw + // everything away and start from scratch with a new cache GUID, which also + // cascades into datatypes throwing away their dangling sync metadata due to + // cache GUID mismatches. sync_prefs_.ClearLocalSyncTransportData(); sync_prefs_.SetCacheGuid(GenerateCacheGUID()); + sync_prefs_.SetGaiaId(authenticated_account_info.gaia); } InitializeBackendTaskRunnerIfNeeded(); @@ -539,8 +594,7 @@ void ProfileSyncService::StartUpSlowEngineComponents() { params.http_factory_getter = base::BindOnce( create_http_post_provider_factory_cb_, MakeUserAgentForSync(channel_), url_loader_factory_->Clone(), network_time_update_callback_); - params.authenticated_account_id = GetAuthenticatedAccountInfo().account_id; - DCHECK(!params.authenticated_account_id.empty() || IsLocalSyncEnabled()); + params.authenticated_account_id = authenticated_account_info.account_id; if (!base::FeatureList::IsEnabled(switches::kSyncE2ELatencyMeasurement)) { invalidation::InvalidationService* invalidator = sync_client_->GetInvalidationService(); diff --git a/components/sync/driver/profile_sync_service_unittest.cc b/components/sync/driver/profile_sync_service_unittest.cc index f768888e06e94d..18070e022acdc5 100644 --- a/components/sync/driver/profile_sync_service_unittest.cc +++ b/components/sync/driver/profile_sync_service_unittest.cc @@ -229,7 +229,7 @@ class ProfileSyncServiceTest : public ::testing::Test { service_.reset(); } - void InitializeForNthSync() { + void PopulatePrefsForNthSync() { // Set first sync time before initialize to simulate a complete sync setup. SyncPrefs sync_prefs(prefs()); sync_prefs.SetCacheGuid(kTestCacheGuid); @@ -241,6 +241,10 @@ class ProfileSyncServiceTest : public ::testing::Test { /*registered_types=*/UserSelectableTypeSet::All(), /*selected_types=*/UserSelectableTypeSet::All()); sync_prefs.SetFirstSetupComplete(); + } + + void InitializeForNthSync() { + PopulatePrefsForNthSync(); service_->Initialize(); } @@ -1564,5 +1568,53 @@ TEST_F(ProfileSyncServiceTestWithStopSyncInPausedState, EXPECT_FALSE(service()->GetDisableReasons().Empty()); } +TEST_F(ProfileSyncServiceTest, ShouldPopulateAccountIdCachedInPrefs) { + SyncPrefs sync_prefs(prefs()); + + SignIn(); + CreateService(ProfileSyncService::AUTO_START); + InitializeForNthSync(); + ASSERT_EQ(SyncService::TransportState::ACTIVE, + service()->GetTransportState()); + ASSERT_EQ(kTestCacheGuid, sync_prefs.GetCacheGuid()); + EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestUser), sync_prefs.GetGaiaId()); +} + +TEST_F(ProfileSyncServiceTest, + ShouldNotPopulateAccountIdCachedInPrefsWithLocalSync) { + SyncPrefs sync_prefs(prefs()); + + SignIn(); + CreateServiceWithLocalSyncBackend(); + InitializeForNthSync(); + ASSERT_EQ(SyncService::TransportState::ACTIVE, + service()->GetTransportState()); + ASSERT_EQ(kTestCacheGuid, sync_prefs.GetCacheGuid()); + EXPECT_TRUE(sync_prefs.GetGaiaId().empty()); +} + +// Verifies that local sync transport data is thrown away if there is a mismatch +// between the account ID cached in SyncPrefs and the actual one. +TEST_F(ProfileSyncServiceTest, + ShouldClearLocalSyncTransportDataDueToAccountIdMismatch) { + SyncPrefs sync_prefs(prefs()); + + SignIn(); + CreateService(ProfileSyncService::AUTO_START); + PopulatePrefsForNthSync(); + ASSERT_EQ(kTestCacheGuid, sync_prefs.GetCacheGuid()); + + // Manually override the authenticated account ID, which should be detected + // during initialization. + sync_prefs.SetGaiaId("corrupt_gaia_id"); + + service()->Initialize(); + + ASSERT_EQ(SyncService::TransportState::ACTIVE, + service()->GetTransportState()); + EXPECT_NE(kTestCacheGuid, sync_prefs.GetCacheGuid()); + EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestUser), sync_prefs.GetGaiaId()); +} + } // namespace } // namespace syncer