Cert Provisioning: Display certificate profile name on certificate details UI

This cl adds certificate profile name field to FailedWorkerInfo and CertificateProvisioningProcess. Then retrieves the certificate profile name from CertificateProvisioningProcess and displays it in certificate details UI. Updates mock_cert_provisioning_worker and tests with the new name field.

Bug: 1137523
Change-Id: If189e84f83ca55ff384622a92b0a4b64f81ca2b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562250Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Michael Ershov <miersh@google.com>
Commit-Queue: Irem Uğuz <iremuguz@google.com>
Cr-Commit-Position: refs/heads/master@{#833278}

chrome/browser/chromeos/cert_provisioning/cert_provisioning_scheduler.cc

@@ -713,6 +713,7 @@ void CertProvisioningSchedulerImpl::UpdateFailedCertProfiles(
 
   FailedWorkerInfo info;
   info.state_before_failure = worker.GetPreviousState();
+  info.cert_profile_name = worker.GetCertProfile().name;
   info.public_key = worker.GetPublicKey();
   info.last_update_time = worker.GetLastUpdateTime();
 

chrome/browser/chromeos/cert_provisioning/cert_provisioning_scheduler.h

@@ -50,6 +50,8 @@ struct FailedWorkerInfo {
       CertProvisioningWorkerState::kInitState;
   // The DER-encoded X.509 SPKI.
   std::string public_key;
+  // Human-readable certificate profile name (UTF-8).
+  std::string cert_profile_name;
   // The time the worker was last updated, i.e. when it transferred to the
   // failed state.
   base::Time last_update_time;

chrome/browser/chromeos/cert_provisioning/mock_cert_provisioning_worker.cc

@@ -53,6 +53,9 @@ MockCertProvisioningWorkerFactory::ExpectDeserializeReturnMock(
 // ================ MockCertProvisioningWorker =================================
 
 MockCertProvisioningWorker::MockCertProvisioningWorker() {
+  // Makes MockCertProvisioningWorker return empty key by default. Because the
+  // return type is a reference, the object must exist to be able to return a
+  // default value.
   static const std::string default_public_key;
   ON_CALL(*this, GetPublicKey).WillByDefault(ReturnRef(default_public_key));
 }

chrome/browser/ui/webui/certificate_provisioning_ui_handler.cc

@@ -105,12 +105,14 @@ base::string16 GetTimeSinceLastUpdate(base::Time last_update_time) {
 
 base::Value CreateProvisioningProcessEntry(
     const std::string& cert_profile_id,
+    const std::string& cert_profile_name,
     bool is_device_wide,
     CertProvisioningWorkerState state,
     base::Time time_since_last_update,
     const std::string& public_key_spki_der) {
   base::Value entry(base::Value::Type::DICTIONARY);
   entry.SetStringKey("certProfileId", cert_profile_id);
+  entry.SetStringKey("certProfileName", cert_profile_name);
   entry.SetBoolKey("isDeviceWide", is_device_wide);
   entry.SetStringKey("status", GetProvisioningProcessStatus(state));
   entry.SetIntKey("stateId", static_cast<int>(state));
@@ -134,14 +136,15 @@ void CollectProvisioningProcesses(
   for (const auto& worker_entry : cert_provisioning_scheduler->GetWorkers()) {
     CertProvisioningWorker* worker = worker_entry.second.get();
     list_to_append_to->Append(CreateProvisioningProcessEntry(
-        worker_entry.first, is_device_wide, worker->GetState(),
-        worker->GetLastUpdateTime(), worker->GetPublicKey()));
+        worker_entry.first, worker->GetCertProfile().name, is_device_wide,
+        worker->GetState(), worker->GetLastUpdateTime(),
+        worker->GetPublicKey()));
   }
   for (const auto& failed_worker_entry :
        cert_provisioning_scheduler->GetFailedCertProfileIds()) {
     const FailedWorkerInfo& worker = failed_worker_entry.second;
     list_to_append_to->Append(CreateProvisioningProcessEntry(
-        failed_worker_entry.first, is_device_wide,
+        failed_worker_entry.first, worker.cert_profile_name, is_device_wide,
         CertProvisioningWorkerState::kFailed, worker.last_update_time,
         worker.public_key));
   }

chrome/browser/ui/webui/certificate_provisioning_ui_handler_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/bind.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/values_test_util.h"
+#include "chrome/browser/chromeos/cert_provisioning/cert_provisioning_common.h"
 #include "chrome/browser/chromeos/cert_provisioning/cert_provisioning_scheduler.h"
 #include "chrome/browser/chromeos/cert_provisioning/cert_provisioning_test_helpers.h"
 #include "chrome/browser/chromeos/cert_provisioning/cert_provisioning_worker.h"
@@ -75,12 +76,23 @@ BA 48 53 4A E2 1C 42 24 EB E5 CD 46 E0 4E 9B 2B
   Public Exponent (24 bits):
   01 00 01)";
 
+// Test values for creating CertProfile for MockCertProvisioningWorker.
+constexpr char kCertProfileVersion[] = "cert_profile_version_1";
+constexpr base::TimeDelta kCertProfileRenewalPeriod =
+    base::TimeDelta::FromSeconds(0);
+constexpr char kDeviceCertProfileId[] = "device_cert_profile_1";
+constexpr char kDeviceCertProfileName[] = "Device Certificate Profile 1";
+constexpr char kUserCertProfileId[] = "user_cert_profile_1";
+constexpr char kUserCertProfileName[] = "User Certificate Profile 1";
+
 void SetupMockCertProvisioningWorker(MockCertProvisioningWorker* worker,
                                      CertProvisioningWorkerState state,
-                                     const std::string* public_key) {
+                                     const std::string* public_key,
+                                     CertProfile& cert_profile) {
   EXPECT_CALL(*worker, GetState).WillRepeatedly(Return(state));
   EXPECT_CALL(*worker, GetLastUpdateTime).WillRepeatedly(Return(base::Time()));
   EXPECT_CALL(*worker, GetPublicKey).WillRepeatedly(ReturnPointee(public_key));
+  ON_CALL(*worker, GetCertProfile).WillByDefault(ReturnRef(cert_profile));
 }
 
 // Recursively visits all strings in |value| and replaces placeholders such as
@@ -263,17 +275,23 @@ TEST_F(CertificateProvisioningUiHandlerTest, NoProcesses) {
 }
 
 TEST_F(CertificateProvisioningUiHandlerTest, HasProcesses) {
+  CertProfile user_cert_profile(
+      kUserCertProfileId, kUserCertProfileName, kCertProfileVersion,
+      /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
   auto user_cert_worker = std::make_unique<MockCertProvisioningWorker>();
   SetupMockCertProvisioningWorker(
       user_cert_worker.get(), CertProvisioningWorkerState::kKeypairGenerated,
-      &der_encoded_spki_);
-  user_workers_["user_cert_profile_1"] = std::move(user_cert_worker);
+      &der_encoded_spki_, user_cert_profile);
+  user_workers_[kUserCertProfileId] = std::move(user_cert_worker);
 
+  CertProfile device_cert_profile(
+      kDeviceCertProfileId, kDeviceCertProfileName, kCertProfileVersion,
+      /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
   auto device_cert_worker = std::make_unique<MockCertProvisioningWorker>();
   SetupMockCertProvisioningWorker(
       device_cert_worker.get(), CertProvisioningWorkerState::kKeypairGenerated,
-      &der_encoded_spki_);
-  device_workers_["device_cert_profile_1"] = std::move(device_cert_worker);
+      &der_encoded_spki_, device_cert_profile);
+  device_workers_[kDeviceCertProfileId] = std::move(device_cert_worker);
 
   // Only the user worker is expected to be displayed in the UI, because the
   // user is not affiliated.
@@ -281,36 +299,42 @@ TEST_F(CertificateProvisioningUiHandlerTest, HasProcesses) {
   std::vector<std::string> profile_ids;
   ASSERT_NO_FATAL_FAILURE(
       RefreshCertProvisioningProcesses(&all_processes, &profile_ids));
-  ASSERT_THAT(profile_ids, UnorderedElementsAre("user_cert_profile_1"));
-
+  ASSERT_THAT(profile_ids, UnorderedElementsAre(kUserCertProfileId));
   EXPECT_EQ(
-      GetByProfileId(all_processes, "user_cert_profile_1"),
+      GetByProfileId(all_processes, kUserCertProfileId),
       FormatJsonDict(
           R"({
-               "certProfileId": "user_cert_profile_1",
+               "certProfileId": "$0",
+               "certProfileName": "$1",
                "isDeviceWide": false,
-               "publicKey": "$0",
+               "publicKey": "$2",
                "stateId": 1,
-               "status": "$1",
+               "status": "$3",
                "timeSinceLastUpdate": ""
              })",
-          {kFormattedPublicKey,
+          {kUserCertProfileId, kUserCertProfileName, kFormattedPublicKey,
            l10n_util::GetStringUTF8(
                IDS_SETTINGS_CERTIFICATE_MANAGER_PROVISIONING_STATUS_PREPARING_CSR_WAITING)}));
 }
 
 TEST_F(CertificateProvisioningUiHandlerAffiliatedTest, HasProcessesAffiliated) {
+  CertProfile user_cert_profile(
+      kUserCertProfileId, kUserCertProfileName, kCertProfileVersion,
+      /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
   auto user_cert_worker = std::make_unique<MockCertProvisioningWorker>();
   SetupMockCertProvisioningWorker(
       user_cert_worker.get(), CertProvisioningWorkerState::kKeypairGenerated,
-      &der_encoded_spki_);
-  user_workers_["user_cert_profile_1"] = std::move(user_cert_worker);
+      &der_encoded_spki_, user_cert_profile);
+  user_workers_[kUserCertProfileId] = std::move(user_cert_worker);
 
+  CertProfile device_cert_profile(
+      kDeviceCertProfileId, kDeviceCertProfileName, kCertProfileVersion,
+      /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
   auto device_cert_worker = std::make_unique<MockCertProvisioningWorker>();
   SetupMockCertProvisioningWorker(device_cert_worker.get(),
                                   CertProvisioningWorkerState::kFailed,
-                                  &der_encoded_spki_);
-  device_workers_["device_cert_profile_1"] = std::move(device_cert_worker);
+                                  &der_encoded_spki_, device_cert_profile);
+  device_workers_[kDeviceCertProfileId] = std::move(device_cert_worker);
 
   // Both user and device-wide workers are expected to be displayed in the UI,
   // because the user is affiliated.
@@ -318,35 +342,37 @@ TEST_F(CertificateProvisioningUiHandlerAffiliatedTest, HasProcessesAffiliated) {
   std::vector<std::string> profile_ids;
   ASSERT_NO_FATAL_FAILURE(
       RefreshCertProvisioningProcesses(&all_processes, &profile_ids));
-  ASSERT_THAT(profile_ids, UnorderedElementsAre("user_cert_profile_1",
-                                                "device_cert_profile_1"));
+  ASSERT_THAT(profile_ids,
+              UnorderedElementsAre(kUserCertProfileId, kDeviceCertProfileId));
 
   EXPECT_EQ(
-      GetByProfileId(all_processes, "user_cert_profile_1"),
+      GetByProfileId(all_processes, kUserCertProfileId),
       FormatJsonDict(
           R"({
-               "certProfileId": "user_cert_profile_1",
+               "certProfileId": "$0",
+               "certProfileName": "$1",
                "isDeviceWide": false,
-               "publicKey": "$0",
+               "publicKey": "$2",
                "stateId": 1,
-               "status": "$1",
+               "status": "$3",
                "timeSinceLastUpdate": ""
              })",
-          {kFormattedPublicKey,
+          {kUserCertProfileId, kUserCertProfileName, kFormattedPublicKey,
            l10n_util::GetStringUTF8(
                IDS_SETTINGS_CERTIFICATE_MANAGER_PROVISIONING_STATUS_PREPARING_CSR_WAITING)}));
   EXPECT_EQ(
-      GetByProfileId(all_processes, "device_cert_profile_1"),
+      GetByProfileId(all_processes, kDeviceCertProfileId),
       FormatJsonDict(
           R"({
-               "certProfileId": "device_cert_profile_1",
+               "certProfileId": "$0",
+               "certProfileName": "$1",
                "isDeviceWide": true,
-               "publicKey": "$0",
+               "publicKey": "$2",
                "stateId": 10,
-               "status": "$1",
+               "status": "$3",
                "timeSinceLastUpdate": ""
              })",
-          {kFormattedPublicKey,
+          {kDeviceCertProfileId, kDeviceCertProfileName, kFormattedPublicKey,
            l10n_util::GetStringUTF8(
                IDS_SETTINGS_CERTIFICATE_MANAGER_PROVISIONING_STATUS_FAILURE)}));
 }
@@ -362,11 +388,14 @@ TEST_F(CertificateProvisioningUiHandlerTest, Updates) {
   ASSERT_THAT(profile_ids, UnorderedElementsAre());
   EXPECT_EQ(1U, handler_->ReadAndResetUiRefreshCountForTesting());
 
+  CertProfile user_cert_profile(
+      kUserCertProfileId, kUserCertProfileName, kCertProfileVersion,
+      /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
   auto user_cert_worker = std::make_unique<MockCertProvisioningWorker>();
   SetupMockCertProvisioningWorker(
       user_cert_worker.get(), CertProvisioningWorkerState::kKeypairGenerated,
-      &der_encoded_spki_);
-  user_workers_["user_cert_profile_1"] = std::move(user_cert_worker);
+      &der_encoded_spki_, user_cert_profile);
+  user_workers_[kUserCertProfileId] = std::move(user_cert_worker);
 
   // The user worker triggers an update
   content::TestWebUIListenerObserver result_waiter_1(
@@ -381,20 +410,21 @@ TEST_F(CertificateProvisioningUiHandlerTest, Updates) {
 
   // Only the user worker is expected to be displayed in the UI, because the
   // user is not affiliated.
-  ASSERT_THAT(profile_ids, UnorderedElementsAre("user_cert_profile_1"));
+  ASSERT_THAT(profile_ids, UnorderedElementsAre(kUserCertProfileId));
 
   EXPECT_EQ(
-      GetByProfileId(all_processes, "user_cert_profile_1"),
+      GetByProfileId(all_processes, kUserCertProfileId),
       FormatJsonDict(
           R"({
-               "certProfileId": "user_cert_profile_1",
+               "certProfileId": "$0",
+               "certProfileName": "$1",
                "isDeviceWide": false,
-               "publicKey": "$0",
+               "publicKey": "$2",
                "stateId": 1,
-               "status": "$1",
+               "status": "$3",
                "timeSinceLastUpdate": ""
              })",
-          {kFormattedPublicKey,
+          {kUserCertProfileId, kUserCertProfileName, kFormattedPublicKey,
            l10n_util::GetStringUTF8(
                IDS_SETTINGS_CERTIFICATE_MANAGER_PROVISIONING_STATUS_PREPARING_CSR_WAITING)}));
 

chrome/test/data/webui/cr_components/certificate_manager_provisioning_test.js

@@ -49,6 +49,7 @@ class TestCertificateProvisioningBrowserProxy extends TestBrowserProxy {
 function createSampleCertificateProvisioningProcess() {
   return {
     certProfileId: 'dummyProfileId',
+    certProfileName: 'Dummy Profile Name',
     isDeviceWide: true,
     publicKey: 'dummyPublicKey',
     stateId: 8,

ui/webui/resources/cr_components/certificate_manager/certificate_provisioning_browser_proxy.js

@@ -17,6 +17,7 @@ import {addSingletonGetter} from 'chrome://resources/js/cr.m.js';
  *
  * @typedef {{
  *   certProfileId: string,
+ *   certProfileName: string,
  *   isDeviceWide: boolean,
  *   status: string,
  *   stateId: number,

ui/webui/resources/cr_components/certificate_manager/certificate_provisioning_details_dialog.html

@@ -30,7 +30,7 @@
   <div slot="body">
     <div class="two-line">
         <div class="label">[[i18n('certificateProvisioningProfile')]]</div>
-        <div class="value">[[model.certProfileId]]</div>
+        <div class="value">[[model.certProfileName]]</div>
     </div>
     <div class="button-box">
       <div class="two-line flex">

ui/webui/resources/cr_components/certificate_manager/certificate_provisioning_entry.html

@@ -8,7 +8,7 @@
   }
 </style>
 <div class="cert-box">
-  <div class="flex" tabindex="0">[[model.certProfileId]]</div>
+  <div class="flex" tabindex="0">[[model.certProfileName]]</div>
   <cr-icon-button class="icon-more-vert" id="dots"
       title="[[i18n('moreActions')]]" on-click="onDotsClick_">
   </cr-icon-button>