diff options
32 files changed, 1089 insertions, 350 deletions
diff --git a/chrome/browser/background/background_contents_service.cc b/chrome/browser/background/background_contents_service.cc index d08e84f..bb25f9b 100644 --- a/chrome/browser/background/background_contents_service.cc +++ b/chrome/browser/background/background_contents_service.cc @@ -325,7 +325,8 @@ void BackgroundContentsService::Observe( switch (content::Details<UnloadedExtensionInfo>(details)->reason) { case extension_misc::UNLOAD_REASON_DISABLE: // Fall through. case extension_misc::UNLOAD_REASON_TERMINATE: // Fall through. - case extension_misc::UNLOAD_REASON_UNINSTALL: + case extension_misc::UNLOAD_REASON_UNINSTALL: // Fall through. + case extension_misc::UNLOAD_REASON_BLACKLIST: ShutdownAssociatedBackgroundContents( ASCIIToUTF16(content::Details<UnloadedExtensionInfo>(details)-> extension->id())); diff --git a/chrome/browser/extensions/admin_policy.cc b/chrome/browser/extensions/admin_policy.cc index c25e31b..ea69dab 100644 --- a/chrome/browser/extensions/admin_policy.cc +++ b/chrome/browser/extensions/admin_policy.cc @@ -41,8 +41,7 @@ bool BlacklistedByDefault(const base::ListValue* blacklist) { return blacklist && blacklist->Find(wildcard) != blacklist->end(); } -bool UserMayLoad(bool is_google_blacklisted, - const base::ListValue* blacklist, +bool UserMayLoad(const base::ListValue* blacklist, const base::ListValue* whitelist, const base::ListValue* forcelist, const Extension* extension, @@ -50,19 +49,17 @@ bool UserMayLoad(bool is_google_blacklisted, if (IsRequired(extension)) return true; - if ((!blacklist || blacklist->empty()) && !is_google_blacklisted) + if (!blacklist || blacklist->empty()) return true; - // Check the whitelist/forcelist first (takes precedence over Google - // blacklist). + // Check the whitelist/forcelist first. base::StringValue id_value(extension->id()); if ((whitelist && whitelist->Find(id_value) != whitelist->end()) || (forcelist && forcelist->Find(id_value) != forcelist->end())) return true; - // Then check both admin and Google blacklists. - bool result = !is_google_blacklisted && - (!blacklist || blacklist->Find(id_value) == blacklist->end()) && + // Then check the admin blacklist. + bool result = (!blacklist || blacklist->Find(id_value) == blacklist->end()) && !BlacklistedByDefault(blacklist); if (error && !result) { *error = l10n_util::GetStringFUTF16( diff --git a/chrome/browser/extensions/admin_policy.h b/chrome/browser/extensions/admin_policy.h index 5d69d07..61cd987 100644 --- a/chrome/browser/extensions/admin_policy.h +++ b/chrome/browser/extensions/admin_policy.h @@ -22,10 +22,8 @@ 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 Google blacklist and admin policy -// white-, black- and forcelists. -bool UserMayLoad(bool is_google_blacklisted, - const base::ListValue* blacklist, +// Returns true if the extension is allowed by the admin policy. +bool UserMayLoad(const base::ListValue* blacklist, const base::ListValue* whitelist, const base::ListValue* forcelist, const Extension* extension, diff --git a/chrome/browser/extensions/admin_policy_unittest.cc b/chrome/browser/extensions/admin_policy_unittest.cc index 875312f..59de6f2 100644 --- a/chrome/browser/extensions/admin_policy_unittest.cc +++ b/chrome/browser/extensions/admin_policy_unittest.cc @@ -50,32 +50,32 @@ TEST_F(ExtensionAdminPolicyTest, BlacklistedByDefault) { // Tests UserMayLoad for required extensions. TEST_F(ExtensionAdminPolicyTest, UserMayLoadRequired) { CreateExtension(Extension::EXTERNAL_POLICY_DOWNLOAD, true); - EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(), NULL)); + EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, NULL, extension_.get(), NULL)); string16 error; - EXPECT_TRUE(ap::UserMayLoad(false, NULL, NULL, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(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(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(), NULL)); blacklist.Append(Value::CreateStringValue("*")); - EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(&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(false, NULL, NULL, NULL, extension_.get(), NULL)); + EXPECT_TRUE(ap::UserMayLoad(NULL, NULL, NULL, extension_.get(), NULL)); base::ListValue blacklist; - EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(), NULL)); string16 error; - EXPECT_TRUE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(), &error)); EXPECT_TRUE(error.empty()); } @@ -86,15 +86,15 @@ TEST_F(ExtensionAdminPolicyTest, UserMayLoadWhitelisted) { base::ListValue whitelist; whitelist.Append(Value::CreateStringValue(extension_->id())); - EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, NULL, extension_.get(), NULL)); base::ListValue blacklist; blacklist.Append(Value::CreateStringValue(extension_->id())); - EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, NULL, extension_.get(), NULL)); string16 error; - EXPECT_TRUE(ap::UserMayLoad(false, NULL, &whitelist, NULL, extension_.get(), + EXPECT_TRUE(ap::UserMayLoad(NULL, &whitelist, NULL, extension_.get(), &error)); EXPECT_TRUE(error.empty()); } @@ -106,30 +106,30 @@ TEST_F(ExtensionAdminPolicyTest, UserMayLoadBlacklisted) { // Blacklisted by default. base::ListValue blacklist; blacklist.Append(Value::CreateStringValue("*")); - EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(), NULL)); string16 error; - EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_FALSE(ap::UserMayLoad(&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(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_FALSE(ap::UserMayLoad(&blacklist, NULL, NULL, extension_.get(), NULL)); blacklist.Clear(); blacklist.Append(Value::CreateStringValue(extension_->id())); - EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, NULL, NULL, extension_.get(), + EXPECT_FALSE(ap::UserMayLoad(&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(false, &blacklist, &whitelist, NULL, + EXPECT_FALSE(ap::UserMayLoad(&blacklist, &whitelist, NULL, extension_.get(), NULL)); whitelist.Append(Value::CreateStringValue("*")); - EXPECT_FALSE(ap::UserMayLoad(false, &blacklist, &whitelist, NULL, + EXPECT_FALSE(ap::UserMayLoad(&blacklist, &whitelist, NULL, extension_.get(), NULL)); } diff --git a/chrome/browser/extensions/blacklist.cc b/chrome/browser/extensions/blacklist.cc index 834d262..3d745a6 100644 --- a/chrome/browser/extensions/blacklist.cc +++ b/chrome/browser/extensions/blacklist.cc @@ -4,6 +4,10 @@ #include "chrome/browser/extensions/blacklist.h" +#include <algorithm> + +#include "base/bind.h" +#include "base/message_loop.h" #include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/common/pref_names.h" @@ -24,12 +28,18 @@ Blacklist::Blacklist(ExtensionPrefs* prefs) : prefs_(prefs) { Blacklist::~Blacklist() { } -bool Blacklist::IsBlacklisted(const Extension* extension) const { - return prefs_->IsExtensionBlacklisted(extension->id()); -} - -bool Blacklist::IsBlacklisted(const std::string& extension_id) const { - return prefs_->IsExtensionBlacklisted(extension_id); +void Blacklist::GetBlacklistedIDs(const std::set<std::string>& ids, + const GetBlacklistedIDsCallback& callback) { + // TODO(kalman): Get the blacklisted IDs from the safebrowsing list. + // This will require going to the IO thread and back. + std::set<std::string> blacklisted_ids; + for (std::set<std::string>::const_iterator it = ids.begin(); + it != ids.end(); ++it) { + if (prefs_->IsExtensionBlacklisted(*it)) + blacklisted_ids.insert(*it); + } + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(callback, blacklisted_ids)); } void Blacklist::SetFromUpdater(const std::vector<std::string>& ids, @@ -43,7 +53,28 @@ void Blacklist::SetFromUpdater(const std::vector<std::string>& ids, LOG(WARNING) << "Got invalid extension ID \"" << *it << "\""; } - prefs_->UpdateBlacklist(ids_as_set); + std::set<std::string> from_prefs = prefs_->GetBlacklistedExtensions(); + + std::set<std::string> no_longer_blacklisted; + std::set_difference(from_prefs.begin(), from_prefs.end(), + ids_as_set.begin(), ids_as_set.end(), + std::inserter(no_longer_blacklisted, + no_longer_blacklisted.begin())); + std::set<std::string> not_yet_blacklisted; + std::set_difference(ids_as_set.begin(), ids_as_set.end(), + from_prefs.begin(), from_prefs.end(), + std::inserter(not_yet_blacklisted, + not_yet_blacklisted.begin())); + + for (std::set<std::string>::iterator it = no_longer_blacklisted.begin(); + it != no_longer_blacklisted.end(); ++it) { + prefs_->SetExtensionBlacklisted(*it, false); + } + for (std::set<std::string>::iterator it = not_yet_blacklisted.begin(); + it != not_yet_blacklisted.end(); ++it) { + prefs_->SetExtensionBlacklisted(*it, true); + } + prefs_->pref_service()->SetString(prefs::kExtensionBlacklistUpdateVersion, version); diff --git a/chrome/browser/extensions/blacklist.h b/chrome/browser/extensions/blacklist.h index 58b12db..89705a5 100644 --- a/chrome/browser/extensions/blacklist.h +++ b/chrome/browser/extensions/blacklist.h @@ -5,9 +5,11 @@ #ifndef CHROME_BROWSER_EXTENSIONS_BLACKLIST_H_ #define CHROME_BROWSER_EXTENSIONS_BLACKLIST_H_ +#include <set> #include <string> #include <vector> +#include "base/callback.h" #include "base/observer_list.h" namespace extensions { @@ -32,18 +34,19 @@ class Blacklist { Blacklist* blacklist_; }; + typedef base::Callback<void(const std::set<std::string>&)> + GetBlacklistedIDsCallback; + // |prefs_| must outlive this. explicit Blacklist(ExtensionPrefs* prefs); ~Blacklist(); - // Gets whether an extension is blacklisted. - // - // Note that this doesn't entirely determine whether an extension is allowed - // to be loaded; there are other considerations (e.g. admin settings). - // See extensions::ManagementPolicy (in particular UserMayLoad). - bool IsBlacklisted(const std::string& extension_id) const; - bool IsBlacklisted(const Extension* extension) const; + // From the set of extension IDs passed in via |ids|, asynchronously checks + // which are blacklisted and includes them in the resulting set passed + // via |callback|, which will be sent on the caller's message loop. + void GetBlacklistedIDs(const std::set<std::string>& ids, + const GetBlacklistedIDsCallback& callback); // Sets the blacklist from the updater to contain the extension IDs in |ids| void SetFromUpdater(const std::vector<std::string>& ids, @@ -58,6 +61,8 @@ class Blacklist { ExtensionPrefs* const prefs_; + std::set<std::string> prefs_blacklist_; + DISALLOW_COPY_AND_ASSIGN(Blacklist); }; diff --git a/chrome/browser/extensions/blacklist_unittest.cc b/chrome/browser/extensions/blacklist_unittest.cc new file mode 100644 index 0000000..9572a5c --- /dev/null +++ b/chrome/browser/extensions/blacklist_unittest.cc @@ -0,0 +1,145 @@ +// Copyright 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 "base/bind.h" +#include "base/message_loop.h" +#include "base/run_loop.h" +#include "chrome/browser/extensions/blacklist.h" +#include "chrome/browser/extensions/extension_prefs.h" +#include "chrome/browser/extensions/test_blacklist.h" +#include "chrome/browser/extensions/test_extension_prefs.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace extensions { +namespace { + +class BlacklistTest : public testing::Test { + public: + BlacklistTest() + : prefs_(message_loop_.message_loop_proxy()), + blacklist_(prefs_.prefs()) { + } + + bool IsBlacklisted(const Extension* extension) { + return TestBlacklist(&blacklist_).IsBlacklisted(extension->id()); + } + + protected: + MessageLoop message_loop_; + + TestExtensionPrefs prefs_; + + Blacklist blacklist_; +}; + +TEST_F(BlacklistTest, SetFromUpdater) { + scoped_refptr<const Extension> extension_a = prefs_.AddExtension("a"); + scoped_refptr<const Extension> extension_b = prefs_.AddExtension("b"); + scoped_refptr<const Extension> extension_c = prefs_.AddExtension("c"); + scoped_refptr<const Extension> extension_d = prefs_.AddExtension("d"); + + // c, d, start blacklisted. + prefs_.prefs()->SetExtensionBlacklisted(extension_c->id(), true); + prefs_.prefs()->SetExtensionBlacklisted(extension_d->id(), true); + + EXPECT_FALSE(IsBlacklisted(extension_a)); + EXPECT_FALSE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsBlacklisted(extension_c)); + EXPECT_TRUE(IsBlacklisted(extension_d)); + + // Mix up the blacklist. + { + std::vector<std::string> blacklist; + blacklist.push_back(extension_b->id()); + blacklist.push_back(extension_c->id()); + blacklist_.SetFromUpdater(blacklist, "1"); + } + EXPECT_FALSE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsBlacklisted(extension_c)); + EXPECT_FALSE(IsBlacklisted(extension_d)); + + // No-op, just in case. + { + std::vector<std::string> blacklist; + blacklist.push_back(extension_b->id()); + blacklist.push_back(extension_c->id()); + blacklist_.SetFromUpdater(blacklist, "2"); + } + EXPECT_FALSE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsBlacklisted(extension_c)); + EXPECT_FALSE(IsBlacklisted(extension_d)); + + // Strictly increase the blacklist. + { + std::vector<std::string> blacklist; + blacklist.push_back(extension_a->id()); + blacklist.push_back(extension_b->id()); + blacklist.push_back(extension_c->id()); + blacklist.push_back(extension_d->id()); + blacklist_.SetFromUpdater(blacklist, "3"); + } + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsBlacklisted(extension_c)); + EXPECT_TRUE(IsBlacklisted(extension_d)); + + // Strictly decrease the blacklist. + { + std::vector<std::string> blacklist; + blacklist.push_back(extension_a->id()); + blacklist.push_back(extension_b->id()); + blacklist_.SetFromUpdater(blacklist, "4"); + } + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_FALSE(IsBlacklisted(extension_c)); + EXPECT_FALSE(IsBlacklisted(extension_d)); + + // Clear the blacklist. + { + std::vector<std::string> blacklist; + blacklist_.SetFromUpdater(blacklist, "5"); + } + EXPECT_FALSE(IsBlacklisted(extension_a)); + EXPECT_FALSE(IsBlacklisted(extension_b)); + EXPECT_FALSE(IsBlacklisted(extension_c)); + EXPECT_FALSE(IsBlacklisted(extension_d)); +} + +void Assign(std::set<std::string> *to, const std::set<std::string>& from) { + *to = from; +} + +TEST_F(BlacklistTest, OnlyIncludesRequestedIDs) { + scoped_refptr<const Extension> extension_a = prefs_.AddExtension("a"); + scoped_refptr<const Extension> extension_b = prefs_.AddExtension("b"); + scoped_refptr<const Extension> extension_c = prefs_.AddExtension("c"); + + { + std::vector<std::string> blacklist; + blacklist.push_back(extension_a->id()); + blacklist.push_back(extension_b->id()); + blacklist_.SetFromUpdater(blacklist, "1"); + base::RunLoop().RunUntilIdle(); + } + + std::set<std::string> blacklist_actual; + { + std::set<std::string> blacklist_query; + blacklist_query.insert(extension_a->id()); + blacklist_query.insert(extension_c->id()); + blacklist_.GetBlacklistedIDs(blacklist_query, + base::Bind(&Assign, &blacklist_actual)); + base::RunLoop().RunUntilIdle(); + } + + std::set<std::string> blacklist_expected; + blacklist_expected.insert(extension_a->id()); + EXPECT_EQ(blacklist_expected, blacklist_actual); +} + +} // namespace extensions +} // namespace diff --git a/chrome/browser/extensions/extension_blacklist_browsertest.cc b/chrome/browser/extensions/extension_blacklist_browsertest.cc new file mode 100644 index 0000000..cc6bc24 --- /dev/null +++ b/chrome/browser/extensions/extension_blacklist_browsertest.cc @@ -0,0 +1,389 @@ +// Copyright 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 "base/run_loop.h" +#include "base/stringprintf.h" +#include "chrome/browser/extensions/blacklist.h" +#include "chrome/browser/extensions/extension_browsertest.h" +#include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_constants.h" +#include "content/public/browser/notification_details.h" +#include "content/public/browser/notification_observer.h" +#include "content/public/browser/notification_registrar.h" +#include "content/public/browser/notification_source.h" + +namespace extensions { + +namespace { + +// Records notifications, but only for extensions with specific IDs. +class FilteringNotificationObserver : public content::NotificationObserver { + public: + FilteringNotificationObserver( + content::NotificationSource source, + const std::set<std::string>& extension_ids) + : extension_ids_(extension_ids) { + registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED, source); + registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_INSTALLED, source); + registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED, source); + } + + // Checks then clears notifications for our extensions. + testing::AssertionResult CheckNotifications(chrome::NotificationType type) { + return CheckNotifications(std::vector<chrome::NotificationType>(1, type)); + } + + // Checks then clears notifications for our extensions. + testing::AssertionResult CheckNotifications(chrome::NotificationType t1, + chrome::NotificationType t2) { + std::vector<chrome::NotificationType> types; + types.push_back(t1); + types.push_back(t2); + return CheckNotifications(types); + } + + // Checks then clears notifications for our extensions. + testing::AssertionResult CheckNotifications(chrome::NotificationType t1, + chrome::NotificationType t2, + chrome::NotificationType t3) { + std::vector<chrome::NotificationType> types; + types.push_back(t1); + types.push_back(t2); + types.push_back(t3); + return CheckNotifications(types); + } + + // Checks then clears notifications for our extensions. + testing::AssertionResult CheckNotifications(chrome::NotificationType t1, + chrome::NotificationType t2, + chrome::NotificationType t3, + chrome::NotificationType t4, + chrome::NotificationType t5, + chrome::NotificationType t6) { + std::vector<chrome::NotificationType> types; + types.push_back(t1); + types.push_back(t2); + types.push_back(t3); + types.push_back(t4); + types.push_back(t5); + types.push_back(t6); + return CheckNotifications(types); + } + + private: + // content::NotificationObserver implementation. + virtual void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) OVERRIDE { + switch (type) { + case chrome::NOTIFICATION_EXTENSION_INSTALLED: { + const Extension* extension = + content::Details<const Extension>(details).ptr(); + if (extension_ids_.count(extension->id())) + notifications_.push_back(static_cast<chrome::NotificationType>(type)); + break; + } + + case chrome::NOTIFICATION_EXTENSION_LOADED: { + const Extension* extension = + content::Details<const Extension>(details).ptr(); + if (extension_ids_.count(extension->id())) + notifications_.push_back(static_cast<chrome::NotificationType>(type)); + break; + } + + case chrome::NOTIFICATION_EXTENSION_UNLOADED: { + UnloadedExtensionInfo* reason = + content::Details<UnloadedExtensionInfo>(details).ptr(); + if (extension_ids_.count(reason->extension->id())) { + notifications_.push_back(static_cast<chrome::NotificationType>(type)); + // The only way that extensions are unloaded in these tests is + // by blacklisting. + EXPECT_EQ(extension_misc::UNLOAD_REASON_BLACKLIST, + reason->reason); + } + break; + } + + default: + NOTREACHED(); + break; + } + } + + // Checks then clears notifications for our extensions. + testing::AssertionResult CheckNotifications( + const std::vector<chrome::NotificationType>& types) { + testing::AssertionResult result = (notifications_ == types) ? + testing::AssertionSuccess() : + testing::AssertionFailure() << "Expected " << Str(types) << ", " << + "Got " << Str(notifications_); + notifications_.clear(); + return result; + } + + std::string Str(const std::vector<chrome::NotificationType>& types) { + std::string str = "["; + bool needs_comma = false; + for (std::vector<chrome::NotificationType>::const_iterator it = + types.begin(); it != types.end(); ++it) { + if (needs_comma) + str += ","; + needs_comma = true; + str += base::StringPrintf("%d", *it); + } + return str + "]"; + } + + const std::set<std::string> extension_ids_; + + std::vector<chrome::NotificationType> notifications_; + + content::NotificationRegistrar registrar_; +}; + +// Stores the paths to CRX files of extensions, and the extension's ID. +// Use arbitrary extensions; we're just testing blacklisting behavior. +class CrxInfo { + public: + CrxInfo(const std::string& path, const std::string& id) + : path_(path), id_(id) {} + + const std::string& path() { return path_; } + const std::string& id() { return id_; } + + private: + const std::string path_; + const std::string id_; +}; + +} // namespace + +class ExtensionBlacklistBrowserTest : public ExtensionBrowserTest { + public: + ExtensionBlacklistBrowserTest() + : info_a_("install/install.crx", "ogdbpbegnmindpdjfafpmpicikegejdj"), + info_b_("autoupdate/v1.crx", "ogjcoiohnmldgjemafoockdghcjciccf"), + info_c_("hosted_app.crx", "kbmnembihfiondgfjekmnmcbddelicoi") {} + + virtual ~ExtensionBlacklistBrowserTest() {} + + protected: + // Returns whether |extension| is strictly safe: in one of ExtensionService's + // non-blacklisted extension sets, and not in its blacklisted extensions. + testing::AssertionResult IsSafe(const Extension* extension) { + std::string id = extension->id(); + int include_mask = ExtensionService::INCLUDE_EVERYTHING & + ~ExtensionService::INCLUDE_BLACKLISTED; + if (!extension_service()->GetExtensionById(id, include_mask)) + return testing::AssertionFailure() << id << " is safe"; + return IsInValidState(extension); + } + + // Returns whether |extension| is strictly blacklisted: in ExtensionService's + // blacklist, and not in any of its other extension sets. + testing::AssertionResult IsBlacklisted(const Extension* extension) { + std::string id = extension->id(); + if (!extension_service()->blacklisted_extensions()->Contains(id)) + return testing::AssertionFailure() << id << " is not blacklisted"; + return IsInValidState(extension); + } + + std::set<std::string> GetTestExtensionIDs() { + std::set<std::string> extension_ids; + extension_ids.insert(info_a_.id()); + extension_ids.insert(info_b_.id()); + extension_ids.insert(info_c_.id()); + return extension_ids; + } + + Blacklist* blacklist() { + return ExtensionSystem::Get(profile())->blacklist(); + } + + CrxInfo info_a_; + CrxInfo info_b_; + CrxInfo info_c_; + + private: + // Returns whether |extension| is either installed or blacklisted, but + // neither both nor neither. + testing::AssertionResult IsInValidState(const Extension* extension) { + std::string id = extension->id(); + bool is_blacklisted = + extension_service()->blacklisted_extensions()->Contains(id); + int safe_mask = ExtensionService::INCLUDE_EVERYTHING & + ~ExtensionService::INCLUDE_BLACKLISTED; + bool is_safe = extension_service()->GetExtensionById(id, safe_mask) != NULL; + if (is_blacklisted && is_safe) { + return testing::AssertionFailure() << + id << " is both safe and in blacklisted_extensions"; + } + if (!is_blacklisted && !is_safe) { + return testing::AssertionFailure() << + id << " is neither safe nor in blacklisted_extensions"; + } + return testing::AssertionSuccess(); + } +}; + +// Stage 1: blacklisting when there weren't any extensions installed when the +// browser started. +IN_PROC_BROWSER_TEST_F(ExtensionBlacklistBrowserTest, PRE_Blacklist) { + FilteringNotificationObserver notifications( + content::NotificationService::AllSources(), GetTestExtensionIDs()); + + scoped_refptr<const Extension> extension_a = + InstallExtension(test_data_dir_.AppendASCII(info_a_.path()), 1); + scoped_refptr<const Extension> extension_b = + InstallExtension(test_data_dir_.AppendASCII(info_b_.path()), 1); + scoped_refptr<const Extension> extension_c = + InstallExtension(test_data_dir_.AppendASCII(info_c_.path()), 1); + + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_INSTALLED, + chrome::NOTIFICATION_EXTENSION_LOADED, + chrome::NOTIFICATION_EXTENSION_INSTALLED, + chrome::NOTIFICATION_EXTENSION_LOADED, + chrome::NOTIFICATION_EXTENSION_INSTALLED, + chrome::NOTIFICATION_EXTENSION_LOADED)); + + ASSERT_TRUE(extension_a); + ASSERT_TRUE(extension_b); + ASSERT_EQ(info_a_.id(), extension_a->id()); + ASSERT_EQ(info_b_.id(), extension_b->id()); + ASSERT_EQ(info_c_.id(), extension_c->id()); + + std::vector<std::string> empty_vector; + std::vector<std::string> vector_a(1, info_a_.id()); + std::vector<std::string> vector_b(1, info_b_.id()); + std::vector<std::string> vector_c(1, info_c_.id()); + std::vector<std::string> vector_ab(1, info_a_.id()); + vector_ab.push_back(info_b_.id()); + std::vector<std::string> vector_bc(1, info_b_.id()); + vector_bc.push_back(info_c_.id()); + std::vector<std::string> vector_abc(1, info_a_.id()); + vector_abc.push_back(info_b_.id()); + vector_abc.push_back(info_c_.id()); + + EXPECT_TRUE(IsSafe(extension_a)); + EXPECT_TRUE(IsSafe(extension_b)); + EXPECT_TRUE(IsSafe(extension_c)); + + // Blacklist a and b. + blacklist()->SetFromUpdater(vector_ab, "1"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsSafe(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_UNLOADED, + chrome::NOTIFICATION_EXTENSION_UNLOADED)); + + // Un-blacklist a. + blacklist()->SetFromUpdater(vector_b, "2"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsSafe(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsSafe(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_LOADED)); + + // Blacklist a then switch with c. + blacklist()->SetFromUpdater(vector_ab, "3"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsSafe(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_UNLOADED)); + + blacklist()->SetFromUpdater(vector_bc, "4"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsSafe(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsBlacklisted(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_LOADED, + chrome::NOTIFICATION_EXTENSION_UNLOADED)); + + // Add a to blacklist. + blacklist()->SetFromUpdater(vector_abc, "5"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsBlacklisted(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_UNLOADED)); + + // Clear blacklist. + blacklist()->SetFromUpdater(empty_vector, "6"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsSafe(extension_a)); + EXPECT_TRUE(IsSafe(extension_b)); + EXPECT_TRUE(IsSafe(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_LOADED, + chrome::NOTIFICATION_EXTENSION_LOADED, + chrome::NOTIFICATION_EXTENSION_LOADED)); + + // Add a and b back again for the next test. + blacklist()->SetFromUpdater(vector_ab, "7"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsSafe(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_UNLOADED, + chrome::NOTIFICATION_EXTENSION_UNLOADED)); +} + +// Stage 2: blacklisting with extensions A and B having been installed, +// with A actually in the blacklist. +IN_PROC_BROWSER_TEST_F(ExtensionBlacklistBrowserTest, Blacklist) { + FilteringNotificationObserver notifications( + content::Source<Profile>(profile()), GetTestExtensionIDs()); + + scoped_refptr<const Extension> extension_a = + extension_service()->blacklisted_extensions()->GetByID(info_a_.id()); + ASSERT_TRUE(extension_a); + + scoped_refptr<const Extension> extension_b = + extension_service()->blacklisted_extensions()->GetByID(info_b_.id()); + ASSERT_TRUE(extension_b); + + scoped_refptr<const Extension> extension_c = + extension_service()->extensions()->GetByID(info_c_.id()); + ASSERT_TRUE(extension_c); + + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsBlacklisted(extension_b)); + EXPECT_TRUE(IsSafe(extension_c)); + + // Make sure that we can still blacklist c and unblacklist b. + std::vector<std::string> vector_ac(1, extension_a->id()); + vector_ac.push_back(extension_c->id()); + blacklist()->SetFromUpdater(vector_ac, "8"); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsBlacklisted(extension_a)); + EXPECT_TRUE(IsSafe(extension_b)); + EXPECT_TRUE(IsBlacklisted(extension_c)); + EXPECT_TRUE(notifications.CheckNotifications( + chrome::NOTIFICATION_EXTENSION_LOADED, + chrome::NOTIFICATION_EXTENSION_UNLOADED)); +} + +} // namespace extensions diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h index b1e82a0..03743b2 100644 --- a/chrome/browser/extensions/extension_browsertest.h +++ b/chrome/browser/extensions/extension_browsertest.h @@ -12,6 +12,8 @@ #include "base/file_path.h" #include "base/files/scoped_temp_dir.h" #include "chrome/browser/extensions/extension_host.h" +#include "chrome/browser/extensions/extension_system.h" +#include "chrome/browser/ui/browser.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/feature_switch.h" #include "chrome/common/extensions/features/feature.h" @@ -21,7 +23,9 @@ #include "content/public/browser/notification_types.h" #include "content/public/browser/web_contents.h" +class ExtensionService; class ExtensionProcessManager; +class Profile; // Base class for extension browser tests. Provides utilities for loading, // unloading, and installing extensions. @@ -50,6 +54,12 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest, ExtensionBrowserTest(); virtual ~ExtensionBrowserTest(); + // Useful accessors. + Profile* profile() { return browser()->profile(); } + ExtensionService* extension_service() { + return extensions::ExtensionSystem::Get(profile())->extension_service(); + } + // InProcessBrowserTest virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE; diff --git a/chrome/browser/extensions/extension_context_menu_model_unittest.cc b/chrome/browser/extensions/extension_context_menu_model_unittest.cc index 66456a0..b431edf 100644 --- a/chrome/browser/extensions/extension_context_menu_model_unittest.cc +++ b/chrome/browser/extensions/extension_context_menu_model_unittest.cc @@ -59,6 +59,9 @@ TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) { // Now the actions are disabled. ASSERT_FALSE(menu->IsCommandIdEnabled(ExtensionContextMenuModel::UNINSTALL)); + + // Don't leave |policy_provider| dangling. + system->management_policy()->UnregisterAllProviders(); } } // namespace diff --git a/chrome/browser/extensions/extension_error_ui.cc b/chrome/browser/extensions/extension_error_ui.cc index 2ba3b14..2cddf94 100644 --- a/chrome/browser/extensions/extension_error_ui.cc +++ b/chrome/browser/extensions/extension_error_ui.cc @@ -49,8 +49,8 @@ string16 ExtensionErrorUI::GenerateMessageSection( for (ExtensionIdSet::const_iterator iter = extensions->begin(); iter != extensions->end(); ++iter) { - const extensions::Extension* e = extension_service_->GetExtensionById(*iter, - true); + const extensions::Extension* e = + extension_service_->GetInstalledExtension(*iter); message += l10n_util::GetStringFUTF16( e->is_app() ? app_template_message_id : extension_template_message_id, UTF8ToUTF16(e->name())) + char16('\n'); diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index 749f4bc..79b11e3 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -748,57 +748,48 @@ void ExtensionPrefs::ClearDisableReasons(const std::string& extension_id) { UpdateExtensionPref(extension_id, kPrefDisableReasons, NULL); } -void ExtensionPrefs::UpdateBlacklist( - const std::set<std::string>& blacklist_set) { - ExtensionIdList remove_pref_ids; - std::set<std::string> used_id_set; +std::set<std::string> ExtensionPrefs::GetBlacklistedExtensions() { + std::set<std::string> ids; + const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref); + if (!extensions) + return ids; - if (extensions) { - for (DictionaryValue::key_iterator extension_id = extensions->begin_keys(); - extension_id != extensions->end_keys(); ++extension_id) { - const DictionaryValue* ext; - if (!extensions->GetDictionaryWithoutPathExpansion(*extension_id, &ext)) { - NOTREACHED() << "Invalid pref for extension " << *extension_id; - continue; - } - const std::string& id(*extension_id); - if (blacklist_set.find(id) == blacklist_set.end()) { - if (!IsBlacklistBitSet(ext)) { - // This extension is not in blacklist. And it was not blacklisted - // before. - continue; - } else { - if (ext->size() == 1) { - // We should remove the entry if the only flag here is blacklist. - remove_pref_ids.push_back(id); - } else { - // Remove the blacklist bit. - UpdateExtensionPref(id, kPrefBlacklist, NULL); - } - } - } else { - if (!IsBlacklistBitSet(ext)) { - // Only set the blacklist if it was not set. - UpdateExtensionPref(id, kPrefBlacklist, - Value::CreateBooleanValue(true)); - } - // Keep the record if this extension is already processed. - used_id_set.insert(id); - } + for (DictionaryValue::Iterator it(*extensions); it.HasNext(); it.Advance()) { + if (!it.value().IsType(Value::TYPE_DICTIONARY)) { + NOTREACHED() << "Invalid pref for extension " << it.key(); + continue; } + if (IsBlacklistBitSet(static_cast<const DictionaryValue*>(&it.value()))) + ids.insert(it.key()); } - // Iterate the leftovers to set blacklist in pref - std::set<std::string>::const_iterator set_itr = blacklist_set.begin(); - for (; set_itr != blacklist_set.end(); ++set_itr) { - if (used_id_set.find(*set_itr) == used_id_set.end()) { - UpdateExtensionPref(*set_itr, kPrefBlacklist, - Value::CreateBooleanValue(true)); - } + return ids; +} + +void ExtensionPrefs::SetExtensionBlacklisted(const std::string& extension_id, + bool is_blacklisted) { + bool currently_blacklisted = IsExtensionBlacklisted(extension_id); + if (is_blacklisted == currently_blacklisted) { + NOTREACHED() << extension_id << " is " << + (currently_blacklisted ? "already" : "not") << + " blacklisted"; + return; } - for (size_t i = 0; i < remove_pref_ids.size(); ++i) { - DeleteExtensionPrefs(remove_pref_ids[i]); + + // Always make sure the "acknowledged" bit is cleared since the blacklist bit + // is changing. + UpdateExtensionPref(extension_id, kPrefBlacklistAcknowledged, NULL); + + if (is_blacklisted) { + UpdateExtensionPref(extension_id, + kPrefBlacklist, + new base::FundamentalValue(true)); + } else { + UpdateExtensionPref(extension_id, kPrefBlacklist, NULL); + const DictionaryValue* dict = GetExtensionPref(extension_id); + if (dict && dict->empty()) + DeleteExtensionPrefs(extension_id); } } @@ -1656,8 +1647,6 @@ scoped_ptr<ExtensionInfo> ExtensionPrefs::GetInstalledExtensionInfo( if (!extensions || !extensions->GetDictionaryWithoutPathExpansion(extension_id, &ext)) return scoped_ptr<ExtensionInfo>(); - if (IsBlacklistBitSet(ext)) - return scoped_ptr<ExtensionInfo>(); int state_value; if (!ext->GetInteger(kPrefState, &state_value) || state_value == Extension::ENABLED_COMPONENT) { diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index 6adb015..1908716 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -175,6 +175,13 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, Extension::DisableReason disable_reason); void ClearDisableReasons(const std::string& extension_id); + // Gets the set of extensions that have been blacklisted in prefs. + std::set<std::string> GetBlacklistedExtensions(); + + // Sets whether the extension with |id| is blacklisted. + void SetExtensionBlacklisted(const std::string& extension_id, + bool is_blacklisted); + // Returns the version string for the currently installed extension, or // the empty string if not found. std::string GetVersionString(const std::string& extension_id); @@ -189,11 +196,12 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, // Returns base extensions install directory. const FilePath& install_directory() const { return install_directory_; } - // Updates the prefs based on the blacklist. - void UpdateBlacklist(const std::set<std::string>& blacklist_set); - - // Returns whether the extension with |id| is blacklisted. - // You probably don't want to call this method, see Blacklist::IsBlacklisted. + // Returns whether the extension with |id| has its blacklist bit set. + // + // WARNING: this only checks the extension's entry in prefs, so by definition + // can only check extensions that prefs knows about. There may be other + // sources of blacklist information, such as safebrowsing. You probably want + // to use Blacklist::GetBlacklistedIDs rather than this method. bool IsExtensionBlacklisted(const std::string& id) const; // Based on extension id, checks prefs to see if it is orphaned. @@ -495,7 +503,8 @@ class ExtensionPrefs : public ContentSettingsStore::Observer, URLPatternSet GetAllowedInstallSites(); private: - friend class ExtensionPrefsUninstallExtension; // Unit test. + friend class ExtensionPrefsBlacklistedExtensions; // Unit test. + friend class ExtensionPrefsUninstallExtension; // Unit test. // See the Create methods. ExtensionPrefs(PrefService* prefs, diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc index 9ddedbd..49e450c 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -246,7 +246,6 @@ class ExtensionPrefsExtensionState : public ExtensionPrefsTest { }; TEST_F(ExtensionPrefsExtensionState, ExtensionState) {} - class ExtensionPrefsEscalatePermissions : public ExtensionPrefsTest { public: virtual void Initialize() { @@ -1189,4 +1188,83 @@ class ExtensionPrefsDisableExtensions : public ExtensionPrefsPrepopulatedTest { }; TEST_F(ExtensionPrefsDisableExtensions, ExtensionPrefsDisableExtensions) {} +// Tests that blacklist state can be queried. +class ExtensionPrefsBlacklistedExtensions : public ExtensionPrefsTest { + public: + virtual ~ExtensionPrefsBlacklistedExtensions() {} + + virtual void Initialize() OVERRIDE { + extension_a_ = prefs_.AddExtension("a"); + extension_b_ = prefs_.AddExtension("b"); + extension_c_ = prefs_.AddExtension("c"); + } + + virtual void Verify() OVERRIDE { + { + std::set<std::string> ids; + EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions()); + } + prefs()->SetExtensionBlacklisted(extension_a_->id(), true); + { + std::set<std::string> ids; + ids.insert(extension_a_->id()); + EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions()); + } + prefs()->SetExtensionBlacklisted(extension_b_->id(), true); + prefs()->SetExtensionBlacklisted(extension_c_->id(), true); + { + std::set<std::string> ids; + ids.insert(extension_a_->id()); + ids.insert(extension_b_->id()); + ids.insert(extension_c_->id()); + EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions()); + } + prefs()->SetExtensionBlacklisted(extension_a_->id(), false); + { + std::set<std::string> ids; + ids.insert(extension_b_->id()); + ids.insert(extension_c_->id()); + EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions()); + } + prefs()->SetExtensionBlacklisted(extension_b_->id(), false); + prefs()->SetExtensionBlacklisted(extension_c_->id(), false); + { + std::set<std::string> ids; + EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions()); + } + + // The interesting part: make sure that we're cleaning up after ourselves + // when we're storing *just* the fact that the extension is blacklisted. + std::string arbitrary_id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + prefs()->SetExtensionBlacklisted(arbitrary_id, true); + prefs()->SetExtensionBlacklisted(extension_a_->id(), true); + + // (And make sure that the acknowledged bit is also cleared). + prefs()->AcknowledgeBlacklistedExtension(arbitrary_id); + + EXPECT_TRUE(prefs()->GetExtensionPref(arbitrary_id)); + { + std::set<std::string> ids; + ids.insert(arbitrary_id); + ids.insert(extension_a_->id()); + EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions()); + } + prefs()->SetExtensionBlacklisted(arbitrary_id, false); + prefs()->SetExtensionBlacklisted(extension_a_->id(), false); + EXPECT_FALSE(prefs()->GetExtensionPref(arbitrary_id)); + { + std::set<std::string> ids; + EXPECT_EQ(ids, prefs()->GetBlacklistedExtensions()); + } + } + + private: + scoped_refptr<const Extension> extension_a_; + scoped_refptr<const Extension> extension_b_; + scoped_refptr<const Extension> extension_c_; +}; +TEST_F(ExtensionPrefsBlacklistedExtensions, + ExtensionPrefsBlacklistedExtensions) {} + } // namespace extensions diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 17e9793..6ca0238 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -5,6 +5,7 @@ #include "chrome/browser/extensions/extension_service.h" #include <algorithm> +#include <iterator> #include <set> #include "base/basictypes.h" @@ -360,6 +361,7 @@ ExtensionService::ExtensionService(Profile* profile, profile_(profile), system_(extensions::ExtensionSystem::Get(profile)), extension_prefs_(extension_prefs), + blacklist_(blacklist), settings_frontend_(extensions::SettingsFrontend::Create(profile)), pending_extension_manager_(*ALLOW_THIS_IN_INITIALIZER_LIST(this)), install_directory_(install_directory), @@ -465,23 +467,29 @@ const ExtensionSet* ExtensionService::terminated_extensions() const { return &terminated_extensions_; } -const ExtensionSet* ExtensionService::GenerateInstalledExtensionsSet() const { - ExtensionSet* installed_extensions = new ExtensionSet(); +const ExtensionSet* ExtensionService::blacklisted_extensions() const { + return &blacklisted_extensions_; +} + +scoped_ptr<const ExtensionSet> + ExtensionService::GenerateInstalledExtensionsSet() const { + scoped_ptr<ExtensionSet> installed_extensions(new ExtensionSet()); installed_extensions->InsertAll(extensions_); installed_extensions->InsertAll(disabled_extensions_); installed_extensions->InsertAll(terminated_extensions_); - return installed_extensions; + installed_extensions->InsertAll(blacklisted_extensions_); + return installed_extensions.PassAs<const ExtensionSet>(); } -const ExtensionSet* ExtensionService::GetWipedOutExtensions() const { - ExtensionSet* extension_set = new ExtensionSet(); +scoped_ptr<const ExtensionSet> ExtensionService::GetWipedOutExtensions() const { + scoped_ptr<ExtensionSet> extension_set(new ExtensionSet()); for (ExtensionSet::const_iterator iter = disabled_extensions_.begin(); iter != disabled_extensions_.end(); ++iter) { int disabled_reason = extension_prefs_->GetDisableReasons((*iter)->id()); if ((disabled_reason & Extension::DISABLE_SIDELOAD_WIPEOUT) != 0) extension_set->Insert(*iter); } - return extension_set; + return extension_set.PassAs<const ExtensionSet>(); } extensions::PendingExtensionManager* @@ -573,9 +581,39 @@ void ExtensionService::Shutdown() { const Extension* ExtensionService::GetExtensionById( const std::string& id, bool include_disabled) const { int include_mask = INCLUDE_ENABLED; - if (include_disabled) - include_mask |= INCLUDE_DISABLED; - return GetExtensionByIdInternal(id, include_mask); + if (include_disabled) { + // Include blacklisted extensions here because there are hundreds of + // callers of this function, and many might assume that this includes those + // that have been disabled due to blacklisting. + include_mask |= INCLUDE_DISABLED | INCLUDE_BLACKLISTED; + } + return GetExtensionById(id, include_mask); +} + +const Extension* ExtensionService::GetExtensionById( + const std::string& id, int include_mask) const { + std::string lowercase_id = StringToLowerASCII(id); + if (include_mask & INCLUDE_ENABLED) { + const Extension* extension = extensions_.GetByID(lowercase_id); + if (extension) + return extension; + } + if (include_mask & INCLUDE_DISABLED) { + const Extension* extension = disabled_extensions_.GetByID(lowercase_id); + if (extension) + return extension; + } + if (include_mask & INCLUDE_TERMINATED) { + const Extension* extension = terminated_extensions_.GetByID(lowercase_id); + if (extension) + return extension; + } + if (include_mask & INCLUDE_BLACKLISTED) { + const Extension* extension = blacklisted_extensions_.GetByID(lowercase_id); + if (extension) + return extension; + } + return NULL; } void ExtensionService::Init() { @@ -631,9 +669,7 @@ bool ExtensionService::UpdateExtension(const std::string& id, const extensions::PendingExtensionInfo* pending_extension_info = pending_extension_manager()->GetById(id); - int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED; - const Extension* extension = - GetExtensionByIdInternal(id, include_mask); + const Extension* extension = GetInstalledExtension(id); if (!pending_extension_info && !extension) { LOG(WARNING) << "Will not update extension " << id << " because it is not installed or pending"; @@ -884,8 +920,10 @@ bool ExtensionService::IsExtensionEnabled( return true; } - if (disabled_extensions_.Contains(extension_id)) + if (disabled_extensions_.Contains(extension_id) || + blacklisted_extensions_.Contains(extension_id)) { return false; + } // If the extension hasn't been loaded yet, check the prefs for it. Assume // enabled unless otherwise noted. @@ -908,8 +946,7 @@ void ExtensionService::EnableExtension(const std::string& extension_id) { extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED); extension_prefs_->ClearDisableReasons(extension_id); - const Extension* extension = GetExtensionByIdInternal(extension_id, - INCLUDE_DISABLED); + const Extension* extension = disabled_extensions_.GetByID(extension_id); // This can happen if sync enables an extension that is not // installed yet. if (!extension) @@ -960,8 +997,8 @@ void ExtensionService::DisableExtension( extension_prefs_->SetExtensionState(extension_id, Extension::DISABLED); extension_prefs_->AddDisableReason(extension_id, disable_reason); - int include_mask = INCLUDE_ENABLED | INCLUDE_TERMINATED; - extension = GetExtensionByIdInternal(extension_id, include_mask); + int include_mask = INCLUDE_EVERYTHING & ~INCLUDE_DISABLED; + extension = GetExtensionById(extension_id, include_mask); if (!extension) return; @@ -1267,6 +1304,7 @@ extensions::ExtensionUpdater* ExtensionService::updater() { 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) { @@ -1830,15 +1868,31 @@ void ExtensionService::IdentifyAlertableExtensions() { bool ExtensionService::PopulateExtensionErrorUI( ExtensionErrorUI* extension_error_ui) { bool needs_alert = false; + + // Extensions that are blacklisted. + for (ExtensionSet::const_iterator it = blacklisted_extensions_.begin(); + it != blacklisted_extensions_.end(); ++it) { + std::string id = (*it)->id(); + if (!extension_prefs_->IsBlacklistedExtensionAcknowledged(id)) { + extension_error_ui->AddBlacklistedExtension(id); + needs_alert = true; + } + } + for (ExtensionSet::const_iterator iter = extensions_.begin(); iter != extensions_.end(); ++iter) { const Extension* e = *iter; + + // Extensions disabled by policy. Note: this no longer includes blacklisted + // extensions, though we still show the same UI. if (!system_->management_policy()->UserMayLoad(e, NULL)) { if (!extension_prefs_->IsBlacklistedExtensionAcknowledged(e->id())) { extension_error_ui->AddBlacklistedExtension(e->id()); needs_alert = true; } } + + // Orphaned extensions. if (extension_prefs_->IsExtensionOrphaned(e->id())) { if (!extension_prefs_->IsOrphanedExtensionAcknowledged(e->id())) { extension_error_ui->AddOrphanedExtension(e->id()); @@ -1846,6 +1900,7 @@ bool ExtensionService::PopulateExtensionErrorUI( } } } + return needs_alert; } @@ -1925,9 +1980,9 @@ void ExtensionService::UnloadExtension( const std::string& extension_id, extension_misc::UnloadedExtensionReason reason) { // Make sure the extension gets deleted after we return from this function. - int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED; + int include_mask = INCLUDE_EVERYTHING & ~INCLUDE_TERMINATED; scoped_refptr<const Extension> extension( - GetExtensionByIdInternal(extension_id, include_mask)); + GetExtensionById(extension_id, include_mask)); // This method can be called via PostTask, so the extension may have been // unloaded by the time this runs. @@ -2085,7 +2140,17 @@ void ExtensionService::AddExtension(const Extension* extension) { // wipeout before, we might disable this extension here. MaybeWipeout(extension); - if (extension_prefs_->IsExtensionDisabled(extension->id())) { + // Communicated to the Blacklist. + std::set<std::string> already_in_blacklist; + + if (extension_prefs_->IsExtensionBlacklisted(extension->id())) { + // Don't check the Blacklist yet because it's asynchronous (we do it at + // the end). This pre-emptive check is because we will always store the + // blacklisted state of *installed* extensions in prefs, and it's important + // not to re-enable blacklisted extensions. + blacklisted_extensions_.Insert(extension); + already_in_blacklist.insert(extension->id()); + } else if (extension_prefs_->IsExtensionDisabled(extension->id())) { disabled_extensions_.Insert(extension); SyncExtensionChangeIfNeeded(*extension); content::NotificationService::current()->Notify( @@ -2097,20 +2162,29 @@ void ExtensionService::AddExtension(const Extension* extension) { Extension::DISABLE_PERMISSIONS_INCREASE) { extensions::AddExtensionDisabledError(this, extension); } - return; - } + } else { + // All apps that are displayed in the launcher are ordered by their ordinals + // so we must ensure they have valid ordinals. + if (extension->RequiresSortOrdinal()) { + extension_prefs_->extension_sorting()->EnsureValidOrdinals( + extension->id(), syncer::StringOrdinal()); + } - // All apps that are displayed in the launcher are ordered by their ordinals - // so we must ensure they have valid ordinals. - if (extension->RequiresSortOrdinal()) { - extension_prefs_->extension_sorting()->EnsureValidOrdinals( - extension->id(), syncer::StringOrdinal()); + extensions_.Insert(extension); + SyncExtensionChangeIfNeeded(*extension); + NotifyExtensionLoaded(extension); + DoPostLoadTasks(extension); } - extensions_.Insert(extension); - SyncExtensionChangeIfNeeded(*extension); - NotifyExtensionLoaded(extension); - DoPostLoadTasks(extension); + // Lastly, begin the process for checking the blacklist status of extensions. + // This may need to go to other threads so is asynchronous. + std::set<std::string> id_set; + id_set.insert(extension->id()); + blacklist_->GetBlacklistedIDs( + id_set, + base::Bind(&ExtensionService::ManageBlacklist, + AsWeakPtr(), + already_in_blacklist)); } void ExtensionService::AddComponentExtension(const Extension* extension) { @@ -2183,9 +2257,7 @@ void ExtensionService::InitializePermissions(const Extension* extension) { // still remember that "omnibox" had been granted, so that if the // extension once again includes "omnibox" in an upgrade, the extension // can upgrade without requiring this user's approval. - int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED; - const Extension* old = - GetExtensionByIdInternal(extension->id(), include_mask); + const Extension* old = GetInstalledExtension(extension->id()); bool is_extension_upgrade = old != NULL; bool is_privilege_increase = false; bool previously_disabled = false; @@ -2366,8 +2438,7 @@ void ExtensionService::OnExtensionInstalled( extension_prefs_->ClearDisableReasons(id); } - int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED; - if (!GetExtensionByIdInternal(extension->id(), include_mask)) { + if (!GetInstalledExtension(extension->id())) { UMA_HISTOGRAM_ENUMERATION("Extensions.InstallType", extension->GetType(), 100); UMA_HISTOGRAM_ENUMERATION("Extensions.InstallSource", @@ -2479,27 +2550,6 @@ const Extension* ExtensionService::GetPendingExtensionUpdate( return pending_extension_updates_.GetByID(id); } -const Extension* ExtensionService::GetExtensionByIdInternal( - const std::string& id, int include_mask) const { - std::string lowercase_id = StringToLowerASCII(id); - if (include_mask & INCLUDE_ENABLED) { - const Extension* extension = extensions_.GetByID(lowercase_id); - if (extension) - return extension; - } - if (include_mask & INCLUDE_DISABLED) { - const Extension* extension = disabled_extensions_.GetByID(lowercase_id); - if (extension) - return extension; - } - if (include_mask & INCLUDE_TERMINATED) { - const Extension* extension = terminated_extensions_.GetByID(lowercase_id); - if (extension) - return extension; - } - return NULL; -} - void ExtensionService::TrackTerminatedExtension(const Extension* extension) { if (!terminated_extensions_.Contains(extension->id())) terminated_extensions_.Insert(make_scoped_refptr(extension)); @@ -2514,13 +2564,16 @@ void ExtensionService::UntrackTerminatedExtension(const std::string& id) { const Extension* ExtensionService::GetTerminatedExtension( const std::string& id) const { - return GetExtensionByIdInternal(id, INCLUDE_TERMINATED); + return GetExtensionById(id, INCLUDE_TERMINATED); } const Extension* ExtensionService::GetInstalledExtension( const std::string& id) const { - int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED | INCLUDE_TERMINATED; - return GetExtensionByIdInternal(id, include_mask); + int include_mask = INCLUDE_ENABLED | + INCLUDE_DISABLED | + INCLUDE_TERMINATED | + INCLUDE_BLACKLISTED; + return GetExtensionById(id, include_mask); } bool ExtensionService::ExtensionBindingsAllowed(const GURL& url) { @@ -3012,9 +3065,7 @@ bool ExtensionService::ShouldDelayExtensionUpdate( if (!install_updates_when_idle_ || !wait_for_idle) return false; - int include_mask = INCLUDE_ENABLED | INCLUDE_DISABLED; - const Extension* old = - GetExtensionByIdInternal(extension_id, include_mask); + const Extension* old = GetInstalledExtension(extension_id); // If there is no old extension, this is not an update, so don't delay. if (!old) return false; @@ -3031,5 +3082,49 @@ bool ExtensionService::ShouldDelayExtensionUpdate( } void ExtensionService::OnBlacklistUpdated() { - CheckManagementPolicy(); + blacklist_->GetBlacklistedIDs( + GenerateInstalledExtensionsSet()->GetIDs(), + base::Bind(&ExtensionService::ManageBlacklist, + AsWeakPtr(), + blacklisted_extensions_.GetIDs())); +} + +void ExtensionService::ManageBlacklist( + const std::set<std::string>& old_blacklisted_ids, + const std::set<std::string>& new_blacklisted_ids) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + std::set<std::string> no_longer_blacklisted; + std::set_difference(old_blacklisted_ids.begin(), old_blacklisted_ids.end(), + new_blacklisted_ids.begin(), new_blacklisted_ids.end(), + std::inserter(no_longer_blacklisted, + no_longer_blacklisted.begin())); + std::set<std::string> not_yet_blacklisted; + std::set_difference(new_blacklisted_ids.begin(), new_blacklisted_ids.end(), + old_blacklisted_ids.begin(), old_blacklisted_ids.end(), + std::inserter(not_yet_blacklisted, + not_yet_blacklisted.begin())); + + for (std::set<std::string>::iterator it = no_longer_blacklisted.begin(); + it != no_longer_blacklisted.end(); ++it) { + scoped_refptr<const Extension> extension = + blacklisted_extensions_.GetByID(*it); + DCHECK(extension); + if (!extension) + continue; + blacklisted_extensions_.Remove(*it); + AddExtension(extension); + } + + for (std::set<std::string>::iterator it = not_yet_blacklisted.begin(); + it != not_yet_blacklisted.end(); ++it) { + scoped_refptr<const Extension> extension = GetInstalledExtension(*it); + DCHECK(extension); + if (!extension) + continue; + blacklisted_extensions_.Insert(extension); + UnloadExtension(*it, extension_misc::UNLOAD_REASON_BLACKLIST); + } + + IdentifyAlertableExtensions(); } diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h index 4b02b51..6e2d1b7 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -210,14 +210,15 @@ class ExtensionService virtual const ExtensionSet* extensions() const OVERRIDE; virtual const ExtensionSet* disabled_extensions() const OVERRIDE; const ExtensionSet* terminated_extensions() const; + const ExtensionSet* blacklisted_extensions() const; - // Returns a set of all installed, disabled, and terminated extensions and - // transfers ownership to caller. - const ExtensionSet* GenerateInstalledExtensionsSet() const; + // Returns a set of all installed, disabled, blacklisted, and terminated + // extensions. + scoped_ptr<const ExtensionSet> GenerateInstalledExtensionsSet() const; // Returns a set of all extensions disabled by the sideload wipeout // initiative. - const ExtensionSet* GetWipedOutExtensions() const; + scoped_ptr<const ExtensionSet> GetWipedOutExtensions() const; // Gets the object managing the set of pending extensions. virtual extensions::PendingExtensionManager* @@ -304,17 +305,34 @@ class ExtensionService // Called when the associated Profile is going to be destroyed. void Shutdown(); - // Look up an extension by ID. Does not include terminated + // Look up an extension by ID. Does not include terminated // extensions. virtual const extensions::Extension* GetExtensionById( const std::string& id, bool include_disabled) const OVERRIDE; + enum IncludeFlag { + INCLUDE_NONE = 0, + INCLUDE_ENABLED = 1 << 0, + INCLUDE_DISABLED = 1 << 1, + INCLUDE_TERMINATED = 1 << 2, + INCLUDE_BLACKLISTED = 1 << 3, + INCLUDE_EVERYTHING = (1 << 4) - 1, + }; + + // Look up an extension by ID, selecting which sets to look in: + // * extensions() --> INCLUDE_ENABLED + // * disabled_extensions() --> INCLUDE_DISABLED + // * terminated_extensions() --> INCLUDE_TERMINATED + // * blacklisted_extensions() --> INCLUDE_BLACKLISTED + const extensions::Extension* GetExtensionById(const std::string& id, + int include_mask) const; + // Looks up a terminated (crashed) extension by ID. const extensions::Extension* GetTerminatedExtension(const std::string& id) const; // Looks up an extension by ID, regardless of whether it's enabled, - // disabled, or terminated. + // disabled, blacklisted, or terminated. virtual const extensions::Extension* GetInstalledExtension( const std::string& id) const OVERRIDE; @@ -741,13 +759,6 @@ class ExtensionService const extensions::ExtensionSyncData& extension_sync_data, syncer::ModelType type); - enum IncludeFlag { - INCLUDE_NONE = 0, - INCLUDE_ENABLED = 1 << 0, - INCLUDE_DISABLED = 1 << 1, - INCLUDE_TERMINATED = 1 << 2 - }; - // Events to be fired after an extension is reloaded. enum PostReloadEvents { EVENT_NONE = 0, @@ -755,12 +766,6 @@ class ExtensionService EVENT_RESTARTED = 1 << 1, }; - // Look up an extension by ID, optionally including either or both of enabled - // and disabled extensions. - const extensions::Extension* GetExtensionByIdInternal( - const std::string& id, - int include_mask) const; - // Adds the given extension to the list of terminated extensions if // it is not already there and unloads it. void TrackTerminatedExtension(const extensions::Extension* extension); @@ -843,15 +848,23 @@ class ExtensionService // extensions::Blacklist::Observer implementation. virtual void OnBlacklistUpdated() OVERRIDE; + // Manages the blacklisted extensions, intended as callback from + // Blacklist::GetBlacklistedIDs. + void ManageBlacklist(const std::set<std::string>& old_blacklisted_ids, + const std::set<std::string>& new_blacklisted_ids); + // The normal profile associated with this ExtensionService. Profile* profile_; // The ExtensionSystem for the profile above. extensions::ExtensionSystem* system_; - // Preferences for the owning profile (weak reference). + // Preferences for the owning profile. extensions::ExtensionPrefs* extension_prefs_; + // Blacklist for the owning profile. + extensions::Blacklist* blacklist_; + // Settings for the owning profile. scoped_ptr<extensions::SettingsFrontend> settings_frontend_; @@ -864,6 +877,12 @@ class ExtensionService // The list of installed extensions that have been terminated. ExtensionSet terminated_extensions_; + // The list of installed extensions that have been blacklisted. Generally + // these shouldn't be considered as installed by the extension platform: we + // only keep them around so that if extensions are blacklisted by mistake + // they can easily be un-blacklisted. + ExtensionSet blacklisted_extensions_; + // The list of extension updates that are waiting to be installed. ExtensionSet pending_extension_updates_; diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index 847c987..def4e8a 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -626,7 +626,8 @@ class ExtensionServiceTest enum InstallState { INSTALL_FAILED, INSTALL_UPDATED, - INSTALL_NEW + INSTALL_NEW, + INSTALL_WITHOUT_LOAD, }; const Extension* PackAndInstallCRX(const FilePath& dir_path, @@ -701,14 +702,19 @@ class ExtensionServiceTest ++expected_extensions_count_; EXPECT_TRUE(installed_) << path.value(); - - EXPECT_EQ(1u, loaded_.size()) << path.value(); EXPECT_EQ(0u, errors.size()) << path.value(); - EXPECT_EQ(expected_extensions_count_, service_->extensions()->size()) << - path.value(); - extension = loaded_[0]; - EXPECT_TRUE(service_->GetExtensionById(extension->id(), false)) << - path.value(); + + if (install_state == INSTALL_WITHOUT_LOAD) { + EXPECT_EQ(0u, loaded_.size()) << path.value(); + } else { + EXPECT_EQ(1u, loaded_.size()) << path.value(); + EXPECT_EQ(expected_extensions_count_, service_->extensions()->size()) << + path.value(); + extension = loaded_[0]; + EXPECT_TRUE(service_->GetExtensionById(extension->id(), false)) << + path.value(); + } + for (std::vector<string16>::iterator err = errors.begin(); err != errors.end(); ++err) { LOG(ERROR) << *err; @@ -2998,7 +3004,7 @@ TEST_F(ExtensionServiceTest, BlacklistedExtensionWillNotInstall) { // We can not install good_crx. FilePath path = data_dir_.AppendASCII("good.crx"); - InstallCRX(path, INSTALL_FAILED); + InstallCRX(path, INSTALL_WITHOUT_LOAD); EXPECT_EQ(0u, service_->extensions()->size()); ValidateBooleanPref(good_crx, "blacklist", true); } @@ -3024,97 +3030,13 @@ TEST_F(ExtensionServiceTest, UnloadBlacklistedExtensionPolicy) { "v1"); // Make sure pref is updated + loop_.RunUntilIdle(); - // 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. + // The good_crx is blacklisted and the whitelist doesn't negate it. 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); - ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist, - "v1"); - - 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); - ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist, - "v1"); - - 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); - ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist, - "v1"); - - 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); - ExtensionSystem::Get(profile_.get())->blacklist()->SetFromUpdater(blacklist, - "v1"); - - InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW); -} - // Test loading extensions from the profile directory, except // blacklisted ones. TEST_F(ExtensionServiceTest, WillNotLoadBlacklistedExtensionsFromDirectory) { @@ -3148,7 +3070,10 @@ TEST_F(ExtensionServiceTest, WillNotLoadBlacklistedExtensionsFromDirectory) { } ASSERT_EQ(2u, loaded_.size()); - EXPECT_FALSE(service_->GetExtensionById(good1, true)); + EXPECT_TRUE(service_->GetInstalledExtension(good1)); + int include_mask = ExtensionService::INCLUDE_EVERYTHING & + ~ExtensionService::INCLUDE_BLACKLISTED; + EXPECT_FALSE(service_->GetExtensionById(good1, include_mask)); } // Will not install extension blacklisted by policy. diff --git a/chrome/browser/extensions/extension_system.cc b/chrome/browser/extensions/extension_system.cc index 3422f6d..2881cec 100644 --- a/chrome/browser/extensions/extension_system.cc +++ b/chrome/browser/extensions/extension_system.cc @@ -103,8 +103,7 @@ void ExtensionSystemImpl::Shared::InitPrefs() { blacklist_.reset(new Blacklist(extension_prefs_.get())); standard_management_policy_provider_.reset( - new StandardManagementPolicyProvider(extension_prefs_.get(), - blacklist_.get())); + new StandardManagementPolicyProvider(extension_prefs_.get())); } void ExtensionSystemImpl::Shared::RegisterManagementPolicyProviders() { diff --git a/chrome/browser/extensions/standard_management_policy_provider.cc b/chrome/browser/extensions/standard_management_policy_provider.cc index 05e2e5f..1a818f2 100644 --- a/chrome/browser/extensions/standard_management_policy_provider.cc +++ b/chrome/browser/extensions/standard_management_policy_provider.cc @@ -14,8 +14,8 @@ namespace extensions { StandardManagementPolicyProvider::StandardManagementPolicyProvider( - ExtensionPrefs* prefs, Blacklist* blacklist) - : prefs_(prefs), blacklist_(blacklist) { + ExtensionPrefs* prefs) + : prefs_(prefs) { } StandardManagementPolicyProvider::~StandardManagementPolicyProvider() { @@ -34,15 +34,14 @@ std::string bool StandardManagementPolicyProvider::UserMayLoad( const Extension* extension, string16* error) const { - bool is_google_blacklisted = blacklist_->IsBlacklisted(extension); const base::ListValue* blacklist = prefs_->pref_service()->GetList(prefs::kExtensionInstallDenyList); const base::ListValue* whitelist = prefs_->pref_service()->GetList(prefs::kExtensionInstallAllowList); const base::ListValue* forcelist = prefs_->pref_service()->GetList(prefs::kExtensionInstallForceList); - return admin_policy::UserMayLoad(is_google_blacklisted, blacklist, whitelist, - forcelist, extension, error); + return admin_policy::UserMayLoad( + blacklist, whitelist, forcelist, extension, error); } bool StandardManagementPolicyProvider::UserMayModifySettings( diff --git a/chrome/browser/extensions/standard_management_policy_provider.h b/chrome/browser/extensions/standard_management_policy_provider.h index 8c849da..4dd2a68 100644 --- a/chrome/browser/extensions/standard_management_policy_provider.h +++ b/chrome/browser/extensions/standard_management_policy_provider.h @@ -17,9 +17,8 @@ class ExtensionPrefs; // extension black/whitelists and admin black/whitelists. class StandardManagementPolicyProvider : public ManagementPolicy::Provider { public: - // |prefs| and |blacklist| must outlive this. - StandardManagementPolicyProvider(ExtensionPrefs* prefs, - Blacklist* blacklist); + // |prefs| must outlive this. + explicit StandardManagementPolicyProvider(ExtensionPrefs* prefs); virtual ~StandardManagementPolicyProvider(); @@ -34,8 +33,6 @@ class StandardManagementPolicyProvider : public ManagementPolicy::Provider { private: ExtensionPrefs* const prefs_; - - Blacklist* const blacklist_; }; } // namespace extensions diff --git a/chrome/browser/extensions/standard_management_policy_provider_unittest.cc b/chrome/browser/extensions/standard_management_policy_provider_unittest.cc index 134e146..8cdc3d8 100644 --- a/chrome/browser/extensions/standard_management_policy_provider_unittest.cc +++ b/chrome/browser/extensions/standard_management_policy_provider_unittest.cc @@ -21,11 +21,9 @@ class StandardManagementPolicyProviderTest : public testing::Test { : ui_thread_(content::BrowserThread::UI, &message_loop_), file_thread_(content::BrowserThread::FILE, &message_loop_), prefs_(message_loop_.message_loop_proxy()), - blacklist_(prefs()), - provider_(prefs(), &blacklist_) { + provider_(prefs()) { } - protected: ExtensionPrefs* prefs() { return prefs_.prefs(); @@ -49,53 +47,9 @@ class StandardManagementPolicyProviderTest : public testing::Test { TestExtensionPrefs prefs_; - Blacklist blacklist_; - StandardManagementPolicyProvider provider_; }; -// Tests various areas of blacklist functionality. -TEST_F(StandardManagementPolicyProviderTest, Blacklist) { - std::string not_installed_id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - - // Install 5 extensions. - ExtensionList extensions; - for (int i = 0; i < 5; i++) { - std::string name = "test" + base::IntToString(i); - extensions.push_back(prefs_.AddExtension(name)); - } - EXPECT_EQ(NULL, - prefs()->GetInstalledExtensionInfo(not_installed_id).get()); - - ExtensionList::const_iterator iter; - for (iter = extensions.begin(); iter != extensions.end(); ++iter) { - EXPECT_TRUE(provider_.UserMayLoad(*iter, NULL)); - } - // Blacklist one installed and one not-installed extension id. - std::vector<std::string> blacklisted_ids; - blacklisted_ids.push_back(extensions[0]->id()); - blacklisted_ids.push_back(not_installed_id); - blacklist_.SetFromUpdater(blacklisted_ids, "version0"); - - // Make sure the id we expect to be blacklisted is. - EXPECT_FALSE(provider_.UserMayLoad(extensions[0], NULL)); - - // Make sure the other id's are not blacklisted. - for (iter = extensions.begin() + 1; iter != extensions.end(); ++iter) - EXPECT_TRUE(provider_.UserMayLoad(*iter, NULL)); - - // Make sure GetInstalledExtensionsInfo returns only the non-blacklisted - // extensions data. - scoped_ptr<ExtensionPrefs::ExtensionsInfo> info( - prefs()->GetInstalledExtensionsInfo()); - EXPECT_EQ(4u, info->size()); - ExtensionPrefs::ExtensionsInfo::iterator info_iter; - for (info_iter = info->begin(); info_iter != info->end(); ++info_iter) { - ExtensionInfo* extension_info = info_iter->get(); - EXPECT_NE(extensions[0]->id(), extension_info->extension_id); - } -} - // Tests the behavior of the ManagementPolicy provider methods for an // extension required by policy. TEST_F(StandardManagementPolicyProviderTest, RequiredExtension) { diff --git a/chrome/browser/extensions/test_blacklist.cc b/chrome/browser/extensions/test_blacklist.cc new file mode 100644 index 0000000..138804d --- /dev/null +++ b/chrome/browser/extensions/test_blacklist.cc @@ -0,0 +1,40 @@ +// Copyright 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/test_blacklist.h" + +#include <set> + +#include "base/bind.h" +#include "base/message_loop.h" +#include "base/run_loop.h" +#include "chrome/browser/extensions/blacklist.h" + +namespace extensions { + +TestBlacklist::TestBlacklist(Blacklist* blacklist) + : blacklist_(blacklist) { +} + +namespace { + +void Assign(std::set<std::string>* out, const std::set<std::string>& in) { + *out = in; +} + +} // namespace + +bool TestBlacklist::IsBlacklisted(const std::string& extension_id) { + std::set<std::string> id_set; + id_set.insert(extension_id); + std::set<std::string> blacklist_set; + blacklist_->GetBlacklistedIDs(id_set, + base::Bind(&Assign, &blacklist_set)); + base::RunLoop run_loop; + MessageLoop::current()->PostTask(FROM_HERE, run_loop.QuitClosure()); + run_loop.Run(); + return blacklist_set.count(extension_id) > 0; +} + +} // namespace extensions diff --git a/chrome/browser/extensions/test_blacklist.h b/chrome/browser/extensions/test_blacklist.h new file mode 100644 index 0000000..a9b5690 --- /dev/null +++ b/chrome/browser/extensions/test_blacklist.h @@ -0,0 +1,34 @@ +// Copyright 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. + +#ifndef CHROME_BROWSER_EXTENSIONS_TEST_BLACKLIST_H_ +#define CHROME_BROWSER_EXTENSIONS_TEST_BLACKLIST_H_ + +#include <string> + +#include "base/basictypes.h" + +namespace extensions { + +class Blacklist; + +// A wrapper for an extensions::Blacklist that provides functionality for +// testing. +class TestBlacklist { + public: + explicit TestBlacklist(Blacklist* blacklist); + + Blacklist* blacklist() { return blacklist_; } + + bool IsBlacklisted(const std::string& extension_id); + + private: + Blacklist* blacklist_; + + DISALLOW_COPY_AND_ASSIGN(TestBlacklist); +}; + +} // namespace extensions + +#endif // CHROME_BROWSER_EXTENSIONS_TEST_BLACKLIST_H_ diff --git a/chrome/browser/extensions/test_extension_system.cc b/chrome/browser/extensions/test_extension_system.cc index 1b8ccd7..ed6eccf 100644 --- a/chrome/browser/extensions/test_extension_system.cc +++ b/chrome/browser/extensions/test_extension_system.cc @@ -84,8 +84,7 @@ ExtensionService* TestExtensionSystem::CreateExtensionService( new ShellWindowGeometryCache(profile_, state_store_.get())); blacklist_.reset(new Blacklist(extension_prefs_.get())); standard_management_policy_provider_.reset( - new StandardManagementPolicyProvider(extension_prefs_.get(), - blacklist_.get())); + new StandardManagementPolicyProvider(extension_prefs_.get())); management_policy_.reset(new ManagementPolicy()); management_policy_->RegisterProvider( standard_management_policy_provider_.get()); diff --git a/chrome/browser/extensions/test_extension_system.h b/chrome/browser/extensions/test_extension_system.h index 3f80dc0..fa66f60 100644 --- a/chrome/browser/extensions/test_extension_system.h +++ b/chrome/browser/extensions/test_extension_system.h @@ -78,8 +78,8 @@ class TestExtensionSystem : public ExtensionSystem { scoped_ptr<Blacklist> blacklist_; scoped_ptr<StandardManagementPolicyProvider> standard_management_policy_provider_; - scoped_ptr<ExtensionService> extension_service_; scoped_ptr<ManagementPolicy> management_policy_; + scoped_ptr<ExtensionService> extension_service_; scoped_ptr<ExtensionProcessManager> extension_process_manager_; scoped_ptr<AlarmManager> alarm_manager_; scoped_refptr<ExtensionInfoMap> info_map_; diff --git a/chrome/browser/extensions/updater/extension_updater_unittest.cc b/chrome/browser/extensions/updater/extension_updater_unittest.cc index 5c78d84..ea3d9a3 100644 --- a/chrome/browser/extensions/updater/extension_updater_unittest.cc +++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc @@ -7,11 +7,16 @@ #include <set> #include <vector> +#include "base/bind.h" +#include "base/bind_helpers.h" #include "base/command_line.h" #include "base/compiler_specific.h" #include "base/file_util.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" +#include "base/message_loop.h" +#include "base/run_loop.h" +#include "base/sequenced_task_runner.h" #include "base/stl_util.h" #include "base/string_number_conversions.h" #include "base/string_split.h" @@ -24,6 +29,7 @@ #include "chrome/browser/extensions/extension_error_reporter.h" #include "chrome/browser/extensions/extension_sync_data.h" #include "chrome/browser/extensions/extension_system.h" +#include "chrome/browser/extensions/test_blacklist.h" #include "chrome/browser/extensions/test_extension_prefs.h" #include "chrome/browser/extensions/test_extension_service.h" #include "chrome/browser/extensions/test_extension_system.h" @@ -1035,9 +1041,10 @@ class ExtensionUpdaterTest : public testing::Test { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; MockService service(prefs_.get()); + TestBlacklist blacklist(service.blacklist()); ExtensionUpdater updater( &service, service.extension_prefs(), service.pref_service(), - service.profile(), service.blacklist(), kUpdateFrequencySecs); + service.profile(), blacklist.blacklist(), kUpdateFrequencySecs); updater.Start(); ResetDownloader( &updater, @@ -1062,7 +1069,7 @@ class ExtensionUpdaterTest : public testing::Test { // Call back the ExtensionUpdater with a 200 response and some test data. std::string extension_data("aaaabbbbcccceeeeaaaabbbbcccceeee"); - EXPECT_FALSE(service.blacklist()->IsBlacklisted(extension_data)); + EXPECT_FALSE(blacklist.IsBlacklisted(extension_data)); fetcher = factory.GetFetcherByID(ExtensionDownloader::kExtensionFetcherId); EXPECT_TRUE(fetcher != NULL && fetcher->delegate() != NULL); @@ -1076,7 +1083,7 @@ class ExtensionUpdaterTest : public testing::Test { RunUntilIdle(); - EXPECT_TRUE(service.blacklist()->IsBlacklisted(extension_data)); + EXPECT_TRUE(blacklist.IsBlacklisted(extension_data)); EXPECT_EQ(version, service.pref_service()-> GetString(prefs::kExtensionBlacklistUpdateVersion)); diff --git a/chrome/browser/protector/protected_prefs_watcher_unittest.cc b/chrome/browser/protector/protected_prefs_watcher_unittest.cc index f69b043..f67d7f7 100644 --- a/chrome/browser/protector/protected_prefs_watcher_unittest.cc +++ b/chrome/browser/protector/protected_prefs_watcher_unittest.cc @@ -132,9 +132,7 @@ TEST_F(ProtectedPrefsWatcherTest, ExtensionPrefChange) { EXPECT_FALSE(IsSignatureValid()); // Blacklisting the extension does update the backup and signature. - std::set<std::string> blacklist; - blacklist.insert(sample_id); - extension_prefs->UpdateBlacklist(blacklist); + extension_prefs->SetExtensionBlacklisted(sample_id, true); EXPECT_TRUE(IsSignatureValid()); } diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index d69e238..f74c916 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -990,6 +990,7 @@ 'browser/extensions/extension_apitest.cc', 'browser/extensions/extension_apitest.h', 'browser/extensions/extension_bindings_apitest.cc', + 'browser/extensions/extension_blacklist_browsertest.cc', 'browser/extensions/extension_browsertest.cc', 'browser/extensions/extension_browsertest.h', 'browser/extensions/extension_context_menu_browsertest.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index ad01328..e58fc98 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -107,6 +107,8 @@ 'browser/download/test_download_shelf.h', 'browser/extensions/mock_extension_special_storage_policy.cc', 'browser/extensions/mock_extension_special_storage_policy.h', + 'browser/extensions/test_blacklist.cc', + 'browser/extensions/test_blacklist.h', 'browser/extensions/test_extension_prefs.cc', 'browser/extensions/test_extension_prefs.h', 'browser/extensions/test_extension_service.cc', @@ -691,6 +693,7 @@ 'browser/extensions/app_notification_test_util.cc', 'browser/extensions/app_notify_channel_setup_unittest.cc', 'browser/extensions/app_sync_data_unittest.cc', + 'browser/extensions/blacklist_unittest.cc', 'browser/extensions/component_loader_unittest.cc', 'browser/extensions/convert_user_script_unittest.cc', 'browser/extensions/convert_web_app_unittest.cc', diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h index e9d4025..bcc010a 100644 --- a/chrome/common/extensions/extension_constants.h +++ b/chrome/common/extensions/extension_constants.h @@ -237,6 +237,7 @@ namespace extension_misc { UNLOAD_REASON_UPDATE, // Extension is being updated to a newer version. UNLOAD_REASON_UNINSTALL, // Extension is being uninstalled. UNLOAD_REASON_TERMINATE, // Extension has terminated. + UNLOAD_REASON_BLACKLIST, // Extension has been blacklisted. }; // The states that an app can be in, as reported by chrome.app.installState diff --git a/chrome/common/extensions/extension_set.cc b/chrome/common/extensions/extension_set.cc index 26ac47c..01f35fd 100644 --- a/chrome/common/extensions/extension_set.cc +++ b/chrome/common/extensions/extension_set.cc @@ -63,8 +63,8 @@ bool ExtensionSet::InsertAll(const ExtensionSet& extensions) { return size() != before; } -void ExtensionSet::Remove(const std::string& id) { - extensions_.erase(id); +bool ExtensionSet::Remove(const std::string& id) { + return extensions_.erase(id) > 0; } void ExtensionSet::Clear() { @@ -137,6 +137,15 @@ const Extension* ExtensionSet::GetByID(const std::string& id) const { return NULL; } +std::set<std::string> ExtensionSet::GetIDs() const { + std::set<std::string> ids; + for (ExtensionMap::const_iterator it = extensions_.begin(); + it != extensions_.end(); ++it) { + ids.insert(it->first); + } + return ids; +} + bool ExtensionSet::ExtensionBindingsAllowed( const ExtensionURLInfo& info) const { if (info.origin().isUnique() || IsSandboxedPage(info)) diff --git a/chrome/common/extensions/extension_set.h b/chrome/common/extensions/extension_set.h index 072feff..2cc553e 100644 --- a/chrome/common/extensions/extension_set.h +++ b/chrome/common/extensions/extension_set.h @@ -90,7 +90,8 @@ class ExtensionSet { bool InsertAll(const ExtensionSet& extensions); // Removes the specified extension. - void Remove(const std::string& id); + // Returns true if the set contained the specified extnesion. + bool Remove(const std::string& id); // Removes all extensions. void Clear(); @@ -123,6 +124,9 @@ class ExtensionSet { // Look up an Extension object by id. const extensions::Extension* GetByID(const std::string& id) const; + // Gets the IDs of all extensions in the set. + std::set<std::string> GetIDs() const; + // Returns true if |info| should get extension api bindings and be permitted // to make api calls. Note that this is independent of what extension // permissions the given extension has been granted. |