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 262 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) {
+  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 67 matching lines @@
       continue;
     }
     FilePath::StringType path_string;
     extension_dict->GetString(kPrefPath, &path_string);
     FilePath path(path_string);
     extension_dict->SetString(kPrefPath,
         MakePathRelative(install_directory_, path));
   }
 }
 
-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);
   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) {
-  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
   // versus a success response. For now nobody is orphaned.
   return false;
 }
 
 bool ExtensionPrefs::IsExternalExtensionAcknowledged(
     const std::string& extension_id) {
   return ReadExtensionPrefBoolean(extension_id, kPrefExternalAcknowledged);
@@ skipping 73 matching lines @@
 #ifdef NDEBUG
   NOTREACHED();
   return std::string();
 #else
   return "admin policy black/white/forcelist, via the ExtensionPrefs";
 #endif
 }
 
 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,
                                            string16* error) const {
   return admin_policy::UserMayModifySettings(extension, error);
 }
 
 bool ExtensionPrefs::MustRemainEnabled(const Extension* extension,
                                        string16* error) const {
   return admin_policy::MustRemainEnabled(extension, error);
@@ skipping 841 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(
-    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
     // extensions.
     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)) {
+  int location_value;
+  if (!ext->GetInteger(kPrefLocation, &location_value))
     return NULL;
-  }
-  int location_value;
-  if (!ext->GetInteger(kPrefLocation, &location_value)) {
+
+  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
   // preferences.
   Extension::Location location =
       static_cast<Extension::Location>(location_value);
   if (location != Extension::INTERNAL &&
       location != Extension::LOAD &&
       !Extension::IsExternalLocation(location)) {
     NOTREACHED();
     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));
   }
 
   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,
                                         const base::Time& fetch_time) {
   DictionaryValue* info = new DictionaryValue();
   info->SetString(kIdleInstallInfoCrxPath, crx_path.value());
   info->SetString(kIdleInstallInfoVersion, version);
   info->SetString(kIdleInstallInfoFetchTime,
                   base::Int64ToString(fetch_time.ToInternalValue()));
   UpdateExtensionPref(extension_id, kIdleInstallInfo, info);
@@ skipping 171 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))
+       it != extension_prefs->end_keys(); ++it)
       result.push_back(*it);

[qfel, 2012/08/14 13:03:14]

Now it returns Google-blacklisted extensions too. IMO that should be fine (it
did include admin-blacklisted extensions, so now it's more uniform), but this
behavior makes ExtensionPrefChange unittest crash (see
protected_prefs_watcher_unittest.cc, the very end of the test). I think we
should remove that part about blacklist update triggering backup change. WDYT?

[Mattias Nissler (ping if slow), 2012/08/14 14:12:31]

Btw, indentation is wrong here. Please fix.

[Mattias Nissler (ping if slow), 2012/08/14 14:12:31]

I'm not so sure about this. Isn't the blacklist bit in the extension prefs a
different thing? I'm not sure when it's present and whit its semantics are.
Matt, can you clarify?

[Matt Perry, 2012/08/15 21:17:07]

I'm not an expert here, but judging by the use case of GetExtensionsFrom, it
makes sense to return the google-blacklisted extensions too. The only caller is
the ProtectedPrefsWatcher, who wants to keep the prefs from changing externally.

-  }
   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) {
     if (GetInstallTime(*ext_id) == base::Time()) {
       LOG(INFO) << "Could not parse installation time of extension "
@@ skipping 257 matching lines @@
                                                 const ExtensionIdSet& strings) {
   ListPrefUpdate update(prefs_, pref);
   ListValue* list_of_values = update.Get();
   list_of_values->Clear();
   for (ExtensionIdSet::const_iterator iter = strings.begin();
        iter != strings.end(); ++iter)
     list_of_values->Append(new StringValue(*iter));
 }
 
 }  // namespace extensions