Store external protocol prompt exceptions per-origin
Modify the external protocol launch prompt behavior, specifically the
behavior of the checkbox that allows the user to opt out of future
prompts. Today that opt-out applies universally for a protocol, across
all origins. The new behavior is that the opt-out applies only for the
current {protocol,origin} tuple. The checkbox is only shown for
trustworthy origins, and only if the relevant group policy
is not explicitly disabled.
This change completely removes the old exceptions and does not
migrate them. These old exceptions are from a privacy/security
point of view "opt-ins", i.e. they are a relaxation of the
default security settings. Clearing the old exceptions recovers users
who had encountered the inferior version of this prompt in the past,
and gets them into a known safe state.
Bug: 1065116
Change-Id: I6c81857894ca479ac723aa642d6ed6795e204f59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124967
Commit-Queue: Todd Sahl <[email protected]>
Reviewed-by: Balazs Engedy <[email protected]>
Reviewed-by: Dominick Ng <[email protected]>
Reviewed-by: Scott Violet <[email protected]>
Reviewed-by: Drew Wilson <[email protected]>
Cr-Commit-Position: refs/heads/master@{#760151}
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 65c2203..adf1ea0 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -7614,8 +7614,8 @@
<message name="IDS_EXTERNAL_PROTOCOL_CANCEL_BUTTON_TEXT" desc="Button in External Protocol Dialog that does not launch the application associated with the protocol">
Cancel
</message>
- <message name="IDS_EXTERNAL_PROTOCOL_CHECKBOX_TEXT" desc="Checkbox to ask the user whether they would like their decision remembered.">
- Always open links of this type in the associated app
+ <message name="IDS_EXTERNAL_PROTOCOL_CHECKBOX_PER_ORIGIN_TEXT" desc="The External Protocol Dialog is shown when a navigation is started from a webpage, such as https://google.com, to a URL where the protocol indicates that a different software application on the machine should be opened, external to the browser. For example, 'skype:' may indicate that the Skype application be opened. This string describes a checkbox that allows the user to opt out of seeing this confirmation dialog in the future from pages with the same origin as the current page, when trying to launch the same protocol currently being launched">
+ Always allow <ph name="ORIGIN">$1<ex>https://google.com</ex></ph> to open links of this type in the associated app
</message>
</if>
<if expr="chromeos">
@@ -7710,7 +7710,7 @@
<message name="IDS_RECENTLY_CLOSED_WINDOW" desc="In Title Case: Title of recently closed windows in Recent Tabs menu. [ICU_Syntax]">
{NUM_TABS, plural, =1 {1 Tab} other {# Tabs}}
</message>
- <message name="IDS_RECENT_TABS_NO_DEVICE_TABS" desc="In Title Case: The label in the Recent Tabs menu in the wrench menu when there's no tabs from other devices.">
+ <message name="IDS_RECENT_TABS_NO_DEVICE_TABS" desc="In Title Case: The label in the Recent Tabs menu in the wrench menu when the re's no tabs from other devices.">
No Tabs From Other Devices
</message>
</if>
diff --git a/chrome/app/generated_resources_grd/IDS_EXTERNAL_PROTOCOL_CHECKBOX_PER_ORIGIN_TEXT.png.sha1 b/chrome/app/generated_resources_grd/IDS_EXTERNAL_PROTOCOL_CHECKBOX_PER_ORIGIN_TEXT.png.sha1
new file mode 100644
index 0000000..79e6f28
--- /dev/null
+++ b/chrome/app/generated_resources_grd/IDS_EXTERNAL_PROTOCOL_CHECKBOX_PER_ORIGIN_TEXT.png.sha1
@@ -0,0 +1 @@
+9d6d06ec926d6d4033c114acf332725349a7aaf8
\ No newline at end of file
diff --git a/chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.cc b/chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.cc
index 25e1e62..e90ea488 100644
--- a/chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.cc
+++ b/chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.cc
@@ -75,7 +75,7 @@
// external protocol handler because we don't want pages to open them, but
// users still can.
const ExternalProtocolHandler::BlockState block_state =
- ExternalProtocolHandler::GetBlockState(scheme, profile_);
+ ExternalProtocolHandler::GetBlockState(scheme, nullptr, profile_);
switch (block_state) {
case ExternalProtocolHandler::DONT_BLOCK:
return metrics::OmniboxInputType::URL;
diff --git a/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc b/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
index ae47f8bf..f5e91b2 100644
--- a/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
+++ b/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
@@ -932,16 +932,24 @@
}
IN_PROC_BROWSER_TEST_F(BrowsingDataRemoverBrowserTest,
- ExternalProtocolHandlerPrefs) {
+ ExternalProtocolHandlerPerOriginPrefs) {
Profile* profile = GetBrowser()->profile();
- base::DictionaryValue prefs;
- prefs.SetBoolean("tel", false);
- profile->GetPrefs()->Set(prefs::kExcludedSchemes, prefs);
+ url::Origin test_origin = url::Origin::Create(GURL("https://example.test/"));
+ const std::string serialized_test_origin = test_origin.Serialize();
+ base::DictionaryValue origin_pref;
+ origin_pref.SetKey(serialized_test_origin,
+ base::Value(base::Value::Type::DICTIONARY));
+ base::Value* allowed_protocols_for_origin =
+ origin_pref.FindDictKey(serialized_test_origin);
+ allowed_protocols_for_origin->SetBoolKey("tel", true);
+ profile->GetPrefs()->Set(prefs::kProtocolHandlerPerOriginAllowedProtocols,
+ origin_pref);
ExternalProtocolHandler::BlockState block_state =
- ExternalProtocolHandler::GetBlockState("tel", profile);
+ ExternalProtocolHandler::GetBlockState("tel", &test_origin, profile);
ASSERT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state);
RemoveAndWait(ChromeBrowsingDataRemoverDelegate::DATA_TYPE_SITE_DATA);
- block_state = ExternalProtocolHandler::GetBlockState("tel", profile);
+ block_state =
+ ExternalProtocolHandler::GetBlockState("tel", &test_origin, profile);
ASSERT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
}
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
index 604d79e..f2256005 100644
--- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
+++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
@@ -1438,20 +1438,31 @@
TEST_F(ChromeBrowsingDataRemoverDelegateTest, RemoveExternalProtocolData) {
TestingProfile* profile = GetProfile();
+ url::Origin test_origin = url::Origin::Create(GURL("https://example.test"));
+ const std::string serialized_test_origin = test_origin.Serialize();
// Add external protocol data on profile.
base::DictionaryValue prefs;
- prefs.SetBoolean("tel", true);
- profile->GetPrefs()->Set(prefs::kExcludedSchemes, prefs);
+ prefs.SetKey(serialized_test_origin,
+ base::Value(base::Value::Type::DICTIONARY));
+ base::Value* allowed_protocols_for_origin =
+ prefs.FindDictKey(serialized_test_origin);
+ allowed_protocols_for_origin->SetBoolKey("tel", true);
+ profile->GetPrefs()->Set(prefs::kProtocolHandlerPerOriginAllowedProtocols,
+ prefs);
EXPECT_FALSE(
- profile->GetPrefs()->GetDictionary(prefs::kExcludedSchemes)->empty());
+ profile->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
BlockUntilBrowsingDataRemoved(
AnHourAgo(), base::Time::Max(),
ChromeBrowsingDataRemoverDelegate::DATA_TYPE_EXTERNAL_PROTOCOL_DATA,
false);
EXPECT_TRUE(
- profile->GetPrefs()->GetDictionary(prefs::kExcludedSchemes)->empty());
+ profile->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
}
// Check that clearing browsing data (either history or cookies with other site
diff --git a/chrome/browser/external_protocol/external_protocol_handler.cc b/chrome/browser/external_protocol/external_protocol_handler.cc
index e4f2b124..4b0b5aeb 100644
--- a/chrome/browser/external_protocol/external_protocol_handler.cc
+++ b/chrome/browser/external_protocol/external_protocol_handler.cc
@@ -22,7 +22,9 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_contents.h"
#include "net/base/escape.h"
+#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "url/gurl.h"
+#include "url/origin.h"
#if !defined(OS_ANDROID)
#include "chrome/browser/sharing/click_to_call/click_to_call_ui_controller.h"
@@ -84,11 +86,13 @@
ExternalProtocolHandler::BlockState GetBlockStateWithDelegate(
const std::string& scheme,
+ const url::Origin* initiating_origin,
ExternalProtocolHandler::Delegate* delegate,
Profile* profile) {
if (delegate)
return delegate->GetBlockState(scheme, profile);
- return ExternalProtocolHandler::GetBlockState(scheme, profile);
+ return ExternalProtocolHandler::GetBlockState(scheme, initiating_origin,
+ profile);
}
void RunExternalProtocolDialogWithDelegate(
@@ -214,9 +218,16 @@
g_external_protocol_handler_delegate = delegate;
}
-// static
+bool ExternalProtocolHandler::MayRememberAllowDecisionsForThisOrigin(
+ const url::Origin* initiating_origin) {
+ return initiating_origin &&
+ network::IsOriginPotentiallyTrustworthy(*initiating_origin);
+}
+
+// static.
ExternalProtocolHandler::BlockState ExternalProtocolHandler::GetBlockState(
const std::string& scheme,
+ const url::Origin* initiating_origin,
Profile* profile) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -245,15 +256,20 @@
PrefService* profile_prefs = profile->GetPrefs();
if (profile_prefs) { // May be NULL during testing.
- const base::DictionaryValue* update_excluded_schemas_profile =
- profile_prefs->GetDictionary(prefs::kExcludedSchemes);
- bool should_block;
- // Ignore stored block decisions. These are now not possible through the UI,
- // and previous block decisions should be ignored to allow users to recover
- // from accidental blocks.
- if (update_excluded_schemas_profile->GetBoolean(scheme, &should_block) &&
- !should_block) {
- return DONT_BLOCK;
+ if (MayRememberAllowDecisionsForThisOrigin(initiating_origin)) {
+ // Check if there is a matching {Origin+Protocol} pair exemption:
+ const base::DictionaryValue* allowed_origin_protocol_pairs =
+ profile_prefs->GetDictionary(
+ prefs::kProtocolHandlerPerOriginAllowedProtocols);
+ const base::Value* allowed_protocols_for_origin =
+ allowed_origin_protocol_pairs->FindDictKey(
+ initiating_origin->Serialize());
+ if (allowed_protocols_for_origin) {
+ base::Optional<bool> allow =
+ allowed_protocols_for_origin->FindBoolKey(scheme);
+ if (allow.has_value() && allow.value())
+ return DONT_BLOCK;
+ }
}
}
@@ -261,25 +277,48 @@
}
// static
-void ExternalProtocolHandler::SetBlockState(const std::string& scheme,
- BlockState state,
- Profile* profile) {
+// This is only called when the "remember" check box is selected from the
+// External Protocol Prompt dialog, and that check box is only shown when there
+// is a non-empty, potentially-trustworthy initiating origin.
+void ExternalProtocolHandler::SetBlockState(
+ const std::string& scheme,
+ const url::Origin& initiating_origin,
+ BlockState state,
+ Profile* profile) {
// Setting the state to BLOCK is no longer supported through the UI.
DCHECK_NE(state, BLOCK);
// Set in the stored prefs.
- PrefService* profile_prefs = profile->GetPrefs();
- if (profile_prefs) { // May be NULL during testing.
- DictionaryPrefUpdate update_excluded_schemas_profile(
- profile_prefs, prefs::kExcludedSchemes);
- if (state == DONT_BLOCK)
- update_excluded_schemas_profile->SetBoolean(scheme, false);
- else
- update_excluded_schemas_profile->Remove(scheme, nullptr);
+ if (MayRememberAllowDecisionsForThisOrigin(&initiating_origin)) {
+ PrefService* profile_prefs = profile->GetPrefs();
+ if (profile_prefs) { // May be NULL during testing.
+ DictionaryPrefUpdate update_allowed_origin_protocol_pairs(
+ profile_prefs, prefs::kProtocolHandlerPerOriginAllowedProtocols);
+
+ const std::string serialized_origin = initiating_origin.Serialize();
+ base::Value* allowed_protocols_for_origin =
+ update_allowed_origin_protocol_pairs->FindDictKey(serialized_origin);
+ if (!allowed_protocols_for_origin) {
+ update_allowed_origin_protocol_pairs->SetKey(
+ serialized_origin, base::Value(base::Value::Type::DICTIONARY));
+ allowed_protocols_for_origin =
+ update_allowed_origin_protocol_pairs->FindDictKey(
+ serialized_origin);
+ }
+ if (state == DONT_BLOCK) {
+ allowed_protocols_for_origin->SetBoolKey(scheme, true);
+ } else {
+ allowed_protocols_for_origin->RemoveKey(scheme);
+ if (allowed_protocols_for_origin->DictEmpty())
+ update_allowed_origin_protocol_pairs->RemoveKey(serialized_origin);
+ }
+ }
}
- if (g_external_protocol_handler_delegate)
- g_external_protocol_handler_delegate->OnSetBlockState(scheme, state);
+ if (g_external_protocol_handler_delegate) {
+ g_external_protocol_handler_delegate->OnSetBlockState(
+ scheme, initiating_origin, state);
+ }
}
// static
@@ -307,7 +346,8 @@
if (web_contents) // Maybe NULL during testing.
profile = Profile::FromBrowserContext(web_contents->GetBrowserContext());
BlockState block_state = GetBlockStateWithDelegate(
- escaped_url.scheme(), g_external_protocol_handler_delegate, profile);
+ escaped_url.scheme(), base::OptionalOrNullptr(initiating_origin),
+ g_external_protocol_handler_delegate, profile);
if (block_state == BLOCK) {
if (g_external_protocol_handler_delegate)
g_external_protocol_handler_delegate->BlockRequest();
@@ -384,11 +424,12 @@
// static
void ExternalProtocolHandler::RegisterPrefs(PrefRegistrySimple* registry) {
- registry->RegisterDictionaryPref(prefs::kExcludedSchemes);
+ registry->RegisterDictionaryPref(
+ prefs::kProtocolHandlerPerOriginAllowedProtocols);
}
// static
void ExternalProtocolHandler::ClearData(Profile* profile) {
PrefService* prefs = profile->GetPrefs();
- prefs->ClearPref(prefs::kExcludedSchemes);
+ prefs->ClearPref(prefs::kProtocolHandlerPerOriginAllowedProtocols);
}
diff --git a/chrome/browser/external_protocol/external_protocol_handler.h b/chrome/browser/external_protocol/external_protocol_handler.h
index ccacbdc3..efab7fd 100644
--- a/chrome/browser/external_protocol/external_protocol_handler.h
+++ b/chrome/browser/external_protocol/external_protocol_handler.h
@@ -16,6 +16,10 @@
class WebContents;
}
+namespace url {
+class Origin;
+}
+
class GURL;
class PrefRegistrySimple;
class Profile;
@@ -61,8 +65,9 @@
virtual void FinishedProcessingCheck() = 0;
virtual void OnSetBlockState(const std::string& scheme,
+ const url::Origin& initiating_origin,
ExternalProtocolHandler::BlockState state) {}
- virtual ~Delegate() {}
+ virtual ~Delegate() = default;
};
// UMA histogram metric names.
@@ -72,11 +77,24 @@
// ExternalProtocolHandler::Delegate for testing code.
static void SetDelegateForTesting(Delegate* delegate);
- // Returns whether we should block a given scheme.
- static BlockState GetBlockState(const std::string& scheme, Profile* profile);
+ // True if |initiating_origin| is not nullptr and is considered
+ // potentially trustworthy.
+ static bool MayRememberAllowDecisionsForThisOrigin(
+ const url::Origin* initiating_origin);
- // Sets whether we should block a given scheme.
+ // Returns whether we should block a given scheme.
+ // |initiating_origin| can be nullptr if the user is performing a
+ // browser initiated top frame navigation, for example by typing in the
+ // address bar or right-clicking a link and selecting 'Open In New Tab'.
+ // Renderer-initiated navigations will set |initiating_origin| to the origin
+ // of the content requesting the navigation.
+ static BlockState GetBlockState(const std::string& scheme,
+ const url::Origin* initiating_origin,
+ Profile* profile);
+
+ // Sets whether we should block a given scheme + origin.
static void SetBlockState(const std::string& scheme,
+ const url::Origin& initiating_origin,
BlockState state,
Profile* profile);
diff --git a/chrome/browser/external_protocol/external_protocol_handler_unittest.cc b/chrome/browser/external_protocol/external_protocol_handler_unittest.cc
index 4116ece..7631df6 100644
--- a/chrome/browser/external_protocol/external_protocol_handler_unittest.cc
+++ b/chrome/browser/external_protocol/external_protocol_handler_unittest.cc
@@ -289,61 +289,171 @@
TEST_F(ExternalProtocolHandlerTest, TestGetBlockStateUnknown) {
ExternalProtocolHandler::BlockState block_state =
- ExternalProtocolHandler::GetBlockState("tel", profile_.get());
+ ExternalProtocolHandler::GetBlockState("tel", nullptr, profile_.get());
EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
EXPECT_TRUE(
- profile_->GetPrefs()->GetDictionary(prefs::kExcludedSchemes)->empty());
+ profile_->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
}
TEST_F(ExternalProtocolHandlerTest, TestGetBlockStateDefaultBlock) {
ExternalProtocolHandler::BlockState block_state =
- ExternalProtocolHandler::GetBlockState("afp", profile_.get());
- EXPECT_EQ(ExternalProtocolHandler::BLOCK, block_state);
- block_state = ExternalProtocolHandler::GetBlockState("res", profile_.get());
+ ExternalProtocolHandler::GetBlockState("afp", nullptr, profile_.get());
EXPECT_EQ(ExternalProtocolHandler::BLOCK, block_state);
block_state =
- ExternalProtocolHandler::GetBlockState("ie.http", profile_.get());
+ ExternalProtocolHandler::GetBlockState("res", nullptr, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::BLOCK, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState("ie.http", nullptr,
+ profile_.get());
EXPECT_EQ(ExternalProtocolHandler::BLOCK, block_state);
EXPECT_TRUE(
- profile_->GetPrefs()->GetDictionary(prefs::kExcludedSchemes)->empty());
+ profile_->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
}
TEST_F(ExternalProtocolHandlerTest, TestGetBlockStateDefaultDontBlock) {
ExternalProtocolHandler::BlockState block_state =
- ExternalProtocolHandler::GetBlockState("mailto", profile_.get());
+ ExternalProtocolHandler::GetBlockState("mailto", nullptr, profile_.get());
EXPECT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state);
EXPECT_TRUE(
- profile_->GetPrefs()->GetDictionary(prefs::kExcludedSchemes)->empty());
+ profile_->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
}
TEST_F(ExternalProtocolHandlerTest, TestSetBlockState) {
- const char kScheme[] = "custom";
+ const char kScheme_1[] = "custom1";
+ const char kScheme_2[] = "custom2";
+ url::Origin example_origin_1 =
+ url::Origin::Create(GURL("https://example.test"));
+ url::Origin example_origin_2 =
+ url::Origin::Create(GURL("https://example2.test"));
ExternalProtocolHandler::BlockState block_state =
- ExternalProtocolHandler::GetBlockState(kScheme, profile_.get());
+ ExternalProtocolHandler::GetBlockState(kScheme_1, &example_origin_1,
+ profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_2, &example_origin_1, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_1, &example_origin_2, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_2, &example_origin_2, profile_.get());
EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
EXPECT_TRUE(
- profile_->GetPrefs()->GetDictionary(prefs::kExcludedSchemes)->empty());
+ profile_->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
- // Set to DONT_BLOCK, and make sure it is written to prefs.
- ExternalProtocolHandler::SetBlockState(
- kScheme, ExternalProtocolHandler::DONT_BLOCK, profile_.get());
- block_state = ExternalProtocolHandler::GetBlockState(kScheme, profile_.get());
+ // Set to DONT_BLOCK for {kScheme_1, example_origin_1}, and make sure it is
+ // written to prefs.
+ ExternalProtocolHandler::SetBlockState(kScheme_1, example_origin_1,
+ ExternalProtocolHandler::DONT_BLOCK,
+ profile_.get());
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_1, &example_origin_1, profile_.get());
EXPECT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state);
- base::Value expected_excluded_schemes(base::Value::Type::DICTIONARY);
- expected_excluded_schemes.SetKey(kScheme, base::Value(false));
- EXPECT_EQ(expected_excluded_schemes,
- *profile_->GetPrefs()->GetDictionary(prefs::kExcludedSchemes));
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_2, &example_origin_1, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_1, &example_origin_2, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_2, &example_origin_2, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+
+ // Set to DONT_BLOCK for {kScheme_2, example_origin_2}, and make sure it is
+ // written to prefs independently of {kScheme_1, example_origin_1}.
+ ExternalProtocolHandler::SetBlockState(kScheme_2, example_origin_2,
+ ExternalProtocolHandler::DONT_BLOCK,
+ profile_.get());
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_1, &example_origin_1, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_2, &example_origin_1, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_1, &example_origin_2, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_2, &example_origin_2, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state);
+
+ const base::DictionaryValue* protocol_origin_pairs =
+ profile_->GetPrefs()->GetDictionary(
+ prefs::kProtocolHandlerPerOriginAllowedProtocols);
+ base::Value expected_allowed_protocols_for_example_origin_1(
+ base::Value::Type::DICTIONARY);
+ expected_allowed_protocols_for_example_origin_1.SetKey(kScheme_1,
+ base::Value(true));
+ const base::Value* allowed_protocols_for_example_origin_1 =
+ protocol_origin_pairs->FindDictKey(example_origin_1.Serialize());
+ EXPECT_EQ(expected_allowed_protocols_for_example_origin_1,
+ *allowed_protocols_for_example_origin_1);
+ base::Value expected_allowed_protocols_for_example_origin_2(
+ base::Value::Type::DICTIONARY);
+ expected_allowed_protocols_for_example_origin_2.SetKey(kScheme_2,
+ base::Value(true));
+ const base::Value* allowed_protocols_for_example_origin_2 =
+ protocol_origin_pairs->FindDictKey(example_origin_2.Serialize());
+ EXPECT_EQ(expected_allowed_protocols_for_example_origin_2,
+ *allowed_protocols_for_example_origin_2);
// Note: BLOCK is no longer supported (it triggers a DCHECK in SetBlockState;
// see https://crbug.com/724919).
// Set back to UNKNOWN, and make sure this results in an empty dictionary.
- ExternalProtocolHandler::SetBlockState(
- kScheme, ExternalProtocolHandler::UNKNOWN, profile_.get());
- block_state = ExternalProtocolHandler::GetBlockState(kScheme, profile_.get());
+ ExternalProtocolHandler::SetBlockState(kScheme_1, example_origin_1,
+ ExternalProtocolHandler::UNKNOWN,
+ profile_.get());
+ ExternalProtocolHandler::SetBlockState(kScheme_2, example_origin_2,
+ ExternalProtocolHandler::UNKNOWN,
+ profile_.get());
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_1, &example_origin_1, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme_2, &example_origin_2, profile_.get());
EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
EXPECT_TRUE(
- profile_->GetPrefs()->GetDictionary(prefs::kExcludedSchemes)->empty());
+ profile_->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
+}
+
+TEST_F(ExternalProtocolHandlerTest, TestSetBlockStateWithUntrustowrthyOrigin) {
+ const char kScheme[] = "custom";
+ // This origin is untrustworthy because it is "http://"
+ url::Origin untrustworthy_origin =
+ url::Origin::Create(GURL("http://example.test"));
+
+ ExternalProtocolHandler::BlockState block_state =
+ ExternalProtocolHandler::GetBlockState(kScheme, &untrustworthy_origin,
+ profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ EXPECT_TRUE(
+ profile_->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
+
+ // Set to DONT_BLOCK for {kScheme, untrustworthy_origin}, and make sure it is
+ // not written to prefs. Calling SetBlockState with a non-trustworthy origin
+ // should not persist any state to prefs.
+ ExternalProtocolHandler::SetBlockState(kScheme, untrustworthy_origin,
+ ExternalProtocolHandler::DONT_BLOCK,
+ profile_.get());
+ block_state = ExternalProtocolHandler::GetBlockState(
+ kScheme, &untrustworthy_origin, profile_.get());
+ EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state);
+ EXPECT_TRUE(
+ profile_->GetPrefs()
+ ->GetDictionary(prefs::kProtocolHandlerPerOriginAllowedProtocols)
+ ->empty());
}
// Test that an opaque initiating origin gets transformed to its precursor
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 4cea944..00ab630 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -524,6 +524,9 @@
const char kPrintingAllowedPageSizes[] = "printing.allowed_page_sizes";
#endif // defined(OS_CHROMEOS)
+// Deprecated 4/2020
+const char kExcludedSchemes[] = "protocol_handler.excluded_schemes";
+
// Register local state used only for migration (clearing or moving to a new
// key).
void RegisterLocalStatePrefsForMigration(PrefRegistrySimple* registry) {
@@ -616,6 +619,8 @@
registry->RegisterIntegerPref(kAmbientModeTopicSource, 0);
registry->RegisterListPref(kPrintingAllowedPageSizes);
#endif // defined(OS_CHROMEOS)
+
+ registry->RegisterDictionaryPref(kExcludedSchemes);
}
} // namespace
@@ -1222,4 +1227,7 @@
// Added 4/2020.
profile_prefs->ClearPref(kPrintingAllowedPageSizes);
#endif
+
+ // Added 4/2020
+ profile_prefs->ClearPref(kExcludedSchemes);
}
diff --git a/chrome/browser/profiles/profile.cc b/chrome/browser/profiles/profile.cc
index 7e0e0fa6..a51e8f3 100644
--- a/chrome/browser/profiles/profile.cc
+++ b/chrome/browser/profiles/profile.cc
@@ -299,7 +299,8 @@
registry->RegisterListPref(prefs::kMediaRouterTabMirroringSources);
registry->RegisterDictionaryPref(prefs::kWebShareVisitedTargets);
- registry->RegisterDictionaryPref(prefs::kExcludedSchemes);
+ registry->RegisterDictionaryPref(
+ prefs::kProtocolHandlerPerOriginAllowedProtocols);
// Instead of registering new prefs here, please create a static method and
// invoke it from RegisterProfilePrefs() in
diff --git a/chrome/browser/ui/browser_ui_prefs.cc b/chrome/browser/ui/browser_ui_prefs.cc
index a8070d77..5919e8f 100644
--- a/chrome/browser/ui/browser_ui_prefs.cc
+++ b/chrome/browser/ui/browser_ui_prefs.cc
@@ -135,6 +135,6 @@
registry->RegisterBooleanPref(prefs::kUserFeedbackAllowed, true);
registry->RegisterBooleanPref(prefs::kAllowSyncXHRInPageDismissal, false);
registry->RegisterBooleanPref(
- prefs::kExternalProtocolDialogShowAlwaysOpenCheckbox, false);
+ prefs::kExternalProtocolDialogShowAlwaysOpenCheckbox, true);
registry->RegisterBooleanPref(prefs::kScreenCaptureAllowed, true);
}
diff --git a/chrome/browser/ui/views/external_protocol_dialog.cc b/chrome/browser/ui/views/external_protocol_dialog.cc
index 4fef48b..0f75ee0 100644
--- a/chrome/browser/ui/views/external_protocol_dialog.cc
+++ b/chrome/browser/ui/views/external_protocol_dialog.cc
@@ -103,10 +103,27 @@
Profile* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
- if (profile->GetPrefs()->GetBoolean(
- prefs::kExternalProtocolDialogShowAlwaysOpenCheckbox)) {
- ShowRememberSelectionCheckbox();
+ // The checkbox allows the user to opt-in to relaxed security
+ // (i.e. skipping future prompts) for the combination of the
+ // protocol and the origin of the page initiating this external
+ // protocol launch. The checkbox is offered so long as the
+ // group policy to show the checkbox is not explicitly disabled
+ // and there is a trustworthy initiating origin.
+ bool show_remember_selection_checkbox =
+ profile->GetPrefs()->GetBoolean(
+ prefs::kExternalProtocolDialogShowAlwaysOpenCheckbox) &&
+ ExternalProtocolHandler::MayRememberAllowDecisionsForThisOrigin(
+ base::OptionalOrNullptr(initiating_origin_));
+
+ if (show_remember_selection_checkbox) {
+ message_box_view_->SetCheckBoxLabel(l10n_util::GetStringFUTF16(
+ IDS_EXTERNAL_PROTOCOL_CHECKBOX_PER_ORIGIN_TEXT,
+ url_formatter::FormatOriginForSecurityDisplay(
+ initiating_origin_.value(),
+ /*scheme_display = */ url_formatter::SchemeDisplay::
+ OMIT_CRYPTOGRAPHIC)));
}
+
constrained_window::ShowWebModalDialogViews(this, web_contents);
chrome::RecordDialogCreation(chrome::DialogIdentifier::EXTERNAL_PROTOCOL);
}
@@ -140,11 +157,13 @@
}
if (remember) {
+ DCHECK(initiating_origin_);
Profile* profile =
Profile::FromBrowserContext(web_contents()->GetBrowserContext());
- ExternalProtocolHandler::SetBlockState(
- url_.scheme(), ExternalProtocolHandler::DONT_BLOCK, profile);
+ ExternalProtocolHandler::SetBlockState(url_.scheme(), *initiating_origin_,
+ ExternalProtocolHandler::DONT_BLOCK,
+ profile);
}
ExternalProtocolHandler::LaunchUrlWithoutSecurityCheck(url_, web_contents());
@@ -166,14 +185,7 @@
return message_box_view_->GetWidget();
}
-void ExternalProtocolDialog::ShowRememberSelectionCheckbox() {
- message_box_view_->SetCheckBoxLabel(
- l10n_util::GetStringUTF16(IDS_EXTERNAL_PROTOCOL_CHECKBOX_TEXT));
-}
-
void ExternalProtocolDialog::SetRememberSelectionCheckboxCheckedForTesting(
bool checked) {
- if (!message_box_view_->HasCheckBox())
- ShowRememberSelectionCheckbox();
message_box_view_->SetCheckBoxSelected(checked);
}
diff --git a/chrome/browser/ui/views/external_protocol_dialog.h b/chrome/browser/ui/views/external_protocol_dialog.h
index 6241718..212102d26 100644
--- a/chrome/browser/ui/views/external_protocol_dialog.h
+++ b/chrome/browser/ui/views/external_protocol_dialog.h
@@ -6,6 +6,7 @@
#define CHROME_BROWSER_UI_VIEWS_EXTERNAL_PROTOCOL_DIALOG_H_
#include "base/macros.h"
+#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/web_contents_observer.h"
#include "ui/views/window/dialog_delegate.h"
#include "url/gurl.h"
@@ -45,7 +46,6 @@
private:
friend class test::ExternalProtocolDialogTestApi;
- void ShowRememberSelectionCheckbox();
void SetRememberSelectionCheckboxCheckedForTesting(bool checked);
void OnDialogAccepted();
diff --git a/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc b/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc
index 6bcc57d..8dc0e65 100644
--- a/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc
+++ b/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc
@@ -55,12 +55,13 @@
}
// DialogBrowserTest:
- void ShowUi(const std::string& name) override {
+ void ShowUi(const std::string& initiating_origin) override {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
- dialog_ = new ExternalProtocolDialog(web_contents, GURL("telnet://12345"),
- base::UTF8ToUTF16("/usr/bin/telnet"),
- base::nullopt);
+ dialog_ = new ExternalProtocolDialog(
+ web_contents, GURL("telnet://12345"),
+ base::UTF8ToUTF16("/usr/bin/telnet"),
+ url::Origin::Create(GURL(initiating_origin)));
}
void SetChecked(bool checked) {
@@ -91,8 +92,11 @@
url_did_launch_ = true;
}
void FinishedProcessingCheck() override {}
- void OnSetBlockState(const std::string& scheme, BlockState state) override {
+ void OnSetBlockState(const std::string& scheme,
+ const url::Origin& initiating_origin,
+ BlockState state) override {
blocked_scheme_ = scheme;
+ blocked_origin_ = initiating_origin;
blocked_state_ = state;
}
@@ -101,6 +105,7 @@
protected:
ExternalProtocolDialog* dialog_ = nullptr;
std::string blocked_scheme_;
+ url::Origin blocked_origin_;
BlockState blocked_state_ = BlockState::UNKNOWN;
bool url_did_launch_ = false;
@@ -109,7 +114,7 @@
};
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest, TestAccept) {
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
dialog_->AcceptDialog();
EXPECT_EQ(blocked_state_, BlockState::UNKNOWN);
EXPECT_TRUE(url_did_launch_);
@@ -120,10 +125,11 @@
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest,
TestAcceptWithChecked) {
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
SetChecked(true);
dialog_->AcceptDialog();
EXPECT_EQ(blocked_scheme_, "telnet");
+ EXPECT_EQ(blocked_origin_, url::Origin::Create(GURL("https://example.test")));
EXPECT_EQ(blocked_state_, BlockState::DONT_BLOCK);
EXPECT_TRUE(url_did_launch_);
histogram_tester_.ExpectBucketCount(
@@ -131,11 +137,23 @@
ExternalProtocolHandler::CHECKED_LAUNCH, 1);
}
+IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest,
+ TestAcceptWithUntrustworthyOrigin) {
+ ShowUi(std::string("http://example.test"));
+ SetChecked(true); // This will no-opt because http:// is not trustworthy
+ dialog_->AcceptDialog();
+ EXPECT_EQ(blocked_state_, BlockState::UNKNOWN);
+ EXPECT_TRUE(url_did_launch_);
+ histogram_tester_.ExpectBucketCount(
+ ExternalProtocolHandler::kHandleStateMetric,
+ ExternalProtocolHandler::LAUNCH, 1);
+}
+
// Regression test for http://crbug.com/835216. The OS owns the dialog, so it
// may may outlive the WebContents it is attached to.
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest,
TestAcceptAfterCloseTab) {
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
SetChecked(true); // |remember_| must be true for the segfault to occur.
browser()->tab_strip_model()->CloseAllTabs();
dialog_->AcceptDialog();
@@ -146,7 +164,7 @@
}
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest, TestCancel) {
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
dialog_->CancelDialog();
EXPECT_EQ(blocked_state_, BlockState::UNKNOWN);
EXPECT_FALSE(url_did_launch_);
@@ -157,7 +175,7 @@
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest,
TestCancelWithChecked) {
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
SetChecked(true);
dialog_->CancelDialog();
// Cancel() should not enforce the remember checkbox.
@@ -170,7 +188,7 @@
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest, TestClose) {
// Closing the dialog should be the same as canceling, except for histograms.
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
EXPECT_TRUE(dialog_->Close());
EXPECT_EQ(blocked_state_, BlockState::UNKNOWN);
EXPECT_FALSE(url_did_launch_);
@@ -182,7 +200,7 @@
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest,
TestCloseWithChecked) {
// Closing the dialog should be the same as canceling, except for histograms.
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
SetChecked(true);
EXPECT_TRUE(dialog_->Close());
EXPECT_EQ(blocked_state_, BlockState::UNKNOWN);
@@ -201,7 +219,7 @@
// Tests that keyboard focus works when the dialog is shown. Regression test for
// https://crbug.com/1025343.
IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest, TestFocus) {
- ShowUi(std::string());
+ ShowUi(std::string("https://example.test"));
gfx::NativeWindow window = browser()->window()->GetNativeWindow();
views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
views::FocusManager* focus_manager = widget->GetFocusManager();
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index 6be0af3..b5c5788 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -1725,10 +1725,13 @@
// Extensions which should be opened upon completion.
const char kDownloadExtensionsToOpen[] = "download.extensions_to_open";
-// Dictionary of schemes used by the external protocol handler. If a scheme
-// is present with value |false|, the protocol may be launched without first
-// prompting the user.
-const char kExcludedSchemes[] = "protocol_handler.excluded_schemes";
+// Dictionary of origins that have permission to launch at least one protocol
+// without first prompting the user. Each origin is a nested dictionary.
+// Within an origin dictionary, if a protocol is present with value |true|,
+// that protocol may be launched by that origin without first prompting
+// the user.
+const char kProtocolHandlerPerOriginAllowedProtocols[] =
+ "protocol_handler.allowed_origin_protocol_pairs";
// String containing the last known intranet redirect URL, if any. See
// intranet_redirect_detector.h for more information.
@@ -2347,7 +2350,7 @@
// (user session is controlled by user profile preference
// kResolveTimezoneByGeolocation)
//
-// Deprecated. Superseeded by kResolveDeviceTimezoneByGeolocationMethod.
+// Deprecated. Superseded by kResolveDeviceTimezoneByGeolocationMethod.
// TODO(alemate): https://crbug.com/783367 Remove outdated prefs.
const char kResolveDeviceTimezoneByGeolocation[] =
"settings.resolve_device_timezone_by_geolocation";
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 89f09a4..d02d33d 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -569,7 +569,7 @@
extern const char kSelectFileLastDirectory[];
-extern const char kExcludedSchemes[];
+extern const char kProtocolHandlerPerOriginAllowedProtocols[];
extern const char kLastKnownIntranetRedirectOrigin[];
extern const char kDNSInterceptionChecksEnabled[];
