Commit b4743e69 authored by Mikel Astiz's avatar Mikel Astiz Committed by Commit Bot

Reflect sync errors on Android if missing keys

This patch is similar in spirit to the desktop equivalent changes in
https://chromium-review.googlesource.com/c/chromium/src/+/1924572.

The overall idea is that, if encryption keys are missing, the UI needs
to display this state as an error, and the user should be able to go
through a reauth flow to retrieve the keys.

The actual logic to trigger the key retrieval process is deferred to
future patches and reflected in TODOs.

Bug: 1012659
Change-Id: I0679e47f8c783fdfd2953989205964165b0f8147
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930796
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721571}
parent dd7f0276
......@@ -1554,6 +1554,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/sync/SyncController.java",
"java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java",
"java/src/org/chromium/chrome/browser/sync/SyncUserDataWiper.java",
"java/src/org/chromium/chrome/browser/sync/TrustedVaultClient.java",
"java/src/org/chromium/chrome/browser/sync/ui/PassphraseActivity.java",
"java/src/org/chromium/chrome/browser/sync/ui/PassphraseCreationDialogFragment.java",
"java/src/org/chromium/chrome/browser/sync/ui/PassphraseDialogFragment.java",
......
......@@ -33,6 +33,7 @@ import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.settings.ChromeSwitchPreference;
import org.chromium.chrome.browser.settings.SettingsUtils;
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.browser.sync.TrustedVaultClient;
import org.chromium.chrome.browser.sync.ui.PassphraseCreationDialogFragment;
import org.chromium.chrome.browser.sync.ui.PassphraseDialogFragment;
import org.chromium.chrome.browser.sync.ui.PassphraseTypeDialogFragment;
......@@ -397,8 +398,12 @@ 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()) {
TrustedVaultClient.displayKeyRetrievalDialog();
} else {
displayPassphraseTypeDialog();
}
......
......@@ -58,6 +58,7 @@ import org.chromium.chrome.browser.signin.IdentityServicesProvider;
import org.chromium.chrome.browser.signin.UnifiedConsentServiceBridge;
import org.chromium.chrome.browser.sync.GoogleServiceAuthError;
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.browser.sync.TrustedVaultClient;
import org.chromium.chrome.browser.sync.ui.PassphraseDialogFragment;
import org.chromium.chrome.browser.util.IntentUtils;
import org.chromium.components.signin.AccountManagerFacade;
......@@ -119,8 +120,10 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
int ANDROID_SYNC_DISABLED = 0;
int AUTH_ERROR = 1;
int PASSPHRASE_REQUIRED = 2;
int CLIENT_OUT_OF_DATE = 3;
int SYNC_SETUP_INCOMPLETE = 4;
int TRUSTED_VAULT_KEY_REQUIRED_FOR_EVERYTHING = 3;
int TRUSTED_VAULT_KEY_REQUIRED_FOR_PASSWORDS = 4;
int CLIENT_OUT_OF_DATE = 5;
int SYNC_SETUP_INCOMPLETE = 6;
int OTHER_ERRORS = 128;
}
......@@ -475,6 +478,13 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
return SyncError.PASSPHRASE_REQUIRED;
}
if (mProfileSyncService.isEngineInitialized()
&& mProfileSyncService.isTrustedVaultKeyRequiredForPreferredDataTypes()) {
return mProfileSyncService.isEncryptEverythingEnabled()
? SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_EVERYTHING
: SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_PASSWORDS;
}
if (ChromeFeatureList.isEnabled(ChromeFeatureList.SYNC_MANUAL_START_ANDROID)
&& wasSigninFlowInterrupted()) {
return SyncError.SYNC_SETUP_INCOMPLETE;
......@@ -492,6 +502,10 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
case SyncError.SYNC_SETUP_INCOMPLETE:
assert ChromeFeatureList.isEnabled(ChromeFeatureList.SYNC_MANUAL_START_ANDROID);
return getString(R.string.sync_settings_not_confirmed_title);
case SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_EVERYTHING:
return getString(R.string.sync_error_card_title);
case SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_PASSWORDS:
return getString(R.string.sync_passwords_error_card_title);
default:
return getString(R.string.sync_error_card_title);
}
......@@ -514,6 +528,9 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
return getString(R.string.hint_other_sync_errors);
case SyncError.PASSPHRASE_REQUIRED:
return getString(R.string.hint_passphrase_required);
case SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_EVERYTHING:
case SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_PASSWORDS:
return getString(R.string.hint_sync_retrieve_keys);
case SyncError.SYNC_SETUP_INCOMPLETE:
assert ChromeFeatureList.isEnabled(ChromeFeatureList.SYNC_MANUAL_START_ANDROID);
return getString(R.string.hint_sync_settings_not_confirmed_description);
......@@ -561,6 +578,12 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
displayPassphraseDialog();
return;
}
if (mCurrentSyncError == SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_EVERYTHING
|| mCurrentSyncError == SyncError.TRUSTED_VAULT_KEY_REQUIRED_FOR_PASSWORDS) {
TrustedVaultClient.displayKeyRetrievalDialog();
return;
}
}
private static void removePreference(PreferenceGroup from, Preference preference) {
......
......@@ -90,6 +90,13 @@ public class SyncPreferenceUtils {
if (profileSyncService.isPassphraseRequiredForPreferredDataTypes()) {
return res.getString(R.string.sync_need_passphrase);
}
if (profileSyncService.isTrustedVaultKeyRequiredForPreferredDataTypes()) {
return profileSyncService.isEncryptEverythingEnabled()
? context.getString(R.string.sync_error_card_title)
: context.getString(R.string.sync_passwords_error_card_title);
}
return context.getString(R.string.sync_and_services_summary_sync_on);
}
return context.getString(R.string.sync_is_disabled);
......@@ -121,7 +128,8 @@ public class SyncPreferenceUtils {
if (profileSyncService.isEngineInitialized()
&& (profileSyncService.hasUnrecoverableError()
|| profileSyncService.getAuthError() != GoogleServiceAuthError.State.NONE
|| profileSyncService.isPassphraseRequiredForPreferredDataTypes())) {
|| profileSyncService.isPassphraseRequiredForPreferredDataTypes()
|| profileSyncService.isTrustedVaultKeyRequiredForPreferredDataTypes())) {
return UiUtils.getTintedDrawable(
context, R.drawable.ic_sync_error_40dp, R.color.default_red);
}
......
......@@ -454,6 +454,17 @@ public class ProfileSyncService {
mNativeProfileSyncServiceAndroid, ProfileSyncService.this);
}
/**
* Checks if trusted vault encryption keys are needed to decrypt a currently-enabled data type.
*
* @return true if we need an encryption key for a type that is currently enabled.
*/
public boolean isTrustedVaultKeyRequiredForPreferredDataTypes() {
assert isEngineInitialized();
return ProfileSyncServiceJni.get().isTrustedVaultKeyRequiredForPreferredDataTypes(
mNativeProfileSyncServiceAndroid, ProfileSyncService.this);
}
/**
* Checks if encrypting all the data types is allowed.
*
......@@ -648,6 +659,8 @@ public class ProfileSyncService {
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isPassphraseRequiredForPreferredDataTypes(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isTrustedVaultKeyRequiredForPreferredDataTypes(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isUsingSecondaryPassphrase(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean setDecryptionPassphrase(
......
......@@ -63,6 +63,8 @@ public class SyncNotificationController implements ProfileSyncService.SyncStateC
createSettingsIntent());
} else if (mProfileSyncService.isEngineInitialized()
&& mProfileSyncService.isPassphraseRequiredForPreferredDataTypes()) {
assert (!mProfileSyncService.isTrustedVaultKeyRequiredForPreferredDataTypes());
if (mProfileSyncService.isPassphrasePrompted()) {
return;
}
......@@ -72,11 +74,23 @@ public class SyncNotificationController implements ProfileSyncService.SyncStateC
case PassphraseType.CUSTOM_PASSPHRASE:
showSyncNotification(R.string.sync_need_passphrase, createPasswordIntent());
break;
case PassphraseType.TRUSTED_VAULT_PASSPHRASE:
assert false : "Passphrase cannot be required with trusted vault passphrase";
return;
case PassphraseType.KEYSTORE_PASSPHRASE: // Falling through intentionally.
default:
mNotificationManager.cancel(NotificationConstants.NOTIFICATION_ID_SYNC);
return;
}
} else if (mProfileSyncService.isEngineInitialized()
&& mProfileSyncService.isTrustedVaultKeyRequiredForPreferredDataTypes()) {
Intent intent = TrustedVaultClient.createKeyRetrievalIntent();
if (intent != null) {
showSyncNotification(mProfileSyncService.isEncryptEverythingEnabled()
? R.string.sync_error_card_title
: R.string.sync_passwords_error_card_title,
intent);
}
} else {
mNotificationManager.cancel(NotificationConstants.NOTIFICATION_ID_SYNC);
return;
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.sync;
import android.content.Intent;
import androidx.annotation.Nullable;
/**
* Client used to communicate with GmsCore about sync encryption keys.
*/
public class TrustedVaultClient {
/**
* Displays a UI that allows the user to reauthenticate and retrieve the sync encryption keys.
*/
public static void displayKeyRetrievalDialog() {
// TODO(crbug.com/1012659): Not implemented.
}
/**
* Creates an intent that launches an activity that triggers the key retrieval UI.
*
* @return the intent for opening the key retrieval activity or null if none is actually
* required
*/
@Nullable
public static Intent createKeyRetrievalIntent() {
// TODO(crbug.com/1012659): Not implemented.
return null;
}
}
......@@ -16,6 +16,7 @@ public class FakeProfileSyncService extends ProfileSyncService {
private boolean mEngineInitialized;
private int mNumberOfSyncedDevices;
private boolean mPassphraseRequiredForPreferredDataTypes;
private boolean mTrustedVaultKeyRequiredForPreferredDataTypes;
private Set<Integer> mChosenTypes = new HashSet<>();
private boolean mCanSyncFeatureStart;
......@@ -66,6 +67,17 @@ public class FakeProfileSyncService extends ProfileSyncService {
mPassphraseRequiredForPreferredDataTypes = passphraseRequiredForPreferredDataTypes;
}
@Override
public boolean isTrustedVaultKeyRequiredForPreferredDataTypes() {
return mTrustedVaultKeyRequiredForPreferredDataTypes;
}
public void setTrustedVaultKeyRequiredForPreferredDataTypes(
boolean trustedVaultKeyRequiredForPreferredDataTypes) {
mTrustedVaultKeyRequiredForPreferredDataTypes =
trustedVaultKeyRequiredForPreferredDataTypes;
}
@Override
public boolean canSyncFeatureStart() {
return mCanSyncFeatureStart;
......
......@@ -244,6 +244,31 @@ public class SyncAndServicesPreferencesTest {
Assert.assertNull("Sync error card should not be shown", getSyncErrorCard(fragment));
}
@Test
@SmallTest
@Feature({"Sync"})
public void testTrustedVaultKeyRequiredShowsSyncErrorCard() throws Exception {
final FakeProfileSyncService pss = overrideProfileSyncService();
mSyncTestRule.setUpTestAccountAndSignIn();
SyncTestUtil.waitForSyncActive();
pss.setEngineInitialized(true);
pss.setTrustedVaultKeyRequiredForPreferredDataTypes(true);
SyncAndServicesPreferences fragment = startSyncAndServicesPreferences();
Assert.assertNotNull("Sync error card should be shown", getSyncErrorCard(fragment));
}
// TODO(crbug.com/1030725): SyncTestRule should support overriding ProfileSyncService.
private FakeProfileSyncService overrideProfileSyncService() {
return TestThreadUtils.runOnUiThreadBlockingNoException(() -> {
// PSS has to be constructed on the UI thread.
FakeProfileSyncService fakeProfileSyncService = new FakeProfileSyncService();
ProfileSyncService.overrideForTests(fakeProfileSyncService);
return fakeProfileSyncService;
});
}
/**
* Start SyncAndServicesPreferences signin screen and dissmiss it without pressing confirm or
* cancel.
......@@ -329,4 +354,4 @@ public class SyncAndServicesPreferencesTest {
Assert.assertTrue(
"The sync switch should be enabled.", getSyncSwitch(fragment).isEnabled());
}
}
\ No newline at end of file
}
......@@ -293,6 +293,15 @@ jboolean ProfileSyncServiceAndroid::IsPassphraseRequiredForPreferredDataTypes(
->IsPassphraseRequiredForPreferredDataTypes();
}
jboolean
ProfileSyncServiceAndroid::IsTrustedVaultKeyRequiredForPreferredDataTypes(
JNIEnv* env,
const JavaParamRef<jobject>& obj) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
return sync_service_->GetUserSettings()
->IsTrustedVaultKeyRequiredForPreferredDataTypes();
}
jboolean ProfileSyncServiceAndroid::IsUsingSecondaryPassphrase(
JNIEnv* env,
const JavaParamRef<jobject>&) {
......
......@@ -92,6 +92,9 @@ class ProfileSyncServiceAndroid : public syncer::SyncServiceObserver {
jboolean IsPassphraseRequiredForPreferredDataTypes(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
jboolean IsTrustedVaultKeyRequiredForPreferredDataTypes(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
jboolean IsUsingSecondaryPassphrase(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
......
......@@ -1831,6 +1831,9 @@ To change this setting, <ph name="BEGIN_LINK">&lt;resetlink&gt;</ph>reset sync<p
<message name="IDS_SYNC_ERROR_CARD_TITLE" desc="Title of the Sync Error Card. [CHAR-LIMIT=32]">
Sync isn't working
</message>
<message name="IDS_SYNC_PASSWORDS_ERROR_CARD_TITLE" desc="Title of the Sync Error Card when it affects passwords only. [CHAR-LIMIT=32]">
Error syncing passwords
</message>
<message name="IDS_SYNC_SETTINGS_NOT_CONFIRMED_TITLE" desc="Title of the error message shown when sync setup was not complete. [CHAR-LIMIT=60]">
Initial sync setup not finished
</message>
......@@ -1840,6 +1843,9 @@ To change this setting, <ph name="BEGIN_LINK">&lt;resetlink&gt;</ph>reset sync<p
<message name="IDS_HINT_SYNC_AUTH_ERROR" desc="Hint message to resolve sync auth error.">
Sign in again to start sync
</message>
<message name="IDS_HINT_SYNC_RETRIEVE_KEYS" desc="Hint message to resolve sync encryption error.">
Fix now
</message>
<message name="IDS_HINT_PASSPHRASE_REQUIRED" desc="Hint message to resolve passphrase required error.">
Enter your passphrase to start sync
</message>
......
4f2e743dff46b212f4c688b74a6956206c58d426
\ No newline at end of file
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