0

[Snooping protection] Used correct capitalization in popup.

Notifier titles that are improper nouns (e.g. "web") should not be
capitalized unless they are the first word in the popup message. This CL
adds new lower-case versions of these titles and the logic to use them
where appropriate.

As the message logic has become more complex, this CL also factors it
out for testing. One complexity: in the absence of a test
`AppRegistryCache` object, we inject one via template "duck typing".

Bug: 1298327
Change-Id: Ic9e2dcbb72899282ac1f56f495ff8f031d23a62e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3539324
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Michael Martis <martis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#984163}
This commit is contained in:
Michael Martis
2022-03-23 03:09:43 +00:00
committed by Chromium LUCI CQ
parent 0aad6b0af9
commit c0e9ad53ad
11 changed files with 295 additions and 91 deletions

@ -1128,6 +1128,7 @@ component("ash") {
"system/holding_space/screen_captures_section.h",
"system/hps/hps_notify_notification_blocker.cc",
"system/hps/hps_notify_notification_blocker.h",
"system/hps/hps_notify_notification_blocker_internal.h",
"system/hps/hps_orientation_controller.cc",
"system/hps/hps_orientation_controller.h",
"system/ime/ime_feature_pod_controller.cc",

@ -4777,21 +4777,30 @@ New install
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_MESSAGE_3_PLUS" desc="Notification message explaining that a popup from three or more applications have been blocked.">
<ph name="APP_1_TITLE">$1<ex>Google Chat</ex></ph>, <ph name="APP_2_TITLE">$2<ex>Chrome</ex></ph> and other notifications are hidden because viewing protection is on
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_SYSTEM_TITLE" desc="Title used for the OS when listing blocked notification sources.">
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_SYSTEM_TITLE_UPPER" desc="Title used for the OS when listing it as the first word in the blocked notifications message (i.e. capitalized).">
System
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE" desc="Title used for a generic application when listing blocked notification sources.">
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_SYSTEM_TITLE_LOWER" desc="Title used for the OS when listing it as a subsequent word in the blocked notifications message.">
system
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE_UPPER" desc="Title used for a generic application when listing it as the first word in the blocked notifications message (i.e. capitalized).">
App
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE_LOWER" desc="Title used for a generic application when listing it as a subsequent word in the blocked notifications message.">
app
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_ARC_TITLE" desc="Title used for an Android application when listing blocked notification sources.">
Android
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_CROSTINI_TITLE" desc="Title used for a Linux application when listing blocked notification sources.">
Linux
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE" desc="Title used for the web browser when listing blocked notification sources.">
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_UPPER" desc="Title used for the web browser when listing it as the first word in the blocked notifications message (i.e. capitalized).">
Web
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_LOWER" desc="Title used for the web browser when listing it as a subsequent word in the blocked notifications message.">
web
</message>
<message name="IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_PHONE_HUB_TITLE" desc="Title used for the Phone Hub feature when listing blocked notification sources.">
Phone Hub
</message>

@ -0,0 +1 @@
edec4cad3e1839f2ad641cebbeb7970ea0029171

@ -0,0 +1 @@
edec4cad3e1839f2ad641cebbeb7970ea0029171

@ -0,0 +1 @@
edec4cad3e1839f2ad641cebbeb7970ea0029171

@ -15,6 +15,7 @@
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/system/hps/hps_notify_notification_blocker_internal.h"
#include "ash/system/model/system_tray_model.h"
#include "ash/system/network/sms_observer.h"
#include "ash/system/status_area_widget.h"
@ -23,18 +24,15 @@
#include "base/bind.h"
#include "base/check_op.h"
#include "base/memory/weak_ptr.h"
#include "base/no_destructor.h"
#include "base/notreached.h"
#include "base/strings/string_util.h"
#include "chromeos/dbus/hps/hps_service.pb.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_service.h"
#include "components/services/app_service/public/cpp/app_registry_cache.h"
#include "components/services/app_service/public/cpp/app_registry_cache_wrapper.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/message_center/message_center.h"
#include "ui/message_center/public/cpp/notification.h"
#include "ui/message_center/public/cpp/notification_types.h"
#include "ui/message_center/public/cpp/notifier_id.h"
namespace ash {
@ -42,93 +40,56 @@ namespace {
constexpr char kNotifierId[] = "hps-notify";
// Returns a human-readable title for the given notification source.
std::u16string GetNotifierTitle(const message_center::NotifierId& id) {
std::u16string title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_SYSTEM_TITLE);
// Returns the capitalized version of an improper-noun notifier title, or the
// unchanged title if it is a proper noun.
std::u16string GetCapitalizedNotifierTitle(const std::u16string& title) {
static base::NoDestructor<std::map<std::u16string, std::u16string>>
kCapitalizedTitles(
{{l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE_LOWER),
l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE_UPPER)},
{l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_SYSTEM_TITLE_LOWER),
l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_SYSTEM_TITLE_UPPER)},
{l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_LOWER),
l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_UPPER)}});
// Assign default title based on notifier type.
switch (id.type) {
case message_center::NotifierType::APPLICATION:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE);
break;
case message_center::NotifierType::ARC_APPLICATION:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_ARC_TITLE);
break;
case message_center::NotifierType::CROSTINI_APPLICATION:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_CROSTINI_TITLE);
break;
case message_center::NotifierType::WEB_PAGE:
title = id.title.value_or(l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE));
break;
case message_center::NotifierType::SYSTEM_COMPONENT:
// Handled by initial value.
break;
case message_center::NotifierType::PHONE_HUB:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_PHONE_HUB_TITLE);
break;
}
// If we can access a more-specific app title, assign it here.
if (id.type == message_center::NotifierType::APPLICATION ||
id.type == message_center::NotifierType::ARC_APPLICATION ||
id.type == message_center::NotifierType::CROSTINI_APPLICATION) {
// Access the registry of human-readable app names.
const AccountId account_id =
Shell::Get()->session_controller()->GetActiveAccountId();
apps::AppRegistryCache* app_cache =
apps::AppRegistryCacheWrapper::Get().GetAppRegistryCache(account_id);
if (!app_cache) {
LOG(ERROR) << "Couldn't find app registry cache for user";
return title;
}
const bool found =
app_cache->ForApp(id.id, [&](const apps::AppUpdate& update) {
const std::string& short_name = update.ShortName();
title = std::u16string(short_name.begin(), short_name.end());
});
if (!found)
LOG(WARNING) << "No matching notifier found for ID " << id.id;
}
return title;
}
// Returns the right message for the number of titles.
std::u16string GetTitlesBlockedMessage(
const std::vector<std::u16string>& titles) {
switch (titles.size()) {
case 0:
return u"";
case 1:
return l10n_util::GetStringFUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_MESSAGE_1, titles[0]);
case 2:
return l10n_util::GetStringFUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_MESSAGE_2, titles[0],
titles[1]);
default:
return l10n_util::GetStringFUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_MESSAGE_3_PLUS, titles[0],
titles[1]);
}
const auto it = kCapitalizedTitles->find(title);
return it == kCapitalizedTitles->end() ? title : it->second;
}
} // namespace
namespace hps_internal {
// Returns the popup message listing the correct notifier titles and the correct
// number of titles.
std::u16string GetTitlesBlockedMessage(
const std::vector<std::u16string>& titles) {
DCHECK(!titles.empty());
const std::u16string& first_title = GetCapitalizedNotifierTitle(titles[0]);
switch (titles.size()) {
case 1:
return l10n_util::GetStringFUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_MESSAGE_1, first_title);
case 2:
return l10n_util::GetStringFUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_MESSAGE_2, first_title,
titles[1]);
default:
return l10n_util::GetStringFUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_MESSAGE_3_PLUS,
first_title, titles[1]);
}
}
} // namespace hps_internal
HpsNotifyNotificationBlocker::HpsNotifyNotificationBlocker(
message_center::MessageCenter* message_center,
HpsNotifyController* controller)
@ -308,7 +269,10 @@ HpsNotifyNotificationBlocker::CreateInfoNotification() const {
continue;
// Use a human readable-title (e.g. "Web" vs "https://somesite.com:443").
const std::u16string& title = GetNotifierTitle(notification->notifier_id());
const std::u16string& title =
hps_internal::GetNotifierTitle<apps::AppRegistryCacheWrapper>(
notification->notifier_id(),
Shell::Get()->session_controller()->GetActiveAccountId());
if (seen_titles.find(title) != seen_titles.end())
continue;
@ -329,7 +293,7 @@ HpsNotifyNotificationBlocker::CreateInfoNotification() const {
message_center::NOTIFICATION_TYPE_SIMPLE, kInfoNotificationId,
l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_TITLE),
GetTitlesBlockedMessage(titles),
hps_internal::GetTitlesBlockedMessage(titles),
/*display_source=*/std::u16string(), /*origin_url=*/GURL(),
message_center::NotifierId(message_center::NotifierType::SYSTEM_COMPONENT,
kNotifierId),

@ -0,0 +1,100 @@
// Copyright 2022 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.
#ifndef ASH_SYSTEM_HPS_HPS_NOTIFY_NOTIFICATION_BLOCKER_INTERNAL_H_
#define ASH_SYSTEM_HPS_HPS_NOTIFY_NOTIFICATION_BLOCKER_INTERNAL_H_
// Logic exposed only for testing on which clients of the notification blocker
// cannot rely.
#include <string>
#include "ash/strings/grit/ash_strings.h"
#include "components/account_id/account_id.h"
#include "components/services/app_service/public/cpp/app_registry_cache.h"
#include "components/services/app_service/public/cpp/app_registry_cache_wrapper.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/message_center/public/cpp/notification_types.h"
#include "ui/message_center/public/cpp/notifier_id.h"
namespace ash {
namespace hps_internal {
// Returns a human-readable title for the given notification source. Improper
// nouns are always returned lower-case and must be subsequently capitalized if
// necessary.
//
// Takes a template argument to allow injection of a duck-typed fake app
// registry cache.
template <typename AppRegistryCacheWrapperType>
std::u16string GetNotifierTitle(const message_center::NotifierId& id,
const AccountId& account_id) {
std::u16string title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_SYSTEM_TITLE_LOWER);
// Assign default title based on notifier type.
switch (id.type) {
case message_center::NotifierType::APPLICATION:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE_LOWER);
break;
case message_center::NotifierType::ARC_APPLICATION:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_ARC_TITLE);
break;
case message_center::NotifierType::CROSTINI_APPLICATION:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_CROSTINI_TITLE);
break;
case message_center::NotifierType::WEB_PAGE:
title = id.title.value_or(l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_LOWER));
break;
case message_center::NotifierType::SYSTEM_COMPONENT:
// Handled by initial value.
break;
case message_center::NotifierType::PHONE_HUB:
title = l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_PHONE_HUB_TITLE);
break;
}
// If we can access a more-specific app title, assign it here.
if (id.type == message_center::NotifierType::APPLICATION ||
id.type == message_center::NotifierType::ARC_APPLICATION ||
id.type == message_center::NotifierType::CROSTINI_APPLICATION) {
// Access the registry of human-readable app names.
auto* app_cache =
AppRegistryCacheWrapperType::Get().GetAppRegistryCache(account_id);
if (!app_cache) {
LOG(ERROR) << "Couldn't find app registry cache for user";
return title;
}
const bool found =
app_cache->ForApp(id.id, [&](const apps::AppUpdate& update) {
const std::string& short_name = update.ShortName();
title = std::u16string(short_name.begin(), short_name.end());
});
if (!found)
LOG(WARNING) << "No matching notifier found for ID " << id.id;
}
return title;
}
std::u16string ASH_EXPORT
GetTitlesBlockedMessage(const std::vector<std::u16string>& titles);
} // namespace hps_internal
} // namespace ash
#endif // ASH_SYSTEM_HPS_HPS_NOTIFY_NOTIFICATION_BLOCKER_INTERNAL_H_

@ -15,6 +15,7 @@
#include "ash/session/session_controller_impl.h"
#include "ash/session/test_session_controller_client.h"
#include "ash/shell.h"
#include "ash/system/hps/hps_notify_notification_blocker_internal.h"
#include "ash/system/message_center/message_center_controller.h"
#include "ash/system/message_center/unified_message_center_bubble.h"
#include "ash/system/network/sms_observer.h"
@ -23,12 +24,17 @@
#include "ash/system/unified/unified_system_tray_bubble.h"
#include "ash/test/ash_test_base.h"
#include "base/memory/scoped_refptr.h"
#include "base/no_destructor.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_command_line.h"
#include "base/test/scoped_feature_list.h"
#include "chromeos/dbus/hps/fake_hps_dbus_client.h"
#include "chromeos/dbus/hps/hps_dbus_client.h"
#include "chromeos/dbus/hps/hps_service.pb.h"
#include "components/account_id/account_id.h"
#include "components/services/app_service/public/cpp/app_types.h"
#include "components/services/app_service/public/cpp/app_update.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/image/image.h"
#include "ui/message_center/message_center.h"
#include "ui/message_center/public/cpp/notification.h"
@ -131,6 +137,42 @@ class IdPopupBlocker : public message_center::NotificationBlocker {
std::string target_id_;
};
// An app registry cache that can be used to register test app names.
class FakeAppRegistryCache {
public:
// The following methods match the interface of apps::AppRegistryCacheWrapper.
static FakeAppRegistryCache& Get() {
static base::NoDestructor<FakeAppRegistryCache> kInstance;
return *kInstance;
}
// In the real class, returns the cache for the given user. We only maintain a
// single fake cache.
FakeAppRegistryCache* GetAppRegistryCache(const AccountId&) { return this; }
template <typename FunctionType>
bool ForApp(const std::string& app_id, FunctionType f) {
for (const std::unique_ptr<apps::AppUpdate>& app : apps_) {
if (app_id == app->AppId()) {
f(*app);
return true;
}
}
return false;
}
// Test-only method to populate fake apps.
void AddApp(std::unique_ptr<apps::AppUpdate> app) {
apps_.push_back(std::move(app));
}
private:
// Use pointers as we need a default constructor.
std::vector<std::unique_ptr<apps::AppUpdate>> apps_;
};
// A test fixture that gives access to the HPS notify controller (to fake
// snooping events).
class HpsNotifyNotificationBlockerTest : public AshTestBase {
@ -444,6 +486,91 @@ TEST_F(HpsNotifyNotificationBlockerTest, SettingsButtonClicked) {
EXPECT_EQ(1, GetNumOsSmartPrivacySettingsOpened());
}
TEST(HpsNotifyNotificationBlockerInternalTest, WebsiteNotifierTitles) {
// Website without title uses a generic "web" string.
const message_center::NotifierId untrusted_notifier(
GURL("http://untrusted.com:443"));
const std::u16string untrusted_title =
hps_internal::GetNotifierTitle<FakeAppRegistryCache>(untrusted_notifier,
AccountId());
EXPECT_EQ(untrusted_title,
l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_LOWER));
// Website with a trusted title uses the title.
const message_center::NotifierId trusted_notifier(
GURL("https://trusted.com:443"), u"Trusted");
const std::u16string trusted_title =
hps_internal::GetNotifierTitle<FakeAppRegistryCache>(trusted_notifier,
AccountId());
EXPECT_EQ(trusted_title, u"Trusted");
}
TEST(HpsNotifyNotificationBlockerInternalTest, AppNotifierTitles) {
// App without known title uses a generic "app" string.
const message_center::NotifierId unknown_app_notifier(
message_center::NotifierType::APPLICATION, "unknown-app");
const std::u16string unknown_app_title =
hps_internal::GetNotifierTitle<FakeAppRegistryCache>(unknown_app_notifier,
AccountId());
EXPECT_EQ(unknown_app_title,
l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_APP_TITLE_LOWER));
// Create an app cache with one used and one unused app.
auto* app_cache =
FakeAppRegistryCache::Get().GetAppRegistryCache(AccountId());
apps::App crostini_app_state(apps::AppType::kCrostini, "crostini-app");
crostini_app_state.short_name = "Signal Messenger";
auto crostini_app = std::make_unique<apps::AppUpdate>(
&crostini_app_state, /*delta=*/nullptr, AccountId());
app_cache->AddApp(std::move(crostini_app));
apps::App arc_app_state(apps::AppType::kArc, "arc-app");
arc_app_state.short_name = "Discord";
auto arc_app = std::make_unique<apps::AppUpdate>(
&arc_app_state, /*delta=*/nullptr, AccountId());
app_cache->AddApp(std::move(arc_app));
// App with a registered name uses that name.
const message_center::NotifierId crostini_app_notifier(
message_center::NotifierType::CROSTINI_APPLICATION, "crostini-app");
const std::u16string crostini_app_title =
hps_internal::GetNotifierTitle<FakeAppRegistryCache>(
crostini_app_notifier, AccountId());
EXPECT_EQ(crostini_app_title, u"Signal Messenger");
}
TEST(HpsNotifyNotificationBlockerInternalTest, PopupMessage) {
// Proper app names should be presented as-is.
const std::vector<std::u16string> list_1 = {u"App title"};
const std::u16string list_1_msg =
hps_internal::GetTitlesBlockedMessage(list_1);
EXPECT_TRUE(list_1_msg.find(u"App title") != std::u16string::npos);
// Improper app names should use a reasonable default. In this case, the
// default should be capitalized since it is the first word in the message.
const std::vector<std::u16string> list_2 = {l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_LOWER)};
const std::u16string list_2_msg =
hps_internal::GetTitlesBlockedMessage(list_2);
EXPECT_TRUE(
list_2_msg.find(l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_UPPER)) !=
std::u16string::npos);
// Subsequent improper app names should not be capitalized.
const std::vector<std::u16string> list_3 = {
u"App title",
l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_LOWER)};
const std::u16string list_3_msg =
hps_internal::GetTitlesBlockedMessage(list_3);
EXPECT_TRUE(
list_3_msg.find(l10n_util::GetStringUTF16(
IDS_ASH_SMART_PRIVACY_SNOOPING_NOTIFICATION_WEB_TITLE_UPPER)) ==
std::u16string::npos);
}
} // namespace
} // namespace ash