Make update URL optional for ExtensionInstallForcelist policy
Don't require the "update" URL to be specified for the
extension force installation policy. It should default to the
Web Store URL.
BUG=chromium:747944
TEST=unit and browser tests
Change-Id: I1dcf028573af5260f15a1dc92608192bd6a554a2
Reviewed-on: https://chromium-review.googlesource.com/1008183
Reviewed-by: Pavol Marko <[email protected]>
Reviewed-by: Devlin <[email protected]>
Commit-Queue: Maksim Ivanov <[email protected]>
Cr-Commit-Position: refs/heads/master@{#550995}diff --git a/chrome/browser/extensions/policy_handlers.cc b/chrome/browser/extensions/policy_handlers.cc
index 6370023..6050a47 100644
--- a/chrome/browser/extensions/policy_handlers.cc
+++ b/chrome/browser/extensions/policy_handlers.cc
@@ -108,21 +108,21 @@
continue;
}
- // Each string item of the list has the following form:
- // <extension_id>;<update_url>
+ // Each string item of the list should be of one of the following forms:
+ // * <extension_id>
+ // * <extension_id>;<update_url>
// Note: The update URL might also contain semicolons.
+ std::string extension_id;
+ std::string update_url;
size_t pos = entry_string.find(';');
if (pos == std::string::npos) {
- if (errors) {
- errors->AddError(policy_name(),
- entry - policy_list_value->begin(),
- IDS_POLICY_VALUE_FORMAT_ERROR);
- }
- continue;
+ extension_id = entry_string;
+ update_url = extension_urls::kChromeWebstoreUpdateURL;
+ } else {
+ extension_id = entry_string.substr(0, pos);
+ update_url = entry_string.substr(pos + 1);
}
- const std::string extension_id = entry_string.substr(0, pos);
- const std::string update_url = entry_string.substr(pos + 1);
if (!crx_file::id_util::IdIsValid(extension_id) ||
!GURL(update_url).is_valid()) {
if (errors) {
diff --git a/chrome/browser/extensions/policy_handlers_unittest.cc b/chrome/browser/extensions/policy_handlers_unittest.cc
index a613dd0..f8cff93 100644
--- a/chrome/browser/extensions/policy_handlers_unittest.cc
+++ b/chrome/browser/extensions/policy_handlers_unittest.cc
@@ -177,6 +177,7 @@
policy::PolicyErrorMap errors;
ExtensionInstallForcelistPolicyHandler handler;
+ // Start with an empty policy.
policy_map.Set(policy::key::kExtensionInstallForcelist,
policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
policy::POLICY_SOURCE_CLOUD, list.CreateDeepCopy(), nullptr);
@@ -184,6 +185,7 @@
EXPECT_TRUE(handler.CheckPolicySettings(policy_map, &errors));
EXPECT_TRUE(errors.empty());
+ // Add a correct entry. No errors should be generated.
list.AppendString("abcdefghijklmnopabcdefghijklmnop;http://example.com");
policy_map.Set(policy::key::kExtensionInstallForcelist,
policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
@@ -211,14 +213,14 @@
EXPECT_TRUE(handler.CheckPolicySettings(policy_map, &errors));
EXPECT_EQ(2U, errors.size());
- // Just an extension ID should also generate an error.
+ // Just an extension ID should be accepted.
list.AppendString("abcdefghijklmnopabcdefghijklmnop");
policy_map.Set(policy::key::kExtensionInstallForcelist,
policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
policy::POLICY_SOURCE_CLOUD, list.CreateDeepCopy(), nullptr);
errors.Clear();
EXPECT_TRUE(handler.CheckPolicySettings(policy_map, &errors));
- EXPECT_EQ(3U, errors.size());
+ EXPECT_EQ(2U, errors.size());
}
TEST(ExtensionInstallForcelistPolicyHandlerTest, ApplyPolicySettings) {
@@ -229,10 +231,12 @@
base::Value* value = NULL;
ExtensionInstallForcelistPolicyHandler handler;
+ // Start with the policy being missing. This shouldn't affect the pref.
handler.ApplyPolicySettings(policy_map, &prefs);
EXPECT_FALSE(prefs.GetValue(pref_names::kInstallForceList, &value));
EXPECT_FALSE(value);
+ // Set the policy to an empty value. This shouldn't affect the pref.
policy_map.Set(policy::key::kExtensionInstallForcelist,
policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
policy::POLICY_SOURCE_CLOUD, policy.CreateDeepCopy(), nullptr);
@@ -240,6 +244,8 @@
EXPECT_TRUE(prefs.GetValue(pref_names::kInstallForceList, &value));
EXPECT_EQ(expected, *value);
+ // Add a correct entry to the policy. The pref should contain a corresponding
+ // entry.
policy.AppendString("abcdefghijklmnopabcdefghijklmnop;http://example.com");
extensions::ExternalPolicyLoader::AddExtension(
&expected, "abcdefghijklmnopabcdefghijklmnop", "http://example.com");
@@ -250,6 +256,23 @@
EXPECT_TRUE(prefs.GetValue(pref_names::kInstallForceList, &value));
EXPECT_EQ(expected, *value);
+ // Add a correct entry with an omitted update URL. The pref should contain now
+ // two entries, with the default update URL substituted for the new entry.
+ // Note: the URL hardcoded below is part of the public policy contract (as
+ // documented in the policy_templates.json file), and therefore any changes to
+ // it must be carefully thought out.
+ policy.AppendString("bcdefghijklmnopabcdefghijklmnopa");
+ extensions::ExternalPolicyLoader::AddExtension(
+ &expected, "bcdefghijklmnopabcdefghijklmnopa",
+ "https://clients2.google.com/service/update2/crx");
+ policy_map.Set(policy::key::kExtensionInstallForcelist,
+ policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
+ policy::POLICY_SOURCE_CLOUD, policy.CreateDeepCopy(), nullptr);
+ handler.ApplyPolicySettings(policy_map, &prefs);
+ EXPECT_TRUE(prefs.GetValue(pref_names::kInstallForceList, &value));
+ EXPECT_EQ(expected, *value);
+
+ // Add an invalid entry. The pref should still contain two previous entries.
policy.AppendString("invalid");
policy_map.Set(policy::key::kExtensionInstallForcelist,
policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
