[sync] Clear gaia-keyed custom passphrase pref
A new gaia-keyed pref for custom passphrase was introduced in
crrev.com/c/5237310 for signed-in non-syncing users on all platforms.
This CL:
* updates KeepAccountSettingsPrefsOnlyForUsers to also clear the new
per-account passphrase pref.
* renames chrome_test_util::ResetSyncSelectedDataTypes to
ResetSyncAccountSettingsPrefs to better reflect its usage
* adds corresponding tests in SyncPrefs, SyncUserSettingsImpl, and
ManageSynSettings egtest.
* not related to this bug: remove unused check in
testAccountSettingsWithRemoteSignout.
Bug: 1471928
Change-Id: Id930b60534c9848175b5077f5157cf8b4246a41b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5250142
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Victor Vianna <victorvianna@google.com>
Commit-Queue: Jood Hajeer <jood@google.com>
Cr-Commit-Position: refs/heads/main@{#1255528}
diff --git a/components/sync/service/sync_prefs.cc b/components/sync/service/sync_prefs.cc
index 58c9461..62b4d42 100644
--- a/components/sync/service/sync_prefs.cc
+++ b/components/sync/service/sync_prefs.cc
@@ -404,24 +404,11 @@
void SyncPrefs::KeepAccountSettingsPrefsOnlyForUsers(
const std::vector<signin::GaiaIdHash>& available_gaia_ids) {
- std::vector<std::string> removed_identities;
- for (std::pair<const std::string&, const base::Value&> account_settings :
- pref_service_->GetDict(prefs::internal::kSelectedTypesPerAccount)) {
- if (!base::Contains(available_gaia_ids, signin::GaiaIdHash::FromBase64(
- account_settings.first))) {
- removed_identities.push_back(account_settings.first);
- }
- }
- if (!removed_identities.empty()) {
- {
- ScopedDictPrefUpdate update_selected_types_dict(
- pref_service_, prefs::internal::kSelectedTypesPerAccount);
- base::Value::Dict& all_accounts = update_selected_types_dict.Get();
- for (const auto& account_id : removed_identities) {
- all_accounts.Remove(account_id);
- }
- }
- }
+ KeepAccountSettingsPrefsOnlyForUsers(
+ available_gaia_ids, prefs::internal::kSelectedTypesPerAccount);
+ KeepAccountSettingsPrefsOnlyForUsers(
+ available_gaia_ids,
+ prefs::internal::kSyncEncryptionBootstrapTokenPerAccount);
}
#if BUILDFLAG(IS_IOS)
@@ -752,6 +739,29 @@
}
#endif // !BUILDFLAG(IS_CHROMEOS_ASH)
+void SyncPrefs::KeepAccountSettingsPrefsOnlyForUsers(
+ const std::vector<signin::GaiaIdHash>& available_gaia_ids,
+ const char* pref_path) {
+ std::vector<std::string> removed_identities;
+ for (std::pair<const std::string&, const base::Value&> account_settings :
+ pref_service_->GetDict(pref_path)) {
+ if (!base::Contains(available_gaia_ids, signin::GaiaIdHash::FromBase64(
+ account_settings.first))) {
+ removed_identities.push_back(account_settings.first);
+ }
+ }
+ if (!removed_identities.empty()) {
+ {
+ ScopedDictPrefUpdate update_account_settings_dict(pref_service_,
+ pref_path);
+ base::Value::Dict& all_accounts = update_account_settings_dict.Get();
+ for (const auto& account_id : removed_identities) {
+ all_accounts.Remove(account_id);
+ }
+ }
+ }
+}
+
// static
void SyncPrefs::RegisterTypeSelectedPref(PrefRegistrySimple* registry,
UserSelectableType type) {
diff --git a/components/sync/service/sync_prefs.h b/components/sync/service/sync_prefs.h
index a029944..d872ffdd 100644
--- a/components/sync/service/sync_prefs.h
+++ b/components/sync/service/sync_prefs.h
@@ -129,6 +129,7 @@
void SetSelectedTypeForAccount(UserSelectableType type,
bool is_type_on,
const signin::GaiaIdHash& gaia_id_hash);
+
// Used to clear per account prefs for all users *except* the ones in the
// passed-in |available_gaia_ids|.
void KeepAccountSettingsPrefsOnlyForUsers(
@@ -291,6 +292,10 @@
void OnFirstSetupCompletePrefChange();
#endif // !BUILDFLAG(IS_CHROMEOS_ASH)
+ void KeepAccountSettingsPrefsOnlyForUsers(
+ const std::vector<signin::GaiaIdHash>& available_gaia_ids,
+ const char* pref_path);
+
// Never null.
const raw_ptr<PrefService> pref_service_;
diff --git a/components/sync/service/sync_prefs_unittest.cc b/components/sync/service/sync_prefs_unittest.cc
index 6f450a31..88c4f08 100644
--- a/components/sync/service/sync_prefs_unittest.cc
+++ b/components/sync/service/sync_prefs_unittest.cc
@@ -83,6 +83,30 @@
sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_2));
}
+TEST_F(SyncPrefsTest, ClearEncryptionBootstrapTokenPerAccount) {
+ ASSERT_TRUE(sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_)
+ .empty());
+ sync_prefs_->SetEncryptionBootstrapTokenForAccount("token", gaia_id_hash_);
+ EXPECT_EQ("token",
+ sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_));
+ auto gaia_id_hash_2 = signin::GaiaIdHash::FromGaiaId("account_gaia_2");
+ EXPECT_TRUE(sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_2)
+ .empty());
+ sync_prefs_->SetEncryptionBootstrapTokenForAccount("token2", gaia_id_hash_2);
+ EXPECT_EQ("token",
+ sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_));
+ EXPECT_EQ("token2",
+ sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_2));
+ // Remove account 2 from device by setting the available_gaia_ids to have the
+ // gaia id of account 1 only.
+ sync_prefs_->KeepAccountSettingsPrefsOnlyForUsers(
+ /*available_gaia_ids=*/{gaia_id_hash_});
+ EXPECT_EQ("token",
+ sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_));
+ EXPECT_TRUE(sync_prefs_->GetEncryptionBootstrapTokenForAccount(gaia_id_hash_2)
+ .empty());
+}
+
TEST_F(SyncPrefsTest, CachedPassphraseType) {
EXPECT_FALSE(sync_prefs_->GetCachedPassphraseType().has_value());
diff --git a/components/sync/service/sync_service_impl.cc b/components/sync/service/sync_service_impl.cc
index 9c38483..9cf34969 100644
--- a/components/sync/service/sync_service_impl.cc
+++ b/components/sync/service/sync_service_impl.cc
@@ -2173,6 +2173,9 @@
sync_prefs_.ClearPassphrasePromptMutedProductVersion();
// The passphrase type is now undefined again.
sync_prefs_.ClearCachedPassphraseType();
+ // TODO(crbug.com/1471928): Update comment to specify that
+ // *EncryptionBootstrapToken will be used for syncing users only, when
+ // kSyncRememberCustomPassphraseAfterSignout is fully rolled-out.
// For explicit passphrase users, clear the encryption key, such that they
// will need to reenter it if sync gets re-enabled.
sync_prefs_.ClearEncryptionBootstrapToken();
diff --git a/components/sync/service/sync_user_settings_impl_unittest.cc b/components/sync/service/sync_user_settings_impl_unittest.cc
index 56c738e..f5a7784 100644
--- a/components/sync/service/sync_user_settings_impl_unittest.cc
+++ b/components/sync/service/sync_user_settings_impl_unittest.cc
@@ -635,6 +635,22 @@
EXPECT_TRUE(sync_user_settings->GetEncryptionBootstrapToken().empty());
}
+TEST_F(SyncUserSettingsImplTest, ClearEncryptionBootstrapTokenPerAccount) {
+ base::test::ScopedFeatureList enable_keep_account_passphrase(
+ kSyncRememberCustomPassphraseAfterSignout);
+ SetSyncAccountState(SyncPrefs::SyncAccountState::kSignedInNotSyncing);
+ std::unique_ptr<SyncUserSettingsImpl> sync_user_settings =
+ MakeSyncUserSettings(GetUserTypes());
+ ASSERT_TRUE(sync_user_settings->GetEncryptionBootstrapToken().empty());
+ sync_user_settings->SetEncryptionBootstrapToken("token");
+ signin::GaiaIdHash gaia_id_hash =
+ signin::GaiaIdHash::FromGaiaId(GetSyncAccountInfoForPrefs().gaia);
+ sync_user_settings->KeepAccountSettingsPrefsOnlyForUsers({gaia_id_hash});
+ EXPECT_EQ("token", sync_user_settings->GetEncryptionBootstrapToken());
+ sync_user_settings->KeepAccountSettingsPrefsOnlyForUsers({});
+ EXPECT_TRUE(sync_user_settings->GetEncryptionBootstrapToken().empty());
+}
+
} // namespace
} // namespace syncer
diff --git a/ios/chrome/browser/ui/settings/google_services/manage_sync_settings_egtest.mm b/ios/chrome/browser/ui/settings/google_services/manage_sync_settings_egtest.mm
index b0e4818..b837861 100644
--- a/ios/chrome/browser/ui/settings/google_services/manage_sync_settings_egtest.mm
+++ b/ios/chrome/browser/ui/settings/google_services/manage_sync_settings_egtest.mm
@@ -183,6 +183,11 @@
config.features_enabled.push_back(
syncer::kSyncEnableContactInfoDataTypeInTransportMode);
}
+ if ([self
+ isRunningTest:@selector(testRememberCustomPassphraseAfterSignout)]) {
+ config.features_enabled.push_back(
+ syncer::kSyncRememberCustomPassphraseAfterSignout);
+ }
return config;
}
@@ -743,7 +748,7 @@
}
// Tests closing the account settings with a remote signout.
-- (void)testAccountSettingsWithRemoteSignout_SyncToSigninEnabled {
+- (void)testAccountSettingsWithRemoteSignout {
FakeSystemIdentity* fakeIdentity = [FakeSystemIdentity fakeIdentity1];
[SigninEarlGrey addFakeIdentity:fakeIdentity];
@@ -760,12 +765,6 @@
[[EarlGrey
selectElementWithMatcher:chrome_test_util::SettingsCollectionView()]
assertWithMatcher:grey_notNil()];
-
- // Verify the error section is not showing.
- [[EarlGrey selectElementWithMatcher:
- grey_accessibilityLabel(l10n_util::GetNSString(
- IDS_IOS_ACCOUNT_TABLE_ERROR_ENTER_PASSPHRASE_BUTTON))]
- assertWithMatcher:grey_notVisible()];
}
// Tests that the batch upload button description in the account settings
@@ -1411,4 +1410,85 @@
assertWithMatcher:grey_notNil()];
}
+// Tests the custom passphrase is remembered per account, kept across signout,
+// and cleared when account is removed from device.
+- (void)testRememberCustomPassphraseAfterSignout {
+ // Enable custom passphrase.
+ [ChromeEarlGrey addBookmarkWithSyncPassphrase:kPassphrase];
+ FakeSystemIdentity* fakeIdentity = [FakeSystemIdentity fakeIdentity1];
+ [SigninEarlGrey addFakeIdentity:fakeIdentity];
+
+ [SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity];
+ [ChromeEarlGreyUI openSettingsMenu];
+ [ChromeEarlGreyUI tapSettingsMenuButton:SettingsAccountButton()];
+
+ // Verify the error section is showing.
+ [[EarlGrey selectElementWithMatcher:
+ grey_accessibilityLabel(l10n_util::GetNSString(
+ IDS_IOS_ACCOUNT_TABLE_ERROR_ENTER_PASSPHRASE_BUTTON))]
+ assertWithMatcher:grey_sufficientlyVisible()];
+
+ // Tap "Enter Passphrase" button.
+ [[EarlGrey selectElementWithMatcher:
+ grey_accessibilityLabel(l10n_util::GetNSString(
+ IDS_IOS_ACCOUNT_TABLE_ERROR_ENTER_PASSPHRASE_BUTTON))]
+ performAction:grey_tap()];
+
+ // Enter the passphrase.
+ [SigninEarlGreyUI submitSyncPassphrase:kPassphrase];
+
+ // Verify it goes back to "manage sync" UI.
+ [[EarlGrey
+ selectElementWithMatcher:grey_accessibilityID(
+ kManageSyncTableViewAccessibilityIdentifier)]
+ assertWithMatcher:grey_sufficientlyVisible()];
+
+ // Verify the error section is not showing anymore.
+ [[EarlGrey selectElementWithMatcher:
+ grey_accessibilityLabel(l10n_util::GetNSString(
+ IDS_IOS_ACCOUNT_TABLE_ERROR_ENTER_PASSPHRASE_BUTTON))]
+ assertWithMatcher:grey_notVisible()];
+
+ // Sign out.
+ SignOutFromAccountSettings();
+ DismissSignOutSnackbar();
+ [ChromeEarlGreyUI waitForAppToIdle];
+ [SigninEarlGrey verifySignedOut];
+
+ // Sign back in with the same identity using the settings sign-in promo.
+ // The history sync opt-in was accepted in the first sign-in earlier in this
+ // test.
+ SignInWithPromoFromAccountSettings(fakeIdentity, /*expect_history_sync=*/NO);
+
+ // Verify the account settings row is showing in the settings menu.
+ [[EarlGrey selectElementWithMatcher:SettingsAccountButton()]
+ assertWithMatcher:grey_sufficientlyVisible()];
+ [ChromeEarlGreyUI tapSettingsMenuButton:SettingsAccountButton()];
+
+ // Verify the "enter passphrase" error section is not showing.
+ [[EarlGrey selectElementWithMatcher:
+ grey_accessibilityLabel(l10n_util::GetNSString(
+ IDS_IOS_ACCOUNT_TABLE_ERROR_ENTER_PASSPHRASE_BUTTON))]
+ assertWithMatcher:grey_notVisible()];
+
+ // Remove fakeIdentity from device.
+ [SigninEarlGrey forgetFakeIdentity:fakeIdentity];
+ [ChromeEarlGreyUI waitForAppToIdle];
+ [[EarlGrey selectElementWithMatcher:SettingsDoneButton()]
+ performAction:grey_tap()];
+
+ // Sign back in.
+ [SigninEarlGrey addFakeIdentity:fakeIdentity];
+
+ [SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity];
+ [ChromeEarlGreyUI openSettingsMenu];
+ [ChromeEarlGreyUI tapSettingsMenuButton:SettingsAccountButton()];
+
+ // Verify the error section is showing because the passphrase was cleared.
+ [[EarlGrey selectElementWithMatcher:
+ grey_accessibilityLabel(l10n_util::GetNSString(
+ IDS_IOS_ACCOUNT_TABLE_ERROR_ENTER_PASSPHRASE_BUTTON))]
+ assertWithMatcher:grey_sufficientlyVisible()];
+}
+
@end
diff --git a/ios/chrome/test/app/signin_test_util.h b/ios/chrome/test/app/signin_test_util.h
index c5459ed..1bdcefc 100644
--- a/ios/chrome/test/app/signin_test_util.h
+++ b/ios/chrome/test/app/signin_test_util.h
@@ -39,8 +39,9 @@
// Resets all preferences related to History Sync Opt-In.
void ResetHistorySyncPreferencesForTesting();
-// Resets all the selected data types to be turned on in the sync engine.
-void ResetSyncSelectedDataTypes();
+// Resets all the selected data types to be turned on in the sync engine. And
+// clear per-account passphrases.
+void ResetSyncAccountSettingsPrefs();
} // namespace chrome_test_util
diff --git a/ios/chrome/test/app/signin_test_util.mm b/ios/chrome/test/app/signin_test_util.mm
index a356c77..ec07e6f7 100644
--- a/ios/chrome/test/app/signin_test_util.mm
+++ b/ios/chrome/test/app/signin_test_util.mm
@@ -181,10 +181,10 @@
history_sync::ResetDeclinePrefs(prefs);
}
-void ResetSyncSelectedDataTypes() {
+void ResetSyncAccountSettingsPrefs() {
ChromeBrowserState* browser_state =
chrome_test_util::GetOriginalBrowserState();
- // Clear the new per-account selected types.
+ // Clear the new per-account selected types and per-account passphrase.
SyncServiceFactory::GetForBrowserState(browser_state)
->GetUserSettings()
->KeepAccountSettingsPrefsOnlyForUsers({});
diff --git a/ios/chrome/test/earl_grey/chrome_test_case_app_interface.mm b/ios/chrome/test/earl_grey/chrome_test_case_app_interface.mm
index 7168b5d..909f140 100644
--- a/ios/chrome/test/earl_grey/chrome_test_case_app_interface.mm
+++ b/ios/chrome/test/earl_grey/chrome_test_case_app_interface.mm
@@ -31,7 +31,7 @@
+ (void)resetAuthentication {
chrome_test_util::ResetSigninPromoPreferences();
chrome_test_util::ResetMockAuthentication();
- chrome_test_util::ResetSyncSelectedDataTypes();
+ chrome_test_util::ResetSyncAccountSettingsPrefs();
chrome_test_util::ResetHistorySyncPreferencesForTesting();
}