diff options
| -rw-r--r-- | chrome/browser/extensions/admin_policy.cc | 19 | ||||
| -rw-r--r-- | chrome/browser/extensions/admin_policy.h | 8 | ||||
| -rw-r--r-- | chrome/browser/extensions/admin_policy_unittest.cc | 46 | ||||
| -rw-r--r-- | chrome/browser/extensions/crx_installer.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_prefs.cc | 189 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_prefs.h | 35 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_prefs_unittest.cc | 12 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_service.cc | 29 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_service.h | 10 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_service_unittest.cc | 108 | ||||
| -rw-r--r-- | chrome/browser/extensions/test_extension_service.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/extensions/test_extension_service.h | 2 | ||||
| -rw-r--r-- | chrome/browser/protector/protected_prefs_watcher.cc | 13 |
13 files changed, 254 insertions, 229 deletions
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 @@ bool BlacklistedByDefault(const base::ListValue* blacklist) { 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 38ff932..5d69d07 100644 --- a/chrome/browser/extensions/admin_policy.h +++ b/chrome/browser/extensions/admin_policy.h @@ -22,10 +22,12 @@ namespace admin_policy { // 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 94afd43..875312f 100644 --- a/chrome/browser/extensions/admin_policy_unittest.cc +++ b/chrome/browser/extensions/admin_policy_unittest.cc @@ -50,28 +50,33 @@ TEST_F(ExtensionAdminPolicyTest, BlacklistedByDefault) { // 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 @@ TEST_F(ExtensionAdminPolicyTest, UserMayLoadWhitelisted) { 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 @@ TEST_F(ExtensionAdminPolicyTest, UserMayLoadBlacklisted) { // 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 da319ae..6f338ed 100644 --- a/chrome/browser/extensions/crx_installer.cc +++ b/chrome/browser/extensions/crx_installer.cc @@ -395,16 +395,6 @@ void CrxInstaller::ConfirmInstall() { 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 aa12933..4b8a442 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -294,6 +294,32 @@ const char* GetToolbarVisibilityKeyName() { 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::MakePathsRelative() { } } -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 @@ bool ExtensionPrefs::ReadExtensionPrefBoolean( 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 @@ void ExtensionPrefs::SetExtensionPrefPermissionSet( } } -// 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 @@ std::string ExtensionPrefs::GetDebugPolicyProviderName() const { 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 @@ const DictionaryValue* ExtensionPrefs::GetExtensionPref( 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 @@ static ExtensionInfo* GetInstalledExtensionInfoImpl( 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 @@ static ExtensionInfo* GetInstalledExtensionInfoImpl( 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 @@ ExtensionPrefs::ExtensionsInfo* ExtensionPrefs::GetInstalledExtensionsInfo() { 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 @@ void ExtensionPrefs::GetExtensions(ExtensionIds* out) { // 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 fa718f4..6a7a7d7 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -85,6 +85,11 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, 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 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, // 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 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, // 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 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, // 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 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, // 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 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, 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 @@ class ExtensionPrefsBlacklist : public ExtensionPrefsTest { 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 @@ class ExtensionPrefsBlacklist : public ExtensionPrefsTest { } 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 5e99cb9..5e6fd54 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -1125,26 +1125,11 @@ void ExtensionService::UpdateExtensionBlacklist( // 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 @@ extensions::ExtensionUpdater* ExtensionService::updater() { 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 @@ bool ExtensionService::PopulateExtensionErrorUI( } } } - 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 @@ void ExtensionService::Observe(int type, 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..8c67cef 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -118,7 +118,7 @@ class ExtensionServiceInterface : public syncer::SyncableService { 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 @@ class ExtensionService 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 @@ TEST_F(ExtensionServiceTest, BlacklistedExtensionWillNotInstall) { 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 @@ TEST_F(ExtensionServiceTest, ManagementPolicyUnloadsAllProhibited) { 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 @@ void TestExtensionService::UpdateExtensionBlacklist( 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 @@ class TestExtensionService : public ExtensionServiceInterface { 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..e51ffd8 100644 --- a/chrome/browser/protector/protected_prefs_watcher.cc +++ b/chrome/browser/protector/protected_prefs_watcher.cc @@ -200,17 +200,10 @@ void ProtectedPrefsWatcher::EnsurePrefsMigration() { } 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); |