diff --git a/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb b/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb index 16c6733047f..99e37c8ad16 100644 --- a/app/controllers/users/piv_cac_setup_from_sign_in_controller.rb +++ b/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 diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index e68db8eb358..1f15da5ac78 100644 --- a/app/services/analytics_events.rb +++ b/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, diff --git a/config/routes.rb b/config/routes.rb index cd0391f6206..72b9d21ffa0 100644 --- a/config/routes.rb +++ b/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' diff --git a/spec/features/sign_in/setup_piv_cac_after_sign_in_spec.rb b/spec/features/sign_in/setup_piv_cac_after_sign_in_spec.rb new file mode 100644 index 00000000000..d41afebdc5c --- /dev/null +++ b/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 diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 20fceb7bd6b..6b5495a85f7 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/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 diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index d8dc562604b..f1b63bfd743 100644 --- a/spec/support/features/session_helper.rb +++ b/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)