Skip to content

Commit

Permalink
Sync UI: Don't allow setting a passphrase if !EncryptEverythingAllowed
Browse files Browse the repository at this point in the history
[MERGE TO RELEASE BRANCH]

If EncryptEverythingAllowed() was false, then PeopleHandler already
skipped the call to EnableEncryptEverything(), but it did still call
SetEncryptionPassphrase(). That could get users into a weird state
where Sync stopped working.
This CL fixes the issue by also skipping the SetEncryptionPassphrase()
call in case EncryptEverythingAllowed() is false.

Bug: 924847
Change-Id: I3b606e3ea9d0a8f86b865b41eda233dd472cc1eb
Reviewed-on: https://chromium-review.googlesource.com/c/1439299
Reviewed-by: Michael Giuffrida <[email protected]>
Commit-Queue: Marc Treib <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#627004}(cherry picked from commit 347c098)
Reviewed-on: https://chromium-review.googlesource.com/c/1477812
Reviewed-by: Marc Treib <[email protected]>
Cr-Commit-Position: refs/branch-heads/3683@{#481}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
  • Loading branch information
Marc Treib committed Feb 19, 2019
1 parent 6cb2ed0 commit 7cdad80
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
4 changes: 3 additions & 1 deletion chrome/browser/ui/webui/settings/people_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -546,8 +546,10 @@ void PeopleHandler::HandleSetEncryption(const base::ListValue* args) {
// Don't allow "encrypt all" 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.
if (!service->GetUserSettings()->IsEncryptEverythingAllowed())
if (!service->GetUserSettings()->IsEncryptEverythingAllowed()) {
configuration.encrypt_all = false;
configuration.set_new_passphrase = false;
}

// Note: Data encryption will not occur until configuration is complete
// (when the PSS receives its CONFIGURE_DONE notification from the sync
Expand Down
19 changes: 14 additions & 5 deletions chrome/browser/ui/webui/settings/people_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,8 @@ TEST_F(PeopleHandlerTest, SetNewCustomPassphrase) {
base::ListValue list_args;
list_args.AppendString(kTestCallbackId);
list_args.AppendString(args);
ON_CALL(*mock_pss_->GetUserSettingsMock(), IsEncryptEverythingAllowed())
.WillByDefault(Return(true));
ON_CALL(*mock_pss_->GetUserSettingsMock(),
IsPassphraseRequiredForDecryption())
.WillByDefault(Return(false));
Expand Down Expand Up @@ -1067,11 +1069,6 @@ TEST_F(PeopleHandlerTest, ShowSetupEncryptAllDisallowed) {
}

TEST_F(PeopleHandlerTest, TurnOnEncryptAllDisallowed) {
std::string args = GetConfiguration(
NULL, SYNC_ALL_DATA, GetAllTypes(), std::string(), ENCRYPT_ALL_DATA);
base::ListValue list_args;
list_args.AppendString(kTestCallbackId);
list_args.AppendString(args);
ON_CALL(*mock_pss_->GetUserSettingsMock(),
IsPassphraseRequiredForDecryption())
.WillByDefault(Return(false));
Expand All @@ -1080,8 +1077,20 @@ TEST_F(PeopleHandlerTest, TurnOnEncryptAllDisallowed) {
SetupInitializedProfileSyncService();
ON_CALL(*mock_pss_->GetUserSettingsMock(), 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_pss_->GetUserSettingsMock(), EnableEncryptEverything())
.Times(0);
EXPECT_CALL(*mock_pss_->GetUserSettingsMock(), SetEncryptionPassphrase(_))
.Times(0);

handler_->HandleSetEncryption(&list_args);

ExpectPageStatusResponse(PeopleHandler::kConfigurePageStatus);
Expand Down

0 comments on commit 7cdad80

Please sign in to comment.