Supervised Users Initiated Installs v2

chrome/browser/supervised_user/supervised_user_service.cc

Index: chrome/browser/supervised_user/supervised_user_service.cc
diff --git a/chrome/browser/supervised_user/supervised_user_service.cc b/chrome/browser/supervised_user/supervised_user_service.cc
index ba82b0f8ba7787fe94e8f2cd20c3d038d4c11bf3..6f3c5b2bc228552c0bb7803fe0a7b25cd5c95abf 100644
--- a/chrome/browser/supervised_user/supervised_user_service.cc
+++ b/chrome/browser/supervised_user/supervised_user_service.cc
@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"
@@ -27,6 +28,7 @@
 #include "chrome/browser/supervised_user/experimental/supervised_user_filtering_switches.h"
 #include "chrome/browser/supervised_user/permission_request_creator.h"
 #include "chrome/browser/supervised_user/supervised_user_constants.h"
+#include "chrome/browser/supervised_user/supervised_user_features.h"
 #include "chrome/browser/supervised_user/supervised_user_service_observer.h"
 #include "chrome/browser/supervised_user/supervised_user_settings_service.h"
 #include "chrome/browser/supervised_user/supervised_user_settings_service_factory.h"
@@ -68,6 +70,8 @@
 
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
 #endif
 
@@ -106,6 +110,13 @@ void CreateURLAccessRequest(
   creator->CreateURLAccessRequest(url, callback);
 }
 
+void CreateExtensionInstallRequest(
+    const std::string& id,
+    PermissionRequestCreator* creator,
+    const SupervisedUserService::SuccessCallback& callback) {
+  creator->CreateExtensionInstallRequest(id, callback);
+}
+
 void CreateExtensionUpdateRequest(
     const std::string& id,
     PermissionRequestCreator* creator,
@@ -113,6 +124,11 @@ void CreateExtensionUpdateRequest(
   creator->CreateExtensionUpdateRequest(id, callback);
 }
 
+// Default callback for AddExtensionInstallRequest.
+void ExtensionInstallRequestSent(const std::string& id, bool success) {
+  VLOG_IF(1, !success) << "Failed sending install request for " << id;
+}
+
 // Default callback for AddExtensionUpdateRequest.
 void ExtensionUpdateRequestSent(const std::string& id, bool success) {
   VLOG_IF(1, !success) << "Failed sending update request for " << id;
@@ -123,46 +139,6 @@ base::FilePath GetBlacklistPath() {
   PathService::Get(chrome::DIR_USER_DATA, &blacklist_dir);
   return blacklist_dir.AppendASCII(kBlacklistFilename);
 }
-
-#if defined(ENABLE_EXTENSIONS)
-enum ExtensionState {
-  EXTENSION_FORCED,
-  EXTENSION_BLOCKED,
-  EXTENSION_ALLOWED
-};
-
-ExtensionState GetExtensionState(const extensions::Extension* extension) {
-  bool was_installed_by_default = extension->was_installed_by_default();
-#if defined(OS_CHROMEOS)
-  // On Chrome OS all external sources are controlled by us so it means that
-  // they are "default". Method was_installed_by_default returns false because
-  // extensions creation flags are ignored in case of default extensions with
-  // update URL(the flags aren't passed to OnExternalExtensionUpdateUrlFound).
-  // TODO(dpolukhin): remove this Chrome OS specific code as soon as creation
-  // flags are not ignored.
-  was_installed_by_default =
-      extensions::Manifest::IsExternalLocation(extension->location());
-#endif
-  // Note: Component extensions are protected from modification/uninstallation
-  // anyway, so there's no need to enforce them again for supervised users.
-  // Also, leave policy-installed extensions alone - they have their own
-  // management; in particular we don't want to override the force-install list.
-  if (extensions::Manifest::IsComponentLocation(extension->location()) ||
-      extensions::Manifest::IsPolicyLocation(extension->location()) ||
-      extension->is_theme() ||
-      extension->from_bookmark() ||
-      extension->is_shared_module() ||
-      was_installed_by_default) {
-    return EXTENSION_ALLOWED;
-  }
-
-  if (extension->was_installed_by_custodian())
-    return EXTENSION_FORCED;
-
-  return EXTENSION_BLOCKED;
-}
-#endif
-
 }  // namespace
 
 SupervisedUserService::~SupervisedUserService() {
@@ -173,6 +149,7 @@ SupervisedUserService::~SupervisedUserService() {
 // static
 void SupervisedUserService::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterDictionaryPref(prefs::kSupervisedUserApprovedExtensions);
   registry->RegisterDictionaryPref(prefs::kSupervisedUserManualHosts);
   registry->RegisterDictionaryPref(prefs::kSupervisedUserManualURLs);
   registry->RegisterIntegerPref(prefs::kDefaultSupervisedUserFilteringBehavior,
@@ -214,6 +191,7 @@ void SupervisedUserService::Init() {
       base::Bind(&SupervisedUserService::OnSiteListsChanged,
                  weak_ptr_factory_.GetWeakPtr()));
 
+  approved_extensions_map_.reset(new std::map<std::string, std::string>());
   SetActive(ProfileIsSupervised());
 }
 
@@ -263,11 +241,28 @@ void SupervisedUserService::ReportURL(const GURL& url,
     callback.Run(false);
 }
 
+void SupervisedUserService::AddExtensionInstallRequest(
+    const std::string& extension_id,
+    const base::Version& version,
+    const SuccessCallback& callback) {
+  std::string id = GetExtensionRequestId(extension_id, version);
+  AddPermissionRequestInternal(base::Bind(CreateExtensionInstallRequest, id),
+                               callback, 0);
+}
+
+void SupervisedUserService::AddExtensionInstallRequest(
+    const std::string& extension_id,
+    const base::Version& version) {
+  std::string id = GetExtensionRequestId(extension_id, version);
+  AddPermissionRequestInternal(base::Bind(CreateExtensionInstallRequest, id),
+                               base::Bind(ExtensionInstallRequestSent, id), 0);
+}
+
 void SupervisedUserService::AddExtensionUpdateRequest(
     const std::string& extension_id,
     const base::Version& version,
     const SuccessCallback& callback) {
-  std::string id = GetExtensionUpdateRequestId(extension_id, version);
+  std::string id = GetExtensionRequestId(extension_id, version);
   AddPermissionRequestInternal(
       base::Bind(CreateExtensionUpdateRequest, id), callback, 0);
 }
@@ -275,13 +270,13 @@ void SupervisedUserService::AddExtensionUpdateRequest(
 void SupervisedUserService::AddExtensionUpdateRequest(
     const std::string& extension_id,
     const base::Version& version) {
-  std::string id = GetExtensionUpdateRequestId(extension_id, version);
+  std::string id = GetExtensionRequestId(extension_id, version);
   AddExtensionUpdateRequest(extension_id, version,
                             base::Bind(ExtensionUpdateRequestSent, id));
 }
 
 // static
-std::string SupervisedUserService::GetExtensionUpdateRequestId(
+std::string SupervisedUserService::GetExtensionRequestId(
     const std::string& extension_id,
     const base::Version& version) {
   return base::StringPrintf("%s:%s", extension_id.c_str(),
@@ -583,6 +578,10 @@ void SupervisedUserService::SetActive(bool active) {
         prefs::kDefaultSupervisedUserFilteringBehavior,
         base::Bind(&SupervisedUserService::OnDefaultFilteringBehaviorChanged,
             base::Unretained(this)));
+    pref_change_registrar_.Add(
+        prefs::kSupervisedUserApprovedExtensions,
+        base::Bind(&SupervisedUserService::UpdateApprovedExtensions,
+                   base::Unretained(this)));
     pref_change_registrar_.Add(prefs::kSupervisedUserSafeSites,
         base::Bind(&SupervisedUserService::OnSafeSitesSettingChanged,
                    base::Unretained(this)));
@@ -604,6 +603,7 @@ void SupervisedUserService::SetActive(bool active) {
     whitelist_service_->Init();
     UpdateManualHosts();
     UpdateManualURLs();
+    UpdateApprovedExtensions();
 
 #if !defined(OS_ANDROID)
     // TODO(bauerb): Get rid of the platform-specific #ifdef here.
@@ -616,6 +616,7 @@ void SupervisedUserService::SetActive(bool active) {
 
     pref_change_registrar_.Remove(
         prefs::kDefaultSupervisedUserFilteringBehavior);
+    pref_change_registrar_.Remove(prefs::kSupervisedUserApprovedExtensions);
     pref_change_registrar_.Remove(prefs::kSupervisedUserManualHosts);
     pref_change_registrar_.Remove(prefs::kSupervisedUserManualURLs);
     for (const char* pref : kCustodianInfoPrefs) {
@@ -911,6 +912,28 @@ void SupervisedUserService::UpdateManualURLs() {
       SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }
 
+void SupervisedUserService::UpdateApprovedExtensions() {
+  const base::DictionaryValue* dict = profile_->GetPrefs()->GetDictionary(
+      prefs::kSupervisedUserApprovedExtensions);
+  std::unique_ptr<std::map<std::string, std::string>> approved_extensions_map(
+      new std::map<std::string, std::string>());
+  for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
+    std::string version = "";

[Marc Treib, 2016/05/23 15:32:42]

No need to initialize with empty string, it'll do that anyway

[mamir, 2016/05/23 19:35:14]

Done.

+    bool result = it.value().GetAsString(&version);
+    DCHECK(result);
+    (*approved_extensions_map)[it.key()] = version;

[Marc Treib, 2016/05/23 15:32:42]

Convert version to an actual base::Version, to make sure it's properly
formatted?

[mamir, 2016/05/23 19:35:14]

Done.

+  }
+  approved_extensions_map_ = std::move(approved_extensions_map);
+
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+  for (const auto& extensions_entry : *approved_extensions_map_) {
+    // Try to Enable the extension, this will call the ManagmentPolicy and

[Marc Treib, 2016/05/23 15:32:42]

nit: don't capitalize "Enable"

[mamir, 2016/05/23 19:35:14]

Done.

+    // properly enable the extension if possible.
+    service->EnableExtension(extensions_entry.first);

[Marc Treib, 2016/05/23 15:32:42]

This will also re-enable extensions that have other disable reasons. Check the
disable reasons, and only re-enable if yours was the only one. Otherwise just
remove that reason.

[mamir, 2016/05/23 19:35:14]

Done.

+  }
+}
+
 std::string SupervisedUserService::GetSupervisedUserName() const {
 #if defined(OS_CHROMEOS)
   // The active user can be NULL in unit tests.
@@ -955,6 +978,56 @@ void SupervisedUserService::Shutdown() {
 }
 
 #if defined(ENABLE_EXTENSIONS)
+SupervisedUserService::ExtensionState SupervisedUserService::GetExtensionState(
+    const extensions::Extension& extension) const {
+  bool was_installed_by_default = extension.was_installed_by_default();
+#if defined(OS_CHROMEOS)
+  // On Chrome OS all external sources are controlled by us so it means that
+  // they are "default". Method was_installed_by_default returns false because
+  // extensions creation flags are ignored in case of default extensions with
+  // update URL(the flags aren't passed to OnExternalExtensionUpdateUrlFound).
+  // TODO(dpolukhin): remove this Chrome OS specific code as soon as creation
+  // flags are not ignored.
+  was_installed_by_default =
+      extensions::Manifest::IsExternalLocation(extension->location());
+#endif
+  // Note: Component extensions are protected from modification/uninstallation
+  // anyway, so there's no need to enforce them again for supervised users.
+  // Also, leave policy-installed extensions alone - they have their own
+  // management; in particular we don't want to override the force-install list.
+  if (extensions::Manifest::IsComponentLocation(extension.location()) ||
+      extensions::Manifest::IsPolicyLocation(extension.location()) ||
+      extension.is_theme() || extension.from_bookmark() ||
+      extension.is_shared_module() || was_installed_by_default) {
+    return ExtensionState::ALLOWED;
+  }
+
+  if (extension.was_installed_by_custodian())
+    return ExtensionState::FORCED;
+
+  // TODO(mamir): if(on blacklist) return EXTENSION_BLOCKED;
+  if (!base::FeatureList::IsEnabled(
+          supervised_users::kSupervisedUserInitiatedExtensionInstall))
+    return ExtensionState::BLOCKED;
+
+  const std::string& id = extension.id();
+  extensions::ExtensionPrefs* extension_prefs =
+      extensions::ExtensionPrefs::Get(profile_);
+
+  // If it wasn't installed before, and it wasn't installed by custodian
+  // which is taken care of by a previous check, then it requires approval.
+  if (!extension_prefs->GetInstalledExtensionInfo(id))
+    return ExtensionState::REQUIRE_APPROVAL;
+
+  if (extension_prefs->HasDisableReason(
+          id, extensions::Extension::DISABLE_CUSTODIAN_APPROVAL_REQUIRED) ||
+      extension_prefs->HasDisableReason(
+          id, extensions::Extension::DISABLE_PERMISSIONS_INCREASE)) {
+    return ExtensionState::REQUIRE_APPROVAL;
+  }
+  return ExtensionState::ALLOWED;
+}
+
 std::string SupervisedUserService::GetDebugPolicyProviderName() const {
   // Save the string space in official builds.
 #ifdef NDEBUG
@@ -968,8 +1041,8 @@ std::string SupervisedUserService::GetDebugPolicyProviderName() const {
 bool SupervisedUserService::UserMayLoad(const extensions::Extension* extension,
                                         base::string16* error) const {
   DCHECK(ProfileIsSupervised());
-  ExtensionState result = GetExtensionState(extension);
-  bool may_load = (result != EXTENSION_BLOCKED);
+  ExtensionState result = GetExtensionState(*extension);
+  bool may_load = (result != ExtensionState::BLOCKED);
   if (!may_load && error)
     *error = GetExtensionsLockedMessage();
   return may_load;
@@ -979,8 +1052,13 @@ bool SupervisedUserService::UserMayModifySettings(
     const extensions::Extension* extension,
     base::string16* error) const {
   DCHECK(ProfileIsSupervised());
-  ExtensionState result = GetExtensionState(extension);
-  bool may_modify = (result == EXTENSION_ALLOWED);
+  ExtensionState result = GetExtensionState(*extension);
+  // While the following check allows the SU to modify the settings and enable
+  // or disable the extension, MustRemainDisabled properly takes care of
+  // keeping an extension disabled when required.
+  // For custodian-installed extensions, the state is always FORCED, even if
+  // it's waiting for an update approval.
+  bool may_modify = (result != ExtensionState::FORCED);
   if (!may_modify && error)
     *error = GetExtensionsLockedMessage();
   return may_modify;
@@ -993,13 +1071,52 @@ bool SupervisedUserService::MustRemainInstalled(
     const extensions::Extension* extension,
     base::string16* error) const {
   DCHECK(ProfileIsSupervised());
-  ExtensionState result = GetExtensionState(extension);
-  bool may_not_uninstall = (result == EXTENSION_FORCED);
+  ExtensionState result = GetExtensionState(*extension);
+  bool may_not_uninstall = (result == ExtensionState::FORCED);
   if (may_not_uninstall && error)
     *error = GetExtensionsLockedMessage();
   return may_not_uninstall;
 }
 
+bool SupervisedUserService::MustRemainDisabled(
+    const extensions::Extension* extension,
+    extensions::Extension::DisableReason* reason,
+    base::string16* error) const {
+  DCHECK(ProfileIsSupervised());
+  extensions::ExtensionPrefs* extension_prefs =
+      extensions::ExtensionPrefs::Get(profile_);
+
+  auto extention_it = approved_extensions_map_->find(extension->id());

[Marc Treib, 2016/05/23 15:32:42]

"extension"
But IMO something like
bool approved = map_.count(id) > 0;
is clearer anyway.

[mamir, 2016/05/23 19:35:14]

Done.

+  bool is_approved_by_custodian =
+      (extention_it != approved_extensions_map_->end());

[Marc Treib, 2016/05/23 15:32:42]

Can we use GetExtensionState here, like all the other ManagementPolicy methods
do? (The approved_extensions_map_ check will have to move in there, which is
probably better anyway)

[mamir, 2016/05/23 19:35:14]

Done.

+
+  bool may_enable =
+      extension->was_installed_by_custodian() || is_approved_by_custodian;
+
+  if (!may_enable) {
+    if (reason)
+      *reason = extensions::Extension::DISABLE_CUSTODIAN_APPROVAL_REQUIRED;
+    if (error)
+      *error = l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOCKED_SUPERVISED_USER);
+    if (base::FeatureList::IsEnabled(
+            supervised_users::kSupervisedUserInitiatedExtensionInstall)) {
+      // If the Extension isn't pending a custodian approval already, send
+      // an approval request.
+      if (!extension_prefs->HasDisableReason(
+              extension->id(),
+              extensions::Extension::DISABLE_CUSTODIAN_APPROVAL_REQUIRED)) {
+        // MustRemainDisabled is a const method and hence cannot call
+        // AddExtensionInstallRequest directly.
+        SupervisedUserService* supervised_user_service =
+            SupervisedUserServiceFactory::GetForProfile(profile_);
+        supervised_user_service->AddExtensionInstallRequest(
+            extension->id(), *extension->version());
+      }
+    }
+  }
+  return !may_enable;
+}
+
 void SupervisedUserService::SetExtensionsActive() {
   extensions::ExtensionSystem* extension_system =
       extensions::ExtensionSystem::Get(profile_);