Convert remote extension custodian approvals to local for child users

According to go/unicros-extensions-dd, we need to keep track of which
extensions have received parental approval. This dictionary will map
extension ids to the approved version. These extensions are safe for
child users to install and enable. Deprecate global preference
profile.managed.approved_extensions for legacy supervised users as
well.

Bug: 1014700
Change-Id: Ida8fdde0f264d42e5b101a0b3057c9866fd78d7e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864516Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711774}

chrome/browser/extensions/extension_service_sync_unittest.cc

@@ -1874,18 +1874,8 @@ class ExtensionServiceTestSupervised
   void SimulateCustodianApprovalChangeViaSync(const std::string& extension_id,
                                               const std::string& version,
                                               SyncChange::SyncChangeType type) {
-    std::string key = SupervisedUserSettingsService::MakeSplitSettingKey(
-        supervised_users::kApprovedExtensions, extension_id);
-    syncer::SyncData sync_data =
-        SupervisedUserSettingsService::CreateSyncDataForSetting(
-            key, base::Value(version));
-
-    SyncChangeList list(1, SyncChange(FROM_HERE, type, sync_data));
-
-    SupervisedUserSettingsService* supervised_user_settings_service =
-        SupervisedUserSettingsServiceFactory::GetForKey(
-            profile()->GetProfileKey());
-    supervised_user_settings_service->ProcessSyncChanges(FROM_HERE, list);
+    supervised_user_service()->UpdateApprovedExtensions(extension_id, version,
+                                                        type);
   }
 
   void CheckDisabledForCustodianApproval(const std::string& extension_id) {
@@ -1900,6 +1890,11 @@ class ExtensionServiceTestSupervised
     return SupervisedUserServiceFactory::GetForProfile(profile());
   }
 
+  SupervisedUserSettingsService* supervised_user_settings_service() {
+    return SupervisedUserSettingsServiceFactory::GetForKey(
+        profile()->GetProfileKey());
+  }
+
   static std::string RequestId(const std::string& extension_id,
                                const std::string& version) {
     return SupervisedUserService::GetExtensionRequestId(
@@ -2209,9 +2204,10 @@ TEST_F(ExtensionServiceTestSupervised,
   // Prefs are updated via Sync.  If the prefs are updated, then the new
   // approved version has been pushed to Sync as well.
   std::string approved_version;
-  PrefService* pref_service = profile()->GetPrefs();
+  std::string key = SupervisedUserSettingsService::MakeSplitSettingKey(
+      supervised_users::kApprovedExtensions, id);
   const base::DictionaryValue* approved_extensions =
-      pref_service->GetDictionary(prefs::kSupervisedUserApprovedExtensions);
+      supervised_user_settings_service()->GetDictionaryAndSplitKey(&key);
   approved_extensions->GetStringWithoutPathExpansion(id, &approved_version);
 
   EXPECT_EQ(base::Version(approved_version), extension->version());

chrome/browser/supervised_user/supervised_user_constants.cc

@@ -10,7 +10,9 @@ namespace supervised_users {
 const char kAccountConsistencyMirrorRequired[] =
     "AccountConsistencyMirrorRequired";
 #endif
+#if BUILDFLAG(ENABLE_EXTENSIONS)
 const char kApprovedExtensions[] = "ApprovedExtensions";
+#endif
 const char kAuthorizationHeaderFormat[] = "Bearer %s";
 const char kCameraMicDisabled[] = "CameraMicDisabled";
 const char kContentPackDefaultFilteringBehavior[] =

chrome/browser/supervised_user/supervised_user_constants.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_SUPERVISED_USER_SUPERVISED_USER_CONSTANTS_H_
 #define CHROME_BROWSER_SUPERVISED_USER_SUPERVISED_USER_CONSTANTS_H_
 
+#include "extensions/buildflags/buildflags.h"
+
 namespace supervised_users {
 
 // Keys for supervised user settings. These are configured remotely and mapped
@@ -12,7 +14,9 @@ namespace supervised_users {
 #if defined(OS_CHROMEOS)
 extern const char kAccountConsistencyMirrorRequired[];
 #endif
+#if BUILDFLAG(ENABLE_EXTENSIONS)
 extern const char kApprovedExtensions[];
+#endif
 extern const char kAuthorizationHeaderFormat[];
 extern const char kCameraMicDisabled[];
 extern const char kContentPackDefaultFilteringBehavior[];

chrome/browser/supervised_user/supervised_user_pref_store.cc

@@ -38,10 +38,6 @@ SupervisedUserSettingsPrefMappingEntry kSupervisedUserSettingsPrefMapping[] = {
         prefs::kAccountConsistencyMirrorRequired,
     },
 #endif
-    {
-        supervised_users::kApprovedExtensions,
-        prefs::kSupervisedUserApprovedExtensions,
-    },
     {
         supervised_users::kContentPackDefaultFilteringBehavior,
         prefs::kDefaultSupervisedUserFilteringBehavior,

chrome/browser/supervised_user/supervised_user_service.cc

@@ -120,7 +120,6 @@ SupervisedUserService::~SupervisedUserService() {
 // static
 void SupervisedUserService::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
-  registry->RegisterDictionaryPref(prefs::kSupervisedUserApprovedExtensions);
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   registry->RegisterBooleanPref(
       prefs::kSupervisedUserExtensionsMayRequestPermissions, false);
@@ -314,6 +313,50 @@ void SupervisedUserService::SetPrimaryPermissionCreatorForTest(
                                std::move(permission_creator));
 }
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+void SupervisedUserService::UpdateApprovedExtensions(
+    const std::string& extension_id,
+    const std::string& version,
+    syncer::SyncChange::SyncChangeType type) {
+  std::string key = SupervisedUserSettingsService::MakeSplitSettingKey(
+      supervised_users::kApprovedExtensions, extension_id);
+  syncer::SyncData sync_data =
+      SupervisedUserSettingsService::CreateSyncDataForSetting(
+          key, base::Value(version));
+
+  syncer::SyncChangeList list(1,
+                              syncer::SyncChange(FROM_HERE, type, sync_data));
+  GetSettingsService()->ProcessSyncChanges(FROM_HERE, list);
+
+  // Keep track of currently approved extensions. We may need to disable them if
+  // they are not in the approved map anymore.
+  std::set<std::string> extensions_to_be_checked;
+  for (const auto& extension : approved_extensions_map_)
+    extensions_to_be_checked.insert(extension.first);
+
+  approved_extensions_map_.clear();
+
+  const base::DictionaryValue* dict =
+      GetSettingsService()->GetDictionaryAndSplitKey(&key);
+  for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
+    std::string version_str;
+    bool result = it.value().GetAsString(&version_str);
+    DCHECK(result);
+    base::Version version(version_str);
+    if (version.IsValid()) {
+      approved_extensions_map_[it.key()] = version;
+      extensions_to_be_checked.insert(it.key());
+    } else {
+      LOG(WARNING) << "Invalid version number " << version_str;
+    }
+  }
+
+  for (const auto& extension_id : extensions_to_be_checked) {
+    ChangeExtensionStateIfNecessary(extension_id);
+  }
+}
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
+
 void SupervisedUserService::SetActive(bool active) {
   if (active_ == active)
     return;
@@ -346,12 +389,6 @@ void SupervisedUserService::SetActive(bool active) {
         base::BindRepeating(
             &SupervisedUserService::OnDefaultFilteringBehaviorChanged,
             base::Unretained(this)));
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-    pref_change_registrar_.Add(
-        prefs::kSupervisedUserApprovedExtensions,
-        base::BindRepeating(&SupervisedUserService::UpdateApprovedExtensions,
-                            base::Unretained(this)));
-#endif
     pref_change_registrar_.Add(
         prefs::kSupervisedUserSafeSites,
         base::BindRepeating(&SupervisedUserService::OnSafeSitesSettingChanged,
@@ -378,10 +415,6 @@ void SupervisedUserService::SetActive(bool active) {
     UpdateManualHosts();
     UpdateManualURLs();
 
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-    UpdateApprovedExtensions();
-#endif
-
 #if !defined(OS_ANDROID)
     // TODO(bauerb): Get rid of the platform-specific #ifdef here.
     // http://crbug.com/313377
@@ -392,9 +425,6 @@ void SupervisedUserService::SetActive(bool active) {
 
     pref_change_registrar_.Remove(
         prefs::kDefaultSupervisedUserFilteringBehavior);
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-    pref_change_registrar_.Remove(prefs::kSupervisedUserApprovedExtensions);
-#endif
     pref_change_registrar_.Remove(prefs::kSupervisedUserManualHosts);
     pref_change_registrar_.Remove(prefs::kSupervisedUserManualURLs);
     for (const char* pref : kCustodianInfoPrefs) {
@@ -781,46 +811,14 @@ void SupervisedUserService::OnExtensionInstalled(
       approved_extensions_map_[id] < version) {
     approved_extensions_map_[id] = version;
 
-    std::string key = SupervisedUserSettingsService::MakeSplitSettingKey(
-        supervised_users::kApprovedExtensions, id);
-    std::unique_ptr<base::Value> version_value(
-        new base::Value(version.GetString()));
-    GetSettingsService()->UpdateSetting(key, std::move(version_value));
+    UpdateApprovedExtensions(id, version.GetString(),
+                             syncer::SyncChange::ACTION_ADD);
   }
   // Upon extension update, the approved version may (or may not) match the
   // installed one. Therefore, a change in extension state might be required.
   ChangeExtensionStateIfNecessary(id);
 }
 
-void SupervisedUserService::UpdateApprovedExtensions() {
-  const base::DictionaryValue* dict = profile_->GetPrefs()->GetDictionary(
-      prefs::kSupervisedUserApprovedExtensions);
-  // Keep track of currently approved extensions. We may need to disable them if
-  // they are not in the approved map anymore.
-  std::set<std::string> extensions_to_be_checked;
-  for (const auto& extension : approved_extensions_map_)
-    extensions_to_be_checked.insert(extension.first);
-
-  approved_extensions_map_.clear();
-
-  for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
-    std::string version_str;
-    bool result = it.value().GetAsString(&version_str);
-    DCHECK(result);
-    base::Version version(version_str);
-    if (version.IsValid()) {
-      approved_extensions_map_[it.key()] = version;
-      extensions_to_be_checked.insert(it.key());
-    } else {
-      LOG(WARNING) << "Invalid version number " << version_str;
-    }
-  }
-
-  for (const auto& extension_id : extensions_to_be_checked) {
-    ChangeExtensionStateIfNecessary(extension_id);
-  }
-}
-
 void SupervisedUserService::ChangeExtensionStateIfNecessary(
     const std::string& extension_id) {
   ExtensionRegistry* registry = ExtensionRegistry::Get(profile_);

chrome/browser/supervised_user/supervised_user_service.h

@@ -30,6 +30,7 @@
 #include "extensions/buildflags/buildflags.h"
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "components/sync/model/sync_change.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_registry_observer.h"
 #include "extensions/browser/management_policy.h"
@@ -177,6 +178,17 @@ class SupervisedUserService : public KeyedService,
   void SetPrimaryPermissionCreatorForTest(
       std::unique_ptr<PermissionRequestCreator> permission_creator);
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  // Updates the map of approved extensions.
+  // If |type| is SyncChangeType::ADD, then add custodian approval for enabling
+  // the extension by adding the approved version to the map of approved
+  // extensions. If |type| is SyncChangeType::DELETE, then remove the extension
+  // from the map of approved extensions.
+  void UpdateApprovedExtensions(const std::string& extension_id,
+                                const std::string& version,
+                                syncer::SyncChange::SyncChangeType type);
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
+
  private:
   friend class SupervisedUserServiceExtensionTestBase;
   friend class SupervisedUserServiceFactory;
@@ -238,11 +250,7 @@ class SupervisedUserService : public KeyedService,
   // Enables/Disables extensions upon change in approved version of the
   // extension_id.
   void ChangeExtensionStateIfNecessary(const std::string& extension_id);
-
-  // Updates the map of approved extensions when the corresponding preference
-  // is changed.
-  void UpdateApprovedExtensions();
-#endif
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
   SupervisedUserSettingsService* GetSettingsService();
 

chrome/browser/supervised_user/supervised_user_settings_service.cc

@@ -139,13 +139,6 @@ void SupervisedUserSettingsService::UploadItem(
   PushItemToSync(key, std::move(value));
 }
 
-void SupervisedUserSettingsService::UpdateSetting(
-    const std::string& key,
-    std::unique_ptr<base::Value> value) {
-  PushItemToSync(key, std::move(value));
-  InformSubscribers();
-}
-
 void SupervisedUserSettingsService::PushItemToSync(
     const std::string& key,
     std::unique_ptr<base::Value> value) {
@@ -438,6 +431,24 @@ SupervisedUserSettingsService::LocalSettingsForTest() const {
   return local_settings_.get();
 }
 
+base::DictionaryValue* SupervisedUserSettingsService::GetDictionaryAndSplitKey(
+    std::string* key) const {
+  size_t pos = key->find_first_of(kSplitSettingKeySeparator);
+  if (pos == std::string::npos)
+    return GetAtomicSettings();
+
+  base::DictionaryValue* split_settings = GetSplitSettings();
+  std::string prefix = key->substr(0, pos);
+  base::DictionaryValue* dict = nullptr;
+  if (!split_settings->GetDictionary(prefix, &dict)) {
+    DCHECK(!split_settings->HasKey(prefix));
+    dict = split_settings->SetDictionary(
+        prefix, std::make_unique<base::DictionaryValue>());
+  }
+  key->erase(0, pos + 1);
+  return dict;
+}
+
 base::DictionaryValue* SupervisedUserSettingsService::GetOrCreateDictionary(
     const std::string& key) const {
   base::Value* value = nullptr;
@@ -466,24 +477,6 @@ base::DictionaryValue* SupervisedUserSettingsService::GetQueuedItems() const {
   return GetOrCreateDictionary(kQueuedItems);
 }
 
-base::DictionaryValue* SupervisedUserSettingsService::GetDictionaryAndSplitKey(
-    std::string* key) const {
-  size_t pos = key->find_first_of(kSplitSettingKeySeparator);
-  if (pos == std::string::npos)
-    return GetAtomicSettings();
-
-  base::DictionaryValue* split_settings = GetSplitSettings();
-  std::string prefix = key->substr(0, pos);
-  base::DictionaryValue* dict = nullptr;
-  if (!split_settings->GetDictionary(prefix, &dict)) {
-    DCHECK(!split_settings->HasKey(prefix));
-    dict = split_settings->SetDictionary(
-        prefix, std::make_unique<base::DictionaryValue>());
-  }
-  key->erase(0, pos + 1);
-  return dict;
-}
-
 std::unique_ptr<base::DictionaryValue>
 SupervisedUserSettingsService::GetSettings() {
   DCHECK(IsReady());

chrome/browser/supervised_user/supervised_user_settings_service.h

@@ -114,12 +114,6 @@ class SupervisedUserSettingsService : public KeyedService,
   // An example of an uploaded item is an access request to a blocked URL.
   void UploadItem(const std::string& key, std::unique_ptr<base::Value> value);
 
-  // Updates supervised user setting and uploads it to the Sync server.
-  // An example is when an extension updates without permission
-  // increase, the approved version information should be updated accordingly.
-  void UpdateSetting(const std::string& key,
-                     std::unique_ptr<base::Value> value);
-
   // Sets the setting with the given |key| to a copy of the given |value|.
   void SetLocalSetting(const std::string& key,
                        std::unique_ptr<base::Value> value);
@@ -150,18 +144,18 @@ class SupervisedUserSettingsService : public KeyedService,
 
   const base::DictionaryValue* LocalSettingsForTest() const;
 
- private:
-  base::DictionaryValue* GetOrCreateDictionary(const std::string& key) const;
-  base::DictionaryValue* GetAtomicSettings() const;
-  base::DictionaryValue* GetSplitSettings() const;
-  base::DictionaryValue* GetQueuedItems() const;
-
   // Returns the dictionary where a given Sync item should be stored, depending
   // on whether the supervised user setting is atomic or split. In case of a
   // split setting, the split setting prefix of |key| is removed, so that |key|
   // can be used to update the returned dictionary.
   base::DictionaryValue* GetDictionaryAndSplitKey(std::string* key) const;
 
+ private:
+  base::DictionaryValue* GetOrCreateDictionary(const std::string& key) const;
+  base::DictionaryValue* GetAtomicSettings() const;
+  base::DictionaryValue* GetSplitSettings() const;
+  base::DictionaryValue* GetQueuedItems() const;
+
   // Returns a dictionary with all supervised user settings if the service is
   // active, or NULL otherwise.
   std::unique_ptr<base::DictionaryValue> GetSettings();

chrome/common/pref_names.cc

@@ -167,11 +167,6 @@ const char kSupervisedUserManualHosts[] = "profile.managed.manual_hosts";
 // Maps URLs to whether the URL is manually allowed or blocked.
 const char kSupervisedUserManualURLs[] = "profile.managed.manual_urls";
 
-// Maps extension ids to the approved version of this extension for a
-// supervised user. Missing extensions are not approved.
-const char kSupervisedUserApprovedExtensions[] =
-    "profile.managed.approved_extensions";
-
 // Stores whether the SafeSites filter is enabled.
 const char kSupervisedUserSafeSites[] = "profile.managed.safe_sites";
 

chrome/common/pref_names.h

@@ -43,7 +43,6 @@ extern const char kSessionExitType[];
 extern const char kObservedSessionTime[];
 extern const char kRecurrentSSLInterstitial[];
 extern const char kSiteEngagementLastUpdateTime[];
-extern const char kSupervisedUserApprovedExtensions[];
 extern const char kSupervisedUserCustodianEmail[];
 extern const char kSupervisedUserCustodianName[];
 extern const char kSupervisedUserCustodianObfuscatedGaiaId[];