Supervised users: Slightly relax restrictions around extensions.

chrome/browser/supervised_user/supervised_user_service.cc

 // Copyright 2014 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/supervised_user/supervised_user_service.h"
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/prefs/pref_service.h"
@@ skipping 65 matching lines @@
   prefs::kSupervisedUserCustodianName,
   prefs::kSupervisedUserCustodianEmail,
   prefs::kSupervisedUserCustodianProfileImageURL,
   prefs::kSupervisedUserCustodianProfileURL,
   prefs::kSupervisedUserSecondCustodianName,
   prefs::kSupervisedUserSecondCustodianEmail,
   prefs::kSupervisedUserSecondCustodianProfileImageURL,
   prefs::kSupervisedUserSecondCustodianProfileURL,
 };
 
+#if defined(ENABLE_EXTENSIONS)
+enum ExtensionState {
+  EXTENSION_FORCED,
+  EXTENSION_BLOCKED,
+  EXTENSION_ALLOWED
+};
+
+ExtensionState GetExtensionState(const extensions::Extension* extension) {
+  // |extension| can be NULL in unit_tests.
+  if (extension && extension->is_theme())
+    return EXTENSION_ALLOWED;
+
+  bool was_installed_by_default = extension->was_installed_by_default();

[Pam (message me for reviews), 2015/02/23 10:18:50]

If indeed |extension| can be NULL in unit tests, kablooie.

[Marc Treib, 2015/02/23 12:07:38]

Huh. Since I got green try runs, looks like extension can't actually be null.
I've removed the check and comment above. Good catch!

+  bool was_installed_by_custodian = extension->was_installed_by_custodian();
+#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
+  if (extensions::Manifest::IsComponentLocation(extension->location()) ||
+      was_installed_by_default ||
+      was_installed_by_custodian) {
+    // Enforce default extensions as well as custodian-installed extensions
+    // (if we'd allow the supervised user to uninstall them, there'd be no way
+    // to get them back).
+    return EXTENSION_FORCED;
+  }
+
+  return EXTENSION_BLOCKED;
+}
+#endif
+
 }  // namespace
 
 base::FilePath SupervisedUserService::Delegate::GetBlacklistPath() const {
   return base::FilePath();
 }
 
 GURL SupervisedUserService::Delegate::GetBlacklistURL() const {
   return GURL();
 }
 
@@ skipping 246 matching lines @@
 void SupervisedUserService::RemoveObserver(
     SupervisedUserServiceObserver* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
 void SupervisedUserService::AddPermissionRequestCreator(
     scoped_ptr<PermissionRequestCreator> creator) {
   permissions_creators_.push_back(creator.release());
 }
 
-#if defined(ENABLE_EXTENSIONS)
-std::string SupervisedUserService::GetDebugPolicyProviderName() const {
-  // Save the string space in official builds.
-#ifdef NDEBUG
-  NOTREACHED();
-  return std::string();
-#else
-  return "Supervised User Service";
-#endif
-}
-
-bool SupervisedUserService::UserMayLoad(const extensions::Extension* extension,
-                                        base::string16* error) const {
-  base::string16 tmp_error;
-  if (ExtensionManagementPolicyImpl(extension, &tmp_error))
-    return true;
-
-  bool was_installed_by_default = extension->was_installed_by_default();
-  bool was_installed_by_custodian = extension->was_installed_by_custodian();
-#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
-  if (extensions::Manifest::IsComponentLocation(extension->location()) ||
-      was_installed_by_default ||
-      was_installed_by_custodian) {
-    return true;
-  }
-
-  if (error)
-    *error = tmp_error;
-  return false;
-}
-
-bool SupervisedUserService::UserMayModifySettings(
-    const extensions::Extension* extension,
-    base::string16* error) const {
-  return ExtensionManagementPolicyImpl(extension, error);
-}
-
-#endif  // defined(ENABLE_EXTENSIONS)
-
 syncer::ModelTypeSet SupervisedUserService::GetPreferredDataTypes() const {
   if (!ProfileIsSupervised())
     return syncer::ModelTypeSet();
 
   syncer::ModelTypeSet result;
   if (IncludesSyncSessionsType())
     result.Put(syncer::SESSIONS);
   result.Put(syncer::EXTENSIONS);
   result.Put(syncer::EXTENSION_SETTINGS);
   result.Put(syncer::APPS);
@@ skipping 69 matching lines @@
   bool sync_everything = false;
   syncer::ModelTypeSet synced_datatypes;
   service->OnUserChoseDatatypes(sync_everything, synced_datatypes);
 
   // Notify ProfileSyncService that we are done with configuration.
   service->SetSetupInProgress(false);
   service->SetSyncSetupCompleted();
 }
 
 #if defined(ENABLE_EXTENSIONS)
-bool SupervisedUserService::ExtensionManagementPolicyImpl(
+std::string SupervisedUserService::GetDebugPolicyProviderName() const {
+  // Save the string space in official builds.
+#ifdef NDEBUG
+  NOTREACHED();
+  return std::string();
+#else
+  return "Supervised User Service";
+#endif
+}
+
+bool SupervisedUserService::UserMayLoad(const extensions::Extension* extension,
+                                        base::string16* error) const {
+  DCHECK(ProfileIsSupervised());
+  ExtensionState result = GetExtensionState(extension);
+  bool may_load = (result != EXTENSION_BLOCKED);
+  if (!may_load && error)
+    *error = l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOCKED_SUPERVISED_USER);
+  return may_load;
+}
+
+// Note: Having MustRemainInstalled always say "true" for custodian-installed
+// extensions does NOT prevent remote uninstalls (which is a bit unexpected, but
+// exactly what we want).
+bool SupervisedUserService::MustRemainInstalled(
     const extensions::Extension* extension,
     base::string16* error) const {
-  // |extension| can be NULL in unit_tests.
-  if (!ProfileIsSupervised() || (extension && extension->is_theme()))
-    return true;
-
-  if (error)
+  DCHECK(ProfileIsSupervised());
+  ExtensionState result = GetExtensionState(extension);
+  bool may_uninstall = (result != EXTENSION_FORCED);

[Pam (message me for reviews), 2015/02/23 10:18:50]

Please change the sense of this (i.e. use may_not_uninstall). The number of
layered negatives is hard to parse.

[Marc Treib, 2015/02/23 12:07:38]

Done.

+  if (!may_uninstall && error)
     *error = l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOCKED_SUPERVISED_USER);
-  return false;
+  return !may_uninstall;
 }
 
 void SupervisedUserService::SetExtensionsActive() {
   extensions::ExtensionSystem* extension_system =
       extensions::ExtensionSystem::Get(profile_);
   extensions::ManagementPolicy* management_policy =
       extension_system->management_policy();
 
   if (management_policy) {
     if (active_)
@@ skipping 439 matching lines @@
   // The active user can be NULL in unit tests.
   if (user_manager::UserManager::Get()->GetActiveUser()) {
     return UTF16ToUTF8(user_manager::UserManager::Get()->GetUserDisplayName(
         user_manager::UserManager::Get()->GetActiveUser()->GetUserID()));
   }
   return std::string();
 #else
   return profile_->GetPrefs()->GetString(prefs::kProfileName);
 #endif
 }