Hardcode ID in TestCertificateProviderExtension

Make the TestCertificateProviderExtension's test extension have a fixed
extension ID (by putting its public key into the manifest.json file),
and update the class to expose this ID as a constant.

This is a preparation step for getting rid of the
TestCertificateProviderExtensionLoginScreenMixin (in favor of
ExtensionForceInstallMixin).

Bug: 1090941
Change-Id: Ic27ed9f0161631b6415a07d3a8e73b8329ec34eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2312817
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Reviewed-by: Fabian Sommer <fabiansommer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791055}

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension.cc

@@ -36,6 +36,8 @@
 
 namespace {
 
+constexpr char kExtensionId[] = "ecmhnokcdiianioonpgakiooenfnonid";
+
 // List of algorithms that the extension claims to support for the returned
 // certificates.
 constexpr extensions::api::certificate_provider::Hash kSupportedHashes[] = {
@@ -116,6 +118,11 @@ bssl::UniquePtr<EVP_PKEY> LoadPrivateKeyFromPem(const base::FilePath& path) {
 
 }  // namespace
 
+// static
+extensions::ExtensionId TestCertificateProviderExtension::extension_id() {
+  return kExtensionId;
+}
+
 // static
 scoped_refptr<net::X509Certificate>
 TestCertificateProviderExtension::GetCertificate() {
@@ -135,15 +142,12 @@ std::string TestCertificateProviderExtension::GetCertificateSpki() {
 }
 
 TestCertificateProviderExtension::TestCertificateProviderExtension(
-    content::BrowserContext* browser_context,
-    const std::string& extension_id)
+    content::BrowserContext* browser_context)
     : browser_context_(browser_context),
-      extension_id_(extension_id),
       certificate_(GetCertificate()),
       private_key_(LoadPrivateKeyFromPem(net::GetTestCertsDirectory().Append(
           FILE_PATH_LITERAL("client_1.key")))) {
   DCHECK(browser_context_);
-  DCHECK(!extension_id_.empty());
   CHECK(certificate_);
   CHECK(private_key_);
   notification_registrar_.Add(this,
@@ -161,7 +165,7 @@ void TestCertificateProviderExtension::Observe(
 
   extensions::TestSendMessageFunction* function =
       content::Source<extensions::TestSendMessageFunction>(source).ptr();
-  if (!function->extension() || function->extension_id() != extension_id_ ||
+  if (!function->extension() || function->extension_id() != kExtensionId ||
       function->browser_context() != browser_context_) {
     // Ignore messages targeted to other extensions.
     return;

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension.h

@@ -15,6 +15,7 @@
 #include "base/values.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "extensions/common/extension_id.h"
 #include "net/cert/x509_certificate.h"
 #include "third_party/boringssl/src/include/openssl/base.h"
 #include "third_party/boringssl/src/include/openssl/evp.h"
@@ -39,12 +40,13 @@ class BrowserContext;
 class TestCertificateProviderExtension final
     : public content::NotificationObserver {
  public:
+  static extensions::ExtensionId extension_id();
   // Returns the certificate provided by the extension.
   static scoped_refptr<net::X509Certificate> GetCertificate();
   static std::string GetCertificateSpki();
 
-  TestCertificateProviderExtension(content::BrowserContext* browser_context,
-                                   const std::string& extension_id);
+  explicit TestCertificateProviderExtension(
+      content::BrowserContext* browser_context);
   ~TestCertificateProviderExtension() override;
 
   int certificate_request_count() const { return certificate_request_count_; }
@@ -83,7 +85,6 @@ class TestCertificateProviderExtension final
                               ReplyToJsCallback callback);
 
   content::BrowserContext* const browser_context_;
-  const std::string extension_id_;
   const scoped_refptr<net::X509Certificate> certificate_;
   const bssl::UniquePtr<EVP_PKEY> private_key_;
   int certificate_request_count_ = 0;

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension_login_screen_mixin.cc

@@ -22,9 +22,6 @@
 
 namespace {
 
-// Extension ID of the test certificate provider extension.
-constexpr char kTestCertProviderExtensionId[] =
-    "ecmhnokcdiianioonpgakiooenfnonid";
 // Path to the update manifest XML file of the test certificate provider
 // extension.
 constexpr char kTestCertProviderExtensionUpdateManifestPath[] =
@@ -36,11 +33,6 @@ Profile* GetProfile() {
 
 }  // namespace
 
-// static
-std::string TestCertificateProviderExtensionLoginScreenMixin::GetExtensionId() {
-  return kTestCertProviderExtensionId;
-}
-
 TestCertificateProviderExtensionLoginScreenMixin::
     TestCertificateProviderExtensionLoginScreenMixin(
         InProcessBrowserTestMixinHost* host,
@@ -49,7 +41,7 @@ TestCertificateProviderExtensionLoginScreenMixin::
     : InProcessBrowserTestMixin(host),
       device_state_mixin_(device_state_mixin),
       load_extension_immediately_(load_extension_immediately),
-      feature_allowlist_(kTestCertProviderExtensionId) {
+      feature_allowlist_(TestCertificateProviderExtension::extension_id()) {
   base::FilePath test_data_dir;
   base::PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
   embedded_test_server_.ServeFilesFromDirectory(test_data_dir);
@@ -65,8 +57,7 @@ TestCertificateProviderExtensionLoginScreenMixin::
 
 void TestCertificateProviderExtensionLoginScreenMixin::SetUpOnMainThread() {
   test_certificate_provider_extension_ =
-      std::make_unique<TestCertificateProviderExtension>(GetProfile(),
-                                                         GetExtensionId());
+      std::make_unique<TestCertificateProviderExtension>(GetProfile());
   ASSERT_TRUE(embedded_test_server_.Start());
   if (load_extension_immediately_) {
     AddExtensionForForceInstallation();
@@ -83,7 +74,10 @@ void TestCertificateProviderExtensionLoginScreenMixin::
   const GURL update_manifest_url = embedded_test_server_.GetURL(
       kTestCertProviderExtensionUpdateManifestPath);
   const std::string policy_item_value = base::ReplaceStringPlaceholders(
-      "$1;$2", {GetExtensionId(), update_manifest_url.spec()}, nullptr);
+      "$1;$2",
+      {TestCertificateProviderExtension::extension_id(),
+       update_manifest_url.spec()},
+      nullptr);
   device_state_mixin_->RequestDevicePolicyUpdate()
       ->policy_payload()
       ->mutable_device_login_screen_extensions()
@@ -93,6 +87,6 @@ void TestCertificateProviderExtensionLoginScreenMixin::
 void TestCertificateProviderExtensionLoginScreenMixin::
     WaitUntilExtensionLoaded() {
   extensions::TestBackgroundPageFirstLoadObserver bg_page_first_load_observer(
-      GetProfile(), GetExtensionId());
+      GetProfile(), TestCertificateProviderExtension::extension_id());
   bg_page_first_load_observer.Wait();
 }

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension_login_screen_mixin.h

@@ -25,8 +25,6 @@ class TestCertificateProviderExtension;
 class TestCertificateProviderExtensionLoginScreenMixin final
     : public InProcessBrowserTestMixin {
  public:
-  static std::string GetExtensionId();
-
   // If |load_extension_immediately| is false,
   // |AddExtensionForForceInstallation()| needs to be called by the test.
   // Otherwise, the extension will be installed during setup.

chrome/browser/chromeos/login/challenge_response_auth_keys_loader_browsertest.cc

@@ -26,6 +26,7 @@
 #include "extensions/browser/pref_names.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/process_manager_observer.h"
+#include "extensions/common/extension_id.h"
 
 namespace chromeos {
 
@@ -76,10 +77,8 @@ class ChallengeResponseAuthKeysLoaderBrowserTest : public OobeBaseTest {
     std::vector<ChallengeResponseKey> challenge_response_keys;
     ChallengeResponseKey challenge_response_key;
     challenge_response_key.set_public_key_spki_der(GetSpki());
-    if (with_extension_id) {
-      challenge_response_key.set_extension_id(
-          cert_provider_extension_mixin_.GetExtensionId());
-    }
+    if (with_extension_id)
+      challenge_response_key.set_extension_id(extension_id());
 
     challenge_response_keys.push_back(challenge_response_key);
     base::Value challenge_response_keys_value =
@@ -139,7 +138,7 @@ class ChallengeResponseAuthKeysLoaderBrowserTest : public OobeBaseTest {
     EXPECT_EQ(pref->GetValue()->DictSize(), static_cast<size_t>(1));
 
     for (const auto& item : pref->GetValue()->DictItems()) {
-      EXPECT_EQ(item.first, GetExtensionId());
+      EXPECT_EQ(item.first, extension_id());
     }
   }
 
@@ -158,8 +157,8 @@ class ChallengeResponseAuthKeysLoaderBrowserTest : public OobeBaseTest {
         ->GetCertificateSpki();
   }
 
-  std::string GetExtensionId() const {
-    return cert_provider_extension_mixin_.GetExtensionId();
+  static extensions::ExtensionId extension_id() {
+    return TestCertificateProviderExtension::extension_id();
   }
 
   AccountId account_id() const { return account_id_; }
@@ -256,7 +255,7 @@ IN_PROC_BROWSER_TEST_F(ChallengeResponseAuthKeysLoaderBrowserTest,
   std::vector<ChallengeResponseKey> challenge_response_keys =
       LoadChallengeResponseKeys();
   ASSERT_EQ(challenge_response_keys.size(), static_cast<size_t>(1));
-  EXPECT_EQ(challenge_response_keys.at(0).extension_id(), GetExtensionId());
+  EXPECT_EQ(challenge_response_keys.at(0).extension_id(), extension_id());
   EXPECT_EQ(challenge_response_keys.at(0).public_key_spki_der(), GetSpki());
 }
 
@@ -278,7 +277,7 @@ IN_PROC_BROWSER_TEST_F(ChallengeResponseAuthKeysLoaderBrowserTest,
   std::vector<ChallengeResponseKey> challenge_response_keys =
       LoadChallengeResponseKeys();
   ASSERT_EQ(challenge_response_keys.size(), static_cast<size_t>(1));
-  EXPECT_EQ(challenge_response_keys.at(0).extension_id(), GetExtensionId());
+  EXPECT_EQ(challenge_response_keys.at(0).extension_id(), extension_id());
   EXPECT_EQ(challenge_response_keys.at(0).public_key_spki_der(), GetSpki());
 }
 
@@ -315,7 +314,7 @@ IN_PROC_BROWSER_TEST_F(ChallengeResponseAuthKeysLoaderBrowserTest,
   std::vector<ChallengeResponseKey> challenge_response_keys =
       LoadChallengeResponseKeys();
   ASSERT_EQ(challenge_response_keys.size(), static_cast<size_t>(1));
-  EXPECT_EQ(challenge_response_keys.at(0).extension_id(), GetExtensionId());
+  EXPECT_EQ(challenge_response_keys.at(0).extension_id(), extension_id());
   EXPECT_EQ(challenge_response_keys.at(0).public_key_spki_der(), GetSpki());
 }
 
@@ -411,7 +410,7 @@ class ChallengeResponseExtensionLoadObserverTest
 
   void OnBackgroundHostCreated(
       extensions::ExtensionHost* extension_host) override {
-    if (extension_host->extension_id() == GetExtensionId()) {
+    if (extension_host->extension_id() == extension_id()) {
       extension_host_ = extension_host;
       extension_host_created_loop_->Quit();
     }

chrome/browser/chromeos/login/security_token_login_browsertest.cc

@@ -225,7 +225,7 @@ class SecurityTokenLoginTest : public MixinBasedInProcessBrowserTest,
     challenge_response_key.set_public_key_spki_der(
         TestCertificateProviderExtension::GetCertificateSpki());
     challenge_response_key.set_extension_id(
-        TestCertificateProviderExtensionLoginScreenMixin::GetExtensionId());
+        TestCertificateProviderExtension::extension_id());
 
     base::Value challenge_response_keys_value =
         SerializeChallengeResponseKeysForKnownUser({challenge_response_key});

chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc

@@ -577,17 +577,19 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, LazyBackgroundPage) {
   extensions::ProcessManager::SetEventPageSuspendingTimeForTesting(1);
 
   // Load the test extension.
-  base::FilePath test_data_dir;
-  base::PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
+  TestCertificateProviderExtension test_certificate_provider_extension(
+      profile());
+  extensions::TestBackgroundPageFirstLoadObserver
+      test_background_page_first_load_observer(
+          profile(), TestCertificateProviderExtension::extension_id());
   const extensions::Extension* const extension =
-      LoadExtension(test_data_dir.AppendASCII("extensions")
+      LoadExtension(base::PathService::CheckedGet(chrome::DIR_TEST_DATA)
+                        .AppendASCII("extensions")
                         .AppendASCII("test_certificate_provider")
                         .AppendASCII("extension"));
   ASSERT_TRUE(extension);
-  TestCertificateProviderExtension test_certificate_provider_extension(
-      profile(), extension->id());
-  extensions::TestBackgroundPageFirstLoadObserver(profile(), extension->id())
-      .Wait();
+  EXPECT_EQ(extension->id(), TestCertificateProviderExtension::extension_id());
+  test_background_page_first_load_observer.Wait();
 
   // Navigate to the page that requests the client authentication. Use the
   // incognito profile in order to force re-authentication in the later request

chrome/test/data/extensions/test_certificate_provider/extension/manifest.json

 {
+  "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4m1mCBTJAmawIS/QWFzsfPCj8+5QAOfVUA4mapvwIBbZ5auGbkcO92U/yfXlNVPAFL6jm44ALqxgI7QLMmucJzNpKGk20e0Kgz4ZURx8kZ5nKHLrRB89vlrUQ8v5B3YlJ5uzk/WT511IcvbwjOru9PMlqNghC/NvyDWIvuo91zbO49twcIhsSFlN/+oYe9n6hKusk6ErX3BvdhBfMlXIjPPlGciOozYl+tOdTgy3nna2sMbmAAsw5k8YB4rgwmCLfMvzJ7+XOUp9sLDGjQmrqX1mCSWCZOsmhOVe2qmEtaV2aJbh2xnFcePQswVB8uys9X5a8Bo5Dgsd0x/8E9TqUwIDAQAB",
   "name": "Test Certificate Provider",
   "version": "1.0",
   "manifest_version": 2,