Extension white and force lists (set by policy) should have priority over auto-updated Google black…

chrome/browser/extensions/extension_prefs.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_prefs.h"
 
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/admin_policy.h"
@@ skipping 253 matching lines @@
     case kExtensionPrefsScopeIncognitoPersistent:
       *result = kPrefIncognitoPreferences;
       return true;
     case kExtensionPrefsScopeIncognitoSessionOnly:
       return false;
   }
   NOTREACHED();
   return false;
 }
 
+// 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) {

[Mattias Nissler (ping if slow), 2012/08/08 08:08:37]

all parameters on separate lines

[qfel, 2012/08/13 16:03:01]

Done.

+  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(
     PrefService* prefs,
     const FilePath& root_dir,
     ExtensionPrefValueMap* extension_pref_value_map)
     : prefs_(prefs),
       install_directory_(root_dir),
       extension_pref_value_map_(extension_pref_value_map),
       ALLOW_THIS_IN_INITIALIZER_LIST(extension_sorting_(
@@ skipping 107 matching lines @@
 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);
   if (!ext) {
     // No such extension yet.
     return false;
   }
   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 {
   const DictionaryValue* ext = GetExtensionPref(extension_id);
   if (!ext) {
     // No such extension yet.
     return false;
   }
   return ReadIntegerFromPref(ext, pref_key, out_value);
 }
@@ skipping 104 matching lines @@
   }
 
   // Set the scriptable host permissions.
   if (!new_value->scriptable_hosts().is_empty()) {
     SetExtensionPrefURLPatternSet(extension_id,
                                   JoinPrefs(pref_key, kPrefScriptableHosts),
                                   new_value->scriptable_hosts());
   }
 }
 
-// static
-bool ExtensionPrefs::IsBlacklistBitSet(const DictionaryValue* ext) {
-  return ReadBooleanFromPref(ext, kPrefBlacklist);
+bool ExtensionPrefs::IsExtensionBlacklisted(
+      const std::string& extension_id) const {
+  return IsExtensionBlacklisted(extension_id, prefs_);
 }
 
-bool ExtensionPrefs::IsExtensionBlacklisted(const std::string& extension_id) {
-  return ReadExtensionPrefBoolean(extension_id, kPrefBlacklist);
+// static
+bool ExtensionPrefs::IsExtensionBlacklisted(const std::string& extension_id,
+                                            const PrefService* pref_service) {
+  const DictionaryValue* dict = pref_service->GetDictionary(kExtensionsPref);
+  if (!dict)
+    return false;
+  const DictionaryValue* ext = NULL;
+  if (!dict->GetDictionary(extension_id, &ext)) {
+    // No such extension yet.
+    return false;
+  }
+  if (!IsBlacklistBitSet(ext))
+    return false;
+
+  // Check if policy settings override the blacklist.
+  const base::ListValue* whitelist =
+      pref_service->GetList(prefs::kExtensionInstallAllowList);
+  const base::ListValue* forcelist =
+      pref_service->GetList(prefs::kExtensionInstallForceList);
+  base::StringValue id(extension_id);
+  return (!whitelist || whitelist->Find(id) == whitelist->end()) &&
+         (!forcelist || forcelist->Find(id) == forcelist->end());

[Mattias Nissler (ping if slow), 2012/08/08 08:08:37]

Blacklisting has recently been abstracted into
chrome/browser/extensions/management_policy.h and we should use that for this as
well.

In fact, the main caller of this in crx_installer does the blacklist check and
the policy check right immediately after each other, so I suggest we consolidate
there.

 }
 
 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
   // versus a success response. For now nobody is orphaned.
   return false;
 }
 
 bool ExtensionPrefs::IsExternalExtensionAcknowledged(
@@ skipping 940 matching lines @@
 const DictionaryValue* ExtensionPrefs::GetExtensionPref(
     const std::string& extension_id) const {
   const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref);
   if (!dict)
     return NULL;
   const DictionaryValue* extension = NULL;
   dict->GetDictionary(extension_id, &extension);
   return extension;
 }
 
-// Helper function for GetInstalledExtensionsInfo.
-static ExtensionInfo* GetInstalledExtensionInfoImpl(
+ExtensionInfo* ExtensionPrefs::GetInstalledExtensionInfoImpl(
     DictionaryValue* extension_data,
-    DictionaryValue::key_iterator extension_id) {
+    DictionaryValue::key_iterator extension_id) const {

[Mattias Nissler (ping if slow), 2012/08/08 08:08:37]

any reason to pass an iterator here instead of the id?

[qfel, 2012/08/13 16:03:01]

I guess the original author did it mechanically when factoring out common code,
but doesn't seem to serve a deeper purpose. Changed.

   DictionaryValue* ext;
   if (!extension_data->GetDictionaryWithoutPathExpansion(*extension_id, &ext)) {
     LOG(WARNING) << "Invalid pref for extension " << *extension_id;
     NOTREACHED();
     return NULL;
   }
-  if (ext->HasKey(kPrefBlacklist)) {
-    bool is_blacklisted = false;
-    if (!ext->GetBoolean(kPrefBlacklist, &is_blacklisted)) {
-      NOTREACHED() << "Invalid blacklist pref:" << *extension_id;
+  if (IsExtensionBlacklisted(*extension_id))

[Mattias Nissler (ping if slow), 2012/08/08 08:08:37]

so you get the extension dictionary passed, and you still make another lookup?
That doesn't seem right.

       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
     // extensions.
     return NULL;
   }
   if (state_value == Extension::EXTERNAL_EXTENSION_UNINSTALLED) {
     LOG(WARNING) << "External extension with id " << *extension_id
                  << " has been uninstalled by the user";
     return NULL;
@@ skipping 248 matching lines @@
   scoped_ptr<ExtensionsInfo> extensions_info(GetInstalledExtensionsInfo());
 
   for (size_t i = 0; i < extensions_info->size(); ++i) {
     ExtensionInfo* info = extensions_info->at(i).get();
     out->push_back(info->extension_id);
   }
 }
 
 // static
 ExtensionPrefs::ExtensionIdSet ExtensionPrefs::GetExtensionsFrom(
-    const base::DictionaryValue* extension_prefs) {
+    const PrefService* pref_service) {
   ExtensionIdSet 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;
-    if (!extension_prefs->GetDictionaryWithoutPathExpansion(*it, &ext)) {
-      NOTREACHED() << "Invalid pref for extension " << *it;
-      continue;
-    }
-    if (!IsBlacklistBitSet(ext))
+    if (!IsExtensionBlacklisted(*it, pref_service))
       result.push_back(*it);
   }
   return result;
 }
 
 void ExtensionPrefs::FixMissingPrefs(const ExtensionIdSet& extension_ids) {
   // Fix old entries that did not get an installation time entry when they
   // were installed or don't have a preferences field.
   for (ExtensionIdSet::const_iterator ext_id = extension_ids.begin();
        ext_id != extension_ids.end(); ++ext_id) {
@@ skipping 232 matching lines @@
                            0,  // default value
                            PrefService::UNSYNCABLE_PREF);
   prefs->RegisterInt64Pref(prefs::kNextExtensionsUpdateCheck,
                            0,  // default value
                            PrefService::UNSYNCABLE_PREF);
   prefs->RegisterListPref(prefs::kExtensionAllowedInstallSites,
                           PrefService::UNSYNCABLE_PREF);
 }
 
 }  // namespace extensions