Commit 97b59d1d authored by Mikel Astiz's avatar Mikel Astiz Committed by Commit Bot

Introduce sync IsTrustedVaultKeyRequired()

This API is analogous to IsPassphraseRequired() and different from
IsTrustedVaultKeyRequiredForPreferredDataTypes() in the sense that it's
not influenced by the currently-selected sync datatypes.

The rationale for introducing is that, when pending keys exist, certain
operations such as setting up a custom passphrase should be disallowed
by the UI, to prevent data loss. This is because having pending keys
means the locally accessible keybag does not contain all keys, so e.g.
setting up a custom passphrase would produce a new keybag where such
keys may be missing. As a consequence, worst-case, this can effectively
lead to user data loss, since there could be synced passwords encrypted
with the keys that have just been lost.

In this patch, setting pages on various platforms are updated to consume
use the new function instead of
IsTrustedVaultKeyRequiredForPreferredDataTypes().

Bug: 1019687
Change-Id: I6b5c69aac12b05ea34b4edc7dc149c84bd23d5c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972140
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarMaksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#732818}
parent f4544aaa
......@@ -268,6 +268,18 @@ public class ManageSyncPreferences extends PreferenceFragmentCompat
closeDialogIfOpen(FRAGMENT_ENTER_PASSPHRASE);
return;
}
if (mProfileSyncService.isTrustedVaultKeyRequired()) {
// The user cannot manually enter trusted vault keys, so it needs to gets treated as an
// error.
closeDialogIfOpen(FRAGMENT_CUSTOM_PASSPHRASE);
closeDialogIfOpen(FRAGMENT_ENTER_PASSPHRASE);
mSyncEncryption.setSummary(mProfileSyncService.isEncryptEverythingEnabled()
? R.string.sync_error_card_title
: R.string.sync_passwords_error_card_title);
return;
}
if (!mProfileSyncService.isPassphraseRequiredForPreferredDataTypes()) {
closeDialogIfOpen(FRAGMENT_ENTER_PASSPHRASE);
}
......@@ -402,11 +414,9 @@ public class ManageSyncPreferences extends PreferenceFragmentCompat
private void onSyncEncryptionClicked() {
if (!mProfileSyncService.isEngineInitialized()) return;
// TODO(crbug.com/1019687): The two below should probably operate independently of the
// preferred datatypes.
if (mProfileSyncService.isPassphraseRequiredForPreferredDataTypes()) {
displayPassphraseDialog();
} else if (mProfileSyncService.isTrustedVaultKeyRequiredForPreferredDataTypes()) {
} else if (mProfileSyncService.isTrustedVaultKeyRequired()) {
CoreAccountInfo primaryAccountInfo =
IdentityServicesProvider.get().getIdentityManager().getPrimaryAccountInfo();
if (primaryAccountInfo != null) {
......
......@@ -454,6 +454,18 @@ public class ProfileSyncService {
mNativeProfileSyncServiceAndroid, ProfileSyncService.this);
}
/**
* Checks if trusted vault encryption keys are needed, independently of the currently-enabled
* data types.
*
* @return true if we need an encryption key.
*/
public boolean isTrustedVaultKeyRequired() {
assert isEngineInitialized();
return ProfileSyncServiceJni.get().isTrustedVaultKeyRequired(
mNativeProfileSyncServiceAndroid, ProfileSyncService.this);
}
/**
* Checks if trusted vault encryption keys are needed to decrypt a currently-enabled data type.
*
......@@ -659,6 +671,8 @@ public class ProfileSyncService {
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isPassphraseRequiredForPreferredDataTypes(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isTrustedVaultKeyRequired(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isTrustedVaultKeyRequiredForPreferredDataTypes(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isUsingSecondaryPassphrase(
......
......@@ -16,6 +16,7 @@ public class FakeProfileSyncService extends ProfileSyncService {
private boolean mEngineInitialized;
private int mNumberOfSyncedDevices;
private boolean mPassphraseRequiredForPreferredDataTypes;
private boolean mTrustedVaultKeyRequired;
private boolean mTrustedVaultKeyRequiredForPreferredDataTypes;
private Set<Integer> mChosenTypes = new HashSet<>();
private boolean mCanSyncFeatureStart;
......@@ -67,6 +68,15 @@ public class FakeProfileSyncService extends ProfileSyncService {
mPassphraseRequiredForPreferredDataTypes = passphraseRequiredForPreferredDataTypes;
}
@Override
public boolean isTrustedVaultKeyRequired() {
return mTrustedVaultKeyRequired;
}
public void setTrustedVaultKeyRequired(boolean trustedVaultKeyRequired) {
mTrustedVaultKeyRequired = trustedVaultKeyRequired;
}
@Override
public boolean isTrustedVaultKeyRequiredForPreferredDataTypes() {
return mTrustedVaultKeyRequiredForPreferredDataTypes;
......
......@@ -293,6 +293,13 @@ jboolean ProfileSyncServiceAndroid::IsPassphraseRequiredForPreferredDataTypes(
->IsPassphraseRequiredForPreferredDataTypes();
}
jboolean ProfileSyncServiceAndroid::IsTrustedVaultKeyRequired(
JNIEnv* env,
const JavaParamRef<jobject>& obj) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
return sync_service_->GetUserSettings()->IsTrustedVaultKeyRequired();
}
jboolean
ProfileSyncServiceAndroid::IsTrustedVaultKeyRequiredForPreferredDataTypes(
JNIEnv* env,
......
......@@ -92,6 +92,9 @@ class ProfileSyncServiceAndroid : public syncer::SyncServiceObserver {
jboolean IsPassphraseRequiredForPreferredDataTypes(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
jboolean IsTrustedVaultKeyRequired(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
jboolean IsTrustedVaultKeyRequiredForPreferredDataTypes(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
......
......@@ -625,6 +625,11 @@ void PeopleHandler::HandleSetEncryption(const base::ListValue* args) {
// 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
......@@ -1036,9 +1041,10 @@ void PeopleHandler::PushSyncPrefs() {
args.SetBoolean("passphraseRequired",
sync_user_settings->IsPassphraseRequired());
args.SetBoolean(
"trustedVaultKeysRequired",
sync_user_settings->IsTrustedVaultKeyRequiredForPreferredDataTypes());
// Same as above, we call IsTrustedVaultKeyRequired() here instead of.
// IsTrustedVaultKeyRequiredForPreferredDataTypes().
args.SetBoolean("trustedVaultKeysRequired",
sync_user_settings->IsTrustedVaultKeyRequired());
syncer::PassphraseType passphrase_type =
sync_user_settings->GetPassphraseType();
......
......@@ -14,6 +14,7 @@
#include "base/json/json_writer.h"
#include "base/macros.h"
#include "base/stl_util.h"
#include "base/test/mock_callback.h"
#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/defaults.h"
......@@ -242,9 +243,7 @@ class PeopleHandlerTest : public ChromeRenderViewHostTestHarness {
ON_CALL(*mock_sync_service_, GetSetupInProgressHandle())
.WillByDefault(
Return(ByMove(std::make_unique<syncer::SyncSetupInProgressHandle>(
base::BindRepeating(
&PeopleHandlerTest::OnSetupInProgressHandleDestroyed,
base::Unretained(this))))));
mock_on_setup_in_progress_handle_destroyed_.Get()))));
handler_ = std::make_unique<TestingPeopleHandler>(&web_ui_, profile());
handler_->AllowJavascript();
......@@ -342,8 +341,8 @@ class PeopleHandlerTest : public ChromeRenderViewHostTestHarness {
return identity_test_env_adaptor_->identity_test_env();
}
MOCK_METHOD0(OnSetupInProgressHandleDestroyed, void());
testing::NiceMock<base::MockCallback<base::RepeatingClosure>>
mock_on_setup_in_progress_handle_destroyed_;
syncer::MockSyncService* mock_sync_service_;
std::unique_ptr<IdentityTestEnvironmentProfileAdaptor>
identity_test_env_adaptor_;
......@@ -485,7 +484,7 @@ TEST_F(PeopleHandlerTest,
// tell it we're through with the setup progress.
testing::InSequence seq;
EXPECT_CALL(*mock_sync_service_, StopAndClear());
EXPECT_CALL(*this, OnSetupInProgressHandleDestroyed());
EXPECT_CALL(mock_on_setup_in_progress_handle_destroyed_, Run());
handler_->CloseSyncSetup();
EXPECT_EQ(
......@@ -973,7 +972,7 @@ TEST_F(PeopleHandlerTest, ShowSetupCustomPassphraseRequired) {
TEST_F(PeopleHandlerTest, ShowSetupTrustedVaultKeysRequired) {
ON_CALL(*mock_sync_service_->GetMockUserSettings(),
IsTrustedVaultKeyRequiredForPreferredDataTypes())
IsTrustedVaultKeyRequired())
.WillByDefault(Return(true));
ON_CALL(*mock_sync_service_->GetMockUserSettings(), GetPassphraseType())
.WillByDefault(Return(syncer::PassphraseType::kTrustedVaultPassphrase));
......@@ -987,8 +986,6 @@ TEST_F(PeopleHandlerTest, ShowSetupTrustedVaultKeysRequired) {
CheckBool(dictionary, "passphraseRequired", false);
CheckBool(dictionary, "trustedVaultKeysRequired", true);
EXPECT_FALSE(dictionary->FindKey("enterPassphraseBody"));
// TODO: See how to verify the appropriate action, once it's actually
// implemented.
}
TEST_F(PeopleHandlerTest, ShowSetupEncryptAll) {
......
......@@ -104,6 +104,10 @@ class SyncUserSettings {
// Whether a passphrase is required to decrypt the data for any currently
// enabled data type.
virtual bool IsPassphraseRequiredForPreferredDataTypes() const = 0;
// Whether trusted vault keys are required for encryption or decryption. Note
// that Sync might still be working fine if the user has disabled all
// encrypted data types.
virtual bool IsTrustedVaultKeyRequired() const = 0;
// Whether trusted vault keys are required for encryption or decryption to
// proceed for any currently enabled data type.
virtual bool IsTrustedVaultKeyRequiredForPreferredDataTypes() const = 0;
......
......@@ -189,6 +189,10 @@ bool SyncUserSettingsImpl::IsPassphraseRequiredForPreferredDataTypes() const {
return IsEncryptedDatatypeEnabled() && IsPassphraseRequired();
}
bool SyncUserSettingsImpl::IsTrustedVaultKeyRequired() const {
return crypto_->IsTrustedVaultKeyRequired();
}
bool SyncUserSettingsImpl::IsTrustedVaultKeyRequiredForPreferredDataTypes()
const {
return IsEncryptedDatatypeEnabled() && crypto_->IsTrustedVaultKeyRequired();
......
......@@ -64,6 +64,7 @@ class SyncUserSettingsImpl : public SyncUserSettings {
ModelTypeSet GetEncryptedDataTypes() const override;
bool IsPassphraseRequired() const override;
bool IsPassphraseRequiredForPreferredDataTypes() const override;
bool IsTrustedVaultKeyRequired() const override;
bool IsTrustedVaultKeyRequiredForPreferredDataTypes() const override;
bool IsUsingSecondaryPassphrase() const override;
base::Time GetExplicitPassphraseTime() const override;
......
......@@ -49,6 +49,7 @@ class SyncUserSettingsMock : public SyncUserSettings {
MOCK_CONST_METHOD0(GetEncryptedDataTypes, ModelTypeSet());
MOCK_CONST_METHOD0(IsPassphraseRequired, bool());
MOCK_CONST_METHOD0(IsPassphraseRequiredForPreferredDataTypes, bool());
MOCK_CONST_METHOD0(IsTrustedVaultKeyRequired, bool());
MOCK_CONST_METHOD0(IsTrustedVaultKeyRequiredForPreferredDataTypes, bool());
MOCK_CONST_METHOD0(IsUsingSecondaryPassphrase, bool());
MOCK_CONST_METHOD0(GetExplicitPassphraseTime, base::Time());
......
......@@ -127,6 +127,10 @@ void TestSyncService::SetPassphraseRequiredForPreferredDataTypes(
user_settings_.SetPassphraseRequiredForPreferredDataTypes(required);
}
void TestSyncService::SetTrustedVaultKeyRequired(bool required) {
user_settings_.SetTrustedVaultKeyRequired(required);
}
void TestSyncService::SetTrustedVaultKeyRequiredForPreferredDataTypes(
bool required) {
user_settings_.SetTrustedVaultKeyRequiredForPreferredDataTypes(required);
......
......@@ -50,6 +50,7 @@ class TestSyncService : public SyncService {
void SetDetailedSyncStatus(bool engine_available, SyncStatus status);
void SetPassphraseRequired(bool required);
void SetPassphraseRequiredForPreferredDataTypes(bool required);
void SetTrustedVaultKeyRequired(bool required);
void SetTrustedVaultKeyRequiredForPreferredDataTypes(bool required);
void SetIsUsingSecondaryPassphrase(bool enabled);
......
......@@ -170,6 +170,10 @@ bool TestSyncUserSettings::IsPassphraseRequiredForPreferredDataTypes() const {
return passphrase_required_for_preferred_data_types_;
}
bool TestSyncUserSettings::IsTrustedVaultKeyRequired() const {
return trusted_vault_key_required_;
}
bool TestSyncUserSettings::IsTrustedVaultKeyRequiredForPreferredDataTypes()
const {
return trusted_vault_key_required_for_preferred_data_types_;
......@@ -213,6 +217,10 @@ void TestSyncUserSettings::SetPassphraseRequiredForPreferredDataTypes(
passphrase_required_for_preferred_data_types_ = required;
}
void TestSyncUserSettings::SetTrustedVaultKeyRequired(bool required) {
trusted_vault_key_required_ = required;
}
void TestSyncUserSettings::SetTrustedVaultKeyRequiredForPreferredDataTypes(
bool required) {
trusted_vault_key_required_for_preferred_data_types_ = required;
......
......@@ -53,6 +53,7 @@ class TestSyncUserSettings : public SyncUserSettings {
syncer::ModelTypeSet GetEncryptedDataTypes() const override;
bool IsPassphraseRequired() const override;
bool IsPassphraseRequiredForPreferredDataTypes() const override;
bool IsTrustedVaultKeyRequired() const override;
bool IsTrustedVaultKeyRequiredForPreferredDataTypes() const override;
bool IsUsingSecondaryPassphrase() const override;
base::Time GetExplicitPassphraseTime() const override;
......@@ -66,6 +67,7 @@ class TestSyncUserSettings : public SyncUserSettings {
void SetEncryptEverythingAllowed(bool allowed);
void SetPassphraseRequired(bool required);
void SetPassphraseRequiredForPreferredDataTypes(bool required);
void SetTrustedVaultKeyRequired(bool required);
void SetTrustedVaultKeyRequiredForPreferredDataTypes(bool required);
void SetIsUsingSecondaryPassphrase(bool enabled);
......@@ -81,6 +83,7 @@ class TestSyncUserSettings : public SyncUserSettings {
bool passphrase_required_ = false;
bool passphrase_required_for_preferred_data_types_ = false;
bool trusted_vault_key_required_ = false;
bool trusted_vault_key_required_for_preferred_data_types_ = false;
bool using_secondary_passphrase_ = false;
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment