[LG-423] Redirect piv/cac errors to cleanup url

app/controllers/two_factor_authentication/piv_cac_verification_controller.rb

@@ -46,11 +46,18 @@ def next_step
 
     def handle_invalid_piv_cac
       clear_piv_cac_information
-      # create new nonce for retry
-      create_piv_cac_nonce
       handle_invalid_otp(type: 'piv_cac')
     end
 
+    # This overrides the method in TwoFactorAuthenticatable so that we
+    # redirect back to ourselves rather than rendering the :show template.
+    # This removes the token from the address bar and preserves the error
+    # in the flash.
+    def render_show_after_invalid
+      flash[:error] = flash.now[:error]
+      redirect_to login_two_factor_piv_cac_url
+    end
+
     def piv_cac_view_data
       {
         two_factor_authentication_method: two_factor_authentication_method,

app/controllers/users/piv_cac_authentication_setup_controller.rb

@@ -13,10 +13,10 @@ class PivCacAuthenticationSetupController < ApplicationController
     def new
       if params.key?(:token)
         process_piv_cac_setup
+      elsif flash[:error_type].present?
+        render_error
       else
-        analytics.track_event(Analytics::USER_REGISTRATION_PIV_CAC_SETUP_VISIT)
-        @presenter = PivCacAuthenticationSetupPresenter.new(user_piv_cac_form)
-        render :new
+        render_prompt
       end
     end
 
@@ -36,6 +36,17 @@ def redirect_to_piv_cac_service
 
     private
 
+    def render_prompt
+      analytics.track_event(Analytics::USER_REGISTRATION_PIV_CAC_SETUP_VISIT)
+      @presenter = PivCacAuthenticationSetupPresenter.new(user_piv_cac_form)
+      render :new
+    end
+
+    def render_error
+      @presenter = PivCacAuthenticationSetupErrorPresenter.new(error: flash[:error_type])
+      render :error
+    end
+
     def two_factor_enabled?
       current_user.two_factor_enabled?
     end
@@ -73,9 +84,9 @@ def next_step
     end
 
     def process_invalid_submission
-      @presenter = PivCacAuthenticationSetupErrorPresenter.new(user_piv_cac_form)
       clear_piv_cac_information
-      render :error
+      flash[:error_type] = user_piv_cac_form.error_type
+      redirect_to setup_piv_cac_url
     end
 
     def authorize_piv_cac_disable

app/presenters/piv_cac_authentication_setup_error_presenter.rb

@@ -1,6 +1,8 @@
 class PivCacAuthenticationSetupErrorPresenter < PivCacAuthenticationSetupBasePresenter
-  def error
-    form.error_type
+  attr_accessor :error
+
+  def initialize(error:)
+    @error = error
   end
 
   def may_select_another_certificate?

app/views/users/piv_cac_authentication_setup/error.html.slim

@@ -1,5 +1,3 @@
-- title @presenter.title
-
 div class='alert alert-error' role='alert'
   = @presenter.title
 
@@ -11,10 +9,9 @@ p.mt-tiny.mb3 = @presenter.description
 
 - cancel = sign_up_or_idv_no_js_link || link
 .mt2.pt1.border-top
-  - if @presenter.may_select_another_certificate?
-    = link_to t('forms.piv_cac_setup.choose_different_certificate'),
-              setup_piv_cac_url, class: 'h5'
-    br
+  = link_to t('forms.piv_cac_setup.choose_different_certificate'),
+            setup_piv_cac_url, class: 'h5'
+  br
   - if user_signing_up? || user_verifying_identity?
     - method = user_signing_up? ? :delete : :get
 
@@ -25,5 +22,3 @@ p.mt-tiny.mb3 = @presenter.description
       user_signing_up: user_signing_up?
   - else
     = link_to cancel_link_text, cancel, class: 'h5'
-
-== javascript_pack_tag 'clipboard'

spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb

@@ -102,8 +102,8 @@
         expect(subject.current_user.reload.second_factor_attempts_count).to eq 1
       end
 
-      it 're-renders the piv/cac entry screen' do
-        expect(response).to render_template(:show)
+      it 'redirects to the piv/cac entry screen' do
+        expect(response).to redirect_to login_two_factor_piv_cac_path
       end
 
       it 'displays flash error message' do
@@ -126,8 +126,8 @@
         expect(subject.current_user.reload.second_factor_attempts_count).to eq 1
       end
 
-      it 're-renders the piv/cac entry screen' do
-        expect(response).to render_template(:show)
+      it 'redirects to the piv/cac entry screen' do
+        expect(response).to redirect_to login_two_factor_piv_cac_path
       end
 
       it 'displays flash error message' do

spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb

@@ -111,7 +111,7 @@
         context 'when redirected with an error token' do
           it 'renders the error template' do
             get :new, params: { token: bad_token }
-            expect(response).to render_template(:error)
+            expect(response).to redirect_to setup_piv_cac_path
           end
 
           it 'resets the piv/cac session information' do

spec/features/users/piv_cac_management_spec.rb

@@ -58,6 +58,40 @@ def find_form(page, attributes)
         expect(user.x509_dn_uuid).to eq uuid
       end
 
+      scenario 'displays error for a bad piv/cac' do
+        stub_piv_cac_service
+
+        sign_in_and_2fa_user(user)
+        visit account_path
+        click_link t('forms.buttons.enable'), href: setup_piv_cac_url
+
+        expect(page).to have_link(t('forms.piv_cac_setup.submit'))
+
+        nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_setup.submit')))
+        visit_piv_cac_service(setup_piv_cac_url,
+                              nonce: nonce,
+                              error: 'certificate.bad')
+        expect(current_path).to eq setup_piv_cac_path
+        expect(page).to have_content(t('headings.piv_cac_setup.certificate.bad'))
+      end
+
+      scenario 'displays error for an expired piv/cac' do
+        stub_piv_cac_service
+
+        sign_in_and_2fa_user(user)
+        visit account_path
+        click_link t('forms.buttons.enable'), href: setup_piv_cac_url
+
+        expect(page).to have_link(t('forms.piv_cac_setup.submit'))
+
+        nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_setup.submit')))
+        visit_piv_cac_service(setup_piv_cac_url,
+                              nonce: nonce,
+                              error: 'certificate.expired')
+        expect(current_path).to eq setup_piv_cac_path
+        expect(page).to have_content(t('headings.piv_cac_setup.certificate.expired'))
+      end
+
       scenario "doesn't allow unassociation of a piv/cac" do
         stub_piv_cac_service
 
@@ -109,7 +143,7 @@ def find_form(page, attributes)
         PivCacService.send(:reset_piv_cac_avaialable_agencies)
       end
 
-      scenario 'does not allow association of a piv/cac with an account' do
+      scenario "doesn't advertise association of a piv/cac with an account" do
         stub_piv_cac_service
 
         sign_in_and_2fa_user(user)

spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb

@@ -1,17 +1,12 @@
 require 'rails_helper'
 
 describe PivCacAuthenticationSetupErrorPresenter do
-  let(:presenter) { described_class.new(form) }
-  let(:form) do
-    OpenStruct.new(
-      error_type: error
-    )
-  end
+  let(:presenter) { described_class.new(error: error) }
   let(:error) { 'certificate.none' }
 
   describe '#error' do
     it 'reflects the form' do
-      expect(presenter.error).to eq form.error_type
+      expect(presenter.error).to eq error
     end
   end