Skip to content

Commit

Permalink
Display Safe Browsing CRX allowlist warnings in chrome://extensions
Browse files Browse the repository at this point in the history
- Forward the Safe Browsing warning state in `ExtensionInfoGenerator`.
- Adjusted the warning string and icon for the Enhanced Safe Browsing
  CRX allowlist warning in 'chrome://extensions'.

Bug: 1188833
Change-Id: I6ddb92553dccbdc50581de12afee310a54997320
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2772760
Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Jeff Cyr <jeffcyr@google.com>
Cr-Commit-Position: refs/heads/master@{#868864}
  • Loading branch information
JeffCyr authored and Chromium LUCI CQ committed Apr 2, 2021
1 parent f6e2c3a commit a6ef791
Show file tree
Hide file tree
Showing 18 changed files with 158 additions and 28 deletions.
3 changes: 0 additions & 3 deletions chrome/app/chromium_strings.grd
Expand Up @@ -586,9 +586,6 @@ Chromium is unable to recover your settings.
<message name="IDS_EXTENSIONS_SHORTCUT_SCOPE_IN_CHROME" desc="The label to indicate that a shortcut will be triggerable only from within the Chrome application.">
In Chromium
</message>
<message name="IDS_EXTENSIONS_BLOCKLISTED_NOT_ALLOWLISTED" desc="The text explaining the reason for disabling extension or app. The extension in question is not included in the Safe Browsing allowlist.">
Chromium recommends disabling this extension because it does not currently meet Enhanced Safe Browsing standards.
</message>

<if expr="enable_extensions">
<message name="IDS_EXTENSIONS_MULTIPLE_UNSUPPORTED_DISABLED_BODY" desc="Body of the dialog shown when multiple unsupported extensions have been disabled.">
Expand Down

This file was deleted.

5 changes: 5 additions & 0 deletions chrome/app/generated_resources.grd
Expand Up @@ -7315,6 +7315,11 @@ Keep your key file in a safe place. You will need it to create new versions of y
Disabled by Chrome. This extension may be unsafe.
</message>

<!-- Enhanced Safe Browsing CRX allowlist warning -->
<message name="IDS_EXTENSIONS_SAFE_BROWSING_CRX_ALLOWLIST_WARNING" desc="A warning shown in 'chrome://extensions' when the extension is not included in the Enhanced Safe Browsing CRX allowlist.">
This extension is not trusted by Enhanced Safe Browsing.
</message>

<!-- Reset Profile Settings strings -->
<message name="IDS_RESET_PROFILE_SETTINGS_EXPLANATION" desc="A label describing the consequences of the 'reset profile settings' feature">
This will reset your startup page, new tab page, search engine, and pinned tabs. It will also disable all extensions and clear temporary data like cookies. Your bookmarks, history and saved passwords will not be cleared.
Expand Down
@@ -0,0 +1 @@
c08534fa7a161f16a1ff2ae9d22ebe1ef4f5638a
3 changes: 0 additions & 3 deletions chrome/app/google_chrome_strings.grd
Expand Up @@ -594,9 +594,6 @@ Google Chrome is unable to recover your settings.
<message name="IDS_EXTENSIONS_SHORTCUT_SCOPE_IN_CHROME" desc="The label to indicate that a shortcut will be triggerable only from within the Chrome application.">
In Chrome
</message>
<message name="IDS_EXTENSIONS_BLOCKLISTED_NOT_ALLOWLISTED" desc="The text explaining the reason for disabling extension or app. The extension in question is not included in the Safe Browsing allowlist.">
Chrome recommends disabling this extension because it does not currently meet Enhanced Safe Browsing standards.
</message>

<if expr="enable_extensions">
<message name="IDS_EXTENSIONS_MULTIPLE_UNSUPPORTED_DISABLED_BODY" desc="Body of the dialog shown when multiple unsupported extensions have been disabled.">
Expand Down

This file was deleted.

Expand Up @@ -32,7 +32,6 @@
#include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
#include "chrome/common/pref_names.h"
#include "chrome/grit/generated_resources.h"
#include "chrome/grit/google_chrome_strings.h"
#include "content/public/browser/render_frame_host.h"
#include "extensions/browser/extension_error.h"
#include "extensions/browser/extension_icon_placeholder.h"
Expand Down Expand Up @@ -507,18 +506,17 @@ void ExtensionInfoGenerator::CreateExtensionInfoHelper(
blocklist_text = IDS_EXTENSIONS_BLOCKLISTED_POTENTIALLY_UNWANTED;
break;
default:
if (extension_system_->extension_service()
->allowlist()
->ShouldDisplayWarning(extension.id())) {
blocklist_text = IDS_EXTENSIONS_BLOCKLISTED_NOT_ALLOWLISTED;
}
break;
}
if (blocklist_text != -1) {
info->blacklist_text.reset(
new std::string(l10n_util::GetStringUTF8(blocklist_text)));
}

if (extension_system_->extension_service()->allowlist()->ShouldDisplayWarning(
extension.id())) {
info->show_safe_browsing_allowlist_warning = true;
}
ExtensionManagement* extension_management =
ExtensionManagementFactory::GetForBrowserContext(browser_context_);
Profile* profile = Profile::FromBrowserContext(browser_context_);
Expand Down
22 changes: 21 additions & 1 deletion chrome/browser/resources/extensions/detail_view.html
Expand Up @@ -89,6 +89,14 @@
width: 18px;
}

#allowlist-warning {
flex: 1;
}

#allowlist-warning .warning-icon {
--iron-icon-fill-color: var(--warning-color);
}

ul {
margin: 0;
padding-inline-start: 20px;
Expand Down Expand Up @@ -173,7 +181,7 @@
</cr-toggle>
</div>
</div>
<div id="warnings" hidden$="[[!hasWarnings_(data.*)]]">
<div id="warnings" hidden$="[[!hasSevereWarnings_(data.*)]]">
<div id="runtime-warnings" hidden$="[[!data.runtimeWarnings.length]]"
class="cr-row continuation warning control-line">
<iron-icon class="warning-icon" icon="cr:error"></iron-icon>
Expand Down Expand Up @@ -220,6 +228,18 @@
<span>$i18n{updateRequiredByPolicy}</span>
</div>
</div>
<div id="allowlist-warning" class="cr-row continuation"
hidden$="[[!showAllowlistWarning_(data.*)]]">
<iron-icon class="warning-icon"
icon="extensions-icons:safebrowsing_warning">
</iron-icon>
<span class="cr-secondary-text">
$i18n{itemAllowlistWarning}
<a href="$i18n{enhancedSafeBrowsingWarningHelpUrl}" target="_blank">
$i18n{learnMore}
</a>
</span>
</div>
<div class="section">
<div class="section-title" role="heading" aria-level="2">
$i18n{itemDescriptionLabel}
Expand Down
14 changes: 13 additions & 1 deletion chrome/browser/resources/extensions/detail_view.js
Expand Up @@ -156,7 +156,7 @@ Polymer({
* @return {boolean}
* @private
*/
hasWarnings_() {
hasSevereWarnings_() {
return this.data.disableReasons.corruptInstall ||
this.data.disableReasons.suspiciousInstall ||
this.data.disableReasons.updateRequired || !!this.data.blacklistText ||
Expand Down Expand Up @@ -378,4 +378,16 @@ Polymer({
return enableControl === EnableControl.ENABLE_TOGGLE ||
enableControl === EnableControl.REPAIR;
},

/**
* @return {boolean} Whether the allowlist warning should be shown.
* @private
*/
showAllowlistWarning_() {
// Only show the allowlist warning if there is no blocklist warning. It
// would be redundant since all blocklisted items are necessarily not
// included in the Safe Browsing allowlist.
return this.data.showSafeBrowsingAllowlistWarning &&
!this.data.blacklistText;
},
});
7 changes: 7 additions & 0 deletions chrome/browser/resources/extensions/icons.html
Expand Up @@ -8,6 +8,13 @@
<path d="M20,5H4A2,2,0,0,0,2,7V17a2,2,0,0,0,2,2H20a2,2,0,0,0,2-2V7A2,2,0,0,0,20,5ZM9,17a5,5,0,1,1,5-5A5,5,0,0,1,9,17Zm11,1a1,1,0,1,1,1-1A1,1,0,0,1,20,18ZM20,8a1,1,0,1,1,1-1A1,1,0,0,1,20,8Z"></path>
</g>

<!-- Material Design icons. -->
<!-- Material icon name: 'gpp_maybe' -->
<g id="safebrowsing_warning">
<path d="M0 0h24v24H0z" fill="none"></path>
<path d="M12 4.24l6 3v4.1c0 3.9-2.55 7.5-6 8.59-3.45-1.09-6-4.7-6-8.59v-4.1l6-3M12 2L4 6v5.33c0 4.93 3.41 9.55 8 10.67 4.59-1.12 8-5.73 8-10.67V6l-8-4zm-1 13h2v2h-2v-2zm2-7h-2v5h2V8z"></path>
</g>

<!-- Copied from iron-icons. -->
<g id="input"><path d="M21 3.01H3c-1.1 0-2 .9-2 2V9h2V4.99h18v14.03H3V15H1v4.01c0 1.1.9 1.98 2 1.98h18c1.1 0 2-.88 2-1.98v-14c0-1.11-.9-2-2-2zM11 16l4-4-4-4v3H1v2h10v3z"></path></g>
<g id="business"><path d="M12 7V3H2v18h20V7H12zM6 19H4v-2h2v2zm0-4H4v-2h2v2zm0-4H4V9h2v2zm0-4H4V5h2v2zm4 12H8v-2h2v2zm0-4H8v-2h2v2zm0-4H8V9h2v2zm0-4H8V5h2v2zm10 12h-8v-2h2v-2h-2v-2h2v-2h-2V9h8v10zm-2-8h-2v2h2v-2zm0 4h-2v2h2v-2z"></path></g>
Expand Down
37 changes: 32 additions & 5 deletions chrome/browser/resources/extensions/item.html
Expand Up @@ -86,13 +86,26 @@
margin-bottom: 8px;
}

#error-icon {
--iron-icon-fill-color: var(--error-color);
#allowlist-warning {
flex: 1;
margin-bottom: 8px;
}

.message-icon {
height: 18px;
margin-inline-end: 4px;
vertical-align: top;
width: 18px;
}

#warnings .message-icon {
--iron-icon-fill-color: var(--error-color);
}

#allowlist-warning .message-icon {
--iron-icon-fill-color: var(--warning-color);
}

#extension-id {
flex-shrink: 0;
}
Expand Down Expand Up @@ -213,13 +226,13 @@
</div>
</div>
<div id="description" class="cr-secondary-text multiline-clippable-text"
hidden$="[[hasWarnings_(data.disableReasons.*, data.*)]]">
hidden$="[[!showDescription_(data.disableReasons.*, data.*)]]">
[[data.description]]
</div>
<template is="dom-if"
if="[[hasWarnings_(data.disableReasons.*, data.*)]]">
if="[[hasSevereWarnings_(data.disableReasons.*, data.*)]]">
<div id="warnings">
<iron-icon id="error-icon" icon="cr:error"></iron-icon>
<iron-icon class="message-icon" icon="cr:error"></iron-icon>
<span id="runtime-warnings" aria-describedby="a11yAssociation"
hidden$="[[!data.runtimeWarnings.length]]">
<template is="dom-repeat" items="[[data.runtimeWarnings]]">
Expand All @@ -242,6 +255,20 @@
--></span>
</div>
</template>
<template is="dom-if"
if="[[showAllowlistWarning_(data.disableReasons.*, data.*)]]">
<div id="allowlist-warning">
<iron-icon class="message-icon"
icon="extensions-icons:safebrowsing_warning">
</iron-icon>
<span class="cr-secondary-text" aria-describedby="a11yAssociation">
$i18n{itemAllowlistWarning}
<a href="$i18n{enhancedSafeBrowsingWarningHelpUrl}" target="_blank">
$i18n{learnMore}
</a>
</span>
</div>
</template>
<template is="dom-if" if="[[inDevMode]]">
<div id="extension-id" class="bounded-text cr-secondary-text">
[[data.id]]
Expand Down
22 changes: 18 additions & 4 deletions chrome/browser/resources/extensions/item.js
Expand Up @@ -432,17 +432,31 @@ Polymer({
* @return {boolean}
* @private
*/
hasWarnings_() {
hasSevereWarnings_() {
return this.data.disableReasons.corruptInstall ||
this.data.disableReasons.suspiciousInstall ||
this.data.runtimeWarnings.length > 0 || !!this.data.blacklistText;
},

/**
* @return {string}
* @return {boolean}
* @private
*/
showDescription_() {
return !this.hasSevereWarnings_() &&
!this.data.showSafeBrowsingAllowlistWarning;
},

/**
* @return {boolean}
* @private
*/
computeWarningsClasses_() {
return this.data.blacklistText ? 'severe' : 'mild';
showAllowlistWarning_() {
// Only show the allowlist warning if there are no other warnings. The item
// card has a fixed height and the content might get cropped if too many
// warnings are displayed. This should be a rare edge case and the allowlist
// warning will still be shown in the item detail view.
return this.data.showSafeBrowsingAllowlistWarning &&
!this.hasSevereWarnings_();
},
});
3 changes: 3 additions & 0 deletions chrome/browser/resources/extensions/shared_vars.html
Expand Up @@ -4,13 +4,16 @@
/* Note: error-color is used for many warnings. There's also an orange-y
* warning color in 1 place. */
--error-color: var(--google-red-700);
/* Copied from 'ui/gfx/color_palette.h' kGoogleYellow700 (#f29900) */
--warning-color: rgb(242, 153, 0);
--extensions-card-height: 160px;
--separator-gap: 9px;
}

@media (prefers-color-scheme: dark) {
html {
--error-color: var(--google-red-refresh-300);
--warning-color: var(--google-yellow-refresh-300);
}
}
</style>
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/webui/extensions/extensions_ui.cc
Expand Up @@ -201,6 +201,8 @@ content::WebUIDataSource* CreateMdExtensionsSource(Profile* profile,
{"itemAllowOnFollowingSites", IDS_EXTENSIONS_ALLOW_ON_FOLLOWING_SITES},
{"itemCollectErrors", IDS_EXTENSIONS_ENABLE_ERROR_COLLECTION},
{"itemCorruptInstall", IDS_EXTENSIONS_CORRUPTED_EXTENSION},
{"itemAllowlistWarning",
IDS_EXTENSIONS_SAFE_BROWSING_CRX_ALLOWLIST_WARNING},
{"itemRepair", IDS_EXTENSIONS_REPAIR_CORRUPTED},
{"itemReload", IDS_EXTENSIONS_RELOAD_TERMINATED},
{"loadErrorCouldNotLoadManifest",
Expand Down Expand Up @@ -289,6 +291,9 @@ content::WebUIDataSource* CreateMdExtensionsSource(Profile* profile,
GURL(chrome::kRemoveNonCWSExtensionURL),
g_browser_process->GetApplicationLocale())
.spec()));
source->AddString(
"enhancedSafeBrowsingWarningHelpUrl",
base::ASCIIToUTF16(chrome::kCwsEnhancedSafeBrowsingLearnMoreURL));
#if BUILDFLAG(IS_CHROMEOS_ASH)
source->AddString(
"kioskDisableBailoutWarningBody",
Expand Down
1 change: 1 addition & 0 deletions chrome/common/extensions/api/developer_private.idl
Expand Up @@ -253,6 +253,7 @@ namespace developerPrivate {
DOMString version;
ExtensionView[] views;
DOMString webStoreUrl;
boolean showSafeBrowsingAllowlistWarning;
};

dictionary ProfileInfo {
Expand Down
32 changes: 31 additions & 1 deletion chrome/test/data/webui/extensions/detail_view_test.js
Expand Up @@ -399,7 +399,7 @@ suite(extension_detail_view_tests.suiteName, function() {
testWarningVisible('#blacklisted-warning', false);
testWarningVisible('#update-required-warning', false);

item.set('data.blacklistText', 'This item is blacklisted');
item.set('data.blacklistText', 'This item is blocklisted');
flush();
testWarningVisible('#runtime-warnings', true);
testWarningVisible('#corrupted-warning', true);
Expand Down Expand Up @@ -433,5 +433,35 @@ suite(extension_detail_view_tests.suiteName, function() {
testWarningVisible('#suspicious-warning', false);
testWarningVisible('#blacklisted-warning', false);
testWarningVisible('#update-required-warning', false);

item.set('data.showSafeBrowsingAllowlistWarning', true);
flush();
testWarningVisible('#runtime-warnings', false);
testWarningVisible('#corrupted-warning', false);
testWarningVisible('#suspicious-warning', false);
testWarningVisible('#blacklisted-warning', false);
testWarningVisible('#update-required-warning', false);
testWarningVisible('#allowlist-warning', true);

item.set('data.disableReasons.suspiciousInstall', true);
flush();
testWarningVisible('#runtime-warnings', false);
testWarningVisible('#corrupted-warning', false);
testWarningVisible('#suspicious-warning', true);
testWarningVisible('#blacklisted-warning', false);
testWarningVisible('#update-required-warning', false);
testWarningVisible('#allowlist-warning', true);

// Test that the allowlist warning is not shown when there is already a
// blocklist message. It would be redundant since all blocklisted extension
// are necessarily not included in the Safe Browsing allowlist.
item.set('data.blacklistText', 'This item is blocklisted');
flush();
testWarningVisible('#runtime-warnings', false);
testWarningVisible('#corrupted-warning', false);
testWarningVisible('#suspicious-warning', true);
testWarningVisible('#blacklisted-warning', true);
testWarningVisible('#update-required-warning', false);
testWarningVisible('#allowlist-warning', false);
});
});
14 changes: 14 additions & 0 deletions chrome/test/data/webui/extensions/item_test.js
Expand Up @@ -255,6 +255,7 @@ suite(extension_item_tests.suiteName, function() {
const kSuspicious = 1 << 1;
const kBlacklisted = 1 << 2;
const kRuntime = 1 << 3;
const kSafeBrowsingAllowlist = 1 << 4;

function assertWarnings(mask) {
assertEquals(
Expand All @@ -266,6 +267,9 @@ suite(extension_item_tests.suiteName, function() {
isChildVisible(item, '#blacklisted-warning'));
assertEquals(
!!(mask & kRuntime), isChildVisible(item, '#runtime-warnings'));
assertEquals(
!!(mask & kSafeBrowsingAllowlist),
isChildVisible(item, '#allowlist-warning'));
}

assertWarnings(0);
Expand Down Expand Up @@ -295,6 +299,16 @@ suite(extension_item_tests.suiteName, function() {
item.set('data.runtimeWarnings', []);
flush();
assertWarnings(0);

item.set('data.showSafeBrowsingAllowlistWarning', true);
flush();
assertWarnings(kSafeBrowsingAllowlist);

// Test that the allowlist warning is not shown when there is already a
// warning message.
item.set('data.disableReasons.suspiciousInstall', true);
flush();
assertWarnings(kSuspicious);
});

test(assert(extension_item_tests.TestNames.SourceIndicator), function() {
Expand Down
5 changes: 3 additions & 2 deletions third_party/closure_compiler/externs/developer_private.js
@@ -1,4 +1,4 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Copyright 2021 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.

Expand Down Expand Up @@ -321,7 +321,8 @@ chrome.developerPrivate.Permissions;
* userMayModify: boolean,
* version: string,
* views: !Array<!chrome.developerPrivate.ExtensionView>,
* webStoreUrl: string
* webStoreUrl: string,
* showSafeBrowsingAllowlistWarning: boolean
* }}
*/
chrome.developerPrivate.ExtensionInfo;
Expand Down

0 comments on commit a6ef791

Please sign in to comment.