Show extension provided certificates in chrome://settings/certificates

chrome/browser/certificate_manager_model.cc

Index: chrome/browser/certificate_manager_model.cc
diff --git a/chrome/browser/certificate_manager_model.cc b/chrome/browser/certificate_manager_model.cc
index 3d366edc97e98d158ddca5d90b70cbd37120be6f..2e9576850eca38311de0502bbf0a20c181c2d98a 100644
--- a/chrome/browser/certificate_manager_model.cc
+++ b/chrome/browser/certificate_manager_model.cc
@@ -9,8 +9,11 @@
 #include "base/bind.h"
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
+#include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
+#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
 #include "chrome/browser/net/nss_context.h"
 #include "chrome/browser/ui/crypto_module_password_dialog_nss.h"
 #include "chrome/common/net/x509_certificate_model.h"
@@ -50,6 +53,20 @@ using content::BrowserThread;
 //                  |
 //               callback
 
+namespace {
+
+std::string GetCertificateOrg(net::X509Certificate* cert) {
+    std::string org;
+    if (!cert->subject().organization_names.empty())
+      org = cert->subject().organization_names[0];
+    if (org.empty())
+      org = cert->subject().GetDisplayName();
+
+    return org;
+}
+
+}  // namespace
+
 // static
 void CertificateManagerModel::Create(
     content::BrowserContext* browser_context,
@@ -62,6 +79,7 @@ void CertificateManagerModel::Create(
       base::Bind(&CertificateManagerModel::GetCertDBOnIOThread,
                  browser_context->GetResourceContext(),
                  observer,
+                 browser_context,
                  callback));
 }
 
@@ -69,12 +87,21 @@ CertificateManagerModel::CertificateManagerModel(
     net::NSSCertDatabase* nss_cert_database,
     bool is_user_db_available,
     bool is_tpm_available,
-    Observer* observer)
+    Observer* observer,
+    content::BrowserContext* browser_context)
     : cert_db_(nss_cert_database),
       is_user_db_available_(is_user_db_available),
       is_tpm_available_(is_tpm_available),
-      observer_(observer) {
+      observer_(observer),
+      weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+#if defined(OS_CHROMEOS)
+  chromeos::CertificateProviderService* service =
+      chromeos::CertificateProviderServiceFactory::GetForBrowserContext(
+          browser_context);

[mattm, 2016/09/06 22:41:04]

don't think browser_context is guaranteed to be valid here. I would do the
createCertificateProvider in CertificateManagerModel::Create and then pass it
through the calls instead of the BrowserContext.

[Ivan Šandrk, 2016/09/07 15:47:34]

Done.

+  certificate_provider_ = service->CreateCertificateProvider();
+#endif
 }
 
 CertificateManagerModel::~CertificateManagerModel() {
@@ -92,6 +119,12 @@ void CertificateManagerModel::Refresh() {
       NULL, // TODO(mattm): supply parent window.
       base::Bind(&CertificateManagerModel::RefreshSlotsUnlocked,
                  base::Unretained(this)));
+
+#if defined(OS_CHROMEOS)
+  certificate_provider_->GetCertificates(base::Bind(
+      &CertificateManagerModel::RefreshExtensionCertificates,
+      weak_ptr_factory_.GetWeakPtr()));
+#endif
 }
 
 void CertificateManagerModel::RefreshSlotsUnlocked() {
@@ -99,7 +132,14 @@ void CertificateManagerModel::RefreshSlotsUnlocked() {
   // TODO(tbarzic): Use async |ListCerts|.
   cert_db_->ListCertsSync(&cert_list_);
   observer_->CertificatesRefreshed();
-  DVLOG(1) << "refresh finished";
+  DVLOG(1) << "refresh finished for platform provided certificates";
+}
+
+void CertificateManagerModel::RefreshExtensionCertificates(
+    const net::CertificateList& new_certs) {
+  extension_cert_list_ = new_certs;
+  observer_->CertificatesRefreshed();

[mattm, 2016/09/06 22:41:04]

a little worried about CertificatesRefreshed being called twice now. If both
happen quickly then it might just be jarring/inefficient, and if they happen
farther apart it could be more confusing (would seem like it was loaded, you
start doing stuff, but then more certs magically appear.)

[Ivan Šandrk, 2016/09/07 15:47:34]

The initial idea was to call it just once, but the problem is that extensions do
not always play nicely when returning a list of certificates. On a normal day
they take a couple of seconds to return the list, and if they misbehave they may
take up to 5 minutes (hard limit), and we don't want to block platform provided
certificates from showing in the list because of that. Note that extensions are
third-party software.

+  DVLOG(1) << "refresh finished for extension provided certificates";
 }
 
 void CertificateManagerModel::FilterAndBuildOrgGroupingMap(
@@ -113,14 +153,17 @@ void CertificateManagerModel::FilterAndBuildOrgGroupingMap(
     if (type != filter_type)
       continue;
 
-    std::string org;
-    if (!cert->subject().organization_names.empty())
-      org = cert->subject().organization_names[0];
-    if (org.empty())
-      org = cert->subject().GetDisplayName();
-
+    std::string org = GetCertificateOrg(cert);
     (*map)[org].push_back(cert);
   }
+
+  // Display extension provided certificates under the "Your Certificates" tab.
+  if (filter_type == net::USER_CERT) {
+    for (auto cert : extension_cert_list_) {
+      std::string org = GetCertificateOrg(cert.get());
+      (*map)[org].push_back(cert);
+    }
+  }
 }
 
 base::string16 CertificateManagerModel::GetColumnText(
@@ -132,6 +175,13 @@ base::string16 CertificateManagerModel::GetColumnText(
       rv = base::UTF8ToUTF16(
           x509_certificate_model::GetCertNameOrNickname(cert.os_cert_handle()));
 
+      // Mark extension provided certificates.
+      if (base::ContainsValue(extension_cert_list_, &cert)) {
+        rv = l10n_util::GetStringFUTF16(
+            IDS_CERT_MANAGER_EXTENSION_PROVIDED_FORMAT,
+            rv);
+      }
+
       // TODO(xiyuan): Put this into a column when we have js tree-table.
       if (IsHardwareBacked(&cert)) {
         rv = l10n_util::GetStringFUTF16(
@@ -222,17 +272,20 @@ void CertificateManagerModel::DidGetCertDBOnUIThread(
     bool is_user_db_available,
     bool is_tpm_available,
     CertificateManagerModel::Observer* observer,
+    content::BrowserContext* browser_context,
     const CreationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   std::unique_ptr<CertificateManagerModel> model(new CertificateManagerModel(
-      cert_db, is_user_db_available, is_tpm_available, observer));
+      cert_db, is_user_db_available, is_tpm_available, observer,
+      browser_context));
   callback.Run(std::move(model));
 }
 
 // static
 void CertificateManagerModel::DidGetCertDBOnIOThread(
     CertificateManagerModel::Observer* observer,
+    content::BrowserContext* browser_context,
     const CreationCallback& callback,
     net::NSSCertDatabase* cert_db) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -250,6 +303,7 @@ void CertificateManagerModel::DidGetCertDBOnIOThread(
                  is_user_db_available,
                  is_tpm_available,
                  observer,
+                 browser_context,
                  callback));
 }
 
@@ -257,13 +311,16 @@ void CertificateManagerModel::DidGetCertDBOnIOThread(
 void CertificateManagerModel::GetCertDBOnIOThread(
     content::ResourceContext* context,
     CertificateManagerModel::Observer* observer,
+    content::BrowserContext* browser_context,
     const CreationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   net::NSSCertDatabase* cert_db = GetNSSCertDatabaseForResourceContext(
       context,
       base::Bind(&CertificateManagerModel::DidGetCertDBOnIOThread,
                  observer,
+                 browser_context,
                  callback));
+
   if (cert_db)
-    DidGetCertDBOnIOThread(observer, callback, cert_db);
+    DidGetCertDBOnIOThread(observer, browser_context, callback, cert_db);
 }