[sync] Rewrite setSyncEncryption() chrome://settings JS API
This API was called when the user created a new sync encryption passphrase
or entered an existing one. This CL improves both its semantics and
implementation.
- Instead of having a set_new_passphrase flag to distinguish new/existing
passphrases, the API is split in set[Encryption/Decryption]Passphrase().
The implementations are mostly disjoint and there's a clear separation
now (the old code would even ignore set_new_passphrase sometimes). This
also mimics the signatures from other layers.
- As per the linked bug, the APIs now receive only what's necessary (the
passphrase string) instead of an entire SyncPrefs object.
- Following a suggestion from an old TODO [1], the "return values" are
now simply whether the passphrase was successfully set or not, instead
of returning what the behavior of the UI should be (PageStatus). This
makes them less coupled with the existing callers.
- The Profile.SyncCustomize histogram is now recorded closer to where the
passphrase updates happen. This may ultimately cause changes in the
metric, but at least something meaningful will be recorded now. Seems the
histogram wasn't actively watched anyways (it expired twice). The obscure
buckets SYNC_ENCRYPT and SYNC_PASSPHRASE are also respectively renamed to
SYNC_CREATED_NEW_PASSPHRASE and SYNC_ENTERED_EXISTING_PASSPHRASE.
[1] https://source.chromium.org/chromium/chromium/src/+/b44f7fc1be34f23c0e052cac0554675eb34b2272:chrome/browser/ui/webui/settings/people_handler.cc;l=593
Change-Id: Id820d08c89fb5d79ffbc62ff9c4f6534c24df4cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2556961
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Monica Basta <msalama@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831393}
diff --git a/chrome/browser/profiles/profile_metrics.h b/chrome/browser/profiles/profile_metrics.h
index 044deee..440fc1c 100644
--- a/chrome/browser/profiles/profile_metrics.h
+++ b/chrome/browser/profiles/profile_metrics.h
@@ -79,10 +79,10 @@
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum ProfileSync {
- SYNC_CUSTOMIZE = 0, // User decided to customize sync
- SYNC_CHOOSE, // User chose what to sync
- SYNC_ENCRYPT, // User has chosen to encrypt all data
- SYNC_PASSPHRASE, // User is using a passphrase
+ SYNC_CUSTOMIZE = 0, // User decided to customize sync
+ SYNC_CHOOSE, // User chose what to sync
+ SYNC_CREATED_NEW_PASSPHRASE, // User created a passphrase to encrypt data
+ SYNC_ENTERED_EXISTING_PASSPHRASE, // User entered an existing passphrase
NUM_PROFILE_SYNC_METRICS
};
diff --git a/chrome/browser/resources/settings/people_page/sync_browser_proxy.js b/chrome/browser/resources/settings/people_page/sync_browser_proxy.js
index eb9e0ef99..a3f55743 100644
--- a/chrome/browser/resources/settings/people_page/sync_browser_proxy.js
+++ b/chrome/browser/resources/settings/people_page/sync_browser_proxy.js
@@ -72,7 +72,6 @@
* extensionsRegistered: boolean,
* extensionsSynced: boolean,
* fullEncryptionBody: string,
- * passphrase: (string|undefined),
* passphraseRequired: boolean,
* passwordsRegistered: boolean,
* passwordsSynced: boolean,
@@ -81,7 +80,6 @@
* preferencesSynced: boolean,
* readingListRegistered: boolean,
* readingListSynced: boolean,
- * setNewPassphrase: (boolean|undefined),
* syncAllDataTypes: boolean,
* tabsRegistered: boolean,
* tabsSynced: boolean,
@@ -198,13 +196,20 @@
setSyncDatatypes(syncPrefs) {}
/**
- * Sets the sync encryption options.
- * @param {!settings.SyncPrefs} syncPrefs
- * @return {!Promise<!settings.PageStatus>}
+ * Attempts to set up a new passphrase to encrypt Sync data.
+ * @param {string} passphrase
+ * @return {!Promise<boolean>} Whether the passphrase was successfully set.
+ * The call can fail, for example, if encrypting the data is disallowed.
*/
- // TODO(crbug.com/1139060): Use a clear signature which doesn't rely on
- // syncPrefs.
- setSyncEncryption(syncPrefs) {}
+ setEncryptionPassphrase(passphrase) {}
+
+ /**
+ * Attempts to set the passphrase to decrypt Sync data.
+ * @param {string} passphrase
+ * @return {!Promise<boolean>} Whether the passphrase was successfully set.
+ * The call can fail, for example, if the passphrase is incorrect.
+ */
+ setDecryptionPassphrase(passphrase) {}
/**
* Start syncing with an account, specified by its email.
@@ -312,9 +317,13 @@
}
/** @override */
- setSyncEncryption(syncPrefs) {
- return cr.sendWithPromise(
- 'SyncSetupSetEncryption', JSON.stringify(syncPrefs));
+ setEncryptionPassphrase(passphrase) {
+ return cr.sendWithPromise('SyncSetupSetEncryptionPassphrase', passphrase);
+ }
+
+ /** @override */
+ setDecryptionPassphrase(passphrase) {
+ return cr.sendWithPromise('SyncSetupSetDecryptionPassphrase', passphrase);
}
/** @override */
diff --git a/chrome/browser/resources/settings/people_page/sync_encryption_options.js b/chrome/browser/resources/settings/people_page/sync_encryption_options.js
index 98d8f48..0708d6c 100644
--- a/chrome/browser/resources/settings/people_page/sync_encryption_options.js
+++ b/chrome/browser/resources/settings/people_page/sync_encryption_options.js
@@ -68,6 +68,13 @@
},
/**
+ * Whether there's a setEncryptionPassphrase() call pending response, in which
+ * case the component should wait before making a new call.
+ * @private {boolean}
+ */
+ isSettingEncryptionPassphrase_: false,
+
+ /**
* Returns the encryption options CrRadioGroupElement.
* @return {?CrRadioGroupElement}
*/
@@ -137,24 +144,26 @@
saveNewPassphrase_() {
assert(this.creatingNewPassphrase_);
chrome.metricsPrivate.recordUserAction('Sync_SaveNewPassphraseClicked');
- // Might happen within the transient time between the request to
- // |setSyncEncryption| and receiving the response.
- if (this.syncPrefs.setNewPassphrase) {
+
+ if (this.isSettingEncryptionPassphrase_) {
return;
}
+
// If a new password has been entered but it is invalid, do not send the
// sync state to the API.
if (!this.validateCreatedPassphrases_()) {
return;
}
- this.syncPrefs.setNewPassphrase = true;
- this.syncPrefs.passphrase = this.passphrase_;
-
+ this.isSettingEncryptionPassphrase_ = true;
settings.SyncBrowserProxyImpl.getInstance()
- .setSyncEncryption(this.syncPrefs)
- .then(pageStatus => {
- this.fire('passphrase-changed', pageStatus);
+ .setEncryptionPassphrase(this.passphrase_)
+ .then(successfullySet => {
+ // TODO(crbug.com/1139060): Rename the event, there is no change if
+ // |successfullySet| is false. It should also mention 'encryption
+ // passphrase' in its name.
+ this.fire('passphrase-changed', {didChange: successfullySet});
+ this.isSettingEncryptionPassphrase_ = false;
});
},
diff --git a/chrome/browser/resources/settings/people_page/sync_page.js b/chrome/browser/resources/settings/people_page/sync_page.js
index 6469b5f..f31b881 100644
--- a/chrome/browser/resources/settings/people_page/sync_page.js
+++ b/chrome/browser/resources/settings/people_page/sync_page.js
@@ -491,22 +491,23 @@
return;
}
- this.syncPrefs.setNewPassphrase = false;
+ this.browserProxy_.setDecryptionPassphrase(this.existingPassphrase_)
+ .then(
+ sucessfullySet => this.handlePageStatusChanged_(
+ sucessfullySet ? settings.PageStatus.DONE :
+ settings.PageStatus.PASSPHRASE_FAILED));
- this.syncPrefs.passphrase = this.existingPassphrase_;
this.existingPassphrase_ = '';
-
- this.browserProxy_.setSyncEncryption(this.syncPrefs)
- .then(this.handlePageStatusChanged_.bind(this));
},
/**
* @private
- * @param {!CustomEvent<!settings.PageStatus>} e
+ * @param {!CustomEvent<!{didChange: boolean}>} e
*/
onPassphraseChanged_(e) {
this.handlePageStatusChanged_(
- /** @type {!settings.PageStatus} */ (e.detail));
+ e.detail.didChange ? settings.PageStatus.DONE :
+ settings.PageStatus.PASSPHRASE_FAILED);
},
/**
diff --git a/chrome/browser/ui/webui/settings/people_handler.cc b/chrome/browser/ui/webui/settings/people_handler.cc
index 66cfbd2b..b31cbad 100644
--- a/chrome/browser/ui/webui/settings/people_handler.cc
+++ b/chrome/browser/ui/webui/settings/people_handler.cc
@@ -84,8 +84,6 @@
bool sync_everything;
syncer::UserSelectableTypeSet selected_types;
bool payments_integration_enabled;
- std::string passphrase;
- bool set_new_passphrase;
};
bool IsSyncSubpage(const GURL& current_url) {
@@ -93,9 +91,7 @@
}
SyncConfigInfo::SyncConfigInfo()
- : sync_everything(false),
- payments_integration_enabled(false),
- set_new_passphrase(false) {}
+ : sync_everything(false), payments_integration_enabled(false) {}
SyncConfigInfo::~SyncConfigInfo() {}
@@ -132,13 +128,6 @@
config->selected_types.Put(type);
}
- // Passphrase settings.
- if (result->GetString("passphrase", &config->passphrase) &&
- !config->passphrase.empty() &&
- !result->GetBoolean("setNewPassphrase", &config->set_new_passphrase)) {
- DLOG(ERROR) << "GetConfiguration() not passed a set_new_passphrase value";
- return false;
- }
return true;
}
@@ -276,8 +265,12 @@
base::BindRepeating(&PeopleHandler::HandleSetDatatypes,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
- "SyncSetupSetEncryption",
- base::BindRepeating(&PeopleHandler::HandleSetEncryption,
+ "SyncSetupSetEncryptionPassphrase",
+ base::BindRepeating(&PeopleHandler::HandleSetEncryptionPassphrase,
+ base::Unretained(this)));
+ web_ui()->RegisterMessageCallback(
+ "SyncSetupSetDecryptionPassphrase",
+ base::BindRepeating(&PeopleHandler::HandleSetDecryptionPassphrase,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"SyncSetupShowSetupUI",
@@ -527,79 +520,72 @@
#endif
}
-void PeopleHandler::HandleSetEncryption(const base::ListValue* args) {
- SyncConfigInfo configuration;
- const base::Value* callback_id = nullptr;
- ParseConfigurationArguments(args, &configuration, &callback_id);
+void PeopleHandler::HandleSetEncryptionPassphrase(const base::ListValue* args) {
+ const base::Value& callback_id = args->GetList()[0];
- // Start configuring the SyncService using the configuration passed to us from
- // the JS layer.
- syncer::SyncService* service = GetSyncService();
-
- // If the sync engine has shutdown for some reason, just close the sync
- // dialog.
- if (!service || !service->IsEngineInitialized()) {
+ // Check the SyncService is up and running before retrieving SyncUserSettings,
+ // which contains the encryption-related APIs.
+ if (!GetSyncService() || !GetSyncService()->IsEngineInitialized()) {
+ // TODO(crbug.com/1139060): HandleSetDatatypes() also returns a success
+ // status in this case. Consider returning a failure in both methods. Maybe
+ // the CloseSyncSetup() call can also be removed.
CloseSyncSetup();
- ResolveJavascriptCallback(*callback_id, base::Value(kDonePageStatus));
+ ResolveJavascriptCallback(callback_id, base::Value(true));
return;
}
+ syncer::SyncUserSettings* sync_user_settings =
+ GetSyncService()->GetUserSettings();
- if (service->GetUserSettings()->IsEncryptEverythingAllowed()) {
- ProfileMetrics::LogProfileSyncInfo(ProfileMetrics::SYNC_ENCRYPT);
+ const std::string& passphrase = args->GetList()[1].GetString();
+ bool successfully_set = false;
+ if (passphrase.empty()) {
+ successfully_set = false;
+ } else if (!sync_user_settings->IsEncryptEverythingAllowed()) {
+ successfully_set = false;
+ } else if (sync_user_settings->IsUsingSecondaryPassphrase()) {
+ // In case a passphrase is already being used, changing to a new one isn't
+ // currently supported (one must reset all the Sync data).
+ successfully_set = false;
+ } else if (sync_user_settings->IsPassphraseRequired() ||
+ sync_user_settings->IsTrustedVaultKeyRequired()) {
+ // Can't re-encrypt the data with |passphrase| if some of it hasn't even
+ // been decrypted yet due to a pending passphrase / trusted vault key.
+ successfully_set = false;
} else {
- // Don't allow "set new passphrase" if the SyncService doesn't allow it.
- // The UI is hidden, but the user may have enabled it e.g. by fiddling with
- // the web inspector.
- configuration.set_new_passphrase = false;
+ sync_user_settings->SetEncryptionPassphrase(passphrase);
+ successfully_set = true;
+ ProfileMetrics::LogProfileSyncInfo(
+ ProfileMetrics::SYNC_CREATED_NEW_PASSPHRASE);
}
+ ResolveJavascriptCallback(callback_id, base::Value(successfully_set));
+}
- bool passphrase_failed = false;
- if (!configuration.passphrase.empty()) {
- // We call IsPassphraseRequired() here (instead of
- // IsPassphraseRequiredForPreferredDataTypes()) because the user may try to
- // enter a passphrase even though no encrypted data types are enabled.
- if (service->GetUserSettings()->IsPassphraseRequired()) {
- // If we have pending keys, try to decrypt them with the provided
- // passphrase. We track if this succeeds or fails because a failed
- // decryption should result in an error even if there aren't any encrypted
- // data types.
- passphrase_failed = !service->GetUserSettings()->SetDecryptionPassphrase(
- configuration.passphrase);
- } else if (service->GetUserSettings()->IsTrustedVaultKeyRequired()) {
- // There are pending keys due to trusted vault keys being required, likely
- // because something changed since the UI was displayed. A passphrase
- // cannot be set in such circumstances.
- passphrase_failed = true;
- } else {
- // OK, the user sent us a passphrase, but we don't have pending keys. So
- // it either means that the pending keys were resolved somehow since the
- // time the UI was displayed (re-encryption, pending passphrase change,
- // etc) or the user wants to re-encrypt.
- if (configuration.set_new_passphrase &&
- !service->GetUserSettings()->IsUsingSecondaryPassphrase()) {
- service->GetUserSettings()->SetEncryptionPassphrase(
- configuration.passphrase);
- }
+void PeopleHandler::HandleSetDecryptionPassphrase(const base::ListValue* args) {
+ const base::Value& callback_id = args->GetList()[0];
+
+ // Check the SyncService is up and running before retrieving SyncUserSettings,
+ // which contains the encryption-related APIs.
+ if (!GetSyncService() || !GetSyncService()->IsEngineInitialized()) {
+ // TODO(crbug.com/1139060): HandleSetDatatypes() also returns a success
+ // status in this case. Consider returning a failure in both methods. Maybe
+ // the CloseSyncSetup() call can also be removed.
+ CloseSyncSetup();
+ ResolveJavascriptCallback(callback_id, base::Value(true));
+ return;
+ }
+ syncer::SyncUserSettings* sync_user_settings =
+ GetSyncService()->GetUserSettings();
+
+ const std::string& passphrase = args->GetList()[1].GetString();
+ bool successfully_set = false;
+ if (!passphrase.empty() && sync_user_settings->IsPassphraseRequired()) {
+ successfully_set = sync_user_settings->SetDecryptionPassphrase(passphrase);
+ if (successfully_set) {
+ ProfileMetrics::LogProfileSyncInfo(
+ ProfileMetrics::SYNC_ENTERED_EXISTING_PASSPHRASE);
}
}
-
- if (passphrase_failed ||
- service->GetUserSettings()->IsPassphraseRequiredForPreferredDataTypes()) {
- // If the user doesn't enter any passphrase, we won't call
- // SetDecryptionPassphrase() (passphrase_failed == false), but we still
- // want to display an error message to let the user know that their blank
- // passphrase entry is not acceptable.
-
- // TODO(tommycli): Switch this to RejectJavascriptCallback once the
- // Sync page JavaScript has been further refactored.
- ResolveJavascriptCallback(*callback_id,
- base::Value(kPassphraseFailedPageStatus));
- } else {
- ResolveJavascriptCallback(*callback_id, base::Value(kConfigurePageStatus));
- }
-
- if (!configuration.set_new_passphrase && !configuration.passphrase.empty())
- ProfileMetrics::LogProfileSyncInfo(ProfileMetrics::SYNC_PASSPHRASE);
+ ResolveJavascriptCallback(callback_id, base::Value(successfully_set));
}
void PeopleHandler::HandleShowSyncSetupUI(const base::ListValue* args) {
diff --git a/chrome/browser/ui/webui/settings/people_handler.h b/chrome/browser/ui/webui/settings/people_handler.h
index b6013d93..c50237f 100644
--- a/chrome/browser/ui/webui/settings/people_handler.h
+++ b/chrome/browser/ui/webui/settings/people_handler.h
@@ -87,14 +87,16 @@
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, TestSyncEverything);
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, TestSyncAllManually);
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, NonRegisteredType);
- FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, TestPassphraseStillRequired);
+ FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, EnterCorrectExistingPassphrase);
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, TestSyncIndividualTypes);
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest,
- EnterExistingFrozenImplicitPassword);
- FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, SetNewCustomPassphrase);
+ SuccessfullyCreateCustomPassphrase);
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, EnterWrongExistingPassphrase);
- FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, EnterBlankExistingPassphrase);
- FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, TurnOnEncryptAllDisallowed);
+ FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, CannotCreateBlankPassphrase);
+ FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest,
+ CannotCreatePassphraseIfEncryptEverythingDisallowed);
+ FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest,
+ CannotOverwritePassphraseWithNewOne);
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest,
UnrecoverableErrorInitializingSync);
FRIEND_TEST_ALL_PREFIXES(PeopleHandlerTest, GaiaErrorInitializingSync);
@@ -152,7 +154,8 @@
void HandleGetProfileInfo(const base::ListValue* args);
void OnDidClosePage(const base::ListValue* args);
void HandleSetDatatypes(const base::ListValue* args);
- void HandleSetEncryption(const base::ListValue* args);
+ void HandleSetEncryptionPassphrase(const base::ListValue* args);
+ void HandleSetDecryptionPassphrase(const base::ListValue* args);
void HandleShowSyncSetupUI(const base::ListValue* args);
void HandleSyncPrefsDispatch(const base::ListValue* args);
#if BUILDFLAG(IS_CHROMEOS_ASH)
diff --git a/chrome/browser/ui/webui/settings/people_handler_unittest.cc b/chrome/browser/ui/webui/settings/people_handler_unittest.cc
index ccc0fcaf..1700d9e7 100644
--- a/chrome/browser/ui/webui/settings/people_handler_unittest.cc
+++ b/chrome/browser/ui/webui/settings/people_handler_unittest.cc
@@ -78,26 +78,12 @@
CHOOSE_WHAT_TO_SYNC
};
-enum EncryptAllConfig {
- ENCRYPT_ALL_DATA,
- ENCRYPT_PASSWORDS
-};
-
// Create a json-format string with the key/value pairs appropriate for a call
-// to HandleSetEncryption(). If |extra_values| is non-null, then the values from
-// the passed dictionary are added to the json.
-std::string GetConfiguration(const base::DictionaryValue* extra_values,
- SyncAllDataConfig sync_all,
- syncer::UserSelectableTypeSet types,
- const std::string& passphrase,
- EncryptAllConfig encrypt_all) {
+// to HandleSetDatatypes().
+std::string GetConfiguration(SyncAllDataConfig sync_all,
+ syncer::UserSelectableTypeSet types) {
base::DictionaryValue result;
- if (extra_values)
- result.MergeDictionary(extra_values);
result.SetBoolean("syncAllDataTypes", sync_all == SYNC_ALL_DATA);
- result.SetBoolean("encryptAllData", encrypt_all == ENCRYPT_ALL_DATA);
- if (!passphrase.empty())
- result.SetString("passphrase", passphrase);
// Add all of our data types.
result.SetBoolean("appsSynced", types.Has(syncer::UserSelectableType::kApps));
result.SetBoolean("autofillSynced",
@@ -314,6 +300,21 @@
EXPECT_EQ(expected_status, status);
}
+ // Expects a call to ResolveJavascriptCallback() with |should_succeed| as its
+ // argument.
+ void ExpectSetPassphraseSuccess(bool should_succeed) {
+ EXPECT_EQ(1u, web_ui_.call_data().size());
+ const auto& data = *web_ui_.call_data()[0];
+ EXPECT_EQ("cr.webUIResponse", data.function_name());
+ EXPECT_TRUE(data.arg2()->is_bool());
+ EXPECT_TRUE(data.arg2()->GetBool())
+ << "Callback should be resolved with a boolean indicating the success, "
+ "never rejected.";
+
+ EXPECT_TRUE(data.arg3()->is_bool());
+ EXPECT_EQ(should_succeed, data.arg3()->GetBool());
+ }
+
const base::DictionaryValue* ExpectSyncPrefsChanged() {
const content::TestWebUI::CallData& data1 = *web_ui_.call_data().back();
EXPECT_EQ("cr.webUIListenerCallback", data1.function_name());
@@ -638,8 +639,7 @@
TEST_F(PeopleHandlerTest, TestSyncEverything) {
SigninUser();
CreatePeopleHandler();
- std::string args = GetConfiguration(nullptr, SYNC_ALL_DATA, GetAllTypes(),
- std::string(), ENCRYPT_PASSWORDS);
+ std::string args = GetConfiguration(SYNC_ALL_DATA, GetAllTypes());
base::ListValue list_args;
list_args.AppendString(kTestCallbackId);
list_args.AppendString(args);
@@ -656,147 +656,119 @@
ExpectPageStatusResponse(PeopleHandler::kConfigurePageStatus);
}
-TEST_F(PeopleHandlerTest, TestPassphraseStillRequired) {
+TEST_F(PeopleHandlerTest, EnterCorrectExistingPassphrase) {
SigninUser();
CreatePeopleHandler();
- std::string args = GetConfiguration(nullptr, SYNC_ALL_DATA, GetAllTypes(),
- std::string(), ENCRYPT_PASSWORDS);
- base::ListValue list_args;
- list_args.AppendString(kTestCallbackId);
- list_args.AppendString(args);
- ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsPassphraseRequiredForPreferredDataTypes())
- .WillByDefault(Return(true));
+ SetupInitializedSyncService();
+
ON_CALL(*mock_sync_service_->GetMockUserSettings(), IsPassphraseRequired())
.WillByDefault(Return(true));
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsUsingSecondaryPassphrase())
- .WillByDefault(Return(false));
- SetupInitializedSyncService();
- SetDefaultExpectationsForConfigPage();
-
- handler_->HandleSetEncryption(&list_args);
- // We should navigate back to the configure page since we need a passphrase.
- ExpectPageStatusResponse(PeopleHandler::kPassphraseFailedPageStatus);
-}
-
-TEST_F(PeopleHandlerTest, EnterExistingFrozenImplicitPassword) {
- SigninUser();
- CreatePeopleHandler();
- base::DictionaryValue dict;
- dict.SetBoolean("setNewPassphrase", false);
- std::string args = GetConfiguration(&dict, SYNC_ALL_DATA, GetAllTypes(),
- "oldGaiaPassphrase", ENCRYPT_PASSWORDS);
- base::ListValue list_args;
- list_args.AppendString(kTestCallbackId);
- list_args.AppendString(args);
- // Act as if an encryption passphrase is required the first time, then never
- // again after that.
- EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsPassphraseRequired())
- .WillOnce(Return(true));
- ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsPassphraseRequiredForPreferredDataTypes())
+ IsTrustedVaultKeyRequired())
.WillByDefault(Return(false));
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
IsUsingSecondaryPassphrase())
- .WillByDefault(Return(false));
- SetupInitializedSyncService();
- EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
- SetDecryptionPassphrase("oldGaiaPassphrase"))
- .WillOnce(Return(true));
-
- handler_->HandleSetEncryption(&list_args);
- ExpectPageStatusResponse(PeopleHandler::kConfigurePageStatus);
-}
-
-TEST_F(PeopleHandlerTest, SetNewCustomPassphrase) {
- SigninUser();
- CreatePeopleHandler();
- base::DictionaryValue dict;
- dict.SetBoolean("setNewPassphrase", true);
- std::string args = GetConfiguration(&dict, SYNC_ALL_DATA, GetAllTypes(),
- "custom_passphrase", ENCRYPT_ALL_DATA);
- base::ListValue list_args;
- list_args.AppendString(kTestCallbackId);
- list_args.AppendString(args);
+ .WillByDefault(Return(true));
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
IsEncryptEverythingAllowed())
.WillByDefault(Return(true));
- ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsPassphraseRequiredForPreferredDataTypes())
- .WillByDefault(Return(false));
+
+ EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
+ SetDecryptionPassphrase("correct_passphrase"))
+ .WillOnce(Return(true));
+
+ base::ListValue list_args;
+ list_args.AppendString(kTestCallbackId);
+ list_args.AppendString("correct_passphrase");
+ handler_->HandleSetDecryptionPassphrase(&list_args);
+
+ ExpectSetPassphraseSuccess(true);
+}
+
+TEST_F(PeopleHandlerTest, SuccessfullyCreateCustomPassphrase) {
+ SigninUser();
+ CreatePeopleHandler();
+ SetupInitializedSyncService();
+
ON_CALL(*mock_sync_service_->GetMockUserSettings(), IsPassphraseRequired())
.WillByDefault(Return(false));
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsTrustedVaultKeyRequired())
+ .WillByDefault(Return(false));
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
IsUsingSecondaryPassphrase())
.WillByDefault(Return(false));
- SetupInitializedSyncService();
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsEncryptEverythingAllowed())
+ .WillByDefault(Return(true));
+
EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
SetEncryptionPassphrase("custom_passphrase"));
- handler_->HandleSetEncryption(&list_args);
- ExpectPageStatusResponse(PeopleHandler::kConfigurePageStatus);
+ base::ListValue list_args;
+ list_args.AppendString(kTestCallbackId);
+ list_args.AppendString("custom_passphrase");
+ handler_->HandleSetEncryptionPassphrase(&list_args);
+
+ ExpectSetPassphraseSuccess(true);
}
TEST_F(PeopleHandlerTest, EnterWrongExistingPassphrase) {
SigninUser();
CreatePeopleHandler();
- base::DictionaryValue dict;
- dict.SetBoolean("setNewPassphrase", false);
- std::string args = GetConfiguration(&dict, SYNC_ALL_DATA, GetAllTypes(),
- "invalid_passphrase", ENCRYPT_ALL_DATA);
- base::ListValue list_args;
- list_args.AppendString(kTestCallbackId);
- list_args.AppendString(args);
- ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsPassphraseRequiredForPreferredDataTypes())
- .WillByDefault(Return(true));
+ SetupInitializedSyncService();
+
ON_CALL(*mock_sync_service_->GetMockUserSettings(), IsPassphraseRequired())
.WillByDefault(Return(true));
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsUsingSecondaryPassphrase())
+ IsTrustedVaultKeyRequired())
.WillByDefault(Return(false));
- SetupInitializedSyncService();
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsUsingSecondaryPassphrase())
+ .WillByDefault(Return(true));
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsEncryptEverythingAllowed())
+ .WillByDefault(Return(true));
+
EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
SetDecryptionPassphrase("invalid_passphrase"))
.WillOnce(Return(false));
- SetDefaultExpectationsForConfigPage();
-
- handler_->HandleSetEncryption(&list_args);
- // We should navigate back to the configure page since we need a passphrase.
- ExpectPageStatusResponse(PeopleHandler::kPassphraseFailedPageStatus);
-}
-
-TEST_F(PeopleHandlerTest, EnterBlankExistingPassphrase) {
- SigninUser();
- CreatePeopleHandler();
- base::DictionaryValue dict;
- dict.SetBoolean("setNewPassphrase", false);
- std::string args = GetConfiguration(&dict,
- SYNC_ALL_DATA,
- GetAllTypes(),
- "",
- ENCRYPT_PASSWORDS);
base::ListValue list_args;
list_args.AppendString(kTestCallbackId);
- list_args.AppendString(args);
- ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsPassphraseRequiredForPreferredDataTypes())
- .WillByDefault(Return(true));
+ list_args.AppendString("invalid_passphrase");
+ handler_->HandleSetDecryptionPassphrase(&list_args);
+
+ ExpectSetPassphraseSuccess(false);
+}
+
+TEST_F(PeopleHandlerTest, CannotCreateBlankPassphrase) {
+ SigninUser();
+ CreatePeopleHandler();
+ SetupInitializedSyncService();
+
ON_CALL(*mock_sync_service_->GetMockUserSettings(), IsPassphraseRequired())
- .WillByDefault(Return(true));
+ .WillByDefault(Return(false));
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsTrustedVaultKeyRequired())
+ .WillByDefault(Return(false));
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
IsUsingSecondaryPassphrase())
.WillByDefault(Return(false));
- SetupInitializedSyncService();
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsEncryptEverythingAllowed())
+ .WillByDefault(Return(true));
- SetDefaultExpectationsForConfigPage();
+ EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
+ SetEncryptionPassphrase)
+ .Times(0);
- handler_->HandleSetEncryption(&list_args);
- // We should navigate back to the configure page since we need a passphrase.
- ExpectPageStatusResponse(PeopleHandler::kPassphraseFailedPageStatus);
+ base::ListValue list_args;
+ list_args.AppendString(kTestCallbackId);
+ list_args.AppendString("");
+ handler_->HandleSetEncryptionPassphrase(&list_args);
+
+ ExpectSetPassphraseSuccess(false);
}
// Walks through each user selectable type, and tries to sync just that single
@@ -808,9 +780,7 @@
for (syncer::UserSelectableType type : GetAllTypes()) {
syncer::UserSelectableTypeSet type_to_set;
type_to_set.Put(type);
- std::string args =
- GetConfiguration(nullptr, CHOOSE_WHAT_TO_SYNC, type_to_set,
- std::string(), ENCRYPT_PASSWORDS);
+ std::string args = GetConfiguration(CHOOSE_WHAT_TO_SYNC, type_to_set);
base::ListValue list_args;
list_args.AppendString(kTestCallbackId);
list_args.AppendString(args);
@@ -833,9 +803,7 @@
SigninUser();
CreatePeopleHandler();
SetDefaultExpectationsForConfigPage();
- std::string args =
- GetConfiguration(nullptr, CHOOSE_WHAT_TO_SYNC, GetAllTypes(),
- std::string(), ENCRYPT_PASSWORDS);
+ std::string args = GetConfiguration(CHOOSE_WHAT_TO_SYNC, GetAllTypes());
base::ListValue list_args;
list_args.AppendString(kTestCallbackId);
list_args.AppendString(args);
@@ -867,9 +835,7 @@
// Simulate "Sync everything" being turned off, but all individual
// toggles left on.
- std::string config =
- GetConfiguration(/*extra_values=*/nullptr, CHOOSE_WHAT_TO_SYNC,
- GetAllTypes(), std::string(), ENCRYPT_PASSWORDS);
+ std::string config = GetConfiguration(CHOOSE_WHAT_TO_SYNC, GetAllTypes());
base::ListValue list_args;
list_args.AppendString(kTestCallbackId);
list_args.AppendString(config);
@@ -1078,34 +1044,62 @@
CheckBool(dictionary, "encryptAllDataAllowed", false);
}
-TEST_F(PeopleHandlerTest, TurnOnEncryptAllDisallowed) {
+TEST_F(PeopleHandlerTest, CannotCreatePassphraseIfEncryptEverythingDisallowed) {
SigninUser();
CreatePeopleHandler();
- ON_CALL(*mock_sync_service_->GetMockUserSettings(),
- IsPassphraseRequiredForPreferredDataTypes())
- .WillByDefault(Return(false));
+ SetupInitializedSyncService();
+
ON_CALL(*mock_sync_service_->GetMockUserSettings(), IsPassphraseRequired())
.WillByDefault(Return(false));
- SetupInitializedSyncService();
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsTrustedVaultKeyRequired())
+ .WillByDefault(Return(false));
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsUsingSecondaryPassphrase())
+ .WillByDefault(Return(false));
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
IsEncryptEverythingAllowed())
.WillByDefault(Return(false));
- base::DictionaryValue dict;
- dict.SetBoolean("setNewPassphrase", true);
- std::string args = GetConfiguration(&dict, SYNC_ALL_DATA, GetAllTypes(),
- "password", ENCRYPT_ALL_DATA);
- base::ListValue list_args;
- list_args.AppendString(kTestCallbackId);
- list_args.AppendString(args);
-
EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
- SetEncryptionPassphrase(_))
+ SetEncryptionPassphrase)
.Times(0);
- handler_->HandleSetEncryption(&list_args);
+ base::ListValue list_args;
+ list_args.AppendString(kTestCallbackId);
+ list_args.AppendString("passphrase123");
+ handler_->HandleSetEncryptionPassphrase(&list_args);
- ExpectPageStatusResponse(PeopleHandler::kConfigurePageStatus);
+ ExpectSetPassphraseSuccess(false);
+}
+
+TEST_F(PeopleHandlerTest, CannotOverwritePassphraseWithNewOne) {
+ SigninUser();
+ CreatePeopleHandler();
+ SetupInitializedSyncService();
+
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(), IsPassphraseRequired())
+ .WillByDefault(Return(false));
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsTrustedVaultKeyRequired())
+ .WillByDefault(Return(false));
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsUsingSecondaryPassphrase())
+ .WillByDefault(Return(true));
+ ON_CALL(*mock_sync_service_->GetMockUserSettings(),
+ IsEncryptEverythingAllowed())
+ .WillByDefault(Return(true));
+
+ EXPECT_CALL(*mock_sync_service_->GetMockUserSettings(),
+ SetEncryptionPassphrase)
+ .Times(0);
+
+ base::ListValue list_args;
+ list_args.AppendString(kTestCallbackId);
+ list_args.AppendString("passphrase123");
+ handler_->HandleSetEncryptionPassphrase(&list_args);
+
+ ExpectSetPassphraseSuccess(false);
}
TEST_F(PeopleHandlerTest, DashboardClearWhileSettingsOpen_ConfirmSoon) {
diff --git a/chrome/test/data/webui/settings/people_page_sync_page_test.js b/chrome/test/data/webui/settings/people_page_sync_page_test.js
index 624cd363..913638f 100644
--- a/chrome/test/data/webui/settings/people_page_sync_page_test.js
+++ b/chrome/test/data/webui/settings/people_page_sync_page_test.js
@@ -337,7 +337,7 @@
assertTrue(passphraseConfirmationInput.invalid);
});
- test('CreatingPassphraseValidPassphrase', function() {
+ test('CreatingPassphraseValidPassphrase', async function() {
encryptWithPassphrase.click();
flush();
@@ -350,34 +350,31 @@
encryptionElement.$$('#passphraseConfirmationInput');
passphraseInput.value = 'foo';
passphraseConfirmationInput.value = 'foo';
+ browserProxy.encryptionPassphraseSuccess = true;
saveNewPassphrase.click();
- function verifySetSyncEncryption(args) {
- assertTrue(args.setNewPassphrase);
- assertEquals('foo', args.passphrase);
+ const passphrase = await browserProxy.whenCalled('setEncryptionPassphrase');
- // Fake backend response.
- const newPrefs = getSyncAllPrefs();
- newPrefs.fullEncryptionBody = 'Encrypted with custom passphrase';
- newPrefs.encryptAllData = true;
- webUIListenerCallback('sync-prefs-changed', newPrefs);
+ assertEquals('foo', passphrase);
- flush();
+ // Fake backend response.
+ const newPrefs = getSyncAllPrefs();
+ newPrefs.fullEncryptionBody = 'Encrypted with custom passphrase';
+ newPrefs.encryptAllData = true;
+ webUIListenerCallback('sync-prefs-changed', newPrefs);
- return waitBeforeNextRender(syncPage).then(() => {
- // Need to re-retrieve this, as a different show passphrase radio
- // button is shown once |syncPrefs.fullEncryptionBody| is non-empty.
- encryptWithPassphrase = encryptionElement.$$(
- 'cr-radio-button[name="encrypt-with-passphrase"]');
+ flush();
- // Assert that the radio boxes are disabled after encryption enabled.
- assertTrue(encryptionRadioGroup.disabled);
- assertEquals(-1, encryptWithGoogle.$.button.tabIndex);
- assertEquals(-1, encryptWithPassphrase.$.button.tabIndex);
- });
- }
- return browserProxy.whenCalled('setSyncEncryption')
- .then(verifySetSyncEncryption);
+ await waitBeforeNextRender(syncPage);
+ // Need to re-retrieve this, as a different show passphrase radio
+ // button is shown once |syncPrefs.fullEncryptionBody| is non-empty.
+ encryptWithPassphrase =
+ encryptionElement.$$('cr-radio-button[name="encrypt-with-passphrase"]');
+
+ // Assert that the radio boxes are disabled after encryption enabled.
+ assertTrue(encryptionRadioGroup.disabled);
+ assertEquals(-1, encryptWithGoogle.$.button.tabIndex);
+ assertEquals(-1, encryptWithPassphrase.$.button.tabIndex);
});
test('RadioBoxesHiddenWhenPassphraseRequired', function() {
@@ -415,7 +412,7 @@
assertFalse(submitExistingPassphrase.disabled);
});
- test('EnterExistingWrongPassphrase', function() {
+ test('EnterExistingWrongPassphrase', async function() {
const prefs = getSyncAllPrefs();
prefs.encryptAllData = true;
prefs.passphraseRequired = true;
@@ -425,21 +422,19 @@
const existingPassphraseInput = syncPage.$$('#existingPassphraseInput');
assertTrue(!!existingPassphraseInput);
existingPassphraseInput.value = 'wrong';
- browserProxy.encryptionResponse = PageStatus.PASSPHRASE_FAILED;
+ browserProxy.decryptionPassphraseSuccess = false;
const submitExistingPassphrase = syncPage.$$('#submitExistingPassphrase');
assertTrue(!!submitExistingPassphrase);
submitExistingPassphrase.click();
- return browserProxy.whenCalled('setSyncEncryption').then(function(args) {
- assertFalse(args.setNewPassphrase);
- assertEquals('wrong', args.passphrase);
- assertTrue(args.passphraseRequired);
- assertTrue(existingPassphraseInput.invalid);
- });
+ const passphrase = await browserProxy.whenCalled('setDecryptionPassphrase');
+
+ assertEquals('wrong', passphrase);
+ assertTrue(existingPassphraseInput.invalid);
});
- test('EnterExistingCorrectPassphrase', function() {
+ test('EnterExistingCorrectPassphrase', async function() {
const prefs = getSyncAllPrefs();
prefs.encryptAllData = true;
prefs.passphraseRequired = true;
@@ -449,29 +444,27 @@
const existingPassphraseInput = syncPage.$$('#existingPassphraseInput');
assertTrue(!!existingPassphraseInput);
existingPassphraseInput.value = 'right';
- browserProxy.encryptionResponse = PageStatus.CONFIGURE;
+ browserProxy.decryptionPassphraseSuccess = true;
const submitExistingPassphrase = syncPage.$$('#submitExistingPassphrase');
assertTrue(!!submitExistingPassphrase);
submitExistingPassphrase.click();
- return browserProxy.whenCalled('setSyncEncryption').then(function(args) {
- assertFalse(args.setNewPassphrase);
- assertEquals('right', args.passphrase);
- assertTrue(args.passphraseRequired);
+ const passphrase = await browserProxy.whenCalled('setDecryptionPassphrase');
- // Fake backend response.
- const newPrefs = getSyncAllPrefs();
- newPrefs.encryptAllData = true;
- webUIListenerCallback('sync-prefs-changed', newPrefs);
+ assertEquals('right', passphrase);
- flush();
+ // Fake backend response.
+ const newPrefs = getSyncAllPrefs();
+ newPrefs.encryptAllData = true;
+ webUIListenerCallback('sync-prefs-changed', newPrefs);
- // Verify that the encryption radio boxes are shown but disabled.
- assertTrue(encryptionRadioGroup.disabled);
- assertEquals(-1, encryptWithGoogle.$.button.tabIndex);
- assertEquals(-1, encryptWithPassphrase.$.button.tabIndex);
- });
+ flush();
+
+ // Verify that the encryption radio boxes are shown but disabled.
+ assertTrue(encryptionRadioGroup.disabled);
+ assertEquals(-1, encryptWithGoogle.$.button.tabIndex);
+ assertEquals(-1, encryptWithPassphrase.$.button.tabIndex);
});
test('SyncAdvancedRow', function() {
diff --git a/chrome/test/data/webui/settings/test_sync_browser_proxy.js b/chrome/test/data/webui/settings/test_sync_browser_proxy.js
index 5537f5f..567f747 100644
--- a/chrome/test/data/webui/settings/test_sync_browser_proxy.js
+++ b/chrome/test/data/webui/settings/test_sync_browser_proxy.js
@@ -19,7 +19,8 @@
'getSyncStatus',
'incrementPromoImpressionCount',
'setSyncDatatypes',
- 'setSyncEncryption',
+ 'setEncryptionPassphrase',
+ 'setDecryptionPassphrase',
'signOut',
'pauseSync',
'sendSyncPrefsChanged',
@@ -37,8 +38,10 @@
this.impressionCount_ = 0;
// Settable fake data.
- /** @type {!settings.PageStatus} */
- this.encryptionResponse = settings.PageStatus.CONFIGURE;
+ /** @type {boolean} */
+ this.encryptionPassphraseSuccess = false;
+ /** @type {boolean} */
+ this.decryptionPassphraseSuccess = false;
/** @type {!Array<!settings.StoredAccount>} */
this.storedAccounts = [];
/** @type {!settings.SyncStatus} */
@@ -111,9 +114,15 @@
}
/** @override */
- setSyncEncryption(syncPrefs) {
- this.methodCalled('setSyncEncryption', syncPrefs);
- return Promise.resolve(this.encryptionResponse);
+ setEncryptionPassphrase(passphrase) {
+ this.methodCalled('setEncryptionPassphrase', passphrase);
+ return Promise.resolve(this.encryptionPassphraseSuccess);
+ }
+
+ /** @override */
+ setDecryptionPassphrase(passphrase) {
+ this.methodCalled('setDecryptionPassphrase', passphrase);
+ return Promise.resolve(this.decryptionPassphraseSuccess);
}
/** @override */
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 9391339..5fc617f 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -60848,8 +60848,8 @@
<int value="2" label="Signed in to sync from secondary profile"/>
<int value="3" label="Customized sync options"/>
<int value="4" label="Chose what to sync"/>
- <int value="5" label="Encrypted all data"/>
- <int value="6" label="Selected a passphrase"/>
+ <int value="5" label="Encrypted data with new passphrase"/>
+ <int value="6" label="Entered existing passphrase"/>
</enum>
<enum name="ProfileSyncCustomize">
