Skip to content

GitLab

T
tangled
Commit fc0d1b93 authored by Xinghui Lu's avatar Xinghui Lu Committed by Chromium LUCI CQ
Browse files
Options

Enable enhanced protection message in interstitials by default. 

To fully replace the old SBER checkbox with the new enhanced protection
message, some tests are removed entirely because the SBER checkbox
doesn't exist anymore:
SafeBrowsingBlockingPageBrowserTest.VisitWhitePaper
SafeBrowsingBlockingPageBrowserTest.ToggleSBEROn
SafeBrowsingBlockingPageBrowserTest.ToggleSBEROff

Some tests are slightly modified by removing the old
extended-reporting-opt-in element:
SafeBrowsingBlockingPageBrowserTest.
  MainFrameBlockedShouldHaveNoDOMDetails
SafeBrowsingBlockingPageBrowserTest.ReloadWhileInterstitialShowing
SafeBrowsingBlockingPageBrowserTest.VerifyHitReportNotSentOnIncognito

For PolicyTest, remove the check of opt-in checkbox in
SafeBrowsingExtendedReportingPolicyManaged and add a new test for
checking the new enhanced-protection-message message with the
kSafeBrowsingProtectionLevel policy.


Bug: 1130721
Change-Id: Iaac03d14158d1b207a24b7799ddb8572d22c0d3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622057
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843356}
parent 135cd032
Hide whitespace changes
Inline Side-by-side
Showing with 93 additions and 147 deletions
chrome/browser/policy/policy_test_utils.cc
View file @ fc0d1b93
......@@ -343,30 +343,6 @@ void PolicyTest::WaitForInterstitial(content::WebContents* tab) {
ASSERT_TRUE(WaitForRenderFrameReady(tab->GetMainFrame()));
}
int PolicyTest::IsExtendedReportingCheckboxVisibleOnInterstitial() {
const std::string command = base::StringPrintf(
"var node = document.getElementById('extended-reporting-opt-in');"
"if (node) {"
" window.domAutomationController.send(node.offsetWidth > 0 || "
" node.offsetHeight > 0 ? %d : %d);"
"} else {"
// The node should be present but not visible, so trigger an error
// by sending false if it's not present.
" window.domAutomationController.send(%d);"
"}",
security_interstitials::CMD_TEXT_FOUND,
security_interstitials::CMD_TEXT_NOT_FOUND,
security_interstitials::CMD_ERROR);
content::WebContents* tab =
browser()->tab_strip_model()->GetActiveWebContents();
WaitForInterstitial(tab);
int result = 0;
EXPECT_TRUE(content::ExecuteScriptAndExtractInt(tab->GetMainFrame(), command,
&result));
return result;
}
void PolicyTest::SendInterstitialCommand(
content::WebContents* tab,
security_interstitials::SecurityInterstitialCommand command) {
......
chrome/browser/policy/policy_test_utils.h
View file @ fc0d1b93
......@@ -105,7 +105,7 @@ class PolicyTest : public InProcessBrowserTest {
void WaitForInterstitial(content::WebContents* tab);
int IsExtendedReportingCheckboxVisibleOnInterstitial();
int IsEnhancedProtectionMessageVisibleOnInterstitial();
void SendInterstitialCommand(
content::WebContents* tab,
......
chrome/browser/policy/safe_browsing_policy_browsertest.cc
View file @ fc0d1b93
......@@ -15,6 +15,7 @@
#include "components/policy/core/common/policy_map.h"
#include "components/policy/policy_constants.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
#include "components/safe_browsing/core/features.h"
#include "content/public/test/browser_test.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
......@@ -25,26 +26,37 @@ using testing::Return;
namespace policy {
// Test that when extended reporting is managed by policy, the opt-in checkbox
// does not appear on SSL blocking pages.
IN_PROC_BROWSER_TEST_F(PolicyTest, SafeBrowsingExtendedReportingPolicyManaged) {
net::EmbeddedTestServer https_server_expired(
net::EmbeddedTestServer::TYPE_HTTPS);
https_server_expired.SetSSLConfig(net::EmbeddedTestServer::CERT_EXPIRED);
https_server_expired.ServeFilesFromSourceDirectory("chrome/test/data");
ASSERT_TRUE(https_server_expired.Start());
int PolicyTest::IsEnhancedProtectionMessageVisibleOnInterstitial() {
const std::string command = base::StringPrintf(
"var node = document.getElementById('enhanced-protection-message');"
"if (node) {"
" window.domAutomationController.send(node.offsetWidth > 0 || "
" node.offsetHeight > 0 ? %d : %d);"
"} else {"
// The node should be present but not visible, so trigger an error
// by sending false if it's not present.
" window.domAutomationController.send(%d);"
"}",
security_interstitials::CMD_TEXT_FOUND,
security_interstitials::CMD_TEXT_NOT_FOUND,
security_interstitials::CMD_ERROR);
content::WebContents* tab =
browser()->tab_strip_model()->GetActiveWebContents();
WaitForInterstitial(tab);
int result = 0;
EXPECT_TRUE(content::ExecuteScriptAndExtractInt(tab->GetMainFrame(), command,
&result));
return result;
}
// Test extended reporting is managed by policy.
IN_PROC_BROWSER_TEST_F(PolicyTest, SafeBrowsingExtendedReportingPolicyManaged) {
// Set the extended reporting pref to True and ensure the enterprise policy
// can overwrite it.
PrefService* prefs = browser()->profile()->GetPrefs();
prefs->SetBoolean(prefs::kSafeBrowsingScoutReportingEnabled, true);
// First, navigate to an SSL error page and make sure the checkbox appears by
// default.
ui_test_utils::NavigateToURL(browser(), https_server_expired.GetURL("/"));
EXPECT_EQ(security_interstitials::CMD_TEXT_FOUND,
IsExtendedReportingCheckboxVisibleOnInterstitial());
// Set the enterprise policy to disable extended reporting.
EXPECT_TRUE(
prefs->GetBoolean(prefs::kSafeBrowsingExtendedReportingOptInAllowed));
......@@ -66,11 +78,46 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, SafeBrowsingExtendedReportingPolicyManaged) {
// deprecated, then SBER's policy management will imply whether the checkbox
// is visible.
EXPECT_TRUE(safe_browsing::IsExtendedReportingOptInAllowed(*prefs));
}
// Test that when Safe Browsing state is managed by policy, the enhanced
// protection message does not appear on SSL blocking pages.
IN_PROC_BROWSER_TEST_F(PolicyTest, SafeBrowsingStatePolicyManaged) {
net::EmbeddedTestServer https_server_expired(
net::EmbeddedTestServer::TYPE_HTTPS);
https_server_expired.SetSSLConfig(net::EmbeddedTestServer::CERT_EXPIRED);
https_server_expired.ServeFilesFromSourceDirectory("chrome/test/data");
ASSERT_TRUE(https_server_expired.Start());
// Set the Safe Browsing state to standard protection.
PrefService* prefs = browser()->profile()->GetPrefs();
safe_browsing::SetSafeBrowsingState(
prefs, safe_browsing::SafeBrowsingState::STANDARD_PROTECTION);
// First, navigate to an SSL error page and make sure the enhanced protection
// message appears by default.
ui_test_utils::NavigateToURL(browser(), https_server_expired.GetURL("/"));
EXPECT_EQ(security_interstitials::CMD_TEXT_FOUND,
IsEnhancedProtectionMessageVisibleOnInterstitial());
// Set the enterprise policy to force standard protection.
PolicyMap policies;
policies.Set(policy::key::kSafeBrowsingProtectionLevel,
policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
policy::POLICY_SOURCE_CLOUD,
base::Value(/* standard protection */ 1), nullptr);
UpdateProviderPolicy(policies);
// Policy should have overwritten the pref, and it should be managed.
EXPECT_EQ(safe_browsing::SafeBrowsingState::STANDARD_PROTECTION,
safe_browsing::GetSafeBrowsingState(*prefs));
EXPECT_TRUE(prefs->IsManagedPreference(prefs::kSafeBrowsingEnabled));
EXPECT_TRUE(prefs->IsManagedPreference(prefs::kSafeBrowsingEnhanced));
// Navigate to an SSL error page, the checkbox should not appear.
// Navigate to an SSL error page, the enhanced protection message should not
// appear.
ui_test_utils::NavigateToURL(browser(), https_server_expired.GetURL("/"));
EXPECT_EQ(security_interstitials::CMD_TEXT_NOT_FOUND,
IsExtendedReportingCheckboxVisibleOnInterstitial());
IsEnhancedProtectionMessageVisibleOnInterstitial());
}
// Test that when safe browsing whitelist domains are set by policy, safe
......
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
View file @ fc0d1b93
......@@ -895,42 +895,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, DontProceed) {
browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
}
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, VisitWhitePaper) {
SetupWarningAndNavigate(browser());
EXPECT_EQ(1, browser()->tab_strip_model()->count());
WebContents* interstitial_tab =
browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(interstitial_tab);
EXPECT_EQ(VISIBLE, GetVisibility("whitepaper-link"));
content::TestNavigationObserver nav_observer(nullptr);
nav_observer.StartWatchingNewWebContents();
EXPECT_TRUE(Click("whitepaper-link"));
nav_observer.Wait();
EXPECT_EQ(2, browser()->tab_strip_model()->count());
EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
// Assert the interstitial is not present in the foreground tab.
AssertNoInterstitial(false);
// Foreground tab displays the help center.
WebContents* new_tab = browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(new_tab);
EXPECT_EQ(GetWhitePaperUrl(), new_tab->GetURL());
// Interstitial should still display in the background tab.
browser()->tab_strip_model()->ActivateTabAt(
0, {TabStripModel::GestureType::kOther});
EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
EXPECT_EQ(interstitial_tab,
browser()->tab_strip_model()->GetActiveWebContents());
EXPECT_TRUE(IsShowingInterstitial(
browser()->tab_strip_model()->GetActiveWebContents()));
}
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, Proceed) {
GURL url = SetupWarningAndNavigate(browser());
......@@ -978,6 +942,7 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, IframeProceed) {
#endif
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
MAYBE_IframeOptInAndReportThreatDetails) {
SetExtendedReportingPrefForTests(browser()->profile()->GetPrefs(), true);
// The extended reporting opt-in is presented in the interstitial for malware,
// phishing, and UwS threats.
const bool expect_threat_details =
......@@ -994,8 +959,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
ThreatDetails* threat_details = details_factory_.get_details();
EXPECT_EQ(expect_threat_details, threat_details != nullptr);
EXPECT_EQ(VISIBLE, GetVisibility("extended-reporting-opt-in"));
EXPECT_TRUE(Click("opt-in-checkbox"));
EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
AssertNoInterstitial(true); // Assert the interstitial is gone
......@@ -1065,6 +1028,7 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
MainFrameBlockedShouldHaveNoDOMDetailsWhenDontProceed) {
SetExtendedReportingPrefForTests(browser()->profile()->GetPrefs(), true);
const bool expect_threat_details =
SafeBrowsingBlockingPage::ShouldReportThreatDetails(
testing::get<0>(GetParam()));
......@@ -1092,8 +1056,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
EXPECT_EQ(expect_threat_details, threat_details != nullptr);
// Go back.
EXPECT_EQ(VISIBLE, GetVisibility("extended-reporting-opt-in"));
EXPECT_TRUE(Click("opt-in-checkbox"));
EXPECT_TRUE(ClickAndWaitForDetach("primary-button"));
AssertNoInterstitial(true); // Assert the interstitial is gone
......@@ -1118,6 +1080,7 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
IN_PROC_BROWSER_TEST_P(
SafeBrowsingBlockingPageBrowserTest,
MainFrameBlockedShouldHaveNoDOMDetailsWhenProceeding) {
SetExtendedReportingPrefForTests(browser()->profile()->GetPrefs(), true);
const bool expect_threat_details =
SafeBrowsingBlockingPage::ShouldReportThreatDetails(
testing::get<0>(GetParam()));
......@@ -1143,8 +1106,6 @@ IN_PROC_BROWSER_TEST_P(
EXPECT_EQ(expect_threat_details, threat_details != nullptr);
// Proceed through the warning.
EXPECT_EQ(VISIBLE, GetVisibility("extended-reporting-opt-in"));
EXPECT_TRUE(Click("opt-in-checkbox"));
EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
AssertNoInterstitial(true); // Assert the interstitial is gone
......@@ -1227,16 +1188,19 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
embedded_test_server()->GetURL(kEmptyPage));
}
// Verifies that the reporting checkbox is still shown if the page is reloaded
// while the interstitial is showing.
// Verifies that the enhanced protection message is still shown if the page is
// reloaded while the interstitial is showing.
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
ReloadWhileInterstitialShowing) {
safe_browsing::SetSafeBrowsingState(
browser()->profile()->GetPrefs(),
safe_browsing::SafeBrowsingState::STANDARD_PROTECTION);
// Start navigation to bad page (kEmptyPage), which will be blocked before it
// is committed.
const GURL url = SetupWarningAndNavigate(browser());
// Checkbox should be showing.
EXPECT_EQ(VISIBLE, GetVisibility("extended-reporting-opt-in"));
EXPECT_EQ(VISIBLE, GetVisibility("enhanced-protection-message"));
WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(tab);
......@@ -1252,7 +1216,7 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
SetupWarningAndNavigate(browser());
// Checkbox should be showing.
EXPECT_EQ(VISIBLE, GetVisibility("extended-reporting-opt-in"));
EXPECT_EQ(VISIBLE, GetVisibility("enhanced-protection-message"));
// Security indicator should be showing.
ExpectSecurityIndicatorDowngrade(tab, 0u);
......@@ -1338,10 +1302,13 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
histograms.ExpectTotalCount(decision_histogram, 1);
histograms.ExpectBucketCount(decision_histogram,
security_interstitials::MetricsHelper::SHOW, 1);
histograms.ExpectTotalCount(interaction_histogram, 1);
histograms.ExpectTotalCount(interaction_histogram, 2);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::TOTAL_VISITS, 1);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::SHOW_ENHANCED_PROTECTION, 1);
// Decision should be recorded.
EXPECT_TRUE(ClickAndWaitForDetach("primary-button"));
......@@ -1350,10 +1317,13 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
histograms.ExpectBucketCount(
decision_histogram, security_interstitials::MetricsHelper::DONT_PROCEED,
1);
histograms.ExpectTotalCount(interaction_histogram, 1);
histograms.ExpectTotalCount(interaction_histogram, 2);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::TOTAL_VISITS, 1);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::SHOW_ENHANCED_PROTECTION, 1);
}
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
......@@ -1382,10 +1352,13 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
histograms.ExpectTotalCount(decision_histogram, 1);
histograms.ExpectBucketCount(decision_histogram,
security_interstitials::MetricsHelper::SHOW, 1);
histograms.ExpectTotalCount(interaction_histogram, 1);
histograms.ExpectTotalCount(interaction_histogram, 2);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::TOTAL_VISITS, 1);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::SHOW_ENHANCED_PROTECTION, 1);
// Decision should be recorded.
EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
......@@ -1393,10 +1366,13 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
histograms.ExpectTotalCount(decision_histogram, 2);
histograms.ExpectBucketCount(
decision_histogram, security_interstitials::MetricsHelper::PROCEED, 1);
histograms.ExpectTotalCount(interaction_histogram, 1);
histograms.ExpectTotalCount(interaction_histogram, 2);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::TOTAL_VISITS, 1);
histograms.ExpectBucketCount(
interaction_histogram,
security_interstitials::MetricsHelper::SHOW_ENHANCED_PROTECTION, 1);
}
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, WhitelistRevisit) {
......@@ -1492,11 +1468,9 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
incognito_browser->profile()->GetPrefs()->SetBoolean(
prefs::kSafeBrowsingScoutReportingEnabled, true); // set up SBER
GURL url = SetupWarningAndNavigate(incognito_browser); // incognito
// Check SBER opt in is not shown.
// Check enhanced protection message is not shown.
EXPECT_EQ(HIDDEN, ::safe_browsing::GetVisibility(
incognito_browser, "extended-reporting-opt-in"));
EXPECT_EQ(HIDDEN, ::safe_browsing::GetVisibility(incognito_browser,
"opt-in-checkbox"));
incognito_browser, "enhanced-protection-message"));
EXPECT_FALSE(hit_report_sent());
}
......@@ -1823,57 +1797,6 @@ IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
EXPECT_EQ(bad_url, contents->GetURL());
}
// Toggle the SBER opt in checkbox and check it enables reporting.
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, ToggleSBEROn) {
// The extended reporting opt-in is presented in the interstitial for malware,
// phishing, and UwS threats.
const bool expect_threat_details =
SafeBrowsingBlockingPage::ShouldReportThreatDetails(
testing::get<0>(GetParam()));
scoped_refptr<content::MessageLoopRunner> threat_report_sent_runner(
new content::MessageLoopRunner);
if (expect_threat_details)
SetReportSentCallback(threat_report_sent_runner->QuitClosure());
// Initially disable SBER.
SetExtendedReportingPrefForTests(browser()->profile()->GetPrefs(), false);
ASSERT_FALSE(IsExtendedReportingEnabled(*browser()->profile()->GetPrefs()));
// Navigate to a site that triggers a warning.
const GURL url = SetupWarningAndNavigate(browser());
// Click the checkbox and click through the warning.
EXPECT_EQ(VISIBLE, GetVisibility("extended-reporting-opt-in"));
EXPECT_TRUE(Click("opt-in-checkbox"));
EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
AssertNoInterstitial(true);
// Check preference is now enabled.
EXPECT_TRUE(IsExtendedReportingEnabled(*browser()->profile()->GetPrefs()));
// If a report should be sent for this type of page, check we got one.
if (expect_threat_details) {
threat_report_sent_runner->Run();
std::string serialized = GetReportSent();
ClientSafeBrowsingReportRequest report;
ASSERT_TRUE(report.ParseFromString(serialized));
EXPECT_TRUE(report.complete());
EXPECT_EQ(url.spec(), report.page_url());
}
}
// Toggle the SBER opt in checkbox and check it disables reporting.
IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, ToggleSBEROff) {
// Initially enable SBER.
SetExtendedReportingPrefForTests(browser()->profile()->GetPrefs(), true);
ASSERT_TRUE(IsExtendedReportingEnabled(*browser()->profile()->GetPrefs()));
// Navigate to a site that triggers a warning.
const GURL url = SetupWarningAndNavigate(browser());
// Click the checkbox and click through the warning.
EXPECT_EQ(VISIBLE, GetVisibility("extended-reporting-opt-in"));
EXPECT_TRUE(Click("opt-in-checkbox"));
EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
AssertNoInterstitial(true);
// Check preference is now disabled.
EXPECT_FALSE(IsExtendedReportingEnabled(*browser()->profile()->GetPrefs()));
}
class SafeBrowsingBlockingPageDelayedWarningBrowserTest
: public InProcessBrowserTest,
public testing::WithParamInterface<
......
components/safe_browsing/core/features.cc
View file @ fc0d1b93
......@@ -60,7 +60,7 @@ const base::Feature kEnhancedProtection{"SafeBrowsingEnhancedProtection",
const base::Feature kEnhancedProtectionMessageInInterstitials{
"SafeBrowsingEnhancedProtectionMessageInInterstitials",
base::FEATURE_DISABLED_BY_DEFAULT};
base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kLimitedListSizeForIOS{"SafeBrowsingLimitedListSizeForIOS",
base::FEATURE_DISABLED_BY_DEFAULT};
......
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