Align PIV/CAC setup-after-sign-in specs to user behavior

app/controllers/users/piv_cac_setup_from_sign_in_controller.rb

@@ -2,85 +2,20 @@
 
 module Users
   class PivCacSetupFromSignInController < ApplicationController
-    include TwoFactorAuthenticatableMethods
     include PivCacConcern
-    include SecureHeadersConcern
     include ReauthenticationRequiredConcern
 
     before_action :confirm_two_factor_authenticated
     before_action :confirm_recently_authenticated_2fa
-    before_action :apply_secure_headers_override, only: :success
     before_action :set_piv_cac_setup_csp_form_action_uris, only: :prompt
 
     def prompt
-      if params.key?(:token)
-        process_piv_cac_setup
-      else
-        render_prompt
-      end
-    end
-
-    def success; end
-
-    def next
-      redirect_to after_sign_in_path_for(current_user)
+      analytics.piv_cac_setup_visited(in_account_creation_flow: false)
     end
 
     def decline
       session.delete(:needs_to_setup_piv_cac_after_sign_in)
       redirect_to after_sign_in_path_for(current_user)
     end
-
-    private
-
-    def render_prompt
-      analytics.piv_cac_setup_visited(in_account_creation_flow: false)
-      render :prompt
-    end
-
-    def process_piv_cac_setup
-      result = user_piv_cac_form.submit
-      properties = result.to_h.merge(analytics_properties)
-      analytics.multi_factor_auth_setup(**properties)
-      if result.success?
-        process_valid_submission
-      else
-        process_invalid_submission
-      end
-    end
-
-    def user_piv_cac_form
-      @user_piv_cac_form ||= UserPivCacSetupForm.new(
-        user: current_user,
-        token: params[:token],
-        nonce: piv_cac_nonce,
-        name: user_session[:piv_cac_nickname],
-      )
-    end
-
-    def process_invalid_submission
-      redirect_to login_piv_cac_error_url(error: user_piv_cac_form.error_type)
-    end
-
-    def process_valid_submission
-      handle_valid_verification_for_confirmation_context(
-        auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC,
-      )
-      session.delete(:needs_to_setup_piv_cac_after_sign_in)
-      save_piv_cac_information(
-        subject: user_piv_cac_form.x509_dn,
-        issuer: user_piv_cac_form.x509_issuer,
-        presented: true,
-      )
-      create_user_event(:piv_cac_enabled)
-      redirect_to login_add_piv_cac_success_url
-    end
-
-    def analytics_properties
-      {
-        in_account_creation_flow: false,
-        enabled_mfa_methods_count: MfaContext.new(current_user).enabled_mfa_methods_count,
-      }
-    end
   end
 end

app/services/analytics_events.rb

@@ -4125,12 +4125,17 @@ def multi_factor_auth_added_phone(
   # Tracks when the user has added the MFA method piv_cac to their account
   # @param [Integer] enabled_mfa_methods_count number of registered mfa methods for the user
   # @param [Boolean] in_account_creation_flow whether user is going through creation flow
-  def multi_factor_auth_added_piv_cac(enabled_mfa_methods_count:, in_account_creation_flow:,
-                                      **extra)
+  # @param ['piv_cac'] method_name Authentication method added
+  def multi_factor_auth_added_piv_cac(
+    enabled_mfa_methods_count:,
+    in_account_creation_flow:,
+    method_name: :piv_cac,
+    **extra
+  )
     track_event(
       :multi_factor_auth_added_piv_cac,
       {
-        method_name: :piv_cac,
+        method_name:,
         enabled_mfa_methods_count:,
         in_account_creation_flow:,
         **extra,

config/routes.rb

@@ -142,8 +142,6 @@
 
       get 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#prompt'
       post 'login/add_piv_cac/prompt' => 'users/piv_cac_setup_from_sign_in#decline'
-      get 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#success'
-      post 'login/add_piv_cac/success' => 'users/piv_cac_setup_from_sign_in#next'
       get 'login/piv_cac_recommended' => 'users/piv_cac_recommended#show'
       post 'login/piv_cac_recommended/add' => 'users/piv_cac_recommended#confirm'
       post 'login/piv_cac_recommended/skip' => 'users/piv_cac_recommended#skip'

spec/features/sign_in/setup_piv_cac_after_sign_in_spec.rb

@@ -0,0 +1,82 @@
+require 'rails_helper'
+
+RSpec.describe 'Setup PIV/CAC after sign-in' do
+  include SamlAuthHelper
+
+  scenario 'user opts to not add piv/cac card' do
+    perform_steps_to_get_to_add_piv_cac_during_sign_up
+
+    click_on t('forms.piv_cac_setup.no_thanks')
+
+    expect(page).to have_current_path(sign_up_completed_path)
+  end
+
+  context 'without an associated service provider' do
+    scenario 'user opts to not add piv/cac card' do
+      perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: nil)
+
+      click_on t('forms.piv_cac_setup.no_thanks')
+
+      expect(page).to have_current_path(account_path)
+    end
+  end
+
+  scenario 'user opts to add piv/cac card' do
+    perform_steps_to_get_to_add_piv_cac_during_sign_up
+
+    fill_in t('forms.piv_cac_setup.nickname'), with: 'Card 1'
+    click_on t('forms.piv_cac_setup.submit')
+    follow_piv_cac_redirect
+
+    expect(page).to have_current_path(sign_up_completed_path)
+  end
+
+  scenario 'user opts to add piv/cac card but gets an error' do
+    perform_steps_to_get_to_add_piv_cac_during_sign_up
+
+    fill_in t('forms.piv_cac_setup.nickname'), with: 'Card 1'
+    stub_piv_cac_service(error: 'certificate.bad')
+    click_on t('forms.piv_cac_setup.submit')
+    follow_piv_cac_redirect
+
+    expect(page).to have_current_path(setup_piv_cac_error_path(error: 'certificate.bad'))
+  end
+
+  scenario 'user opts to add piv/cac card and has piv cac redirect in CSP' do
+    allow(Identity::Hostdata).to receive(:env).and_return('test')
+    allow(Identity::Hostdata).to receive(:domain).and_return('example.com')
+
+    perform_steps_to_get_to_add_piv_cac_during_sign_up
+
+    expected_form_action = <<-STR.squish
+      form-action https://*.pivcac.test.example.com 'self'
+      http://localhost:7654 https://example.com
+    STR
+
+    expect(page.response_headers['Content-Security-Policy']).
+      to(include(expected_form_action))
+  end
+
+  def perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: :oidc)
+    user = create(:user, :fully_registered, :with_phone)
+    if sp
+      visit_idp_from_sp_with_ial1(sp)
+    else
+      visit new_user_session_path
+    end
+
+    click_on t('account.login.piv_cac')
+    stub_piv_cac_service
+    click_on t('forms.piv_cac_login.submit')
+
+    follow_piv_cac_redirect
+    expect(page).to have_current_path(login_piv_cac_error_path(error: 'user.not_found'))
+    click_on t('instructions.mfa.piv_cac.back_to_sign_in')
+
+    fill_in_credentials_and_submit(user.email, user.password)
+    fill_in_code_with_last_phone_otp
+    click_submit_default
+    expect(current_path).to eq login_add_piv_cac_prompt_path
+    fill_in t('forms.piv_cac_setup.nickname'), with: 'Card 1'
+  end
+end

spec/features/users/sign_in_spec.rb

@@ -32,20 +32,6 @@
       to have_link t('devise.failure.not_found_in_database_link_text', href: link_url)
   end
 
-  scenario 'user opts to not add piv/cac card' do
-    perform_steps_to_get_to_add_piv_cac_during_sign_up
-    click_on t('forms.piv_cac_setup.no_thanks')
-    expect(current_path).to eq sign_up_completed_path
-  end
-
-  context 'without an associated service provider' do
-    scenario 'user opts to not add piv/cac card' do
-      perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: nil)
-      click_on t('forms.piv_cac_setup.no_thanks')
-      expect(current_path).to eq account_path
-    end
-  end
-
   scenario 'user is suspended, gets show please call page after 2fa' do
     user = create(:user, :fully_registered, :suspended)
     service_provider = ServiceProvider.find_by(issuer: OidcAuthHelper::OIDC_IAL1_ISSUER)
@@ -61,22 +47,6 @@
     expect(current_path).to eq(user_please_call_path)
   end
 
-  scenario 'user opts to add piv/cac card' do
-    perform_steps_to_get_to_add_piv_cac_during_sign_up
-    nonce = piv_cac_nonce_from_form_action
-    visit_piv_cac_service(
-      current_url,
-      nonce: nonce,
-      dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234',
-      uuid: SecureRandom.uuid,
-      subject: 'SomeIgnoredSubject',
-    )
-
-    expect(current_path).to eq login_add_piv_cac_success_path
-    click_continue
-    expect(current_path).to eq sign_up_completed_path
-  end
-
   scenario 'user with old terms of use can accept and continue to IAL1 SP' do
     user = create(
       :user,
@@ -132,36 +102,6 @@
     expect(oidc_redirect_url).to start_with service_provider.redirect_uris.first
   end
 
-  scenario 'user opts to add piv/cac card but gets an error' do
-    perform_steps_to_get_to_add_piv_cac_during_sign_up
-    nonce = piv_cac_nonce_from_form_action
-    visit_piv_cac_service(
-      current_url,
-      nonce: nonce,
-      dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234',
-      uuid: SecureRandom.uuid,
-      error: 'certificate.bad',
-      subject: 'SomeIgnoredSubject',
-    )
-
-    expect(page).to have_current_path(login_piv_cac_error_path(error: 'certificate.bad'))
-  end
-
-  scenario 'user opts to add piv/cac card and has piv cac redirect in CSP' do
-    allow(Identity::Hostdata).to receive(:env).and_return('test')
-    allow(Identity::Hostdata).to receive(:domain).and_return('example.com')
-
-    perform_steps_to_get_to_add_piv_cac_during_sign_up
-
-    expected_form_action = <<-STR.squish
-      form-action https://*.pivcac.test.example.com 'self'
-      http://localhost:7654 https://example.com
-    STR
-
-    expect(page.response_headers['Content-Security-Policy']).
-      to(include(expected_form_action))
-  end
-
   scenario 'User with gov/mil email directed to recommended PIV page' do
     user = create(:user, :with_phone, { email: 'example@example.gov' })
 
@@ -1095,35 +1035,6 @@
     end
   end
 
-  def perform_steps_to_get_to_add_piv_cac_during_sign_up(sp: :oidc)
-    user = create(:user, :fully_registered, :with_phone)
-    if sp
-      visit_idp_from_sp_with_ial1(sp)
-    else
-      visit new_user_session_path
-    end
-    click_on t('account.login.piv_cac')
-    allow(FeatureManagement).to receive(:development_and_identity_pki_disabled?).and_return(false)
-
-    stub_piv_cac_service
-    nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_login.submit')))
-    visit_piv_cac_service(
-      current_url,
-      nonce: nonce,
-      dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234',
-      uuid: SecureRandom.uuid,
-      subject: 'SomeIgnoredSubject',
-    )
-
-    expect(page).to have_current_path(login_piv_cac_error_path(error: 'user.not_found'))
-    visit new_user_session_path
-    fill_in_credentials_and_submit(user.email, user.password)
-    fill_in_code_with_last_phone_otp
-    click_submit_default
-    expect(current_path).to eq login_add_piv_cac_prompt_path
-    fill_in 'name', with: 'Card 1'
-  end
-
   def with_forgery_protection
     original_allow_forgery_protection = ActionController::Base.allow_forgery_protection
     ActionController::Base.allow_forgery_protection = true

spec/support/features/session_helper.rb

@@ -559,7 +559,7 @@ def sign_in_via_branded_page(user)
       click_submit_default
     end
 
-    def stub_piv_cac_service
+    def stub_piv_cac_service(error: nil)
       allow(IdentityConfig.store).to receive(:identity_pki_disabled).and_return(false)
       allow(IdentityConfig.store).to receive(:piv_cac_service_url).
         and_return('http://piv.example.com/')
@@ -570,6 +570,34 @@ def stub_piv_cac_service
           body: CGI.unescape(request.body.sub(/^token=/, '')),
         }
       end
+
+      stub_request(:post, 'piv.example.com').
+        with(query: hash_including('nonce', 'redirect_uri')).
+        to_return do |request|
+          query = UriService.params(request.uri)
+          {
+            status: 302,
+            headers: {
+              location: UriService.add_params(
+                query['redirect_uri'],
+                token: {
+                  dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234',
+                  uuid: SecureRandom.uuid,
+                  subject: 'SomeIgnoredSubject',
+                  nonce: query['nonce'],
+                  error:,
+                }.compact.to_json,
+              ),
+            },
+          }
+        end
+    end
+
+    def follow_piv_cac_redirect
+      # RackTest won't do an external redirect to the stubbed PKI service, but we can manually
+      # submit a request to the `current_url` and get the redirect header.
+      redirect_url = Faraday.post(current_url).headers['location']
+      visit redirect_url
     end
 
     def visit_piv_cac_service(idp_url, token_data)