Certificate cleanup

app/assets/stylesheets/application.css.scss.erb

@@ -93,3 +93,20 @@ $image-path: 'img';
   }
 }
 
+// Override to get <summary> disclosure triangle to render correctly in Chrome
+// This can be removed entirely after LG-3762 upgrades USWDS
+.summary-fix {
+  display: list-item !important;
+}
+
+.certificate-expired,
+.usa-table td.certificate-expired {
+  color: white;
+  background: red;
+}
+
+.certificate-warning,
+.usa-table td.certificate-warning {
+  color: black;
+  background: yellow;
+}

app/models/service_provider.rb

@@ -78,10 +78,11 @@ def redirect_uris=(uris)
     super uris.select(&:present?)
   end
 
+  # @return [ServiceProviderCertificate]
   def certificate
     @certificate ||= begin
       if saml_client_cert
-        ServiceProviderCertificate.new saml_client_cert
+        ServiceProviderCertificate.new(OpenSSL::X509::Certificate.new(saml_client_cert))
       else
         null_certificate
       end

app/models/service_provider_certificate.rb

@@ -1,14 +1,29 @@
-# A class to provide colorized expiration text
-class ServiceProviderCertificate < OpenSSL::X509::Certificate
-  def expiration_time_to_colorized_s
-    self.class.expiration_time_to_colorized_s(not_after)
+# A class to decorate a certificate, for easier warning and expiration
+class ServiceProviderCertificate
+  attr_reader :cert
+
+  # @param [OpenSSL::X509::Certificate,String] cert
+  def initialize(cert)
+    @cert = cert
+  end
+
+  def method_missing(name, *args, &block)
+    if cert.respond_to?(name)
+      cert.send(name, *args, &block)
+    else
+      super
+    end
   end
 
-  def self.expiration_time_to_colorized_s(time)
-    time_s = time.to_s
-    if time < Time.zone.now
+  def respond_to_missing?(name)
+    cert.respond_to_missing?(name) || super
+  end
+
+  def expiration_time_to_colorized_s
+    time_s = not_after.to_s
+    if not_after < Time.zone.now
       time_s.colorize(color: :black, background: :red)
-    elsif time < warning_period
+    elsif not_after < self.class.warning_period
       time_s.colorize(color: :black, background: :light_yellow)
     else
       time_s
@@ -18,4 +33,12 @@ def self.expiration_time_to_colorized_s(time)
   def self.warning_period
     (Figaro.env.certificate_expiration_warning_period || 60).to_i.days.from_now
   end
+
+  def expiration_css_class
+    if not_after < Time.zone.now
+      'certificate-expired'
+    elsif not_after < self.class.warning_period
+      'certificate-warning'
+    end
+  end
 end

app/views/service_providers/_certificate_expiration_td.html.erb

@@ -1,14 +1,12 @@
-<% cert = app.certificate %>
-<% if cert.not_after < Time.zone.now %>
-  <td style='color: white; background: red'>
-<% elsif cert.not_after < ServiceProviderCertificate.warning_period %>
-  <td style='color: black; background: yellow'>
-<% else %>
-  <td>
+<%
+ # locals: app
+ cert = app.certificate
+%>
+
+<%= content_tag(:td, class: cert.expiration_css_class) do %>
+  <% if cert.issuer.to_s == 'Null Certificate' %>
+    Invalid
+  <% else %>
+    <%= cert.not_after.localtime.strftime("%F") %>
+  <% end %>
 <% end %>
-    <% if cert.issuer.to_s == 'Null Certificate' %>
-      Invalid
-    <% else %>
-      <%= cert.not_after.localtime.strftime("%F") %>
-    <% end %>
-  </td>

app/views/service_providers/show.html.erb

@@ -58,27 +58,51 @@
   <% end %>
 <% end %>
 
-    <% if service_provider.identity_protocol == 'saml' %>
-    <h2><label for="acs_url">Assertion Consumer Service URL:</label></h2>
-<p class="font-mono-xs margin-top-0" name="acs_url"><%= service_provider.acs_url %></p>
-
-<h2><label for="assertion_consumer_logout_service_url">Assertion Consumer Logout Service URL:</label></h2>
-<p class="font-mono-xs margin-top-0" name="assertion_consumer_logout_service_url"><%= service_provider.assertion_consumer_logout_service_url %></p>
+<% if service_provider.identity_protocol == 'saml' %>
+  <h2><label for="acs_url">Assertion Consumer Service URL:</label></h2>
+  <p class="font-mono-xs margin-top-0" name="acs_url"><%= service_provider.acs_url %></p>
 
-<h2><label for="assertion_consumer_logout_service_url">SP Initiated Login URL:</label></h2>
-<p class="font-mono-xs margin-top-0" name="assertion_consumer_logout_service_url"><%= service_provider.sp_initiated_login_url %></p>
+  <h2><label for="assertion_consumer_logout_service_url">Assertion Consumer Logout Service URL:</label></h2>
+  <p class="font-mono-xs margin-top-0" name="assertion_consumer_logout_service_url"><%= service_provider.assertion_consumer_logout_service_url %></p>
 
-<h2><label for="block_encryption">SAML Assertion Encryption:</label></h2>
-<p class="font-mono-xs margin-top-0" name="block_encryption"><%= service_provider.block_encryption %></p>
-    <% end %>
+  <h2><label for="assertion_consumer_logout_service_url">SP Initiated Login URL:</label></h2>
+  <p class="font-mono-xs margin-top-0" name="assertion_consumer_logout_service_url"><%= service_provider.sp_initiated_login_url %></p>
 
+  <h2><label for="block_encryption">SAML Assertion Encryption:</label></h2>
+  <p class="font-mono-xs margin-top-0" name="block_encryption"><%= service_provider.block_encryption %></p>
+<% end %>
 
-<h2><label for="saml_client_cert">Public certificate:</label></h2>
-<pre><code><%= service_provider.saml_client_cert %></code></pre>
-        <% unless service_provider.saml_client_cert.blank? %>
-        <p class="font-mono-xs margin-top-0" name="saml_client_cert">Expires:
-        <%= render partial: 'certificate_expiration', locals: { app: service_provider } %></p>
-        <% end %>
+<h2>
+  <label for="saml_client_cert">Public certificate:</label>
+</h2>
+
+<% unless service_provider.saml_client_cert.blank? %>
+  <dl>
+    <dt>Issuer</dt>
+    <dd><%= service_provider.certificate.issuer %></dd>
+
+    <dt>Subject</dt>
+    <dd><%= service_provider.certificate.subject %></dd>
+
+    <dt>Serial Number</dt>
+    <dd class="font-mono-xs"><%= service_provider.certificate.serial %></dd>
+
+    <dt>Expiration</dt>
+    <dd>
+      <%= content_tag(:span, class: ['font-mono-xs', service_provider.certificate.expiration_css_class]) do %>
+        <%= service_provider.certificate.not_after %>
+      <% end %>
+      (<%= time_ago_in_words service_provider.certificate.not_after %>)
+    </dd>
+  </dl>
+
+  <details name="saml_client_cert">
+    <summary class="summary-fix">
+      Raw Certificate
+    </summary>
+    <pre><code><%= service_provider.saml_client_cert %></code></pre>
+  </details>
+<% end %>
 
 <h2><label for="return_to_sp_url">Return to App URL:</label></h2>
 <p class="font-mono-xs margin-top-0" name="return_to_sp_url"><%= service_provider.return_to_sp_url %></p>

lib/tasks/check_certificates.rake

@@ -13,8 +13,7 @@ namespace :dashboard do
 
     puts "\nExpiration                Issuer"
     sps.each do |sp|
-      exp = collection[sp]&.not_after
-      puts "#{ServiceProviderCertificate.expiration_time_to_colorized_s(exp)}:  #{sp}"
+      puts "#{collection[sp]&.expiration_time_to_colorized_s}   #{sp}"
     end
   end
 end

spec/models/service_provider_certificate_spec.rb

@@ -1,29 +1,54 @@
 require 'rails_helper'
 
-describe ServiceProviderCertificate do
+RSpec.describe ServiceProviderCertificate do
   before do
     allow(Figaro.env).to receive(:certificate_expiration_warning_period).and_return('5')
   end
 
+  let(:plain_cert) do
+    instance_double('OpenSSL::X509::Certificate', not_after: not_after)
+  end
+
+  subject(:cert) do
+    ServiceProviderCertificate.new(plain_cert)
+  end
+
   context 'certificate is expired' do
+    let(:not_after) { 1.day.ago }
+
     it 'wraps the expiration in ansi color codes to make it black on red' do
-      expired_time = 1.day.ago
-      expect(ServiceProviderCertificate.expiration_time_to_colorized_s(expired_time)).
-        to match(/\A\e\[0;30;41m#{expired_time.to_s}\e\[0m\z/)
+      expect(cert.expiration_time_to_colorized_s).
+        to match(/\A\e\[0;30;41m#{not_after.to_s}\e\[0m\z/)
+    end
+
+    it 'has an expired CSS style' do
+      expect(cert.expiration_css_class).to eq('certificate-expired')
     end
   end
+
   context 'certificate is near expiration' do
+    let(:not_after) { (5.days - 10.seconds).from_now }
+
     it 'wraps the expiration in ansi color codes to make it black on yellow' do
-      expired_time = (5.days - 10.seconds).from_now
-      expect(ServiceProviderCertificate.expiration_time_to_colorized_s(expired_time)).
-        to match(/\A\e\[0;30;103m#{expired_time.to_s}\e\[0m\z/)
+      expect(cert.expiration_time_to_colorized_s).
+        to match(/\A\e\[0;30;103m#{not_after.to_s}\e\[0m\z/)
+    end
+
+    it 'has a warning CSS style' do
+      expect(cert.expiration_css_class).to eq('certificate-warning')
     end
   end
+
   context 'certificate is not near expiration' do
-    it 'does not wraps the expiration in ansi color codes' do
-      expired_time = (5.days + 10.seconds).from_now
-      expect(ServiceProviderCertificate.expiration_time_to_colorized_s(expired_time)).
-        to match(/\A#{expired_time.to_s}\z/)
+    let(:not_after) { (5.days + 10.seconds).from_now }
+
+    it 'does not wrap the expiration in ansi color codes' do
+      expect(cert.expiration_time_to_colorized_s).
+        to match(/\A#{not_after.to_s}\z/)
+    end
+
+    it 'does not have a CSS style' do
+      expect(cert.expiration_css_class).to be_nil
     end
   end
 end