Skip to content

Commit

Permalink
Add trusted vault opt-in link to settings WebUI
Browse files Browse the repository at this point in the history
The link is shown if:
- A newly-introduced feature toggle is enabled
and
- Sync-the-transport is active
and
- kKeystorePassphrase is the sync passphrase type
and
- IsPassphraseRequired() is false (this would only be true for a keystore
passphrase in very rare cases).

A temporary URL is used for the moment and TODO is added to set the
correct URL once it's available.

Implementation screenshot:
crbug.com/1202088#c2
Mock (ignore strings, since they are not final):
https://docs.google.com/presentation/d/1FhpO2px56jU0YIrByqNysSdLcZ2s9CkIy-J_QS3s1PY/edit#slide=id.g97310fee6c_1_0

Bug: 1202088
Change-Id: I124b361a364abc0cec3c76a0c7a3079bbfdea465
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2859026
Reviewed-by: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#877837}
  • Loading branch information
Victor Hugo Vianna Silva authored and Chromium LUCI CQ committed Apr 30, 2021
1 parent cdb6202 commit 86010a1
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 21 deletions.
8 changes: 8 additions & 0 deletions chrome/app/settings_strings.grdp
Expand Up @@ -682,6 +682,14 @@
<message name="IDS_SETTINGS_PASSWORD_ROW_FEDERATED_MORE_ACTIONS" desc="The ARIA (accessibility) message for the More Actions button, which sits in every row of the password list. For this row does not include a password, only information of the account. It opens a menu with a list of actions, which apply to the account presented on this row.">
More actions, saved account for <ph name="USERNAME">$1<ex>example@gmail.com</ex></ph> on <ph name="DOMAIN">$2<ex>www.google.com</ex></ph>
</message>
<!-- TODO(crbug.com/1202088): Remove "translateable=false" once definitive string is available, and update description. -->
<message translateable="false" name="IDS_SETTINGS_TRUSTED_VAULT_OPT_IN_LABEL" desc="Label for the link to the page to opt in to trusted vault encryption">
Trusted vault
</message>
<!-- TODO(crbug.com/1202088): Remove "translateable=false" once definitive string is available, and update description. -->
<message translateable="false" name="IDS_SETTINGS_TRUSTED_VAULT_OPT_IN_SUB_LABEL" desc="Sub-label for the link to the page to opt in to trusted vault encryption">
Opt in to trusted vault
</message>


<!-- Default Browser Page -->
Expand Down
Expand Up @@ -101,6 +101,11 @@
label="$i18n{passwordsAutosigninLabel}"
sub-label="$i18n{passwordsAutosigninDescription}">
</settings-toggle-button>
<cr-link-row external on-click="onTrustedVaultOptInClick_"
hidden$="[[!shouldOfferTrustedVaultOptIn_]]"
label="$i18n{trustedVaultOptInLabel}"
sub-label="$i18n{trustedVaultOptInSubLabel}">
</cr-link-row>
<div id="checkPasswordsBannerContainer" class="cr-row"
hidden$="[[!shouldShowBanner_]]">
<picture>
Expand Down
Expand Up @@ -19,6 +19,7 @@ import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js';
import 'chrome://resources/cr_elements/cr_link_row/cr_link_row.js';
import 'chrome://resources/cr_elements/icons.m.js';
import 'chrome://resources/cr_elements/shared_style_css.m.js';
import {OpenWindowProxyImpl} from '../open_window_proxy.js';
import {assert} from 'chrome://resources/js/assert.m.js';
import {focusWithoutInk} from 'chrome://resources/js/cr/ui/focus_without_ink.m.js';
import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
Expand Down Expand Up @@ -198,6 +199,16 @@ Polymer({
'isOptedInForAccountStorage_, numberOfDevicePasswords_)',
},

/**
* Whether the entry point leading to enroll in trusted vault encryption
* should be shown.
* @private
*/
shouldOfferTrustedVaultOptIn_: {
type: Boolean,
value: false,
},

/** @private */
hasLeakedCredentials_: {
type: Boolean,
Expand Down Expand Up @@ -338,6 +349,12 @@ Polymer({
this.addWebUIListener('stored-accounts-updated', storedAccountsChanged);
// </if>

syncBrowserProxy.sendOfferTrustedVaultOptInChanged();
this.addWebUIListener(
'offer-trusted-vault-opt-in-changed', (offerOptIn) => {
this.shouldOfferTrustedVaultOptIn_ = offerOptIn;
});

afterNextRender(this, function() {
IronA11yAnnouncer.requestAvailability();
});
Expand Down Expand Up @@ -463,6 +480,15 @@ Polymer({
PasswordManagerProxy.PasswordCheckReferrer.PASSWORD_SETTINGS);
},

/**
* Shows the page to opt in to trusted vault encryption.
* @private
*/
onTrustedVaultOptInClick_() {
OpenWindowProxyImpl.getInstance().openURL(
loadTimeData.getString('trustedVaultOptInUrl'));
},

/**
* Shows the 'device passwords' page.
* @private
Expand Down
Expand Up @@ -230,6 +230,11 @@ import {addSingletonGetter, sendWithPromise} from 'chrome://resources/js/cr.m.js
* manager in passwords section on page load.
*/
sendSyncPrefsChanged() {}

/**
* Forces an offer-trusted-vault-opt-in-changed event to be fired.
*/
sendOfferTrustedVaultOptInChanged() {}
}

/**
Expand Down Expand Up @@ -341,6 +346,11 @@ import {addSingletonGetter, sendWithPromise} from 'chrome://resources/js/cr.m.js
sendSyncPrefsChanged() {
chrome.send('SyncPrefsDispatch');
}

/** @override */
sendOfferTrustedVaultOptInChanged() {
chrome.send('SyncOfferTrustedVaultOptInDispatch');
}
}

addSingletonGetter(SyncBrowserProxyImpl);
50 changes: 50 additions & 0 deletions chrome/browser/ui/webui/settings/people_handler.cc
Expand Up @@ -50,6 +50,7 @@
#include "components/strings/grit/components_strings.h"
#include "components/sync/base/passphrase_enums.h"
#include "components/sync/base/user_selectable_type.h"
#include "components/sync/driver/sync_driver_switches.h"
#include "components/sync/driver/sync_user_settings.h"
#include "components/unified_consent/unified_consent_metrics.h"
#include "content/public/browser/render_view_host.h"
Expand All @@ -76,6 +77,9 @@ using signin::ConsentLevel;

namespace {

const char kOfferTrustedVaultOptInChangedEvent[] =
"offer-trusted-vault-opt-in-changed";

// A structure which contains all the configuration information for sync.
struct SyncConfigInfo {
SyncConfigInfo();
Expand Down Expand Up @@ -288,6 +292,10 @@ void PeopleHandler::RegisterMessages() {
"SyncPrefsDispatch",
base::BindRepeating(&PeopleHandler::HandleSyncPrefsDispatch,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"SyncOfferTrustedVaultOptInDispatch",
base::BindRepeating(&PeopleHandler::HandleOfferTrustedVaultOptInDispatch,
base::Unretained(this)));
#if BUILDFLAG(IS_CHROMEOS_ASH)
web_ui()->RegisterMessageCallback(
"AttemptUserExit",
Expand Down Expand Up @@ -741,6 +749,13 @@ void PeopleHandler::HandleSyncPrefsDispatch(const base::ListValue* args) {
PushSyncPrefs();
}

void PeopleHandler::HandleOfferTrustedVaultOptInDispatch(
const base::ListValue* args) {
AllowJavascript();
FireWebUIListener(kOfferTrustedVaultOptInChangedEvent,
base::Value(ShouldOfferTrustedVaultOptIn()));
}

void PeopleHandler::CloseSyncSetup() {
// Stop a timer to handle timeout in waiting for checking network connection.
engine_start_timer_.reset();
Expand Down Expand Up @@ -816,6 +831,39 @@ void PeopleHandler::InitializeSyncBlocker() {
}
}

bool PeopleHandler::ShouldOfferTrustedVaultOptIn() const {
syncer::SyncService* sync_service = GetSyncService();
if (!sync_service) {
return false;
}

if (sync_service->GetTransportState() !=
syncer::SyncService::TransportState::ACTIVE) {
// Transport state must be active so SyncUserSettings::GetPassphraseType()
// changes once the opt-in completes, and the UI is notified.
return false;
}

switch (sync_service->GetUserSettings()->GetPassphraseType()) {
case syncer::PassphraseType::kImplicitPassphrase:
case syncer::PassphraseType::kFrozenImplicitPassphrase:
case syncer::PassphraseType::kCustomPassphrase:
case syncer::PassphraseType::kTrustedVaultPassphrase:
// Either trusted vault is already set or a transition from this
// passphrase type to trusted vault is disallowed.
return false;
case syncer::PassphraseType::kKeystorePassphrase:
if (sync_service->GetUserSettings()->IsPassphraseRequired()) {
// This should be extremely rare.
return false;
}
return base::FeatureList::IsEnabled(
switches::kSyncSupportTrustedVaultPassphraseRecovery) &&
base::FeatureList::IsEnabled(
switches::kSyncOfferTrustedVaultOptIn);
}
}

void PeopleHandler::FocusUI() {
WebContents* web_contents = web_ui()->GetWebContents();
web_contents->GetDelegate()->ActivateContents(web_contents);
Expand Down Expand Up @@ -850,6 +898,8 @@ void PeopleHandler::OnStateChanged(syncer::SyncService* sync) {
// MaybeMarkSyncConfiguring() then.
MaybeMarkSyncConfiguring();
PushSyncPrefs();
FireWebUIListener(kOfferTrustedVaultOptInChangedEvent,
base::Value(ShouldOfferTrustedVaultOptIn()));
}

void PeopleHandler::BeforeUnloadDialogCancelled() {
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/webui/settings/people_handler.h
Expand Up @@ -156,6 +156,7 @@ class PeopleHandler : public SettingsPageUIHandler,
void HandleSetDecryptionPassphrase(const base::ListValue* args);
void HandleShowSyncSetupUI(const base::ListValue* args);
void HandleSyncPrefsDispatch(const base::ListValue* args);
void HandleOfferTrustedVaultOptInDispatch(const base::ListValue* args);
#if BUILDFLAG(IS_CHROMEOS_ASH)
void HandleAttemptUserExit(const base::ListValue* args);
void HandleTurnOnSync(const base::ListValue* args);
Expand Down Expand Up @@ -207,6 +208,10 @@ class PeopleHandler : public SettingsPageUIHandler,
// |sync_blocker_|.
void InitializeSyncBlocker();

// Whether the entry in settings to opt in to trusted vault encryption
// should be visible.
bool ShouldOfferTrustedVaultOptIn() const;

// Weak pointer.
Profile* profile_;

Expand Down
54 changes: 34 additions & 20 deletions chrome/browser/ui/webui/settings/people_handler_unittest.cc
Expand Up @@ -315,29 +315,43 @@ class PeopleHandlerTest : public ChromeRenderViewHostTestHarness {
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());

std::string event;
EXPECT_TRUE(data1.arg1()->GetAsString(&event));
EXPECT_EQ(event, "sync-prefs-changed");
std::vector<const base::Value*> GetAllFiredValuesForEventName(
const std::string& event_name) {
std::vector<const base::Value*> arguments;
for (const std::unique_ptr<content::TestWebUI::CallData>& data :
web_ui_.call_data()) {
if (data->function_name() == "cr.webUIListenerCallback" &&
data->arg1()->is_string() &&
data->arg1()->GetString() == event_name) {
arguments.push_back(data->arg2());
}
}
return arguments;
}

const base::DictionaryValue* dictionary = nullptr;
EXPECT_TRUE(data1.arg2()->GetAsDictionary(&dictionary));
// Must be called at most once per test to check if a sync-prefs-changed
// event happened. Returns the single fired value.
const base::DictionaryValue* ExpectSyncPrefsChanged() {
std::vector<const base::Value*> args =
GetAllFiredValuesForEventName("sync-prefs-changed");
EXPECT_EQ(1U, args.size());
EXPECT_NE(args[0], nullptr);
EXPECT_TRUE(args[0]->is_dict());
const base::DictionaryValue* dictionary;
args[0]->GetAsDictionary(&dictionary);
return dictionary;
}

// Must be called at most once per test to check if a sync-status-changed
// event happened. Returns the single fired value.
const base::DictionaryValue* ExpectSyncStatusChanged() {
const content::TestWebUI::CallData& data = *web_ui_.call_data().back();
EXPECT_EQ("cr.webUIListenerCallback", data.function_name());

std::string event;
EXPECT_TRUE(data.arg1()->GetAsString(&event));
EXPECT_EQ(event, "sync-status-changed");

const base::DictionaryValue* dictionary = nullptr;
EXPECT_TRUE(data.arg2()->GetAsDictionary(&dictionary));
std::vector<const base::Value*> args =
GetAllFiredValuesForEventName("sync-status-changed");
EXPECT_EQ(1U, args.size());
EXPECT_NE(args[0], nullptr);
EXPECT_TRUE(args[0]->is_dict());
const base::DictionaryValue* dictionary;
args[0]->GetAsDictionary(&dictionary);
return dictionary;
}

Expand Down Expand Up @@ -463,8 +477,8 @@ TEST_F(PeopleHandlerTest,
.WillByDefault(Return(syncer::SyncService::TransportState::ACTIVE));
NotifySyncStateChanged();

// Updates for the sync status and the sync prefs are sent.
EXPECT_EQ(2U, web_ui_.call_data().size());
// Updates for the sync status, sync prefs and trusted vault opt-in are sent.
EXPECT_EQ(3U, web_ui_.call_data().size());

const base::DictionaryValue* dictionary = ExpectSyncPrefsChanged();
CheckBool(dictionary, "syncAllDataTypes", true);
Expand Down
Expand Up @@ -1027,7 +1027,10 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
{"managePasswordsPlaintext",
IDS_SETTINGS_PASSWORDS_MANAGE_PASSWORDS_PLAINTEXT},
{"savedToThisDeviceOnly",
IDS_SETTINGS_PAYMENTS_SAVED_TO_THIS_DEVICE_ONLY}};
IDS_SETTINGS_PAYMENTS_SAVED_TO_THIS_DEVICE_ONLY},
{"trustedVaultOptInLabel", IDS_SETTINGS_TRUSTED_VAULT_OPT_IN_LABEL},
{"trustedVaultOptInSubLabel",
IDS_SETTINGS_TRUSTED_VAULT_OPT_IN_SUB_LABEL}};

GURL google_password_manager_url = GetGooglePasswordManagerURL(
password_manager::ManagePasswordsReferrer::kChromeSettings);
Expand Down Expand Up @@ -1090,6 +1093,9 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
l10n_util::GetStringFUTF16(
IDS_SETTINGS_SIGNED_OUT_USER_HAS_COMPROMISED_CREDENTIALS_LABEL,
base::ASCIIToUTF16(chrome::kSyncLearnMoreURL)));
// TODO(crbug.com/1202088): Add the correct URL here when that's available.
html_source->AddString("trustedVaultOptInUrl",
google_password_manager_url.spec());

bool is_guest_mode = false;
#if BUILDFLAG(IS_CHROMEOS_ASH)
Expand Down
6 changes: 6 additions & 0 deletions chrome/test/data/webui/settings/test_sync_browser_proxy.js
Expand Up @@ -25,6 +25,7 @@ export class TestSyncBrowserProxy extends TestBrowserProxy {
'signOut',
'pauseSync',
'sendSyncPrefsChanged',
'sendOfferTrustedVaultOptInChanged',
'startSignIn',
'startSyncingWithEmail',
];
Expand Down Expand Up @@ -131,6 +132,11 @@ export class TestSyncBrowserProxy extends TestBrowserProxy {
this.methodCalled('sendSyncPrefsChanged');
}

/** @override */
sendOfferTrustedVaultOptInChanged() {
this.methodCalled('sendOfferTrustedVaultOptInChanged');
}

/** @override */
attemptUserExit() {}

Expand Down
5 changes: 5 additions & 0 deletions components/sync/driver/sync_driver_switches.cc
Expand Up @@ -90,4 +90,9 @@ const base::Feature kSyncSupportTrustedVaultPassphraseRecovery{
"SyncSupportTrustedVaultPassphraseRecovery",
base::FEATURE_DISABLED_BY_DEFAULT};

// Whether the entry point to opt in to trusted vault in settings should be
// shown.
const base::Feature kSyncOfferTrustedVaultOptIn{
"SyncOfferTrustedVaultOptIn", base::FEATURE_DISABLED_BY_DEFAULT};

} // namespace switches
2 changes: 2 additions & 0 deletions components/sync/driver/sync_driver_switches.h
Expand Up @@ -41,6 +41,8 @@ extern const base::FeatureParam<base::TimeDelta> kSyncPolicyLoadTimeout;

extern const base::Feature kSyncSupportTrustedVaultPassphraseRecovery;

extern const base::Feature kSyncOfferTrustedVaultOptIn;

} // namespace switches

#endif // COMPONENTS_SYNC_DRIVER_SYNC_DRIVER_SWITCHES_H_

0 comments on commit 86010a1

Please sign in to comment.