Extension white and force lists (set by policy) should have priority over auto-updated Google blacklist.
BUG=93246
TEST=unit_tests
Review URL: https://chromiumcodereview.appspot.com/10854009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152548 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/admin_policy.cc b/chrome/browser/extensions/admin_policy.cc
index 8ec3f53..c25e31b 100644
--- a/chrome/browser/extensions/admin_policy.cc
+++ b/chrome/browser/extensions/admin_policy.cc
@@ -41,24 +41,29 @@
return blacklist && blacklist->Find(wildcard) != blacklist->end();
}
-bool UserMayLoad(const base::ListValue* blacklist,
+bool UserMayLoad(bool is_google_blacklisted,
+ const base::ListValue* blacklist,
const base::ListValue* whitelist,
+ const base::ListValue* forcelist,
const Extension* extension,
string16* error) {
if (IsRequired(extension))
return true;
- if (!blacklist || blacklist->empty())
+ if ((!blacklist || blacklist->empty()) && !is_google_blacklisted)
return true;
- // Check the whitelist first.
+ // Check the whitelist/forcelist first (takes precedence over Google
+ // blacklist).
base::StringValue id_value(extension->id());
- if (whitelist && whitelist->Find(id_value) != whitelist->end())
+ if ((whitelist && whitelist->Find(id_value) != whitelist->end()) ||
+ (forcelist && forcelist->Find(id_value) != forcelist->end()))
return true;
- // Then check the blacklist (the admin blacklist, not the Google blacklist).
- bool result = blacklist->Find(id_value) == blacklist->end() &&
- !BlacklistedByDefault(blacklist);
+ // Then check both admin and Google blacklists.
+ bool result = !is_google_blacklisted &&
+ (!blacklist || blacklist->Find(id_value) == blacklist->end()) &&
+ !BlacklistedByDefault(blacklist);
if (error && !result) {
*error = l10n_util::GetStringFUTF16(
IDS_EXTENSION_CANT_INSTALL_POLICY_BLACKLIST,
diff --git a/chrome/browser/extensions/admin_policy.h b/chrome/browser/extensions/admin_policy.h
index 38ff9325..5d69d07 100644
--- a/chrome/browser/extensions/admin_policy.h
+++ b/chrome/browser/extensions/admin_policy.h
@@ -22,10 +22,12 @@
// from the command line, or when loaded as an unpacked extension).
bool BlacklistedByDefault(const base::ListValue* blacklist);
-// Returns true if the extension is allowed by admin policy white- and
-// blacklists.
-bool UserMayLoad(const base::ListValue* blacklist,
+// Returns true if the extension is allowed by Google blacklist and admin policy
+// white-, black- and forcelists.
+bool UserMayLoad(bool is_google_blacklisted,
+ const base::ListValue* blacklist,
const base::ListValue* whitelist,
+ const base::ListValue* forcelist,
const Extension* extension,
string16* error);
diff --git a/chrome/browser/extensions/admin_policy_unittest.cc b/chrome/browser/extensions/admin_policy_unittest.cc
index 94afd433..875312f 100644
--- a/chrome/browser/extensions/admin_policy_unittest.cc
+++ b/chrome/browser/extensions/admin_policy_unittest.cc
@@ -50,28 +50,33 @@
// Tests UserMayLoad for required extensions.
TEST_F(ExtensionAdminPolicyTest, UserMayLoadRequired) {
CreateExtension(Extension::EXTERNAL_POLICY_DOWNLOAD, true);
- EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(), NULL));
string16 error;
- EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, extension_.get(), &error));
+ EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(),
+ &error));
EXPECT_TRUE(error.empty());
// Required extensions may load even if they're on the blacklist.
base::ListValue blacklist;
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ NULL));
blacklist.Append(Value::CreateStringValue("*"));
- EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ NULL));
}
// Tests UserMayLoad when no blacklist exists, or it's empty.
TEST_F(ExtensionAdminPolicyTest, UserMayLoadNoBlacklist) {
CreateExtension(Extension::INTERNAL, false);
- EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(), NULL));
base::ListValue blacklist;
- EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ NULL));
string16 error;
- EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), &error));
+ EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ &error));
EXPECT_TRUE(error.empty());
}
@@ -81,13 +86,16 @@
base::ListValue whitelist;
whitelist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(),
+ NULL));
base::ListValue blacklist;
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, extension_.get(), NULL));
+ EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(),
+ NULL));
string16 error;
- EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, extension_.get(), &error));
+ EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(),
+ &error));
EXPECT_TRUE(error.empty());
}
@@ -98,25 +106,31 @@
// Blacklisted by default.
base::ListValue blacklist;
blacklist.Append(Value::CreateStringValue("*"));
- EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), NULL));
+ EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ NULL));
string16 error;
- EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), &error));
+ EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ &error));
EXPECT_FALSE(error.empty());
// Extension on the blacklist, with and without wildcard.
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), NULL));
+ EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ NULL));
blacklist.Clear();
blacklist.Append(Value::CreateStringValue(extension_->id()));
- EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, extension_.get(), NULL));
+ EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(),
+ NULL));
// With a whitelist. There's no such thing as a whitelist wildcard.
base::ListValue whitelist;
whitelist.Append(
Value::CreateStringValue("behllobkkfkfnphdnhnkndlbkcpglgmj"));
- EXPECT_FALSE(ap::UserMayLoad(&blacklist, &whitelist, extension_.get(), NULL));
+ EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, &whitelist, NULL,
+ extension_.get(), NULL));
whitelist.Append(Value::CreateStringValue("*"));
- EXPECT_FALSE(ap::UserMayLoad(&blacklist, &whitelist, extension_.get(), NULL));
+ EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, &whitelist, NULL,
+ extension_.get(), NULL));
}
TEST_F(ExtensionAdminPolicyTest, UserMayModifySettings) {
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index da319ae7..6f338ed 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -395,16 +395,6 @@
if (!frontend_weak_.get())
return;
- if (frontend_weak_->extension_prefs()
- ->IsExtensionBlacklisted(extension_->id())) {
- VLOG(1) << "This extension: " << extension_->id()
- << " is blacklisted. Install failed.";
- ReportFailureFromUIThread(
- CrxInstallerError(
- l10n_util::GetStringUTF16(IDS_EXTENSION_CANT_INSTALL_BLACKLISTED)));
- return;
- }
-
string16 error;
if (!ExtensionSystem::Get(profile_)->management_policy()->
UserMayLoad(extension_, &error)) {
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index aa12933b..4b8a442 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -294,6 +294,32 @@
kBrowserActionPinned : kBrowserActionVisible;
}
+// Reads a boolean pref from |ext| with key |pref_key|.
+// Return false if the value is false or |pref_key| does not exist.
+bool ReadBooleanFromPref(const DictionaryValue* ext,
+ const std::string& pref_key) {
+ bool bool_value = false;
+ ext->GetBoolean(pref_key, &bool_value);
+ return bool_value;
+}
+
+// Reads an integer pref from |ext| with key |pref_key|.
+// Return false if the value does not exist.
+bool ReadIntegerFromPref(const DictionaryValue* ext,
+ const std::string& pref_key,
+ int* out_value) {
+ if (!ext->GetInteger(pref_key, out_value))
+ return false;
+ return out_value != NULL;
+}
+
+// Checks if kPrefBlacklist is set to true in the DictionaryValue.
+// Return false if the value is false or kPrefBlacklist does not exist.
+// This is used to decide if an extension is blacklisted.
+bool IsBlacklistBitSet(const DictionaryValue* ext) {
+ return ReadBooleanFromPref(ext, kPrefBlacklist);
+}
+
} // namespace
ExtensionPrefs::ExtensionPrefs(
@@ -381,54 +407,6 @@
}
}
-void ExtensionPrefs::MakePathsAbsolute(DictionaryValue* dict) {
- if (!dict || dict->empty())
- return;
-
- for (DictionaryValue::key_iterator i = dict->begin_keys();
- i != dict->end_keys(); ++i) {
- DictionaryValue* extension_dict = NULL;
- if (!dict->GetDictionaryWithoutPathExpansion(*i, &extension_dict)) {
- NOTREACHED();
- continue;
- }
-
- int location_value;
- if (extension_dict->GetInteger(kPrefLocation, &location_value) &&
- location_value == Extension::LOAD) {
- // Unpacked extensions will already have absolute paths.
- continue;
- }
-
- FilePath::StringType path_string;
- if (!extension_dict->GetString(kPrefPath, &path_string))
- continue;
-
- DCHECK(location_value == Extension::COMPONENT ||
- !FilePath(path_string).IsAbsolute());
- extension_dict->SetString(
- kPrefPath, install_directory_.Append(path_string).value());
- }
-}
-
-DictionaryValue* ExtensionPrefs::CopyCurrentExtensions() {
- const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
- if (extensions) {
- DictionaryValue* copy = extensions->DeepCopy();
- MakePathsAbsolute(copy);
- return copy;
- }
- return new DictionaryValue;
-}
-
-// static
-bool ExtensionPrefs::ReadBooleanFromPref(
- const DictionaryValue* ext, const std::string& pref_key) {
- bool bool_value = false;
- ext->GetBoolean(pref_key, &bool_value);
- return bool_value;
-}
-
bool ExtensionPrefs::ReadExtensionPrefBoolean(
const std::string& extension_id, const std::string& pref_key) const {
const DictionaryValue* ext = GetExtensionPref(extension_id);
@@ -439,15 +417,6 @@
return ReadBooleanFromPref(ext, pref_key);
}
-// static
-bool ExtensionPrefs::ReadIntegerFromPref(
- const DictionaryValue* ext, const std::string& pref_key, int* out_value) {
- if (!ext->GetInteger(pref_key, out_value))
- return false;
-
- return out_value != NULL;
-}
-
bool ExtensionPrefs::ReadExtensionPrefInteger(
const std::string& extension_id, const std::string& pref_key,
int* out_value) const {
@@ -610,15 +579,6 @@
}
}
-// static
-bool ExtensionPrefs::IsBlacklistBitSet(const DictionaryValue* ext) {
- return ReadBooleanFromPref(ext, kPrefBlacklist);
-}
-
-bool ExtensionPrefs::IsExtensionBlacklisted(const std::string& extension_id) {
- return ReadExtensionPrefBoolean(extension_id, kPrefBlacklist);
-}
-
bool ExtensionPrefs::IsExtensionOrphaned(const std::string& extension_id) {
// TODO(miket): we believe that this test will hinge on the number of
// consecutive times that an update check has returned a certain response
@@ -712,13 +672,17 @@
bool ExtensionPrefs::UserMayLoad(const Extension* extension,
string16* error) const {
+ const DictionaryValue* ext_prefs = GetExtensionPref(extension->id());
+ bool is_google_blacklisted = ext_prefs && IsBlacklistBitSet(ext_prefs);
const base::ListValue* blacklist =
prefs_->GetList(prefs::kExtensionInstallDenyList);
const base::ListValue* whitelist =
prefs_->GetList(prefs::kExtensionInstallAllowList);
- return admin_policy::UserMayLoad(blacklist, whitelist, extension,
- error);
+ const base::ListValue* forcelist =
+ prefs_->GetList(prefs::kExtensionInstallForceList);
+ return admin_policy::UserMayLoad(is_google_blacklisted, blacklist, whitelist,
+ forcelist, extension, error);
}
bool ExtensionPrefs::UserMayModifySettings(const Extension* extension,
@@ -1615,26 +1579,15 @@
return extension;
}
-// Helper function for GetInstalledExtensionsInfo.
-static ExtensionInfo* GetInstalledExtensionInfoImpl(
- DictionaryValue* extension_data,
- DictionaryValue::key_iterator extension_id) {
- DictionaryValue* ext;
- if (!extension_data->GetDictionaryWithoutPathExpansion(*extension_id, &ext)) {
- LOG(WARNING) << "Invalid pref for extension " << *extension_id;
- NOTREACHED();
+ExtensionInfo* ExtensionPrefs::GetInstalledExtensionInfo(
+ const std::string& extension_id) const {
+ const DictionaryValue* ext;
+ const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+ if (!extensions ||
+ !extensions->GetDictionaryWithoutPathExpansion(extension_id, &ext))
return NULL;
- }
- if (ext->HasKey(kPrefBlacklist)) {
- bool is_blacklisted = false;
- if (!ext->GetBoolean(kPrefBlacklist, &is_blacklisted)) {
- NOTREACHED() << "Invalid blacklist pref:" << *extension_id;
+ if (IsBlacklistBitSet(ext))
return NULL;
- }
- if (is_blacklisted) {
- return NULL;
- }
- }
int state_value;
if (!ext->GetInteger(kPrefState, &state_value)) {
// This can legitimately happen if we store preferences for component
@@ -1642,17 +1595,24 @@
return NULL;
}
if (state_value == Extension::EXTERNAL_EXTENSION_UNINSTALLED) {
- LOG(WARNING) << "External extension with id " << *extension_id
+ LOG(WARNING) << "External extension with id " << extension_id
<< " has been uninstalled by the user";
return NULL;
}
FilePath::StringType path;
- if (!ext->GetString(kPrefPath, &path)) {
- return NULL;
- }
int location_value;
- if (!ext->GetInteger(kPrefLocation, &location_value)) {
+ if (!ext->GetInteger(kPrefLocation, &location_value))
return NULL;
+
+ if (!ext->GetString(kPrefPath, &path))
+ return NULL;
+
+ // Make path absolute. Unpacked extensions will already have absolute paths,
+ // otherwise make it so.
+ if (location_value != Extension::LOAD) {
+ DCHECK(location_value == Extension::COMPONENT ||
+ !FilePath(path).IsAbsolute());
+ path = install_directory_.Append(path).value();
}
// Only the following extension types can be installed permanently in the
@@ -1666,29 +1626,27 @@
return NULL;
}
- DictionaryValue* manifest = NULL;
+ const DictionaryValue* manifest = NULL;
if (location != Extension::LOAD &&
!ext->GetDictionary(kPrefManifest, &manifest)) {
- LOG(WARNING) << "Missing manifest for extension " << *extension_id;
+ LOG(WARNING) << "Missing manifest for extension " << extension_id;
// Just a warning for now.
}
- return new ExtensionInfo(manifest, *extension_id, FilePath(path), location);
+ return new ExtensionInfo(manifest, extension_id, FilePath(path), location);
}
-ExtensionPrefs::ExtensionsInfo* ExtensionPrefs::GetInstalledExtensionsInfo() {
- scoped_ptr<DictionaryValue> extension_data(CopyCurrentExtensions());
-
+ExtensionPrefs::ExtensionsInfo* ExtensionPrefs::GetInstalledExtensionsInfo(
+ ) const {
ExtensionsInfo* extensions_info = new ExtensionsInfo;
- for (DictionaryValue::key_iterator extension_id(
- extension_data->begin_keys());
- extension_id != extension_data->end_keys(); ++extension_id) {
+ const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+ for (DictionaryValue::key_iterator extension_id = extensions->begin_keys();
+ extension_id != extensions->end_keys(); ++extension_id) {
if (!Extension::IdIsValid(*extension_id))
continue;
- ExtensionInfo* info = GetInstalledExtensionInfoImpl(extension_data.get(),
- extension_id);
+ ExtensionInfo* info = GetInstalledExtensionInfo(*extension_id);
if (info)
extensions_info->push_back(linked_ptr<ExtensionInfo>(info));
}
@@ -1696,22 +1654,6 @@
return extensions_info;
}
-ExtensionInfo* ExtensionPrefs::GetInstalledExtensionInfo(
- const std::string& extension_id) {
- scoped_ptr<DictionaryValue> extension_data(CopyCurrentExtensions());
-
- for (DictionaryValue::key_iterator extension_iter(
- extension_data->begin_keys());
- extension_iter != extension_data->end_keys(); ++extension_iter) {
- if (*extension_iter == extension_id) {
- return GetInstalledExtensionInfoImpl(extension_data.get(),
- extension_iter);
- }
- }
-
- return NULL;
-}
-
void ExtensionPrefs::SetIdleInstallInfo(const std::string& extension_id,
const FilePath& crx_path,
const std::string& version,
@@ -1903,8 +1845,17 @@
// static
ExtensionPrefs::ExtensionIds ExtensionPrefs::GetExtensionsFrom(
- const base::DictionaryValue* extension_prefs) {
+ const PrefService* pref_service) {
ExtensionIds result;
+
+ const base::DictionaryValue* extension_prefs;
+ const base::Value* extension_prefs_value =
+ pref_service->GetUserPrefValue(kExtensionsPref);
+ if (!extension_prefs_value ||
+ !extension_prefs_value->GetAsDictionary(&extension_prefs)) {
+ return result; // Empty set
+ }
+
for (base::DictionaryValue::key_iterator it = extension_prefs->begin_keys();
it != extension_prefs->end_keys(); ++it) {
const DictionaryValue* ext;
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index fa718f4f..6a7a7d7 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -85,6 +85,11 @@
ExtensionPrefValueMap* extension_pref_value_map);
virtual ~ExtensionPrefs();
+ // Returns all installed extensions from extension preferences provided by
+ // |pref_service|. This is exposed for ProtectedPrefsWatcher because it needs
+ // access to the extension ID list before the ExtensionService is initialized.
+ static ExtensionIds GetExtensionsFrom(const PrefService* pref_service);
+
// If |extensions_disabled| is true, extension controlled preferences and
// content settings do not become effective.
void Init(bool extensions_disabled);
@@ -130,12 +135,6 @@
// Returns all installed extensions
void GetExtensions(ExtensionIds* out);
- // Returns all installed extensions from |extension_prefs|. This is exposed
- // for ProtectedPrefsWatcher because it needs access to the extension ID list
- // before the ExtensionService is initialized.
- static ExtensionIds GetExtensionsFrom(
- const base::DictionaryValue* extension_prefs);
-
// Getter and setter for browser action visibility.
bool GetBrowserActionVisibility(const Extension* extension);
void SetBrowserActionVisibility(const Extension* extension,
@@ -175,9 +174,6 @@
// Updates the prefs based on the blacklist.
void UpdateBlacklist(const std::set<std::string>& blacklist_set);
- // Based on extension id, checks prefs to see if it is blacklisted.
- bool IsExtensionBlacklisted(const std::string& id);
-
// Based on extension id, checks prefs to see if it is orphaned.
bool IsExtensionOrphaned(const std::string& id);
@@ -340,11 +336,12 @@
// version directory and the location. Blacklisted extensions won't be saved
// and neither will external extensions the user has explicitly uninstalled.
// Caller takes ownership of returned structure.
- ExtensionsInfo* GetInstalledExtensionsInfo();
+ ExtensionsInfo* GetInstalledExtensionsInfo() const;
// Returns the ExtensionInfo from the prefs for the given extension. If the
// extension is not present, NULL is returned.
- ExtensionInfo* GetInstalledExtensionInfo(const std::string& extension_id);
+ ExtensionInfo* GetInstalledExtensionInfo(
+ const std::string& extension_id) const;
// We've downloaded an updated .crx file for the extension, but are waiting
// for idle time to install it.
@@ -494,17 +491,6 @@
// consumers who expect full paths.
void MakePathsAbsolute(base::DictionaryValue* dict);
- // Reads a boolean pref from |ext| with key |pref_key|.
- // Return false if the value is false or |pref_key| does not exist.
- static bool ReadBooleanFromPref(const base::DictionaryValue* ext,
- const std::string& pref_key);
-
- // Reads an integer pref from |ext| with key |pref_key|.
- // Return false if the value does not exist.
- static bool ReadIntegerFromPref(const base::DictionaryValue* ext,
- const std::string& pref_key,
- int* out_value);
-
// Interprets the list pref, |pref_key| in |extension_id|'s preferences, as a
// URLPatternSet. The |valid_schemes| specify how to parse the URLPatterns.
bool ReadExtensionPrefURLPatternSet(const std::string& extension_id,
@@ -538,11 +524,6 @@
void LoadExtensionControlledPrefs(const std::string& id,
ExtensionPrefsScope scope);
- // Checks if kPrefBlacklist is set to true in the DictionaryValue.
- // Return false if the value is false or kPrefBlacklist does not exist.
- // This is used to decide if an extension is blacklisted.
- static bool IsBlacklistBitSet(const base::DictionaryValue* ext);
-
// Fix missing preference entries in the extensions that are were introduced
// in a later Chrome version.
void FixMissingPrefs(const ExtensionIds& extension_ids);
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index a9351f4..728af45 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -492,7 +492,7 @@
ExtensionList::const_iterator iter;
for (iter = extensions_.begin(); iter != extensions_.end(); ++iter) {
- EXPECT_FALSE(prefs()->IsExtensionBlacklisted((*iter)->id()));
+ EXPECT_TRUE(prefs()->UserMayLoad(*iter, NULL));
}
// Blacklist one installed and one not-installed extension id.
std::set<std::string> blacklisted_ids;
@@ -502,15 +502,13 @@
}
virtual void Verify() {
- // Make sure the two id's we expect to be blacklisted are.
- EXPECT_TRUE(prefs()->IsExtensionBlacklisted(extensions_[0]->id()));
- EXPECT_TRUE(prefs()->IsExtensionBlacklisted(not_installed_id_));
+ // Make sure the id we expect to be blacklisted is.
+ EXPECT_FALSE(prefs()->UserMayLoad(extensions_[0], NULL));
// Make sure the other id's are not blacklisted.
ExtensionList::const_iterator iter;
- for (iter = extensions_.begin() + 1; iter != extensions_.end(); ++iter) {
- EXPECT_FALSE(prefs()->IsExtensionBlacklisted((*iter)->id()));
- }
+ for (iter = extensions_.begin() + 1; iter != extensions_.end(); ++iter)
+ EXPECT_TRUE(prefs()->UserMayLoad(*iter, NULL));
// Make sure GetInstalledExtensionsInfo returns only the non-blacklisted
// extensions data.
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 5e99cb9d..5e6fd54c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -1125,26 +1125,11 @@
// Use this set to indicate if an extension in the blacklist has been used.
std::set<std::string> blacklist_set;
for (unsigned int i = 0; i < blacklist.size(); ++i) {
- if (Extension::IdIsValid(blacklist[i])) {
+ if (Extension::IdIsValid(blacklist[i]))
blacklist_set.insert(blacklist[i]);
- }
}
extension_prefs_->UpdateBlacklist(blacklist_set);
- std::vector<std::string> to_be_removed;
- // Loop current extensions, unload installed extensions.
- for (ExtensionSet::const_iterator iter = extensions_.begin();
- iter != extensions_.end(); ++iter) {
- const Extension* extension = (*iter);
- if (blacklist_set.find(extension->id()) != blacklist_set.end()) {
- to_be_removed.push_back(extension->id());
- }
- }
-
- // UnloadExtension will change the extensions_ list. So, we should
- // call it outside the iterator loop.
- for (unsigned int i = 0; i < to_be_removed.size(); ++i) {
- UnloadExtension(to_be_removed[i], extension_misc::UNLOAD_REASON_DISABLE);
- }
+ CheckManagementPolicy();
}
Profile* ExtensionService::profile() {
@@ -1171,15 +1156,14 @@
return updater_.get();
}
-void ExtensionService::CheckAdminBlacklist() {
+void ExtensionService::CheckManagementPolicy() {
std::vector<std::string> to_be_removed;
// Loop through extensions list, unload installed extensions.
for (ExtensionSet::const_iterator iter = extensions_.begin();
iter != extensions_.end(); ++iter) {
const Extension* extension = (*iter);
- if (!system_->management_policy()->UserMayLoad(extension, NULL)) {
+ if (!system_->management_policy()->UserMayLoad(extension, NULL))
to_be_removed.push_back(extension->id());
- }
}
// UnloadExtension will change the extensions_ list. So, we should
@@ -1747,7 +1731,7 @@
}
}
}
- if (extension_prefs_->IsExtensionBlacklisted(e->id())) {
+ if (!extension_prefs_->UserMayLoad(e, NULL)) {
if (!extension_prefs_->IsBlacklistedExtensionAcknowledged(e->id())) {
extension_error_ui->AddBlacklistedExtension(e->id());
needs_alert = true;
@@ -2429,7 +2413,8 @@
std::string* pref_name = content::Details<std::string>(details).ptr();
if (*pref_name == prefs::kExtensionInstallAllowList ||
*pref_name == prefs::kExtensionInstallDenyList) {
- CheckAdminBlacklist();
+ IdentifyAlertableExtensions();
+ CheckManagementPolicy();
} else {
NOTREACHED() << "Unexpected preference name.";
}
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index a5ad7eb..8c67cef3 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -118,7 +118,7 @@
virtual void UpdateExtensionBlacklist(
const std::vector<std::string>& blacklist) = 0;
- virtual void CheckAdminBlacklist() = 0;
+ virtual void CheckManagementPolicy() = 0;
// Safe to call multiple times in a row.
//
@@ -407,10 +407,10 @@
virtual void UpdateExtensionBlacklist(
const std::vector<std::string>& blacklist) OVERRIDE;
- // Go through each extension and unload those that the network admin has
- // put on the blacklist (not to be confused with the Google-managed blacklist)
- // set of extensions.
- virtual void CheckAdminBlacklist() OVERRIDE;
+ // Go through each extension and unload those that are not allowed to run by
+ // management policy providers (ie. network admin and Google-managed
+ // blacklist).
+ virtual void CheckManagementPolicy() OVERRIDE;
virtual void CheckForUpdatesSoon() OVERRIDE;
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index e8f3def..bfa27f6 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -2786,6 +2786,112 @@
ValidateBooleanPref(good_crx, "blacklist", true);
}
+// Unload blacklisted extension on policy change.
+TEST_F(ExtensionServiceTest, UnloadBlacklistedExtensionPolicy) {
+ InitializeEmptyExtensionService();
+ FilePath path = data_dir_.AppendASCII("good.crx");
+
+ const Extension* good = InstallCRX(path, INSTALL_NEW);
+ EXPECT_EQ(good_crx, good->id());
+ UpdateExtension(good_crx, path, FAILED_SILENTLY);
+ EXPECT_EQ(1u, service_->extensions()->size());
+
+ base::ListValue whitelist;
+ PrefService* prefs = service_->extension_prefs()->pref_service();
+ whitelist.Append(base::Value::CreateStringValue(good_crx));
+ prefs->Set(prefs::kExtensionInstallAllowList, whitelist);
+
+ std::vector<std::string> blacklist;
+ blacklist.push_back(good_crx);
+ service_->UpdateExtensionBlacklist(blacklist);
+ // Make sure pref is updated
+
+ // Now, the good_crx is blacklisted but whitelist negates it.
+ ValidateBooleanPref(good_crx, "blacklist", true);
+ EXPECT_EQ(1u, service_->extensions()->size());
+
+ whitelist.Clear();
+ prefs->Set(prefs::kExtensionInstallAllowList, whitelist);
+
+ // Now, the good_crx is blacklisted for good.
+ ValidateBooleanPref(good_crx, "blacklist", true);
+ EXPECT_EQ(0u, service_->extensions()->size());
+}
+
+// Allow Google-blacklisted extension if policy explicitly allows it (blacklist
+// then set policy).
+TEST_F(ExtensionServiceTest, WhitelistGoogleBlacklistedExtension) {
+ InitializeEmptyExtensionService();
+
+ std::vector<std::string> blacklist;
+ blacklist.push_back(good_crx);
+ service_->UpdateExtensionBlacklist(blacklist);
+
+ FilePath path = data_dir_.AppendASCII("good.crx");
+ InstallCRX(path, INSTALL_FAILED);
+
+ base::ListValue whitelist;
+ whitelist.Append(base::Value::CreateStringValue(good_crx));
+ service_->extension_prefs()->pref_service()->Set(
+ prefs::kExtensionInstallAllowList, whitelist);
+
+ InstallCRX(path, INSTALL_NEW);
+}
+
+// Allow Google-blacklisted extension if policy requires it (blacklist then set
+// policy).
+TEST_F(ExtensionServiceTest, ForcelistGoogleBlacklistedExtension) {
+ InitializeEmptyExtensionService();
+
+ std::vector<std::string> blacklist;
+ blacklist.push_back(good_crx);
+ service_->UpdateExtensionBlacklist(blacklist);
+
+ FilePath path = data_dir_.AppendASCII("good.crx");
+ InstallCRX(path, INSTALL_FAILED);
+
+ base::ListValue forcelist;
+ forcelist.Append(base::Value::CreateStringValue(good_crx));
+ service_->extension_prefs()->pref_service()->Set(
+ prefs::kExtensionInstallAllowList, forcelist);
+
+ InstallCRX(path, INSTALL_NEW);
+}
+
+// Allow Google-blacklisted extension if policy explicitly allows it (set policy
+// then blacklist).
+TEST_F(ExtensionServiceTest, GoogleBlacklistWhitelistedExtension) {
+ InitializeEmptyExtensionService();
+
+ base::ListValue whitelist;
+ whitelist.Append(base::Value::CreateStringValue(good_crx));
+ service_->extension_prefs()->pref_service()->Set(
+ prefs::kExtensionInstallAllowList, whitelist);
+
+ std::vector<std::string> blacklist;
+ blacklist.push_back(good_crx);
+ service_->UpdateExtensionBlacklist(blacklist);
+
+ InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
+}
+
+// Allow Google-blacklisted extension if policy requires it (set policy then
+// blacklist).
+TEST_F(ExtensionServiceTest, GoogleBlacklistForcelistedExtension) {
+ InitializeEmptyExtensionService();
+
+ base::ListValue forcelist;
+ forcelist.Append(base::Value::CreateStringValue(good_crx));
+ service_->extension_prefs()->pref_service()->Set(
+ prefs::kExtensionInstallAllowList, forcelist);
+
+ std::vector<std::string> blacklist;
+ blacklist.push_back(good_crx);
+ service_->UpdateExtensionBlacklist(blacklist);
+
+ InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
+}
+
// Test loading extensions from the profile directory, except
// blacklisted ones.
TEST_F(ExtensionServiceTest, WillNotLoadBlacklistedExtensionsFromDirectory) {
@@ -3067,7 +3173,7 @@
management_policy_->RegisterProvider(&provider);
// Run the policy check.
- service_->CheckAdminBlacklist();
+ service_->CheckManagementPolicy();
EXPECT_EQ(0u, service_->extensions()->size());
EXPECT_EQ(0u, service_->disabled_extensions()->size());
}
diff --git a/chrome/browser/extensions/test_extension_service.cc b/chrome/browser/extensions/test_extension_service.cc
index f9cd7a8..16d5bc2 100644
--- a/chrome/browser/extensions/test_extension_service.cc
+++ b/chrome/browser/extensions/test_extension_service.cc
@@ -66,7 +66,7 @@
ADD_FAILURE();
}
-void TestExtensionService::CheckAdminBlacklist() {
+void TestExtensionService::CheckManagementPolicy() {
ADD_FAILURE();
}
diff --git a/chrome/browser/extensions/test_extension_service.h b/chrome/browser/extensions/test_extension_service.h
index 5ad4909..0c0b27f 100644
--- a/chrome/browser/extensions/test_extension_service.h
+++ b/chrome/browser/extensions/test_extension_service.h
@@ -48,7 +48,7 @@
virtual void UpdateExtensionBlacklist(
const std::vector<std::string>& blacklist) OVERRIDE;
- virtual void CheckAdminBlacklist() OVERRIDE;
+ virtual void CheckManagementPolicy() OVERRIDE;
virtual void CheckForUpdatesSoon() OVERRIDE;
virtual syncer::SyncError MergeDataAndStartSyncing(
diff --git a/chrome/browser/protector/protected_prefs_watcher.cc b/chrome/browser/protector/protected_prefs_watcher.cc
index 46ac3a3..e51ffd84 100644
--- a/chrome/browser/protector/protected_prefs_watcher.cc
+++ b/chrome/browser/protector/protected_prefs_watcher.cc
@@ -200,17 +200,10 @@
}
bool ProtectedPrefsWatcher::UpdateCachedPrefs() {
- // Direct access to the extensions prefs is required becase ExtensionService
- // may not yet have been initialized.
- const base::DictionaryValue* extension_prefs;
- const base::Value* extension_prefs_value =
- profile_->GetPrefs()->GetUserPrefValue(ExtensionPrefs::kExtensionsPref);
- if (!extension_prefs_value ||
- !extension_prefs_value->GetAsDictionary(&extension_prefs)) {
- return false;
- }
+ // ExtensionService may not yet have been initialized, so using static method
+ // exposed for this purpose.
ExtensionPrefs::ExtensionIds extension_ids =
- ExtensionPrefs::GetExtensionsFrom(extension_prefs);
+ ExtensionPrefs::GetExtensionsFrom(profile_->GetPrefs());
if (extension_ids == cached_extension_ids_)
return false;
cached_extension_ids_.swap(extension_ids);
