Exempt component and policy-installed extensions from policy blacklist check.
Component extensions and policy-installed extensions shouldn't be affected by the policy blacklist: The former are considered internal UI that happens to be part of an extension, and they should have first-class policy knobs if we want to disable them. For the latter, the admin specifying them is enough indication that they should be installed, and requiring them to explicitly white-listed in some cases is redundant.
BUG=chromium-os:16702, chromium-os:21281
TEST=On ChromeOS, configure ExtensionInstallBlacklist = * and load chrome://bookmarks
Review URL: http://codereview.chromium.org/8380006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107707 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 514ebe2..c348719 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -378,7 +378,7 @@
}
if (!frontend_weak_->extension_prefs()->IsExtensionAllowedByPolicy(
- extension_->id())) {
+ extension_->id(), install_source_)) {
ReportFailureFromUIThread(
l10n_util::GetStringUTF8(IDS_EXTENSION_CANT_INSTALL_POLICY_BLACKLIST));
return;
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 8485f88..9241264 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -11,10 +11,10 @@
#include "chrome/browser/prefs/pref_notifier.h"
#include "chrome/browser/prefs/scoped_user_pref_update.h"
#include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/url_constants.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/url_pattern.h"
#include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
#include "content/public/browser/notification_service.h"
using base::Time;
@@ -619,9 +619,15 @@
}
bool ExtensionPrefs::IsExtensionAllowedByPolicy(
- const std::string& extension_id) {
+ const std::string& extension_id,
+ Extension::Location location) {
std::string string_value;
+ if (location == Extension::COMPONENT ||
+ location == Extension::EXTERNAL_POLICY_DOWNLOAD) {
+ return true;
+ }
+
const ListValue* blacklist =
prefs_->GetList(prefs::kExtensionInstallDenyList);
if (!blacklist || blacklist->empty())
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index d993914..d7760a7 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -182,7 +182,8 @@
// Is the extension with |extension_id| allowed by policy (checking both
// whitelist and blacklist).
- bool IsExtensionAllowedByPolicy(const std::string& extension_id);
+ bool IsExtensionAllowedByPolicy(const std::string& extension_id,
+ Extension::Location location);
// Returns the last value set via SetLastPingDay. If there isn't such a
// pref, the returned Time will return true for is_null().
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 53fac60c..531d3b6 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -7,8 +7,8 @@
#include <algorithm>
#include <set>
-#include "base/bind.h"
#include "base/basictypes.h"
+#include "base/bind.h"
#include "base/callback.h"
#include "base/command_line.h"
#include "base/file_util.h"
@@ -92,8 +92,8 @@
#include "content/browser/plugin_service.h"
#include "content/browser/renderer_host/render_process_host.h"
#include "content/browser/user_metrics.h"
-#include "content/public/browser/notification_service.h"
#include "content/common/pepper_plugin_registry.h"
+#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
#include "googleurl/src/gurl.h"
#include "net/base/registry_controlled_domain.h"
@@ -1400,7 +1400,12 @@
bool write_to_prefs) {
std::string error;
scoped_refptr<const Extension> extension(NULL);
- if (!extension_prefs_->IsExtensionAllowedByPolicy(info.extension_id)) {
+
+ // An explicit check against policy is required to behave correctly during
+ // startup. This is because extensions that were previously OK might have
+ // been blacklisted in policy while Chrome was not running.
+ if (!extension_prefs_->IsExtensionAllowedByPolicy(info.extension_id,
+ info.extension_location)) {
error = errors::kDisabledByPolicy;
} else if (info.extension_manifest.get()) {
extension = Extension::Create(
@@ -1694,8 +1699,10 @@
for (ExtensionList::const_iterator iter = extensions_.begin();
iter != extensions_.end(); ++iter) {
const Extension* extension = (*iter);
- if (!extension_prefs_->IsExtensionAllowedByPolicy(extension->id()))
+ if (!extension_prefs_->IsExtensionAllowedByPolicy(extension->id(),
+ extension->location())) {
to_be_removed.push_back(extension->id());
+ }
}
// UnloadExtension will change the extensions_ list. So, we should
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index b2da4aa..b9083a2 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -2599,6 +2599,93 @@
EXPECT_EQ(0u, service_->extensions()->size());
}
+// Tests that component extensions are not blacklisted by policy.
+TEST_F(ExtensionServiceTest, ComponentExtensionWhitelisted) {
+ InitializeEmptyExtensionService();
+
+ // Blacklist everything.
+ {
+ ListPrefUpdate update(profile_->GetPrefs(),
+ prefs::kExtensionInstallDenyList);
+ ListValue* blacklist = update.Get();
+ blacklist->Append(Value::CreateStringValue("*"));
+ }
+
+ // Install a component extension.
+ FilePath path = data_dir_
+ .AppendASCII("good")
+ .AppendASCII("Extensions")
+ .AppendASCII(good0)
+ .AppendASCII("1.0.0.0");
+ std::string manifest;
+ ASSERT_TRUE(file_util::ReadFileToString(
+ path.Append(Extension::kManifestFilename), &manifest));
+ service_->register_component_extension(
+ ExtensionService::ComponentExtensionInfo(manifest, path));
+ service_->Init();
+
+ // Extension should be installed despite blacklist.
+ ASSERT_EQ(1u, service_->extensions()->size());
+ EXPECT_EQ(good0, service_->extensions()->at(0)->id());
+
+ // Poke external providers and make sure the extension is still present.
+ service_->CheckForExternalUpdates();
+ ASSERT_EQ(1u, service_->extensions()->size());
+ EXPECT_EQ(good0, service_->extensions()->at(0)->id());
+
+ // Extension should not be uninstalled on blacklist changes.
+ {
+ ListPrefUpdate update(profile_->GetPrefs(),
+ prefs::kExtensionInstallDenyList);
+ ListValue* blacklist = update.Get();
+ blacklist->Append(Value::CreateStringValue(good0));
+ }
+ loop_.RunAllPending();
+ ASSERT_EQ(1u, service_->extensions()->size());
+ EXPECT_EQ(good0, service_->extensions()->at(0)->id());
+}
+
+// Tests that policy-installed extensions are not blacklisted by policy.
+TEST_F(ExtensionServiceTest, PolicyInstalledExtensionsWhitelisted) {
+ InitializeEmptyExtensionService();
+
+ // Blacklist everything.
+ {
+ ListPrefUpdate update(profile_->GetPrefs(),
+ prefs::kExtensionInstallDenyList);
+ ListValue* blacklist = update.Get();
+ blacklist->Append(Value::CreateStringValue("*"));
+ }
+
+ // Have policy force-install an extension.
+ MockExtensionProvider* provider =
+ new MockExtensionProvider(service_,
+ Extension::EXTERNAL_POLICY_DOWNLOAD);
+ AddMockExternalProvider(provider);
+ provider->UpdateOrAddExtension(good_crx, "1.0.0.0",
+ data_dir_.AppendASCII("good.crx"));
+
+ // Reloading extensions should find our externally registered extension
+ // and install it.
+ service_->CheckForExternalUpdates();
+ loop_.RunAllPending();
+
+ // Extension should be installed despite blacklist.
+ ASSERT_EQ(1u, service_->extensions()->size());
+ EXPECT_EQ(good_crx, service_->extensions()->at(0)->id());
+
+ // Blacklist update should not uninstall the extension.
+ {
+ ListPrefUpdate update(profile_->GetPrefs(),
+ prefs::kExtensionInstallDenyList);
+ ListValue* blacklist = update.Get();
+ blacklist->Append(Value::CreateStringValue(good0));
+ }
+ loop_.RunAllPending();
+ ASSERT_EQ(1u, service_->extensions()->size());
+ EXPECT_EQ(good_crx, service_->extensions()->at(0)->id());
+}
+
// Tests disabling extensions
TEST_F(ExtensionServiceTest, DisableExtension) {
InitializeEmptyExtensionService();
@@ -3145,8 +3232,7 @@
// Now test the case where user uninstalls and then the extension is removed
// from the external provider.
-
- provider->UpdateOrAddExtension(good_crx, "1.0.0.1", source_path);
+ provider->UpdateOrAddExtension(good_crx, "1.0.0.1", source_path);
service_->CheckForExternalUpdates();
loop_.RunAllPending();
