diff options
Diffstat (limited to 'chrome/browser/extensions')
25 files changed, 1001 insertions, 84 deletions
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc index 32023cf..e294201 100644 --- a/chrome/browser/extensions/crx_installer.cc +++ b/chrome/browser/extensions/crx_installer.cc @@ -26,6 +26,7 @@ #include "chrome/browser/extensions/default_apps_trial.h" #include "chrome/browser/extensions/extension_error_reporter.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/extensions/permissions_updater.h" #include "chrome/browser/extensions/webstore_installer.h" #include "chrome/browser/profiles/profile.h" @@ -254,6 +255,9 @@ bool CrxInstaller::AllowInstall(const Extension* extension, } // The checks below are skipped for themes and external installs. + // TODO(pamg): After ManagementPolicy refactoring is complete, remove this + // and other uses of install_source_ that are no longer needed now that the + // SandboxedExtensionUnpacker sets extension->location. if (extension->is_theme() || Extension::IsExternalLocation(install_source_)) return true; @@ -383,12 +387,10 @@ void CrxInstaller::ConfirmInstall() { return; } - if (!frontend_weak_->extension_prefs()->IsExtensionAllowedByPolicy( - extension_->id(), install_source_)) { - ReportFailureFromUIThread(l10n_util::GetStringFUTF16( - IDS_EXTENSION_CANT_INSTALL_POLICY_BLACKLIST, - UTF8ToUTF16(extension_->name()), - UTF8ToUTF16(extension_->id()))); + string16 error; + if (!ExtensionSystem::Get(profile_)->management_policy()->UserMayLoad( + extension_, &error)) { + ReportFailureFromUIThread(error); return; } diff --git a/chrome/browser/extensions/extension_context_menu_browsertest.cc b/chrome/browser/extensions/extension_context_menu_browsertest.cc index 975cc06..8045e2a 100644 --- a/chrome/browser/extensions/extension_context_menu_browsertest.cc +++ b/chrome/browser/extensions/extension_context_menu_browsertest.cc @@ -5,9 +5,12 @@ #include "base/utf_string_conversions.h" #include "chrome/app/chrome_command_ids.h" #include "chrome/browser/extensions/extension_browsertest.h" +#include "chrome/browser/extensions/extension_context_menu_model.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/extensions/extension_test_message_listener.h" #include "chrome/browser/extensions/lazy_background_page_test_util.h" +#include "chrome/browser/extensions/test_management_policy.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/tab_contents/render_view_context_menu.h" #include "chrome/browser/ui/browser.h" @@ -512,6 +515,41 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuBrowserTest, Enabled) { TestEnabledContextMenu(false); } +// Tests that applicable menu items are disabled when a ManagementPolicy +// prohibits them. +IN_PROC_BROWSER_TEST_F(ExtensionContextMenuBrowserTest, PolicyDisablesItems) { + ASSERT_TRUE(LoadContextMenuExtension("simple")); + ExtensionService* service = browser()->profile()->GetExtensionService(); + ASSERT_TRUE(service != NULL); + ASSERT_FALSE(service->extensions()->is_empty()); + + // We need an extension to pass to the menu constructor, but we don't care + // which one. + ExtensionSet::const_iterator i = service->extensions()->begin(); + const extensions::Extension* extension = *i; + ASSERT_TRUE(extension != NULL); + + scoped_refptr<ExtensionContextMenuModel> menu( + new ExtensionContextMenuModel(extension, browser())); + + ExtensionSystem::Get( + browser()->profile())->management_policy()->UnregisterAllProviders(); + + // Actions should be enabled. + ASSERT_TRUE(menu->IsCommandIdEnabled(ExtensionContextMenuModel::DISABLE)); + ASSERT_TRUE(menu->IsCommandIdEnabled(ExtensionContextMenuModel::UNINSTALL)); + + extensions::TestManagementPolicyProvider policy_provider( + extensions::TestManagementPolicyProvider::PROHIBIT_MODIFY_STATUS); + ExtensionSystem::Get( + browser()->profile())->management_policy()->RegisterProvider( + &policy_provider); + + // Now the actions are disabled. + ASSERT_FALSE(menu->IsCommandIdEnabled(ExtensionContextMenuModel::DISABLE)); + ASSERT_FALSE(menu->IsCommandIdEnabled(ExtensionContextMenuModel::UNINSTALL)); +} + class ExtensionContextMenuBrowserLazyTest : public ExtensionContextMenuBrowserTest { void SetUpCommandLine(CommandLine* command_line) { diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc index 92eceed..3d32b8b 100644 --- a/chrome/browser/extensions/extension_context_menu_model.cc +++ b/chrome/browser/extensions/extension_context_menu_model.cc @@ -6,6 +6,7 @@ #include "base/utf_string_conversions.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/extensions/extension_tab_util.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" @@ -25,15 +26,6 @@ using content::Referrer; using content::WebContents; using extensions::Extension; -enum MenuEntries { - NAME = 0, - CONFIGURE, - HIDE, - DISABLE, - UNINSTALL, - MANAGE -}; - ExtensionContextMenuModel::ExtensionContextMenuModel( const Extension* extension, Browser* browser) @@ -65,7 +57,8 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const { return extension->GetHomepageURL().is_valid(); } else if (command_id == DISABLE || command_id == UNINSTALL) { // Some extension types can not be disabled or uninstalled. - return Extension::UserMayDisable(extension->location()); + return ExtensionSystem::Get( + profile_)->management_policy()->UserMayModifySettings(extension, NULL); } return true; } diff --git a/chrome/browser/extensions/extension_context_menu_model.h b/chrome/browser/extensions/extension_context_menu_model.h index 1f0134c..282dd17 100644 --- a/chrome/browser/extensions/extension_context_menu_model.h +++ b/chrome/browser/extensions/extension_context_menu_model.h @@ -28,6 +28,15 @@ class ExtensionContextMenuModel public ui::SimpleMenuModel::Delegate, public ExtensionUninstallDialog::Delegate { public: + enum MenuEntries { + NAME = 0, + CONFIGURE, + HIDE, + DISABLE, + UNINSTALL, + MANAGE + }; + // Creates a menu model for the given extension action. ExtensionContextMenuModel(const extensions::Extension* extension, Browser* browser); diff --git a/chrome/browser/extensions/extension_management_api.cc b/chrome/browser/extensions/extension_management_api.cc index 5666e07..3f508d2 100644 --- a/chrome/browser/extensions/extension_management_api.cc +++ b/chrome/browser/extensions/extension_management_api.cc @@ -17,7 +17,9 @@ #include "chrome/browser/extensions/extension_event_router.h" #include "chrome/browser/extensions/extension_management_api_constants.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/extensions/extension_uninstall_dialog.h" +#include "chrome/browser/extensions/management_policy.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/extensions/application_launch.h" #include "chrome/browser/ui/webui/extensions/extension_icon_source.h" @@ -72,11 +74,17 @@ static DictionaryValue* CreateExtensionInfo(const Extension& extension, bool enabled = service->IsExtensionEnabled(extension.id()); extension.GetBasicInfo(enabled, info); + const extensions::ManagementPolicy* policy = ExtensionSystem::Get( + service->profile())->management_policy(); + info->SetBoolean(keys::kMayDisableKey, + policy->UserMayModifySettings(&extension, NULL)); + info->SetBoolean(keys::kIsAppKey, extension.is_app()); if (!enabled) { - bool permissions_escalated = service->extension_prefs()-> - DidExtensionEscalatePermissions(extension.id()); + ExtensionPrefs* prefs = service->extension_prefs(); + bool permissions_escalated = + prefs->DidExtensionEscalatePermissions(extension.id()); const char* reason = permissions_escalated ? keys::kDisabledReasonPermissionsIncrease : keys::kDisabledReasonUnknown; info->SetString(keys::kDisabledReasonKey, reason); @@ -183,7 +191,7 @@ bool GetPermissionWarningsByIdFunction::RunImpl() { const Extension* extension = service()->GetExtensionById(ext_id, true); if (!extension) { error_ = ExtensionErrorUtils::FormatErrorMessage(keys::kNoExtensionError, - ext_id); + ext_id); return false; } @@ -203,7 +211,7 @@ namespace { class SafeManifestJSONParser : public UtilityProcessHostClient { public: SafeManifestJSONParser(GetPermissionWarningsByManifestFunction* client, - const std::string& manifest) + const std::string& manifest) : client_(client), manifest_(manifest) {} @@ -381,9 +389,11 @@ bool SetEnabledFunction::RunImpl() { return false; } - if (!Extension::UserMayDisable(extension->location())) { + const extensions::ManagementPolicy* policy = ExtensionSystem::Get( + profile())->management_policy(); + if (!policy->UserMayModifySettings(extension, NULL)) { error_ = ExtensionErrorUtils::FormatErrorMessage( - keys::kUserCantDisableError, extension_id_); + keys::kUserCantModifyError, extension_id_); return false; } @@ -454,12 +464,10 @@ bool UninstallFunction::RunImpl() { return false; } - ExtensionPrefs* prefs = service()->extension_prefs(); - - if (!Extension::UserMayDisable( - prefs->GetInstalledExtensionInfo(extension_id_)->extension_location)) { + if (!ExtensionSystem::Get( + profile())->management_policy()->UserMayModifySettings(extension, NULL)) { error_ = ExtensionErrorUtils::FormatErrorMessage( - keys::kUserCantDisableError, extension_id_); + keys::kUserCantModifyError, extension_id_); return false; } diff --git a/chrome/browser/extensions/extension_management_api_constants.cc b/chrome/browser/extensions/extension_management_api_constants.cc index ec2b8d1..02d765b 100644 --- a/chrome/browser/extensions/extension_management_api_constants.cc +++ b/chrome/browser/extensions/extension_management_api_constants.cc @@ -11,6 +11,7 @@ const char kDisabledReasonKey[] = "disabledReason"; const char kHostPermissionsKey[] = "hostPermissions"; const char kIconsKey[] = "icons"; const char kIsAppKey[] = "isApp"; +const char kMayDisableKey[] = "mayDisable"; const char kPermissionsKey[] = "permissions"; const char kShowConfirmDialogKey[] = "showConfirmDialog"; const char kSizeKey[] = "size"; @@ -28,8 +29,8 @@ const char kGestureNeededForEscalationError[] = const char kManifestParseError[] = "Failed to parse manifest."; const char kNoExtensionError[] = "Failed to find extension with id *"; const char kNotAnAppError[] = "Extension * is not an App"; +const char kUserCantModifyError[] = "Extension * cannot be modified by user"; const char kUninstallCanceledError[] = "Extension * uninstall canceled by user"; -const char kUserCantDisableError[] = "Extension * can not be disabled by user"; const char kUserDidNotReEnableError[] = "The user did not accept the re-enable dialog"; diff --git a/chrome/browser/extensions/extension_management_api_constants.h b/chrome/browser/extensions/extension_management_api_constants.h index ed4174a..e1e77fa 100644 --- a/chrome/browser/extensions/extension_management_api_constants.h +++ b/chrome/browser/extensions/extension_management_api_constants.h @@ -14,6 +14,7 @@ extern const char kDisabledReasonKey[]; extern const char kHostPermissionsKey[]; extern const char kIconsKey[]; extern const char kIsAppKey[]; +extern const char kMayDisableKey[]; extern const char kPermissionsKey[]; extern const char kShowConfirmDialogKey[]; extern const char kSizeKey[]; @@ -30,8 +31,8 @@ extern const char kGestureNeededForEscalationError[]; extern const char kManifestParseError[]; extern const char kNoExtensionError[]; extern const char kNotAnAppError[]; +extern const char kUserCantModifyError[]; extern const char kUninstallCanceledError[]; -extern const char kUserCantDisableError[]; extern const char kUserDidNotReEnableError[]; } // namespace extension_management_api_constants diff --git a/chrome/browser/extensions/extension_management_apitest.cc b/chrome/browser/extensions/extension_management_apitest.cc index e1d0d8a..e00c58c 100644 --- a/chrome/browser/extensions/extension_management_apitest.cc +++ b/chrome/browser/extensions/extension_management_apitest.cc @@ -2,9 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <map> + #include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/extensions/extension_test_message_listener.h" +#include "chrome/browser/extensions/test_management_policy.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_finder.h" @@ -13,6 +17,8 @@ #include "chrome/common/chrome_switches.h" #include "chrome/test/base/ui_test_utils.h" +using extensions::Extension; + namespace { // Find a browser other than |browser|. @@ -41,16 +47,16 @@ class ExtensionManagementApiTest : public ExtensionApiTest { FilePath basedir = test_data_dir_.AppendASCII("management"); // Load 4 enabled items. - ASSERT_TRUE(LoadExtension(basedir.AppendASCII("enabled_extension"))); - ASSERT_TRUE(LoadExtension(basedir.AppendASCII("enabled_app"))); - ASSERT_TRUE(LoadExtension(basedir.AppendASCII("description"))); - ASSERT_TRUE(LoadExtension(basedir.AppendASCII("permissions"))); + InstallNamedExtension(basedir, "enabled_extension"); + InstallNamedExtension(basedir, "enabled_app"); + InstallNamedExtension(basedir, "description"); + InstallNamedExtension(basedir, "permissions"); // Load 2 disabled items. - ASSERT_TRUE(LoadExtension(basedir.AppendASCII("disabled_extension"))); - DisableExtension(last_loaded_extension_id_); - ASSERT_TRUE(LoadExtension(basedir.AppendASCII("disabled_app"))); - DisableExtension(last_loaded_extension_id_); + InstallNamedExtension(basedir, "disabled_extension"); + DisableExtension(extension_ids_["disabled_extension"]); + InstallNamedExtension(basedir, "disabled_app"); + DisableExtension(extension_ids_["disabled_app"]); } // Load an app, and wait for a message from app "management/launch_on_install" @@ -65,6 +71,16 @@ class ExtensionManagementApiTest : public ExtensionApiTest { ASSERT_TRUE(launched_app.WaitUntilSatisfied()); } + + protected: + void InstallNamedExtension(FilePath basedir, std::string name) { + const Extension* extension = LoadExtension(basedir.AppendASCII(name)); + ASSERT_TRUE(extension); + extension_ids_[name] = extension->id(); + } + + // Maps installed extension names to their IDs.. + std::map<std::string, std::string> extension_ids_; }; IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, Basics) { @@ -77,6 +93,42 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, Uninstall) { ASSERT_TRUE(RunExtensionSubtest("management/test", "uninstall.html")); } +// Tests actions on extensions when no management policy is in place. +IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, ManagementPolicyAllowed) { + InstallExtensions(); + ExtensionService* service = browser()->profile()->GetExtensionService(); + EXPECT_TRUE(service->GetExtensionById(extension_ids_["enabled_extension"], + false)); + + // Ensure that all actions are allowed. + ExtensionSystem::Get( + browser()->profile())->management_policy()->UnregisterAllProviders(); + + ASSERT_TRUE(RunExtensionSubtest("management/management_policy", + "allowed.html")); + // The last thing the test does is uninstall the "enabled_extension". + EXPECT_FALSE(service->GetExtensionById(extension_ids_["enabled_extension"], + true)); +} + +// Tests actions on extensions when management policy prohibits those actions. +IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, ManagementPolicyProhibited) { + InstallExtensions(); + ExtensionService* service = browser()->profile()->GetExtensionService(); + EXPECT_TRUE(service->GetExtensionById(extension_ids_["enabled_extension"], + false)); + + // Prohibit status changes. + extensions::ManagementPolicy* policy = + ExtensionSystem::Get(browser()->profile())->management_policy(); + policy->UnregisterAllProviders(); + extensions::TestManagementPolicyProvider provider( + extensions::TestManagementPolicyProvider::PROHIBIT_MODIFY_STATUS); + policy->RegisterProvider(&provider); + ASSERT_TRUE(RunExtensionSubtest("management/management_policy", + "prohibited.html")); +} + IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, LaunchPanelApp) { ExtensionService* service = browser()->profile()->GetExtensionService(); diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index 52af11b..fdcd35d 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -22,6 +22,8 @@ #include "chrome/common/pref_names.h" #include "chrome/common/url_constants.h" #include "content/public/browser/notification_service.h" +#include "grit/generated_resources.h" +#include "ui/base/l10n/l10n_util.h" using extensions::Extension; using extensions::ExtensionInfo; @@ -549,6 +551,24 @@ void ExtensionPrefs::SetExtensionPrefPermissionSet( } } +bool ExtensionPrefs::ManagementPolicyImpl(const Extension* extension, + string16* error, + bool modifiable_value) const { + // An extension that is required (by admin policy, for example) cannot be + // modified. + bool modifiable = !Extension::IsRequired(extension->location()); + // Some callers equate "no restriction" to true, others to false. + if (modifiable) + return modifiable_value; + + if (error) { + *error = l10n_util::GetStringFUTF16( + IDS_EXTENSION_CANT_MODIFY_POLICY_REQUIRED, + UTF8ToUTF16(extension->name())); + } + return !modifiable_value; +} + // static bool ExtensionPrefs::IsBlacklistBitSet(DictionaryValue* ext) { return ReadBooleanFromPref(ext, kPrefBlacklist); @@ -640,15 +660,16 @@ void ExtensionPrefs::SetAppNotificationDisabled( Value::CreateBooleanValue(value)); } -bool ExtensionPrefs::IsExtensionAllowedByPolicy( - const std::string& extension_id, - Extension::Location location) const { - base::StringValue id_value(extension_id); +std::string ExtensionPrefs::GetPolicyProviderName() const { + return "admin policy black/whitelist, via the ExtensionPrefs"; +} + +bool ExtensionPrefs::UserMayLoad(const extensions::Extension* extension, + string16* error) const { + base::StringValue id_value(extension->id()); - if (location == Extension::COMPONENT || - location == Extension::EXTERNAL_POLICY_DOWNLOAD) { + if (extensions::Extension::IsRequired(extension->location())) return true; - } const base::ListValue* blacklist = prefs_->GetList(prefs::kExtensionInstallDenyList); @@ -662,8 +683,25 @@ bool ExtensionPrefs::IsExtensionAllowedByPolicy( return true; // Then check the blacklist (the admin blacklist, not the Google blacklist). - return blacklist->Find(id_value) == blacklist->end() && - !ExtensionsBlacklistedByDefault(); + bool result = blacklist->Find(id_value) == blacklist->end() && + !ExtensionsBlacklistedByDefault(); + if (error && !result) { + *error = l10n_util::GetStringFUTF16( + IDS_EXTENSION_CANT_INSTALL_POLICY_BLACKLIST, + UTF8ToUTF16(extension->name()), + UTF8ToUTF16(extension->id())); + } + return result; +} + +bool ExtensionPrefs::UserMayModifySettings(const Extension* extension, + string16* error) const { + return ManagementPolicyImpl(extension, error, true); +} + +bool ExtensionPrefs::MustRemainEnabled(const Extension* extension, + string16* error) const { + return ManagementPolicyImpl(extension, error, false); } bool ExtensionPrefs::ExtensionsBlacklistedByDefault() const { diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index bef7f2a..bb7e48a 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -17,6 +17,7 @@ #include "chrome/browser/extensions/extension_menu_manager.h" #include "chrome/browser/extensions/extension_prefs_scope.h" #include "chrome/browser/extensions/extension_scoped_prefs.h" +#include "chrome/browser/extensions/management_policy.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/string_ordinal.h" @@ -46,6 +47,7 @@ struct ExtensionOmniboxSuggestion; // PrefValueStore::extension_prefs(), which this class populates and // maintains as the underlying extensions change. class ExtensionPrefs : public extensions::ContentSettingsStore::Observer, + public extensions::ManagementPolicy::Provider, public ExtensionScopedPrefs { public: // Key name for a preference that keeps track of per-extension settings. This @@ -207,10 +209,22 @@ class ExtensionPrefs : public extensions::ContentSettingsStore::Observer, bool IsAppNotificationDisabled(const std::string& extension_id) const; void SetAppNotificationDisabled(const std::string& extension_id, bool value); - // Is the extension with |extension_id| allowed by policy (checking both - // whitelist and blacklist). - bool IsExtensionAllowedByPolicy(const std::string& extension_id, - extensions::Extension::Location location) const; + // ManagementPolicy::Provider + virtual std::string GetPolicyProviderName() const OVERRIDE; + // Returns true if the extension is allowed by admin policy white- and + // blacklists. + virtual bool UserMayLoad(const extensions::Extension* extension, + string16* error) const OVERRIDE; + // Returns false if the extension is required to remain running, as determined + // by Extension::IsRequired(). In practice this enforces the admin policy + // forcelist. + virtual bool UserMayModifySettings(const extensions::Extension* extension, + string16* error) const OVERRIDE; + // Returns true if the extension is required to remain running, as determined + // by Extension::IsRequired(). In practice this enforces the admin policy + // forcelist. + virtual bool MustRemainEnabled(const extensions::Extension* extension, + string16* error) const OVERRIDE; // Checks if extensions are blacklisted by default, by policy. When true, this // means that even extensions without an ID should be blacklisted (e.g. @@ -514,6 +528,12 @@ class ExtensionPrefs : public extensions::ContentSettingsStore::Observer, const std::string& id, bool incognito) const; + // Internal implementation for certain ManagementPolicy::Delegate methods. + // Returns |modifiable_value| if the extension can be modified. + bool ManagementPolicyImpl(const extensions::Extension* extension, + string16* error, + bool modifiable_value) const; + // 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. diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc index b614f240..6d2da47 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -1138,3 +1138,70 @@ class ExtensionPrefsDisableExtensions : public ExtensionPrefsPrepopulatedTest { int iteration_; }; TEST_F(ExtensionPrefsDisableExtensions, ExtensionPrefsDisableExtensions) {} + +// Parent class for testing the ManagementPolicy provider methods. +class ExtensionPrefsManagementPolicyProvider : public ExtensionPrefsTest { + public: + virtual void Initialize() {} + virtual void Verify() {} + + void InitializeWithLocation(Extension::Location location, bool required) { + ASSERT_EQ(required, Extension::IsRequired(location)); + + DictionaryValue values; + values.SetString(keys::kName, "test"); + values.SetString(keys::kVersion, "0.1"); + std::string error; + extension_ = Extension::Create(FilePath(), location, values, + Extension::NO_FLAGS, &error); + ASSERT_TRUE(extension_.get()); + } + + protected: + scoped_refptr<Extension> extension_; +}; + +// Tests the behavior of the ManagementPolicy provider methods for an +// extension required by policy. +class ExtensionPrefsRequiredExtension + : public ExtensionPrefsManagementPolicyProvider { + public: + virtual void Initialize() { + InitializeWithLocation(Extension::EXTERNAL_POLICY_DOWNLOAD, true); + } + + virtual void Verify() { + string16 error16; + EXPECT_TRUE(prefs()->UserMayLoad(extension_.get(), &error16)); + EXPECT_EQ(string16(), error16); + + // We won't check the exact wording of the error, but it should say + // something. + EXPECT_FALSE(prefs()->UserMayModifySettings(extension_.get(), &error16)); + EXPECT_NE(string16(), error16); + EXPECT_TRUE(prefs()->MustRemainEnabled(extension_.get(), &error16)); + EXPECT_NE(string16(), error16); + } +}; +TEST_F(ExtensionPrefsRequiredExtension, RequiredExtension) {} + +// Tests the behavior of the ManagementPolicy provider methods for an +// extension required by policy. +class ExtensionPrefsNotRequiredExtension + : public ExtensionPrefsManagementPolicyProvider { + public: + virtual void Initialize() { + InitializeWithLocation(Extension::INTERNAL, false); + } + + virtual void Verify() { + string16 error16; + EXPECT_TRUE(prefs()->UserMayLoad(extension_.get(), &error16)); + EXPECT_EQ(string16(), error16); + EXPECT_TRUE(prefs()->UserMayModifySettings(extension_.get(), &error16)); + EXPECT_EQ(string16(), error16); + EXPECT_FALSE(prefs()->MustRemainEnabled(extension_.get(), &error16)); + EXPECT_EQ(string16(), error16); + } +}; +TEST_F(ExtensionPrefsNotRequiredExtension, NotRequiredExtension) {} diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 8747212..3d5d054 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -17,7 +17,6 @@ #include "base/metrics/histogram.h" #include "base/path_service.h" #include "base/stl_util.h" -#include "base/string16.h" #include "base/string_number_conversions.h" #include "base/string_util.h" #include "base/stringprintf.h" @@ -105,9 +104,11 @@ #include "content/public/browser/render_process_host.h" #include "content/public/common/pepper_plugin_info.h" #include "googleurl/src/gurl.h" +#include "grit/generated_resources.h" #include "net/base/registry_controlled_domain.h" #include "sync/api/sync_change.h" #include "sync/api/sync_error_factory.h" +#include "ui/base/l10n/l10n_util.h" #include "webkit/database/database_tracker.h" #include "webkit/database/database_util.h" @@ -304,7 +305,7 @@ bool ExtensionService::UninstallExtensionHelper( // The following call to UninstallExtension will not allow an uninstall of a // policy-controlled extension. - std::string error; + string16 error; if (!extensions_service->UninstallExtension(extension_id, false, &error)) { LOG(WARNING) << "Cannot uninstall extension with id " << extension_id << ": " << error; @@ -678,7 +679,7 @@ void ExtensionService::ReloadExtension(const std::string& extension_id) { bool ExtensionService::UninstallExtension( std::string extension_id, bool external_uninstall, - std::string* error) { + string16* error) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); scoped_refptr<const Extension> extension(GetInstalledExtension(extension_id)); @@ -689,15 +690,13 @@ bool ExtensionService::UninstallExtension( // Policy change which triggers an uninstall will always set // |external_uninstall| to true so this is the only way to uninstall // managed extensions. - if (!Extension::UserMayDisable(extension->location()) && - !external_uninstall) { + if (!external_uninstall && + !system_->management_policy()->UserMayModifySettings( + extension.get(), error)) { content::NotificationService::current()->Notify( chrome::NOTIFICATION_EXTENSION_UNINSTALL_NOT_ALLOWED, content::Source<Profile>(profile_), content::Details<const Extension>(extension)); - if (error != NULL) { - *error = errors::kCannotUninstallManagedExtension; - } return false; } @@ -848,8 +847,10 @@ void ExtensionService::DisableExtension( const Extension* extension = GetInstalledExtension(extension_id); // |extension| can be NULL if sync disables an extension that is not // installed yet. - if (extension && !Extension::UserMayDisable(extension->location())) + if (extension && + !system_->management_policy()->UserMayModifySettings(extension, NULL)) { return; + } extension_prefs_->SetExtensionState(extension_id, Extension::DISABLED); extension_prefs_->SetDisableReason(extension_id, disable_reason); @@ -1154,8 +1155,7 @@ void ExtensionService::CheckAdminBlacklist() { for (ExtensionSet::const_iterator iter = extensions_.begin(); iter != extensions_.end(); ++iter) { const Extension* extension = (*iter); - if (!extension_prefs_->IsExtensionAllowedByPolicy(extension->id(), - extension->location())) { + if (!system_->management_policy()->UserMayLoad(extension, NULL)) { to_be_removed.push_back(extension->id()); } } @@ -2097,7 +2097,7 @@ void ExtensionService::OnExtensionInstalled( // installation disabled the extension, make sure it is now enabled. bool initial_enable = !extension_prefs_->IsExtensionDisabled(id) || - !Extension::UserMayDisable(extension->location()); + system_->management_policy()->MustRemainEnabled(extension, NULL); PendingExtensionInfo pending_extension_info; if (pending_extension_manager()->GetById(id, &pending_extension_info)) { pending_extension_manager()->Remove(id); diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h index 90e31f5..911e52a 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -20,6 +20,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "base/property_bag.h" +#include "base/string16.h" #include "base/time.h" #include "base/tuple.h" #include "chrome/browser/extensions/api/api_resource_controller.h" @@ -329,7 +330,7 @@ class ExtensionService // to ExtensionPrefs some other way. virtual bool UninstallExtension(std::string extension_id, bool external_uninstall, - std::string* error); + string16* error); virtual bool IsExtensionEnabled( const std::string& extension_id) const OVERRIDE; @@ -410,7 +411,7 @@ class ExtensionService const std::vector<std::string>& blacklist) OVERRIDE; // Go through each extension and unload those that the network admin has - // put on the blacklist (not to be confused with the Google managed blacklist + // put on the blacklist (not to be confused with the Google-managed blacklist) // set of extensions. virtual void CheckAdminBlacklist() OVERRIDE; diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index 6d2bc0e0..8d6f721 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -46,6 +46,7 @@ #include "chrome/browser/extensions/pending_extension_info.h" #include "chrome/browser/extensions/pending_extension_manager.h" #include "chrome/browser/extensions/test_extension_system.h" +#include "chrome/browser/extensions/test_management_policy.h" #include "chrome/browser/extensions/unpacked_installer.h" #include "chrome/browser/extensions/updater/extension_updater.h" #include "chrome/browser/plugin_prefs_factory.h" @@ -372,6 +373,7 @@ class MockProviderVisitor ExtensionServiceTestBase::ExtensionServiceTestBase() : loop_(MessageLoop::TYPE_IO), service_(NULL), + management_policy_(NULL), expected_extensions_count_(0), ui_thread_(BrowserThread::UI, &loop_), db_thread_(BrowserThread::DB, &loop_), @@ -421,6 +423,9 @@ void ExtensionServiceTestBase::InitializeExtensionService( service_->set_extensions_enabled(true); service_->set_show_extensions_prompts(false); + management_policy_ = static_cast<TestExtensionSystem*>( + ExtensionSystem::Get(profile))->CreateManagementPolicy(); + // When we start up, we want to make sure there is no external provider, // since the ExtensionService on Windows will use the Registry as a default // provider and if there is something already registered there then it will @@ -616,6 +621,8 @@ class ExtensionServiceTest return PackAndInstallCRX(dir_path, FilePath(), install_state); } + // Attempts to install an extension. Use INSTALL_FAILED if the installation + // is expected to fail. const Extension* InstallCRX(const FilePath& path, InstallState install_state) { StartCRXInstall(path); @@ -642,7 +649,8 @@ class ExtensionServiceTest return WaitForCrxInstall(crx_path, install_state); } - // Wait for a CrxInstaller to finish. Used by InstallCRX. + // Wait for a CrxInstaller to finish. Used by InstallCRX. Set the + // |install_state| to INSTALL_FAILED if the installation is expected to fail. // Returns an Extension pointer if the install succeeded, NULL otherwise. const Extension* WaitForCrxInstall(const FilePath& path, InstallState install_state) { @@ -2107,7 +2115,6 @@ TEST_F(ExtensionServiceTest, InstallAppsWithUnlimitedStorage) { EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()-> IsStorageUnlimited(origin1)); - // Uninstall the other, unlimited storage should be revoked. UninstallExtension(id2, false); EXPECT_EQ(0u, service_->extensions()->size()); @@ -2945,6 +2952,142 @@ TEST_F(ExtensionServiceTest, PolicyInstalledExtensionsWhitelisted) { EXPECT_TRUE(service_->GetExtensionById(good_crx, false)); } +// Tests that extensions cannot be installed if the policy provider prohibits +// it. This functionality is implemented in CrxInstaller::ConfirmInstall(). +TEST_F(ExtensionServiceTest, ManagementPolicyProhibitsInstall) { + InitializeEmptyExtensionService(); + + management_policy_->UnregisterAllProviders(); + extensions::TestManagementPolicyProvider provider_( + extensions::TestManagementPolicyProvider::PROHIBIT_LOAD); + management_policy_->RegisterProvider(&provider_); + + InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_FAILED); + EXPECT_EQ(0u, service_->extensions()->size()); +} + +// Tests that extensions cannot be loaded from prefs if the policy provider +// prohibits it. This functionality is implemented in InstalledLoader::Load(). +TEST_F(ExtensionServiceTest, ManagementPolicyProhibitsLoadFromPrefs) { + InitializeEmptyExtensionService(); + + // Create a fake extension to be loaded as though it were read from prefs. + FilePath path = data_dir_.AppendASCII("management") + .AppendASCII("simple_extension"); + DictionaryValue manifest; + manifest.SetString(keys::kName, "simple_extension"); + manifest.SetString(keys::kVersion, "1"); + // LOAD is for extensions loaded from the command line. We use it here, even + // though we're testing loading from prefs, so that we don't need to provide + // an extension key. + extensions::ExtensionInfo extension_info(&manifest, "", path, + Extension::LOAD); + + // Ensure we can load it with no management policy in place. + management_policy_->UnregisterAllProviders(); + EXPECT_EQ(0u, service_->extensions()->size()); + extensions::InstalledLoader(service_).Load(extension_info, false); + EXPECT_EQ(1u, service_->extensions()->size()); + + const Extension* extension = *(service_->extensions()->begin()); + EXPECT_TRUE(service_->UninstallExtension(extension->id(), false, NULL)); + EXPECT_EQ(0u, service_->extensions()->size()); + + // Ensure we cannot load it if management policy prohibits installation. + extensions::TestManagementPolicyProvider provider_( + extensions::TestManagementPolicyProvider::PROHIBIT_LOAD); + management_policy_->RegisterProvider(&provider_); + + extensions::InstalledLoader(service_).Load(extension_info, false); + EXPECT_EQ(0u, service_->extensions()->size()); +} + +// Tests disabling an extension when prohibited by the ManagementPolicy. +TEST_F(ExtensionServiceTest, ManagementPolicyProhibitsDisable) { + InitializeEmptyExtensionService(); + + InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW); + EXPECT_EQ(1u, service_->extensions()->size()); + EXPECT_EQ(0u, service_->disabled_extensions()->size()); + + management_policy_->UnregisterAllProviders(); + extensions::TestManagementPolicyProvider provider( + extensions::TestManagementPolicyProvider::PROHIBIT_MODIFY_STATUS); + management_policy_->RegisterProvider(&provider); + + // Attempt to disable it. + service_->DisableExtension(good_crx, Extension::DISABLE_USER_ACTION); + + EXPECT_EQ(1u, service_->extensions()->size()); + EXPECT_TRUE(service_->GetExtensionById(good_crx, false)); + EXPECT_EQ(0u, service_->disabled_extensions()->size()); +} + +// Tests uninstalling an extension when prohibited by the ManagementPolicy. +TEST_F(ExtensionServiceTest, ManagementPolicyProhibitsUninstall) { + InitializeEmptyExtensionService(); + + InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW); + EXPECT_EQ(1u, service_->extensions()->size()); + EXPECT_EQ(0u, service_->disabled_extensions()->size()); + + management_policy_->UnregisterAllProviders(); + extensions::TestManagementPolicyProvider provider( + extensions::TestManagementPolicyProvider::PROHIBIT_MODIFY_STATUS); + management_policy_->RegisterProvider(&provider); + + // Attempt to uninstall it. + EXPECT_FALSE(service_->UninstallExtension(good_crx, false, NULL)); + + EXPECT_EQ(1u, service_->extensions()->size()); + EXPECT_TRUE(service_->GetExtensionById(good_crx, false)); +} + +// Tests that previously installed extensions that are now prohibited from +// being installed are removed. +TEST_F(ExtensionServiceTest, ManagementPolicyUnloadsAllProhibited) { + InitializeEmptyExtensionService(); + + InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW); + InstallCRX(data_dir_.AppendASCII("page_action.crx"), INSTALL_NEW); + EXPECT_EQ(2u, service_->extensions()->size()); + EXPECT_EQ(0u, service_->disabled_extensions()->size()); + + management_policy_->UnregisterAllProviders(); + extensions::TestManagementPolicyProvider provider( + extensions::TestManagementPolicyProvider::PROHIBIT_LOAD); + management_policy_->RegisterProvider(&provider); + + // Run the policy check. + service_->CheckAdminBlacklist(); + EXPECT_EQ(0u, service_->extensions()->size()); + EXPECT_EQ(0u, service_->disabled_extensions()->size()); +} + +// Tests that previously disabled extensions that are now required to be +// enabled are re-enabled on reinstall. +TEST_F(ExtensionServiceTest, ManagementPolicyRequiresEnable) { + InitializeEmptyExtensionService(); + + // Install, then disable, an extension. + InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW); + EXPECT_EQ(1u, service_->extensions()->size()); + service_->DisableExtension(good_crx, Extension::DISABLE_USER_ACTION); + EXPECT_EQ(1u, service_->disabled_extensions()->size()); + + // Register an ExtensionMnagementPolicy that requires the extension to remain + // enabled. + management_policy_->UnregisterAllProviders(); + extensions::TestManagementPolicyProvider provider( + extensions::TestManagementPolicyProvider::MUST_REMAIN_ENABLED); + management_policy_->RegisterProvider(&provider); + + // Reinstall the extension. + InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_UPDATED); + EXPECT_EQ(1u, service_->extensions()->size()); + EXPECT_EQ(0u, service_->disabled_extensions()->size()); +} + TEST_F(ExtensionServiceTest, ExternalExtensionAutoAcknowledgement) { InitializeEmptyExtensionService(); set_extensions_enabled(true); @@ -3085,7 +3228,9 @@ TEST_F(ExtensionServiceTest, ReloadExtensions) { TEST_F(ExtensionServiceTest, UninstallExtension) { InitializeEmptyExtensionService(); InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW); + EXPECT_EQ(1u, service_->extensions()->size()); UninstallExtension(good_crx, false); + EXPECT_EQ(0u, service_->extensions()->size()); } TEST_F(ExtensionServiceTest, UninstallTerminatedExtension) { @@ -3468,8 +3613,10 @@ void ExtensionServiceTest::TestExternalProvider( loop_.RunAllPending(); FilePath install_path = extensions_install_dir_.AppendASCII(id); - // It should not be possible to uninstall a policy controlled extension. - if (Extension::UserMayDisable(location)) { + if (Extension::IsRequired(location)) { + // Policy controlled extensions should not have been touched by uninstall. + ASSERT_TRUE(file_util::PathExists(install_path)); + } else { // The extension should also be gone from the install directory. ASSERT_FALSE(file_util::PathExists(install_path)); loaded_.clear(); @@ -3488,15 +3635,14 @@ void ExtensionServiceTest::TestExternalProvider( service_->CheckForExternalUpdates(); loop_.RunAllPending(); ASSERT_EQ(1u, loaded_.size()); - } else { - // Policy controlled extesions should not have been touched by uninstall. - ASSERT_TRUE(file_util::PathExists(install_path)); } ValidatePrefKeyCount(1); ValidateIntegerPref(good_crx, "state", Extension::ENABLED); ValidateIntegerPref(good_crx, "location", location); - if (Extension::UserMayDisable(location)) { + if (Extension::IsRequired(location)) { + EXPECT_EQ(2, provider->visit_count()); + } else { // Now test an externally triggered uninstall (deleting the registry key or // the pref entry). provider->RemoveExtension(good_crx); @@ -3536,8 +3682,6 @@ void ExtensionServiceTest::TestExternalProvider( ValidatePrefKeyCount(1); EXPECT_EQ(5, provider->visit_count()); - } else { - EXPECT_EQ(2, provider->visit_count()); } } diff --git a/chrome/browser/extensions/extension_service_unittest.h b/chrome/browser/extensions/extension_service_unittest.h index f4267d0..3d6d85d 100644 --- a/chrome/browser/extensions/extension_service_unittest.h +++ b/chrome/browser/extensions/extension_service_unittest.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -57,6 +57,7 @@ class ExtensionServiceTestBase : public testing::Test { FilePath data_dir_; // Managed by ExtensionSystemFactory. ExtensionService* service_; + extensions::ManagementPolicy* management_policy_; size_t expected_extensions_count_; content::TestBrowserThread ui_thread_; content::TestBrowserThread db_thread_; diff --git a/chrome/browser/extensions/extension_system.cc b/chrome/browser/extensions/extension_system.cc index 1cc0ccc..621a497 100644 --- a/chrome/browser/extensions/extension_system.cc +++ b/chrome/browser/extensions/extension_system.cc @@ -26,6 +26,7 @@ #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_system_factory.h" #include "chrome/browser/extensions/lazy_background_task_queue.h" +#include "chrome/browser/extensions/management_policy.h" #include "chrome/browser/extensions/unpacked_installer.h" #include "chrome/browser/extensions/user_script_master.h" #include "chrome/browser/prefs/pref_service.h" @@ -84,6 +85,11 @@ void ExtensionSystemImpl::Shared::InitPrefs() { extension_prefs_->Init(extensions_disabled); } +void ExtensionSystemImpl::Shared::RegisterManagementPolicyProviders() { + DCHECK(extension_prefs_.get()); + management_policy_->RegisterProvider(extension_prefs_.get()); +} + void ExtensionSystemImpl::Shared::InitInfoMap() { // The ExtensionInfoMap needs to be created before the // ExtensionProcessManager. @@ -120,6 +126,11 @@ void ExtensionSystemImpl::Shared::Init(bool extensions_enabled) { autoupdate_enabled, extensions_enabled)); + // The ManagementPolicy providers msut be registered before the + // ExtensionService tries to load any extensions. + management_policy_.reset(new extensions::ManagementPolicy); + RegisterManagementPolicyProviders(); + extension_service_->component_loader()->AddDefaultComponentExtensions(); if (command_line->HasSwitch(switches::kLoadComponentExtension)) { CommandLine::StringType path_list = command_line->GetSwitchValueNative( @@ -189,6 +200,10 @@ ExtensionService* ExtensionSystemImpl::Shared::extension_service() { return extension_service_.get(); } +extensions::ManagementPolicy* ExtensionSystemImpl::Shared::management_policy() { + return management_policy_.get(); +} + UserScriptMaster* ExtensionSystemImpl::Shared::user_script_master() { return user_script_master_.get(); } @@ -261,6 +276,10 @@ ExtensionService* ExtensionSystemImpl::extension_service() { return shared_->extension_service(); } +extensions::ManagementPolicy* ExtensionSystemImpl::management_policy() { + return shared_->management_policy(); +} + UserScriptMaster* ExtensionSystemImpl::user_script_master() { return shared_->user_script_master(); } diff --git a/chrome/browser/extensions/extension_system.h b/chrome/browser/extensions/extension_system.h index 2ae71cc..676bdb1 100644 --- a/chrome/browser/extensions/extension_system.h +++ b/chrome/browser/extensions/extension_system.h @@ -28,6 +28,7 @@ namespace extensions { class AlarmManager; class Extension; class LazyBackgroundTaskQueue; +class ManagementPolicy; class RulesRegistryService; } @@ -56,6 +57,11 @@ class ExtensionSystem : public ProfileKeyedService { // The ExtensionService is created at startup. virtual ExtensionService* extension_service() = 0; + // The class controlling whether users are permitted to perform certain + // actions on extensions (install, uninstall, disable, etc.). + // The ManagementPolicy is created at startup. + virtual extensions::ManagementPolicy* management_policy() = 0; + // The ExtensionDevToolsManager is created at startup. virtual ExtensionDevToolsManager* devtools_manager() = 0; @@ -115,6 +121,7 @@ class ExtensionSystemImpl : public ExtensionSystem { virtual void Init(bool extensions_enabled) OVERRIDE; virtual ExtensionService* extension_service() OVERRIDE; // shared + virtual extensions::ManagementPolicy* management_policy() OVERRIDE; // shared virtual UserScriptMaster* user_script_master() OVERRIDE; // shared virtual ExtensionDevToolsManager* devtools_manager() OVERRIDE; virtual ExtensionProcessManager* process_manager() OVERRIDE; @@ -147,10 +154,13 @@ class ExtensionSystemImpl : public ExtensionSystem { // Initialization takes place in phases. virtual void InitPrefs(); + // This must not be called until all the providers have been created. + void RegisterManagementPolicyProviders(); void InitInfoMap(); void Init(bool extensions_enabled); ExtensionService* extension_service(); + extensions::ManagementPolicy* management_policy(); UserScriptMaster* user_script_master(); ExtensionInfoMap* info_map(); extensions::LazyBackgroundTaskQueue* lazy_background_task_queue(); @@ -163,10 +173,11 @@ class ExtensionSystemImpl : public ExtensionSystem { // The services that are shared between normal and incognito profiles. - // Keep extension_prefs_ on top of extension_service_ because the latter - // maintains a pointer to the first and shall be destructed first. + // Keep extension_prefs_ above extension_service_, because the latter + // maintains a pointer to the former and must be destructed first. scoped_ptr<ExtensionPrefs> extension_prefs_; scoped_ptr<ExtensionService> extension_service_; + scoped_ptr<extensions::ManagementPolicy> management_policy_; scoped_refptr<UserScriptMaster> user_script_master_; // extension_info_map_ needs to outlive extension_process_manager_. scoped_refptr<ExtensionInfoMap> extension_info_map_; diff --git a/chrome/browser/extensions/installed_loader.cc b/chrome/browser/extensions/installed_loader.cc index 62384c2..3ec5d13 100644 --- a/chrome/browser/extensions/installed_loader.cc +++ b/chrome/browser/extensions/installed_loader.cc @@ -11,6 +11,7 @@ #include "base/values.h" #include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_system.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_file_util.h" @@ -68,13 +69,7 @@ InstalledLoader::~InstalledLoader() { void InstalledLoader::Load(const ExtensionInfo& info, bool write_to_prefs) { std::string error; scoped_refptr<const Extension> extension(NULL); - // An explicit check against policy is required to behave correctly during - // startup. This is because extensions that were previously OK might have - // been blacklisted in policy while Chrome was not running. - if (!extension_prefs_->IsExtensionAllowedByPolicy(info.extension_id, - info.extension_location)) { - error = errors::kDisabledByPolicy; - } else if (info.extension_manifest.get()) { + if (info.extension_manifest.get()) { extension = Extension::Create( info.extension_path, info.extension_location, @@ -96,6 +91,18 @@ void InstalledLoader::Load(const ExtensionInfo& info, bool write_to_prefs) { content::RecordAction(UserMetricsAction("Extensions.IDChangedError")); } + // Check policy on every load in case an extension was blacklisted while + // Chrome was not running. + const ManagementPolicy* policy = ExtensionSystem::Get( + extension_service_->profile())->management_policy(); + if (extension && + !policy->UserMayLoad(extension, NULL)) { + // The error message from UserMayInstall() often contains the extension ID + // and is therefore not well suited to this UI. + error = errors::kDisabledByPolicy; + extension = NULL; + } + if (!extension) { extension_service_-> ReportExtensionLoadError(info.extension_path, error, false); diff --git a/chrome/browser/extensions/management_policy.cc b/chrome/browser/extensions/management_policy.cc new file mode 100644 index 0000000..2524116 --- /dev/null +++ b/chrome/browser/extensions/management_policy.cc @@ -0,0 +1,90 @@ +// 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/management_policy.h" + +namespace extensions { +ManagementPolicy::ManagementPolicy() { +} + +ManagementPolicy::~ManagementPolicy() { +} + +bool ManagementPolicy::Provider::UserMayLoad(const Extension* extension, + string16* error) const { + return true; +} + +bool ManagementPolicy::Provider::UserMayModifySettings( + const Extension* extension, string16* error) const { + return true; +} + +bool ManagementPolicy::Provider::MustRemainEnabled(const Extension* extension, + string16* error) const { + return false; +} + +void ManagementPolicy::RegisterProvider(Provider* provider) { + providers_.insert(provider); +} + +void ManagementPolicy::UnregisterProvider(Provider* provider) { + providers_.erase(provider); +} + +bool ManagementPolicy::UserMayLoad(const Extension* extension, + string16* error) const { + for (ProviderList::const_iterator it = providers_.begin(); + it != providers_.end(); ++it) { + if (!(*it)->UserMayLoad(extension, error)) { + // The extension may be NULL in testing. + std::string id = extension ? extension->id() : "[test]"; + DLOG(WARNING) << "Installation of extension " << id + << " prohibited by " << (*it)->GetPolicyProviderName(); + return false; + } + } + return true; +} + +bool ManagementPolicy::UserMayModifySettings(const Extension* extension, + string16* error) const { + for (ProviderList::const_iterator it = providers_.begin(); + it != providers_.end(); ++it) { + if (!(*it)->UserMayModifySettings(extension, error)) { + // The extension may be NULL in testing. + std::string id = extension ? extension->id() : "[test]"; + DLOG(WARNING) << "Modification of extension " << id + << " prohibited by " << (*it)->GetPolicyProviderName(); + return false; + } + } + return true; +} + +bool ManagementPolicy::MustRemainEnabled(const Extension* extension, + string16* error) const { + for (ProviderList::const_iterator it = providers_.begin(); + it != providers_.end(); ++it) { + // The extension may be NULL in testing. + std::string id = extension ? extension->id() : "[test]"; + if ((*it)->MustRemainEnabled(extension, error)) { + DLOG(WARNING) << "Extension " << id + << " required to remain enabled by " + << (*it)->GetPolicyProviderName(); + return true; + } + } + return false; +} + +void ManagementPolicy::UnregisterAllProviders() { + providers_.clear(); +} + +int ManagementPolicy::GetNumProviders() const { + return providers_.size(); +} +} // namespace diff --git a/chrome/browser/extensions/management_policy.h b/chrome/browser/extensions/management_policy.h new file mode 100644 index 0000000..f2d16f5 --- /dev/null +++ b/chrome/browser/extensions/management_policy.h @@ -0,0 +1,112 @@ +// 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. + +#ifndef CHROME_BROWSER_EXTENSIONS_MANAGEMENT_POLICY_H_ +#define CHROME_BROWSER_EXTENSIONS_MANAGEMENT_POLICY_H_ +#pragma once + +#include <set> +#include <string> + +#include "base/basictypes.h" +#include "chrome/common/extensions/extension.h" + +namespace extensions { + +// This class registers providers that want to prohibit certain actions from +// being applied to extensions. It must be called, via the ExtensionService, +// before allowing a user or a user-level mechanism to perform the respective +// action. (That is, installing or otherwise modifying an extension in order +// to conform to enterprise administrator policy must be exempted from these +// checks.) +// +// This "policy" and its providers should not be confused with administrator +// policy, although admin policy is one of the sources ("Providers") of +// restrictions registered with and exposed by the ManagementPolicy. +class ManagementPolicy { + public: + // Each mechanism that wishes to limit users' ability to control extensions, + // whether one individual extension or the whole system, should implement + // the methods of this Provider interface that it needs. In each case, if the + // provider does not need to control a certain action, that method does not + // need to be implemented. + // + // It is not guaranteed that a particular Provider's methods will be called + // each time a user tries to perform one of the controlled actions (the list + // of providers is short-circuited as soon as a decision is possible), so + // implementations of these methods must have no side effects. + // + // For all of the Provider methods below, if |error| is not NULL and the + // method imposes a restriction on the desired action, |error| may be set + // to an applicable error message, but this is not required. + class Provider { + public: + Provider() {} + virtual ~Provider() {} + + // A human-readable name for this provider, for use in debug messages. + virtual std::string GetPolicyProviderName() const = 0; + + // Providers should return false if a user may not install the |extension|, + // or load or run it if it has already been installed. + virtual bool UserMayLoad(const Extension* extension, + string16* error) const; + + // Providers should return false if a user may not enable, disable, or + // uninstall the |extension|, or change its usage options (incognito + // permission, file access, etc.). + virtual bool UserMayModifySettings(const Extension* extension, + string16* error) const; + + // Providers should return true if the |extension| must always remain + // enabled. This is distinct from UserMayModifySettings() in that the latter + // also prohibits enabling the extension if it is currently disabled. + // Providers implementing this method should also implement the others + // above, if they wish to completely lock in an extension. + virtual bool MustRemainEnabled(const Extension* extension, + string16* error) const; + + private: + DISALLOW_COPY_AND_ASSIGN(Provider); + }; + + ManagementPolicy(); + ~ManagementPolicy(); + + // Registers or unregisters a provider, causing it to be added to or removed + // from the list of providers queried. Ownership of the provider remains with + // the caller. Providers do not need to be unregistered on shutdown. + void RegisterProvider(Provider* provider); + void UnregisterProvider(Provider* provider); + + // Returns true if the user is permitted to install, load, and run the given + // extension. If not, |error| may be set to an appropriate message. + bool UserMayLoad(const Extension* extension, string16* error) const; + + // Returns true if the user is permitted to enable, disable, or uninstall the + // given extension, or change the extension's usage options (incognito mode, + // file access, etc.). If not, |error| may be set to an appropriate message. + bool UserMayModifySettings(const Extension* extension, + string16* error) const; + + // Returns true if the extension must remain enabled at all times (e.g. a + // compoment extension). In that case, |error| may be set to an appropriate + // message. + bool MustRemainEnabled(const Extension* extension, + string16* error) const; + + // For use in testing. + void UnregisterAllProviders(); + int GetNumProviders() const; + + private: + typedef std::set<Provider*> ProviderList; + ProviderList providers_; + + DISALLOW_COPY_AND_ASSIGN(ManagementPolicy); +}; + +} // namespace + +#endif // CHROME_BROWSER_EXTENSIONS_MANAGEMENT_POLICY_H_ diff --git a/chrome/browser/extensions/management_policy_unittest.cc b/chrome/browser/extensions/management_policy_unittest.cc new file mode 100644 index 0000000..e80b4e4 --- /dev/null +++ b/chrome/browser/extensions/management_policy_unittest.cc @@ -0,0 +1,177 @@ +// 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 "base/utf_string_conversions.h" +#include "chrome/browser/extensions/management_policy.h" +#include "chrome/browser/extensions/test_management_policy.h" +#include "testing/gtest/include/gtest/gtest.h" + +typedef extensions::TestManagementPolicyProvider TestProvider; +using extensions::Extension; + +class ManagementPolicyTest : public testing::Test { + public: + void SetUp() { + allow_all_.SetProhibitedActions(TestProvider::ALLOW_ALL); + no_modify_status_.SetProhibitedActions( + TestProvider::PROHIBIT_MODIFY_STATUS); + no_load_.SetProhibitedActions(TestProvider::PROHIBIT_LOAD); + must_remain_enabled_.SetProhibitedActions( + TestProvider::MUST_REMAIN_ENABLED); + restrict_all_.SetProhibitedActions(TestProvider::PROHIBIT_MODIFY_STATUS | + TestProvider::PROHIBIT_LOAD | + TestProvider::MUST_REMAIN_ENABLED); + } + + protected: + extensions::ManagementPolicy policy_; + + TestProvider allow_all_; + TestProvider no_modify_status_; + TestProvider no_load_; + TestProvider must_remain_enabled_; + TestProvider restrict_all_; +}; + +TEST_F(ManagementPolicyTest, RegisterAndUnregister) { + EXPECT_EQ(0, policy_.GetNumProviders()); + policy_.RegisterProvider(&allow_all_); + EXPECT_EQ(1, policy_.GetNumProviders()); + policy_.RegisterProvider(&allow_all_); + EXPECT_EQ(1, policy_.GetNumProviders()); + + policy_.RegisterProvider(&no_modify_status_); + EXPECT_EQ(2, policy_.GetNumProviders()); + policy_.UnregisterProvider(&allow_all_); + EXPECT_EQ(1, policy_.GetNumProviders()); + policy_.UnregisterProvider(&allow_all_); + EXPECT_EQ(1, policy_.GetNumProviders()); + policy_.UnregisterProvider(&no_modify_status_); + EXPECT_EQ(0, policy_.GetNumProviders()); + + policy_.RegisterProvider(&allow_all_); + policy_.RegisterProvider(&no_modify_status_); + EXPECT_EQ(2, policy_.GetNumProviders()); + policy_.UnregisterAllProviders(); + EXPECT_EQ(0, policy_.GetNumProviders()); +} + +TEST_F(ManagementPolicyTest, UserMayLoad) { + // No providers registered. + string16 error; + // The extension and location are irrelevant to the + // TestManagementPolicyProviders. + EXPECT_TRUE(policy_.UserMayLoad(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // One provider, no relevant restriction. + policy_.RegisterProvider(&no_modify_status_); + EXPECT_TRUE(policy_.UserMayLoad(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // Two providers, no relevant restrictions. + policy_.RegisterProvider(&must_remain_enabled_); + EXPECT_TRUE(policy_.UserMayLoad(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // Three providers, one with a relevant restriction. + policy_.RegisterProvider(&no_load_); + EXPECT_FALSE(policy_.UserMayLoad(NULL, &error)); + EXPECT_FALSE(error.empty()); + + // Remove the restriction. + policy_.UnregisterProvider(&no_load_); + error.clear(); + EXPECT_TRUE(policy_.UserMayLoad(NULL, &error)); + EXPECT_TRUE(error.empty()); +} +TEST_F(ManagementPolicyTest, UserMayModifySettings) { + // No providers registered. + string16 error; + EXPECT_TRUE(policy_.UserMayModifySettings(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // One provider, no relevant restriction. + policy_.RegisterProvider(&allow_all_); + EXPECT_TRUE(policy_.UserMayModifySettings(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // Two providers, no relevant restrictions. + policy_.RegisterProvider(&no_load_); + EXPECT_TRUE(policy_.UserMayModifySettings(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // Three providers, one with a relevant restriction. + policy_.RegisterProvider(&no_modify_status_); + EXPECT_FALSE(policy_.UserMayModifySettings(NULL, &error)); + EXPECT_FALSE(error.empty()); + + // Remove the restriction. + policy_.UnregisterProvider(&no_modify_status_); + error.clear(); + EXPECT_TRUE(policy_.UserMayModifySettings(NULL, &error)); + EXPECT_TRUE(error.empty()); +} + +TEST_F(ManagementPolicyTest, MustRemainEnabled) { + // No providers registered. + string16 error; + EXPECT_FALSE(policy_.MustRemainEnabled(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // One provider, no relevant restriction. + policy_.RegisterProvider(&allow_all_); + EXPECT_FALSE(policy_.MustRemainEnabled(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // Two providers, no relevant restrictions. + policy_.RegisterProvider(&no_modify_status_); + EXPECT_FALSE(policy_.MustRemainEnabled(NULL, &error)); + EXPECT_TRUE(error.empty()); + + // Three providers, one with a relevant restriction. + policy_.RegisterProvider(&must_remain_enabled_); + EXPECT_TRUE(policy_.MustRemainEnabled(NULL, &error)); + EXPECT_FALSE(error.empty()); + + // Remove the restriction. + policy_.UnregisterProvider(&must_remain_enabled_); + error.clear(); + EXPECT_FALSE(policy_.MustRemainEnabled(NULL, &error)); + EXPECT_TRUE(error.empty()); +} + +// Tests error handling in the ManagementPolicy. +TEST_F(ManagementPolicyTest, ErrorHandling) { + // The error parameter should be unchanged if no restriction was found. + std::string original_error = "Ceci est en effet une erreur."; + string16 original_error16 = UTF8ToUTF16(original_error); + string16 error = original_error16; + EXPECT_TRUE(policy_.UserMayLoad(NULL, &error)); + EXPECT_EQ(original_error, UTF16ToUTF8(error)); + EXPECT_TRUE(policy_.UserMayModifySettings(NULL, &error)); + EXPECT_EQ(original_error, UTF16ToUTF8(error)); + EXPECT_FALSE(policy_.MustRemainEnabled(NULL, &error)); + EXPECT_EQ(original_error, UTF16ToUTF8(error)); + + // Ensure no crashes if no error message was requested. + EXPECT_TRUE(policy_.UserMayLoad(NULL, NULL)); + EXPECT_TRUE(policy_.UserMayModifySettings(NULL, NULL)); + EXPECT_FALSE(policy_.MustRemainEnabled(NULL, NULL)); + policy_.RegisterProvider(&restrict_all_); + EXPECT_FALSE(policy_.UserMayLoad(NULL, NULL)); + EXPECT_FALSE(policy_.UserMayModifySettings(NULL, NULL)); + EXPECT_TRUE(policy_.MustRemainEnabled(NULL, NULL)); + + // Make sure returned error is correct. + error = original_error16; + EXPECT_FALSE(policy_.UserMayLoad(NULL, &error)); + EXPECT_EQ(UTF8ToUTF16(TestProvider::expected_error()), error); + error = original_error16; + EXPECT_FALSE(policy_.UserMayModifySettings(NULL, &error)); + EXPECT_EQ(UTF8ToUTF16(TestProvider::expected_error()), error); + error = original_error16; + EXPECT_TRUE(policy_.MustRemainEnabled(NULL, &error)); + EXPECT_EQ(UTF8ToUTF16(TestProvider::expected_error()), error); +} diff --git a/chrome/browser/extensions/test_extension_system.cc b/chrome/browser/extensions/test_extension_system.cc index 1dcaa01..00d69c6 100644 --- a/chrome/browser/extensions/test_extension_system.cc +++ b/chrome/browser/extensions/test_extension_system.cc @@ -64,10 +64,22 @@ ExtensionService* TestExtensionSystem::CreateExtensionService( return extension_service_.get(); } +extensions::ManagementPolicy* TestExtensionSystem::CreateManagementPolicy() { + management_policy_.reset(new extensions::ManagementPolicy()); + DCHECK(extension_prefs_.get()); + management_policy_->RegisterProvider(extension_prefs_.get()); + + return management_policy(); +} + ExtensionService* TestExtensionSystem::extension_service() { return extension_service_.get(); } +extensions::ManagementPolicy* TestExtensionSystem::management_policy() { + return management_policy_.get(); +} + void TestExtensionSystem::SetExtensionService(ExtensionService* service) { extension_service_.reset(service); } diff --git a/chrome/browser/extensions/test_extension_system.h b/chrome/browser/extensions/test_extension_system.h index 0ccb778..0a29c0c 100644 --- a/chrome/browser/extensions/test_extension_system.h +++ b/chrome/browser/extensions/test_extension_system.h @@ -26,6 +26,10 @@ class TestExtensionSystem : public ExtensionSystem { const FilePath& install_directory, bool autoupdate_enabled); + // Creates and returns a ManagementPolicy with the ExtensionService and + // ExtensionPrefs registered. If not invoked, the ManagementPolicy is NULL. + extensions::ManagementPolicy* CreateManagementPolicy(); + // Creates an ExtensionProcessManager. If not invoked, the // ExtensionProcessManager is NULL. void CreateExtensionProcessManager(); @@ -35,6 +39,7 @@ class TestExtensionSystem : public ExtensionSystem { virtual void Init(bool extensions_enabled) OVERRIDE {} virtual ExtensionService* extension_service() OVERRIDE; + virtual extensions::ManagementPolicy* management_policy() OVERRIDE; void SetExtensionService(ExtensionService* service); virtual UserScriptMaster* user_script_master() OVERRIDE; virtual ExtensionDevToolsManager* devtools_manager() OVERRIDE; @@ -58,6 +63,7 @@ class TestExtensionSystem : public ExtensionSystem { // invoked. scoped_ptr<ExtensionPrefs> extension_prefs_; scoped_ptr<ExtensionService> extension_service_; + scoped_ptr<extensions::ManagementPolicy> management_policy_; scoped_ptr<ExtensionProcessManager> extension_process_manager_; scoped_ptr<extensions::AlarmManager> alarm_manager_; }; diff --git a/chrome/browser/extensions/test_management_policy.cc b/chrome/browser/extensions/test_management_policy.cc new file mode 100755 index 0000000..16d317e --- /dev/null +++ b/chrome/browser/extensions/test_management_policy.cc @@ -0,0 +1,54 @@ +// 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/test_management_policy.h" + +#include "base/utf_string_conversions.h" + +namespace extensions { +TestManagementPolicyProvider::TestManagementPolicyProvider() + : may_load_(true), + may_modify_status_(true), + must_remain_enabled_(false) { + error_message_ = UTF8ToUTF16(expected_error()); +} + +TestManagementPolicyProvider::TestManagementPolicyProvider( + int prohibited_actions) { + SetProhibitedActions(prohibited_actions); + error_message_ = UTF8ToUTF16(expected_error()); +} + +void TestManagementPolicyProvider::SetProhibitedActions( + int prohibited_actions) { + may_load_ = (prohibited_actions & PROHIBIT_LOAD) == 0; + may_modify_status_ = (prohibited_actions & PROHIBIT_MODIFY_STATUS) == 0; + must_remain_enabled_ = (prohibited_actions & MUST_REMAIN_ENABLED) != 0; +} + +std::string TestManagementPolicyProvider::GetPolicyProviderName() const { + return "the test management policy provider"; +} + +bool TestManagementPolicyProvider::UserMayLoad(const Extension* extension, + string16* error) const { + if (error && !may_load_) + *error = error_message_; + return may_load_; +} + +bool TestManagementPolicyProvider::UserMayModifySettings( + const Extension* extension, string16* error) const { + if (error && !may_modify_status_) + *error = error_message_; + return may_modify_status_; +} + +bool TestManagementPolicyProvider::MustRemainEnabled(const Extension* extension, + string16* error) const { + if (error && must_remain_enabled_) + *error = error_message_; + return must_remain_enabled_; +} +} // namespace diff --git a/chrome/browser/extensions/test_management_policy.h b/chrome/browser/extensions/test_management_policy.h new file mode 100644 index 0000000..bcb300d --- /dev/null +++ b/chrome/browser/extensions/test_management_policy.h @@ -0,0 +1,54 @@ +// 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. + +#ifndef CHROME_BROWSER_EXTENSIONS_TEST_MANAGEMENT_POLICY_H_ +#define CHROME_BROWSER_EXTENSIONS_TEST_MANAGEMENT_POLICY_H_ +#pragma once + +#include <string> + +#include "base/string16.h" +#include "chrome/browser/extensions/extension_service.h" + +namespace extensions { +// This class provides a simple way to create providers with specific +// restrictions and a known error message, for use in testing. +class TestManagementPolicyProvider : public ManagementPolicy::Provider { + public: + enum AllowedActionFlag { + ALLOW_ALL = 0, + PROHIBIT_LOAD = 1 << 0, + PROHIBIT_MODIFY_STATUS = 1 << 1, + MUST_REMAIN_ENABLED = 1 << 2 + }; + + static std::string expected_error() { + return "Action prohibited by test provider."; + } + + TestManagementPolicyProvider(); + explicit TestManagementPolicyProvider(int prohibited_actions); + + void SetProhibitedActions(int prohibited_actions); + + virtual std::string GetPolicyProviderName() const OVERRIDE; + + virtual bool UserMayLoad(const Extension* extension, + string16* error) const OVERRIDE; + + virtual bool UserMayModifySettings(const Extension* extension, + string16* error) const OVERRIDE; + + virtual bool MustRemainEnabled(const Extension* extension, + string16* error) const OVERRIDE; + + private: + bool may_load_; + bool may_modify_status_; + bool must_remain_enabled_; + + string16 error_message_; +}; +} // namespace +#endif // CHROME_BROWSER_EXTENSIONS_TEST_MANAGEMENT_POLICY_H_ |