Avoid explicit extensions ids for policy migrator

`ExtensionPolicyMigrator::CopyPoliciesIfUnset()` now takes the hashed
extension id, instead of the extension id itself, and linearly searches
in the policy bundle to find a matching policy.

Where this function was called with an extension id, the extension id is
replaced with the hashed one.

Note that HashedExtensionId generates a hash that uses upper-case
letters.

Bug: 878528
Change-Id: I6290fe894cfcc6e0c440b639908bd1f0866d49ba
Reviewed-on: https://chromium-review.googlesource.com/1207891Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590311}

chrome/browser/BUILD.gn

@@ -3739,6 +3739,8 @@ jumbo_split_static_library("browser") {
       "media/webrtc/tab_capture_access_handler.h",
       "metrics/extensions_metrics_provider.cc",
       "metrics/extensions_metrics_provider.h",
+      "policy/chrome_extension_policy_migrator.cc",
+      "policy/chrome_extension_policy_migrator.h",
       "renderer_context_menu/context_menu_content_type_app_mode.cc",
       "renderer_context_menu/context_menu_content_type_app_mode.h",
       "renderer_context_menu/context_menu_content_type_extension_popup.cc",

chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_policy_migrator.cc

@@ -11,12 +11,14 @@ namespace enterprise_reporting {
 
 namespace {
 
-// Extension ID for the Chrome Reporting Extension.
+// Hash of the Extension ID for the Chrome Reporting Extension.
 // https://chrome.google.com/webstore/detail/chrome-reporting-extensio/emahakmocgideepebncgnmlmliepgpgb
-const char kStableExtensionId[] = "emahakmocgideepebncgnmlmliepgpgb";
+const char kStableHashedExtensionId[] =
+    "FD15C63ABA854733FDCBC1D4D34A71E963A12ABD";
 
-// Beta extension ID.
-const char kBetaExtensionId[] = "kigjhoekjcpdfjpimbdjegmgecmlicaf";
+// Hash of the beta extension ID.
+const char kBetaHashedExtensionId[] =
+    "08455FA7CB8734168378F731B00B354CEEE0088F";
 
 const policy::ExtensionPolicyMigrator::Migration kMigrations[] = {
     {"report_version_data", policy::key::kReportVersionData},
@@ -32,8 +34,8 @@ EnterpriseReportingPolicyMigrator::EnterpriseReportingPolicyMigrator() {}
 EnterpriseReportingPolicyMigrator::~EnterpriseReportingPolicyMigrator() {}
 
 void EnterpriseReportingPolicyMigrator::Migrate(policy::PolicyBundle* bundle) {
-  CopyPoliciesIfUnset(bundle, kStableExtensionId, kMigrations);
-  CopyPoliciesIfUnset(bundle, kBetaExtensionId, kMigrations);
+  CopyPoliciesIfUnset(bundle, kStableHashedExtensionId, kMigrations);
+  CopyPoliciesIfUnset(bundle, kBetaHashedExtensionId, kMigrations);
 }
 
 }  // namespace enterprise_reporting

chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_policy_migrator.h

@@ -5,13 +5,13 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_API_ENTERPRISE_REPORTING_PRIVATE_ENTERPRISE_REPORTING_POLICY_MIGRATOR_H_
 #define CHROME_BROWSER_EXTENSIONS_API_ENTERPRISE_REPORTING_PRIVATE_ENTERPRISE_REPORTING_POLICY_MIGRATOR_H_
 
-#include "components/policy/core/common/extension_policy_migrator.h"
+#include "chrome/browser/policy/chrome_extension_policy_migrator.h"
 
 namespace extensions {
 namespace enterprise_reporting {
 
 class EnterpriseReportingPolicyMigrator
-    : public policy::ExtensionPolicyMigrator {
+    : public policy::ChromeExtensionPolicyMigrator {
  public:
   EnterpriseReportingPolicyMigrator();
   ~EnterpriseReportingPolicyMigrator() override;

components/policy/core/common/extension_policy_migrator.cc → chrome/browser/policy/chrome_extension_policy_migrator.cc

@@ -2,26 +2,43 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/policy/core/common/extension_policy_migrator.h"
+#include "chrome/browser/policy/chrome_extension_policy_migrator.h"
+#include "extensions/common/hashed_extension_id.h"
 
 namespace policy {
 
-ExtensionPolicyMigrator::~ExtensionPolicyMigrator() {}
-
-void ExtensionPolicyMigrator::CopyPoliciesIfUnset(
+void ChromeExtensionPolicyMigrator::CopyPoliciesIfUnset(
     PolicyBundle* bundle,
-    const std::string& extension_id,
+    const std::string& hashed_extension_id,
     base::span<const Migration> migrations) {
-  PolicyMap& extension_map = bundle->Get(
-      PolicyNamespace(PolicyDomain::POLICY_DOMAIN_EXTENSIONS, extension_id));
-  if (extension_map.empty())
+  // HashedExtensionId gives an all-uppercase output, so make sure the input is
+  // all uppercase.
+  std::string hashed_extension_id_uppercase = hashed_extension_id;
+  std::transform(hashed_extension_id_uppercase.begin(),
+                 hashed_extension_id_uppercase.end(),
+                 hashed_extension_id_uppercase.begin(), ::toupper);
+
+  // Look for an extension with this hash.
+  PolicyMap* extension_map = nullptr;
+  for (auto& policy : *bundle) {
+    const PolicyNamespace& policy_namespace = policy.first;
+
+    if (policy_namespace.domain == PolicyDomain::POLICY_DOMAIN_EXTENSIONS &&
+        extensions::HashedExtensionId(policy_namespace.component_id).value() ==
+            hashed_extension_id_uppercase) {
+      extension_map = policy.second.get();
+      break;
+    }
+  }
+
+  if (extension_map == nullptr || extension_map->empty())
     return;
 
   PolicyMap& chrome_map = bundle->Get(PolicyNamespace(
       PolicyDomain::POLICY_DOMAIN_CHROME, /* component_id */ std::string()));
 
   for (const auto& migration : migrations) {
-    PolicyMap::Entry* entry = extension_map.GetMutable(migration.old_name);
+    PolicyMap::Entry* entry = extension_map->GetMutable(migration.old_name);
     if (entry) {
       if (!chrome_map.Get(migration.new_name)) {
         chrome_map.Set(migration.new_name, entry->DeepCopy());

chrome/browser/policy/chrome_extension_policy_migrator.h

+// Copyright 2018 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_POLICY_CHROME_EXTENSION_POLICY_MIGRATOR_H_
+#define CHROME_BROWSER_POLICY_CHROME_EXTENSION_POLICY_MIGRATOR_H_
+
+#include "components/policy/core/common/extension_policy_migrator.h"
+
+namespace policy {
+
+// ExtensionPolicyMigrator with chrome-specific helper functions.
+class ChromeExtensionPolicyMigrator : public ExtensionPolicyMigrator {
+ public:
+  ~ChromeExtensionPolicyMigrator() override {}
+
+ protected:
+  // Helper function intended for implementers who want to rename policies and
+  // copy them from an extension domain to the Chrome domain. If one of the
+  // Chrome domain policies is already set, it is not overridden.
+  // hashed_extension_id is the SHA1-hashed extension_id to avoid the necessity
+  // of possibly hardcoding extension ids.
+  static void CopyPoliciesIfUnset(PolicyBundle* bundle,
+                                  const std::string& hashed_extension_id,
+                                  base::span<const Migration> migrations);
+};
+
+}  // namespace policy
+
+#endif  // CHROME_BROWSER_POLICY_CHROME_EXTENSION_POLICY_MIGRATOR_H_

components/policy/core/common/extension_policy_migrator_unittest.cc → chrome/browser/policy/chrome_extension_policy_migrator_unittest.cc

@@ -2,12 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/policy/core/common/extension_policy_migrator.h"
+#include "chrome/browser/policy/chrome_extension_policy_migrator.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "extensions/common/hashed_extension_id.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace policy {
@@ -43,16 +44,18 @@ void SetPolicy(PolicyMap* policy,
               POLICY_SOURCE_CLOUD, std::move(value), nullptr);
 }
 
-class TestingPolicyMigrator : public ExtensionPolicyMigrator {
+class TestingPolicyMigrator : public ChromeExtensionPolicyMigrator {
  public:
   void Migrate(PolicyBundle* bundle) override {
-    CopyPoliciesIfUnset(bundle, kExtensionId, kMigrations);
+    CopyPoliciesIfUnset(bundle,
+                        extensions::HashedExtensionId(kExtensionId).value(),
+                        kMigrations);
   }
 };
 
 }  // namespace
 
-TEST(ExtensionPolicyMigratorTest, CopyPoliciesIfUnset) {
+TEST(ChromeExtensionPolicyMigratorTest, CopyPoliciesIfUnset) {
   PolicyBundle bundle;
 
   PolicyMap& chrome_map = bundle.Get(

chrome/test/BUILD.gn

@@ -3650,6 +3650,7 @@ test("unit_tests") {
       "../browser/media_galleries/chromeos/mtp_device_object_enumerator_unittest.cc",
       "../browser/metrics/extensions_metrics_provider_unittest.cc",
       "../browser/notifications/notification_system_observer_unittest.cc",
+      "../browser/policy/chrome_extension_policy_migrator_unittest.cc",
       "../browser/renderer_context_menu/context_menu_content_type_unittest.cc",
       "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc",
       "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_unittest.cc",

components/policy/core/common/BUILD.gn

@@ -89,7 +89,6 @@ source_set("internal") {
     "config_dir_policy_loader.h",
     "configuration_policy_provider.cc",
     "configuration_policy_provider.h",
-    "extension_policy_migrator.cc",
     "extension_policy_migrator.h",
     "external_data_fetcher.cc",
     "external_data_fetcher.h",
@@ -315,7 +314,6 @@ source_set("unit_tests") {
     "cloud/device_management_service_unittest.cc",
     "cloud/policy_header_service_unittest.cc",
     "cloud/user_info_fetcher_unittest.cc",
-    "extension_policy_migrator_unittest.cc",
     "generate_policy_source_unittest.cc",
     "plist_writer_unittest.cc",
     "policy_bundle_unittest.cc",

components/policy/core/common/extension_policy_migrator.h

@@ -19,7 +19,7 @@ namespace policy {
 // |LegacyPoliciesDeprecatingPolicyHandler| instead.
 class POLICY_EXPORT ExtensionPolicyMigrator {
  public:
-  virtual ~ExtensionPolicyMigrator();
+  virtual ~ExtensionPolicyMigrator() {}
 
   // If there are deprecated policies in |bundle|, set the value of the new
   // policies accordingly.
@@ -33,14 +33,6 @@ class POLICY_EXPORT ExtensionPolicyMigrator {
     // New name for the policy, in the Chrome domain.
     const char* new_name;
   };
-
- protected:
-  // Helper function intended for implementers who want to rename policies and
-  // copy them from an extension domain to the Chrome domain. If one of the
-  // Chrome domain policies is already set, it is not overridden.
-  void CopyPoliciesIfUnset(PolicyBundle* bundle,
-                           const std::string& extension_id,
-                           base::span<const Migration> migrations);
 };
 
 }  // namespace policy

extensions/common/BUILD.gn

@@ -405,6 +405,7 @@ if (enable_extensions) {
       "features/feature_provider_unittest.cc",
       "features/simple_feature_unittest.cc",
       "file_util_unittest.cc",
+      "hashed_extension_id_unittest.cc",
       "image_util_unittest.cc",
       "manifest_handler_perf_test.cc",
       "manifest_handler_unittest.cc",

extensions/common/hashed_extension_id_unittest.cc

+// Copyright 2018 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 "extensions/common/hashed_extension_id.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+TEST(HashedExtensionIdTest, Basic) {
+  const std::string kExtensionId = "abcdefghijklmnopabcdefghijklmnop";
+  const std::string kExpectedHash = "ACD66AF886BA7B085B41B4382BC39D1855BC18FE";
+
+  EXPECT_EQ(kExpectedHash, HashedExtensionId(kExtensionId).value());
+}
+
+}  // namespace extensions