Add management policy function allowing extensions to be disabled.

This CL adds the MustRemainDisabled function to the ManagementPolicy::Provider
interface and calls it in appropriate places.

BUG=314276

Review URL: https://codereview.chromium.org/54903011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@232976 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/developer_private/developer_private_api.cc

@@ -716,18 +716,24 @@ bool DeveloperPrivateEnableFunction::RunImpl() {
   std::string extension_id = params->item_id;
 
   ExtensionSystem* system = ExtensionSystem::Get(GetProfile());
-  ManagementPolicy* management_policy = system->management_policy();
+  ManagementPolicy* policy = system->management_policy();
   ExtensionService* service = GetProfile()->GetExtensionService();
 
   const Extension* extension = service->GetInstalledExtension(extension_id);
-  if (!extension ||
-      !management_policy->UserMayModifySettings(extension, NULL)) {
-    LOG(ERROR) << "Attempt to enable an extension that is non-usermanagable "
-               "was made. Extension id: " << extension_id.c_str();
+  if (!extension) {
+    LOG(ERROR) << "Did not find extension with id " << extension_id;
+    return false;
+  }
+  bool enable = params->enable;
+  if (!policy->UserMayModifySettings(extension, NULL) ||
+      (!enable && policy->MustRemainEnabled(extension, NULL)) ||
+      (enable && policy->MustRemainDisabled(extension, NULL, NULL))) {
+    LOG(ERROR) << "Attempt to change enable state denied by management policy. "
+               << "Extension id: " << extension_id.c_str();
     return false;
   }
 
-  if (params->enable) {
+  if (enable) {
     ExtensionPrefs* prefs = service->extension_prefs();
     if (prefs->DidExtensionEscalatePermissions(extension_id)) {
       ShellWindowRegistry* registry = ShellWindowRegistry::Get(GetProfile());

chrome/browser/extensions/api/management/management_api.cc

@@ -470,7 +470,9 @@ bool ManagementSetEnabledFunction::RunImpl() {
 
   const ManagementPolicy* policy =
       ExtensionSystem::Get(GetProfile())->management_policy();
-  if (!policy->UserMayModifySettings(extension, NULL)) {
+  if (!policy->UserMayModifySettings(extension, NULL) ||
+      (!params->enabled && policy->MustRemainEnabled(extension, NULL)) ||
+      (params->enabled && policy->MustRemainDisabled(extension, NULL, NULL))) {
     error_ = ErrorUtils::FormatErrorMessage(
         keys::kUserCantModifyError, extension_id_);
     return false;

chrome/browser/extensions/extension_service.cc

@@ -119,6 +119,7 @@ using extensions::Extension;
 using extensions::ExtensionIdSet;
 using extensions::ExtensionInfo;
 using extensions::FeatureSwitch;
+using extensions::ManagementPolicy;
 using extensions::Manifest;
 using extensions::PermissionMessage;
 using extensions::PermissionMessages;
@@ -889,11 +890,17 @@ void ExtensionService::EnableExtension(const std::string& extension_id) {
 
   if (IsExtensionEnabled(extension_id))
     return;
+  const Extension* extension = disabled_extensions_.GetByID(extension_id);
+
+  ManagementPolicy* policy = system_->management_policy();
+  if (extension && policy->MustRemainDisabled(extension, NULL, NULL)) {
+    UMA_HISTOGRAM_COUNTS_100("Extensions.EnableDeniedByPolicy", 1);
+    return;
+  }
 
   extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED);
   extension_prefs_->ClearDisableReasons(extension_id);
 
-  const Extension* extension = disabled_extensions_.GetByID(extension_id);
   // This can happen if sync enables an extension that is not
   // installed yet.
   if (!extension)
@@ -1203,20 +1210,28 @@ extensions::ExtensionUpdater* ExtensionService::updater() {
 }
 
 void ExtensionService::CheckManagementPolicy() {
-  std::vector<std::string> to_be_removed;
+  std::vector<std::string> to_unload;
+  std::map<std::string, Extension::DisableReason> to_disable;
 
-  // Loop through extensions list, unload installed extensions.
+  // Loop through the extensions list, finding extensions we need to unload or
+  // disable.
   for (ExtensionSet::const_iterator iter = extensions_.begin();
        iter != extensions_.end(); ++iter) {
     const Extension* extension = (iter->get());
     if (!system_->management_policy()->UserMayLoad(extension, NULL))
-      to_be_removed.push_back(extension->id());
+      to_unload.push_back(extension->id());
+    Extension::DisableReason disable_reason = Extension::DISABLE_NONE;
+    if (system_->management_policy()->MustRemainDisabled(
+            extension, &disable_reason, NULL))
+      to_disable[extension->id()] = disable_reason;
   }
 
-  // UnloadExtension will change the extensions_ list. So, we should
-  // call it outside the iterator loop.
-  for (size_t i = 0; i < to_be_removed.size(); ++i)
-    UnloadExtension(to_be_removed[i], UnloadedExtensionInfo::REASON_DISABLE);
+  for (size_t i = 0; i < to_unload.size(); ++i)
+    UnloadExtension(to_unload[i], UnloadedExtensionInfo::REASON_DISABLE);
+
+  for (std::map<std::string, Extension::DisableReason>::const_iterator i =
+           to_disable.begin(); i != to_disable.end(); ++i)
+    DisableExtension(i->first, i->second);
 }
 
 void ExtensionService::CheckForUpdatesSoon() {

chrome/browser/extensions/installed_loader.cc

@@ -166,11 +166,18 @@ void InstalledLoader::Load(const ExtensionInfo& info, bool write_to_prefs) {
   // Chrome was not running.
   const ManagementPolicy* policy = extensions::ExtensionSystem::Get(
       extension_service_->profile())->management_policy();
-  if (extension.get() && !policy->UserMayLoad(extension.get(), 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.get()) {
+    Extension::DisableReason disable_reason = Extension::DISABLE_NONE;
+    if (!policy->UserMayLoad(extension.get(), 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;
+    } else if (!extension_prefs_->IsExtensionDisabled(extension->id()) &&
+               policy->MustRemainDisabled(extension, &disable_reason, NULL)) {
+      extension_prefs_->SetExtensionState(extension->id(), Extension::DISABLED);
+      extension_prefs_->AddDisableReason(extension->id(), disable_reason);
+    }
   }
 
   if (!extension.get()) {

chrome/browser/extensions/management_policy.cc

@@ -39,6 +39,13 @@ bool ManagementPolicy::Provider::MustRemainEnabled(const Extension* extension,
   return false;
 }
 
+bool ManagementPolicy::Provider::MustRemainDisabled(
+    const Extension* extension,
+    Extension::DisableReason* reason,
+    string16* error) const {
+  return false;
+}
+
 void ManagementPolicy::RegisterProvider(Provider* provider) {
   providers_.insert(provider);
 }
@@ -65,6 +72,17 @@ bool ManagementPolicy::MustRemainEnabled(const Extension* extension,
                              false, extension, error);
 }
 
+bool ManagementPolicy::MustRemainDisabled(const Extension* extension,
+                                          Extension::DisableReason* reason,
+                                          string16* error) const {
+  for (ProviderList::const_iterator it = providers_.begin();
+       it != providers_.end(); ++it)
+    if ((*it)->MustRemainDisabled(extension, reason, error))
+      return true;
+
+  return false;
+}
+
 void ManagementPolicy::UnregisterAllProviders() {
   providers_.clear();
 }

chrome/browser/extensions/management_policy.h

@@ -68,6 +68,12 @@ class ManagementPolicy {
     virtual bool MustRemainEnabled(const Extension* extension,
                                    string16* error) const;
 
+    // Similar to MustRemainEnabled, but for whether an extension must remain
+    // disabled, and returns an error and/or reason if the caller needs it.
+    virtual bool MustRemainDisabled(const Extension* extension,
+                                    Extension::DisableReason* reason,
+                                    string16* error) const;
+
    private:
     DISALLOW_COPY_AND_ASSIGN(Provider);
   };
@@ -97,6 +103,12 @@ class ManagementPolicy {
   bool MustRemainEnabled(const Extension* extension,
                          string16* error) const;
 
+  // Returns true immediately if any registered provider's MustRemainDisabled
+  // function returns true.
+  bool MustRemainDisabled(const Extension* extension,
+                          Extension::DisableReason* reason,
+                          string16* error) const;
+
   // For use in testing.
   void UnregisterAllProviders();
   int GetNumProviders() const;

chrome/browser/extensions/management_policy_unittest.cc

@@ -19,6 +19,9 @@ class ManagementPolicyTest : public testing::Test {
     no_load_.SetProhibitedActions(TestProvider::PROHIBIT_LOAD);
     must_remain_enabled_.SetProhibitedActions(
         TestProvider::MUST_REMAIN_ENABLED);
+    must_remain_disabled_.SetProhibitedActions(
+        TestProvider::MUST_REMAIN_DISABLED);
+    must_remain_disabled_.SetDisableReason(Extension::DISABLE_SIDELOAD_WIPEOUT);
     restrict_all_.SetProhibitedActions(TestProvider::PROHIBIT_MODIFY_STATUS |
                                        TestProvider::PROHIBIT_LOAD |
                                        TestProvider::MUST_REMAIN_ENABLED);
@@ -31,6 +34,7 @@ class ManagementPolicyTest : public testing::Test {
   TestProvider no_modify_status_;
   TestProvider no_load_;
   TestProvider must_remain_enabled_;
+  TestProvider must_remain_disabled_;
   TestProvider restrict_all_;
 };
 
@@ -142,6 +146,36 @@ TEST_F(ManagementPolicyTest, MustRemainEnabled) {
   EXPECT_TRUE(error.empty());
 }
 
+TEST_F(ManagementPolicyTest, MustRemainDisabled) {
+  // No providers registered.
+  string16 error;
+  EXPECT_FALSE(policy_.MustRemainDisabled(NULL, NULL, &error));
+  EXPECT_TRUE(error.empty());
+
+  // One provider, no relevant restriction.
+  policy_.RegisterProvider(&allow_all_);
+  EXPECT_FALSE(policy_.MustRemainDisabled(NULL, NULL, &error));
+  EXPECT_TRUE(error.empty());
+
+  // Two providers, no relevant restrictions.
+  policy_.RegisterProvider(&no_modify_status_);
+  EXPECT_FALSE(policy_.MustRemainDisabled(NULL, NULL, &error));
+  EXPECT_TRUE(error.empty());
+
+  // Three providers, one with a relevant restriction.
+  Extension::DisableReason reason = Extension::DISABLE_NONE;
+  policy_.RegisterProvider(&must_remain_disabled_);
+  EXPECT_TRUE(policy_.MustRemainDisabled(NULL, &reason, &error));
+  EXPECT_FALSE(error.empty());
+  EXPECT_EQ(Extension::DISABLE_SIDELOAD_WIPEOUT, reason);
+
+  // Remove the restriction.
+  policy_.UnregisterProvider(&must_remain_disabled_);
+  error.clear();
+  EXPECT_FALSE(policy_.MustRemainDisabled(NULL, 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.

chrome/browser/extensions/test_management_policy.cc

@@ -7,10 +7,13 @@
 #include "base/strings/utf_string_conversions.h"
 
 namespace extensions {
+
 TestManagementPolicyProvider::TestManagementPolicyProvider()
     : may_load_(true),
       may_modify_status_(true),
-      must_remain_enabled_(false) {
+      must_remain_enabled_(false),
+      must_remain_disabled_(false),
+      disable_reason_(Extension::DISABLE_NONE) {
   error_message_ = UTF8ToUTF16(expected_error());
 }
 
@@ -25,6 +28,12 @@ void TestManagementPolicyProvider::SetProhibitedActions(
   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;
+  must_remain_disabled_ = (prohibited_actions & MUST_REMAIN_DISABLED) != 0;
+}
+
+void TestManagementPolicyProvider::SetDisableReason(
+    Extension::DisableReason reason) {
+  disable_reason_ = reason;
 }
 
 std::string TestManagementPolicyProvider::GetDebugPolicyProviderName() const {
@@ -51,4 +60,19 @@ bool TestManagementPolicyProvider::MustRemainEnabled(const Extension* extension,
     *error = error_message_;
   return must_remain_enabled_;
 }
-}  // namespace
+
+bool TestManagementPolicyProvider::MustRemainDisabled(
+    const Extension* extension,
+    Extension::DisableReason* reason,
+    string16* error) const {
+  if (must_remain_disabled_) {
+    if (error)
+      *error = error_message_;
+    if (reason)
+      *reason = disable_reason_;
+  }
+  return must_remain_disabled_;
+}
+
+
+}  // namespace extensions

chrome/browser/extensions/test_management_policy.h

@@ -12,6 +12,7 @@
 #include "chrome/browser/extensions/management_policy.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 {
@@ -20,7 +21,8 @@ class TestManagementPolicyProvider : public ManagementPolicy::Provider {
     ALLOW_ALL = 0,
     PROHIBIT_LOAD = 1 << 0,
     PROHIBIT_MODIFY_STATUS = 1 << 1,
-    MUST_REMAIN_ENABLED = 1 << 2
+    MUST_REMAIN_ENABLED = 1 << 2,
+    MUST_REMAIN_DISABLED = 1 << 3
   };
 
   static std::string expected_error() {
@@ -31,6 +33,7 @@ class TestManagementPolicyProvider : public ManagementPolicy::Provider {
   explicit TestManagementPolicyProvider(int prohibited_actions);
 
   void SetProhibitedActions(int prohibited_actions);
+  void SetDisableReason(Extension::DisableReason reason);
 
   virtual std::string GetDebugPolicyProviderName() const OVERRIDE;
 
@@ -43,12 +46,20 @@ class TestManagementPolicyProvider : public ManagementPolicy::Provider {
   virtual bool MustRemainEnabled(const Extension* extension,
                                  string16* error) const OVERRIDE;
 
+  virtual bool MustRemainDisabled(const Extension* extension,
+                                  Extension::DisableReason* reason,
+                                  string16* error) const OVERRIDE;
+
  private:
   bool may_load_;
   bool may_modify_status_;
   bool must_remain_enabled_;
+  bool must_remain_disabled_;
+  Extension::DisableReason disable_reason_;
 
   string16 error_message_;
 };
-}  // namespace
+
+}  // namespace extensions
+
 #endif  // CHROME_BROWSER_EXTENSIONS_TEST_MANAGEMENT_POLICY_H_