diff --git a/app/controllers/concerns/piv_cac_concern.rb b/app/controllers/concerns/piv_cac_concern.rb new file mode 100644 index 00000000000..29c528917bb --- /dev/null +++ b/app/controllers/concerns/piv_cac_concern.rb @@ -0,0 +1,15 @@ +module PivCacConcern + extend ActiveSupport::Concern + + def create_piv_cac_nonce + user_session[:piv_cac_nonce] = SecureRandom.base64(20) + end + + def piv_cac_nonce + user_session[:piv_cac_nonce] + end + + def clear_piv_cac_nonce + user_session[:piv_cac_nonce] = nil + end +end diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb index fa0040ae5dc..4cbcf075681 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -1,6 +1,7 @@ module TwoFactorAuthentication class PivCacVerificationController < ApplicationController include TwoFactorAuthenticatable + include PivCacConcern before_action :confirm_piv_cac_enabled before_action :reset_attempt_count_if_user_no_longer_locked_out, only: :show @@ -9,8 +10,7 @@ def show if params[:token] process_token else - @piv_cac_nonce = SecureRandom.base64(10) - user_session[:piv_cac_nonce] = @piv_cac_nonce + create_piv_cac_nonce @presenter = presenter_for_two_factor_authentication_method end end @@ -21,8 +21,11 @@ def process_token result = piv_cac_verfication_form.submit analytics.track_event(Analytics::MULTI_FACTOR_AUTH, result.to_h.merge(analytics_properties)) if result.success? + clear_piv_cac_nonce handle_valid_piv_cac else + # create new nonce for retry + create_piv_cac_nonce handle_invalid_otp(type: 'piv_cac') end end @@ -39,6 +42,8 @@ def piv_cac_view_data two_factor_authentication_method: two_factor_authentication_method, user_email: current_user.email, remember_device_available: false, + totp_enabled: current_user.totp_enabled?, + piv_cac_nonce: piv_cac_nonce, }.merge(generic_data) end @@ -46,7 +51,7 @@ def piv_cac_verfication_form @piv_cac_verification_form ||= UserPivCacVerificationForm.new( user: current_user, token: params[:token], - nonce: user_session[:piv_cac_nonce] + nonce: piv_cac_nonce ) end diff --git a/app/controllers/users/piv_cac_authentication_setup_controller.rb b/app/controllers/users/piv_cac_authentication_setup_controller.rb index 6dbe0a69bc8..a84685a8313 100644 --- a/app/controllers/users/piv_cac_authentication_setup_controller.rb +++ b/app/controllers/users/piv_cac_authentication_setup_controller.rb @@ -1,6 +1,7 @@ module Users class PivCacAuthenticationSetupController < ApplicationController include UserAuthenticator + include PivCacConcern before_action :confirm_two_factor_authenticated before_action :authorize_piv_cac_setup, only: :new @@ -12,8 +13,8 @@ def new else # add a nonce that we track for the return analytics.track_event(Analytics::USER_REGISTRATION_PIV_CAC_SETUP_VISIT) - @piv_cac_nonce = SecureRandom.base64(10) - user_session[:piv_cac_nonce] = @piv_cac_nonce + create_piv_cac_nonce + @presenter = PivCacAuthenticationSetupPresenter.new(user_piv_cac_form) render :new end end @@ -42,7 +43,7 @@ def user_piv_cac_form @user_piv_cac_form ||= UserPivCacSetupForm.new( user: current_user, token: params[:token], - nonce: user_session[:piv_cac_nonce] + nonce: piv_cac_nonce ) end @@ -52,7 +53,8 @@ def process_valid_submission end def process_invalid_submission - @presenter = PivCacAuthenticationSetupPresenter.new(user_piv_cac_form) + create_piv_cac_nonce + @presenter = PivCacAuthenticationSetupErrorPresenter.new(user_piv_cac_form) render :error end diff --git a/app/presenters/piv_cac_authentication_setup_base_presenter.rb b/app/presenters/piv_cac_authentication_setup_base_presenter.rb new file mode 100644 index 00000000000..e51ec8a7387 --- /dev/null +++ b/app/presenters/piv_cac_authentication_setup_base_presenter.rb @@ -0,0 +1,22 @@ +class PivCacAuthenticationSetupBasePresenter + include Rails.application.routes.url_helpers + include ActionView::Helpers::TranslationHelper + + attr_reader :form + + def initialize(form) + @form = form + end + + def piv_cac_nonce + @form.nonce + end + + def piv_cac_capture_text + t('forms.piv_cac_setup.submit') + end + + def piv_cac_service_link + PivCacService.piv_cac_service_link(piv_cac_nonce) + end +end diff --git a/app/presenters/piv_cac_authentication_setup_error_presenter.rb b/app/presenters/piv_cac_authentication_setup_error_presenter.rb new file mode 100644 index 00000000000..0b00fbdbb84 --- /dev/null +++ b/app/presenters/piv_cac_authentication_setup_error_presenter.rb @@ -0,0 +1,22 @@ +class PivCacAuthenticationSetupErrorPresenter < PivCacAuthenticationSetupBasePresenter + def error + form.error_type + end + + def may_select_another_certificate? + error.start_with?('certificate.') && error != 'certificate.none' || + error == 'token.invalid' || error == 'piv_cac.already_associated' + end + + def title + t("titles.piv_cac_setup.#{error}") + end + + def heading + t("headings.piv_cac_setup.#{error}") + end + + def description + t("forms.piv_cac_setup.#{error}_html") + end +end diff --git a/app/presenters/piv_cac_authentication_setup_presenter.rb b/app/presenters/piv_cac_authentication_setup_presenter.rb index f83d1ae2aea..493f6aea1f5 100644 --- a/app/presenters/piv_cac_authentication_setup_presenter.rb +++ b/app/presenters/piv_cac_authentication_setup_presenter.rb @@ -1,29 +1,13 @@ -class PivCacAuthenticationSetupPresenter - include Rails.application.routes.url_helpers - include ActionView::Helpers::TranslationHelper - - def initialize(form) - @form = form - end - - def error - @form.error_type - end - - def may_select_another_certificate? - error.start_with?('certificate.') && error != 'certificate.none' || - error == 'token.invalid' || error == 'piv_cac.already_associated' - end - +class PivCacAuthenticationSetupPresenter < PivCacAuthenticationSetupBasePresenter def title - t("titles.piv_cac_setup.#{error}") + t('titles.piv_cac_setup.new') end def heading - t("headings.piv_cac_setup.#{error}") + t('headings.piv_cac_setup.new') end def description - t("forms.piv_cac_setup.#{error}_html") + t('forms.piv_cac_setup.piv_cac_intro_html') end end diff --git a/app/presenters/two_factor_auth_code/piv_cac_authentication_presenter.rb b/app/presenters/two_factor_auth_code/piv_cac_authentication_presenter.rb index bb5d6c969e6..6a1d7017347 100644 --- a/app/presenters/two_factor_auth_code/piv_cac_authentication_presenter.rb +++ b/app/presenters/two_factor_auth_code/piv_cac_authentication_presenter.rb @@ -20,6 +20,7 @@ def piv_cac_capture_text def fallback_links [ otp_fallback_options, + auth_app_fallback, personal_key_link, ].compact end @@ -33,9 +34,13 @@ def cancel_link end end + def piv_cac_service_link + PivCacService.piv_cac_service_link(piv_cac_nonce) + end + private - attr_reader :user_email, :two_factor_authentication_method + attr_reader :user_email, :two_factor_authentication_method, :totp_enabled, :piv_cac_nonce def otp_fallback_options t( @@ -48,16 +53,25 @@ def otp_fallback_options def sms_link view.link_to( t('devise.two_factor_authentication.totp_fallback.sms_link_text'), - otp_send_path(locale: LinkLocaleResolver.locale, otp_delivery_selection_form: - { otp_delivery_preference: 'sms' }) + login_two_factor_path(locale: LinkLocaleResolver.locale, otp_delivery_preference: 'sms') ) end def voice_link view.link_to( t('devise.two_factor_authentication.totp_fallback.voice_link_text'), - otp_send_path(locale: LinkLocaleResolver.locale, otp_delivery_selection_form: - { otp_delivery_preference: 'voice' }) + login_two_factor_path(locale: LinkLocaleResolver.locale, otp_delivery_preference: 'voice') + ) + end + + def auth_app_fallback + safe_join([auth_app_fallback_tag, '.']) if totp_enabled + end + + def auth_app_fallback_tag + view.link_to( + t('links.two_factor_authentication.app'), + login_two_factor_authenticator_path(locale: LinkLocaleResolver.locale) ) end end diff --git a/app/services/piv_cac_service.rb b/app/services/piv_cac_service.rb index 18dcb298964..8bd37558c34 100644 --- a/app/services/piv_cac_service.rb +++ b/app/services/piv_cac_service.rb @@ -43,7 +43,7 @@ def token_decoded(token) end def decode_token_response(res) - return { 'error' => 'token.bad' } unless res.code == '200' + return { 'error' => 'token.bad' } unless res.code.to_i == 200 result = res.body JSON.parse(result) rescue JSON::JSONError diff --git a/app/views/two_factor_authentication/piv_cac_verification/show.html.slim b/app/views/two_factor_authentication/piv_cac_verification/show.html.slim index 51a135e418c..eb7143187c1 100644 --- a/app/views/two_factor_authentication/piv_cac_verification/show.html.slim +++ b/app/views/two_factor_authentication/piv_cac_verification/show.html.slim @@ -4,7 +4,7 @@ h1.h3.my0 = @presenter.header p.mt-tiny.mb3 = @presenter.help_text = link_to @presenter.piv_cac_capture_text, - PivCacService.piv_cac_service_link(@piv_cac_nonce), + @presenter.piv_cac_service_link, class: 'btn btn-primary' = render 'shared/fallback_links', presenter: @presenter diff --git a/app/views/users/piv_cac_authentication_setup/new.html.slim b/app/views/users/piv_cac_authentication_setup/new.html.slim index c85076406fa..c62a521abdf 100644 --- a/app/views/users/piv_cac_authentication_setup/new.html.slim +++ b/app/views/users/piv_cac_authentication_setup/new.html.slim @@ -1,10 +1,10 @@ -- title t('titles.piv_cac_setup.new') +- title @presenter.title -h1.h3.my0 = t('headings.piv_cac_setup.new') -p.mt-tiny.mb3 = t('forms.piv_cac_setup.piv_cac_intro_html') +h1.h3.my0 = @presenter.heading +p.mt-tiny.mb3 = @presenter.description -= link_to t('forms.piv_cac_setup.submit'), - PivCacService.piv_cac_service_link(@piv_cac_nonce), += link_to @presenter.piv_cac_capture_text, + @presenter.piv_cac_service_link, class: 'btn btn-primary' = render 'shared/cancel', link: account_path diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index 3c202afd25c..09f748259cd 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -490,6 +490,95 @@ def submit_prefilled_otp_code end end + describe 'when the user is PIV/CAC enabled' do + it 'allows SMS and Voice fallbacks' do + user = user_with_piv_cac + sign_in_before_2fa(user) + + click_link t('devise.two_factor_authentication.piv_cac_fallback.link') + + expect(current_path).to eq login_two_factor_piv_cac_path + + expect(page).not_to have_link(t('links.two_factor_authentication.app')) + + click_link t('devise.two_factor_authentication.totp_fallback.sms_link_text') + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') + + visit login_two_factor_piv_cac_path + + click_link t('devise.two_factor_authentication.totp_fallback.voice_link_text') + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'voice') + end + + it 'allows totp fallback when configured' do + user = create(:user, :signed_up, :with_piv_or_cac, otp_secret_key: 'foo') + sign_in_before_2fa(user) + + click_link t('devise.two_factor_authentication.piv_cac_fallback.link') + + expect(current_path).to eq login_two_factor_piv_cac_path + + click_link t('links.two_factor_authentication.app') + + expect(current_path).to eq login_two_factor_authenticator_path + end + + scenario 'user can cancel PIV/CAC process' do + user = create(:user, :signed_up, :with_piv_or_cac) + sign_in_before_2fa(user) + click_link t('devise.two_factor_authentication.piv_cac_fallback.link') + + expect(current_path).to eq login_two_factor_piv_cac_path + click_link t('links.cancel') + + expect(current_path).to eq root_path + end + + scenario 'user uses PIV/CAC as their second factor' do + stub_piv_cac_service + + user = user_with_piv_cac + sign_in_before_2fa(user) + + nonce = visit_login_two_factor_piv_cac_and_get_nonce + + visit_piv_cac_service(login_two_factor_piv_cac_path, { + uuid: user.x509_dn_uuid, + dn: "C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234", + nonce: nonce + }) + expect(current_path).to eq account_path + end + + scenario 'user uses incorrect PIV/CAC as their second factor' do + stub_piv_cac_service + + user = user_with_piv_cac + sign_in_before_2fa(user) + + nonce = visit_login_two_factor_piv_cac_and_get_nonce + + visit_piv_cac_service(login_two_factor_piv_cac_path, { + uuid: user.x509_dn_uuid + 'X', + dn: "C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.12345", + nonce: nonce + }) + expect(current_path).to eq login_two_factor_piv_cac_path + expect(page).to have_content(t("devise.two_factor_authentication.invalid_piv_cac")) + end + end + + describe 'when the user is not piv/cac enabled' do + it 'has no link to piv/cac during login' do + user = create(:user, :signed_up) + sign_in_before_2fa(user) + + expect(page).not_to have_link(t('devise.two_factor_authentication.piv_cac_fallback.link')) + end + end + describe 'when the user is TOTP enabled' do it 'allows SMS and Voice fallbacks' do user = create(:user, :signed_up, otp_secret_key: 'foo') diff --git a/spec/features/users/piv_cac_management_spec.rb b/spec/features/users/piv_cac_management_spec.rb new file mode 100644 index 00000000000..877256dbfaa --- /dev/null +++ b/spec/features/users/piv_cac_management_spec.rb @@ -0,0 +1,85 @@ +require 'rails_helper' + +feature 'PIV/CAC Management' do + + def find_form(page, attributes) + page.all('form').detect do |form| + attributes.all? { |key, value| form[key] == value } + end + end + + context 'with no piv/cac associated yet' do + let(:uuid) { SecureRandom.uuid } + + scenario 'allows association of a piv/cac with an account' do + stub_piv_cac_service + + user = create(:user, :signed_up, phone: '+1 202-555-1212') + sign_in_and_2fa_user(user) + visit account_path + expect(page).to have_link(t('forms.buttons.enable'), href: setup_piv_cac_url) + + visit 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, + uuid: uuid, + subject: 'SomeIgnoredSubject' + }) + + expect(current_path).to eq account_path + + form = find_form(page, action: disable_piv_cac_url) + expect(form).to_not be_nil + expect(page).not_to have_link(t('forms.buttons.enable'), href: setup_piv_cac_url) + + user.reload + expect(user.x509_dn_uuid).to eq uuid + end + + scenario "doesn't allow unassociation of a piv/cac" do + stub_piv_cac_service + + user = create(:user, :signed_up, phone: '+1 202-555-1212') + sign_in_and_2fa_user(user) + visit account_path + form = find_form(page, action: disable_piv_cac_url) + expect(form).to be_nil + end + end + + context 'with a piv/cac associated' do + scenario "doesn't allow association of another piv/cac with the account" do + stub_piv_cac_service + + user = create(:user, :signed_up, :with_piv_or_cac, phone: '+1 202-555-1212') + sign_in_and_2fa_user(user) + visit account_path + expect(page).not_to have_link(t('forms.buttons.enable'), href: setup_piv_cac_url) + end + + scenario 'allows disassociation of the piv/cac' do + stub_piv_cac_service + + user = create(:user, :signed_up, :with_piv_or_cac, phone: '+1 202-555-1212') + sign_in_and_2fa_user(user) + visit account_path + + form = find_form(page, action: disable_piv_cac_url) + expect(form).to_not be_nil + + form.click_button(t('forms.buttons.disable')) + + expect(current_path).to eq account_path + + form = find_form(page, action: disable_piv_cac_url) + expect(form).to be_nil + expect(page).to have_link(t('forms.buttons.enable'), href: setup_piv_cac_url) + + user.reload + expect(user.x509_dn_uuid).to be_nil + end + end +end diff --git a/spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb b/spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb new file mode 100644 index 00000000000..121595efaa6 --- /dev/null +++ b/spec/presenters/piv_cac_authentication_setup_error_presenter_spec.rb @@ -0,0 +1,56 @@ +require 'rails_helper' + +describe PivCacAuthenticationSetupErrorPresenter do + let(:presenter) { described_class.new(form) } + let(:form) do + OpenStruct.new( + error_type: error + ) + end + let(:error) { 'certificate.none' } + + describe '#error' do + it 'reflects the form' do + expect(presenter.error).to eq form.error_type + end + end + + describe '#may_select_another_certificate?' do + let(:may_select_another_certificate) { presenter.may_select_another_certificate? } + context 'when token.invalid' do + let(:error) { 'token.invalid' } + + it { expect(may_select_another_certificate).to be_truthy } + end + + context 'not when certificate.none' do + let(:error) { 'certificate.none' } + + it { expect(may_select_another_certificate).to be_falsey } + end + + context 'when certificate.*' do + let(:error) { 'certificate.revoked' } + + it { expect(may_select_another_certificate).to be_truthy } + end + end + + describe '#title' do + let(:expected_title) { t('titles.piv_cac_setup.' + error ) } + + it { expect(presenter.title).to eq expected_title } + end + + describe '#heading' do + let(:expected_heading) { t('headings.piv_cac_setup.' + error ) } + + it { expect(presenter.heading).to eq expected_heading } + end + + describe '#description' do + let(:expected_description) { t('forms.piv_cac_setup.' + error + '_html') } + + it { expect(presenter.description).to eq expected_description } + end +end diff --git a/spec/presenters/piv_cac_authentication_setup_presenter_spec.rb b/spec/presenters/piv_cac_authentication_setup_presenter_spec.rb index 307c50eaafd..ecbb20ff8fa 100644 --- a/spec/presenters/piv_cac_authentication_setup_presenter_spec.rb +++ b/spec/presenters/piv_cac_authentication_setup_presenter_spec.rb @@ -4,52 +4,23 @@ let(:presenter) { described_class.new(form) } let(:form) do OpenStruct.new( - error_type: error ) end - let(:error) { 'certificate.none' } - - describe '#error' do - it 'reflects the form' do - expect(presenter.error).to eq form.error_type - end - end - - describe '#may_select_another_certificate?' do - let(:may_select_another_certificate) { presenter.may_select_another_certificate? } - context 'when token.invalid' do - let(:error) { 'token.invalid' } - - it { expect(may_select_another_certificate).to be_truthy } - end - - context 'not when certificate.none' do - let(:error) { 'certificate.none' } - - it { expect(may_select_another_certificate).to be_falsey } - end - - context 'when certificate.*' do - let(:error) { 'certificate.revoked' } - - it { expect(may_select_another_certificate).to be_truthy } - end - end describe '#title' do - let(:expected_title) { t('titles.piv_cac_setup.' + error ) } + let(:expected_title) { t('titles.piv_cac_setup.new' ) } it { expect(presenter.title).to eq expected_title } end describe '#heading' do - let(:expected_heading) { t('headings.piv_cac_setup.' + error ) } + let(:expected_heading) { t('headings.piv_cac_setup.new' ) } it { expect(presenter.heading).to eq expected_heading } end describe '#description' do - let(:expected_description) { t('forms.piv_cac_setup.' + error + '_html') } + let(:expected_description) { t('forms.piv_cac_setup.piv_cac_intro_html') } it { expect(presenter.description).to eq expected_description } end diff --git a/spec/presenters/two_factor_auth_code/piv_cac_authentication_presenter_spec.rb b/spec/presenters/two_factor_auth_code/piv_cac_authentication_presenter_spec.rb index 1b87ae78705..5ad624c5fef 100644 --- a/spec/presenters/two_factor_auth_code/piv_cac_authentication_presenter_spec.rb +++ b/spec/presenters/two_factor_auth_code/piv_cac_authentication_presenter_spec.rb @@ -4,6 +4,10 @@ include Rails.application.routes.url_helpers include ActionView::Helpers::TagHelper + def presenter_with(arguments = {}, view = ActionController::Base.new.view_context) + TwoFactorAuthCode::PivCacAuthenticationPresenter.new(data: arguments, view: view) + end + let(:user_email) { 'user@example.com' } let(:reauthn) { } let(:presenter) { presenter_with(reauthn: reauthn, user_email: user_email) } diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index e7babdcfb7e..84a0b2c871f 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -1,3 +1,5 @@ +require 'cgi' + module Features module SessionHelper VALID_PASSWORD = 'Val!d Pass w0rd'.freeze @@ -97,6 +99,13 @@ def user_with_2fa create(:user, :signed_up, phone: '+1 (555) 555-0000', password: VALID_PASSWORD) end + def user_with_piv_cac + create(:user, :signed_up, :with_piv_or_cac, + phone: '+1 (555) 555-0000', + password: VALID_PASSWORD + ) + end + def confirm_last_user @raw_confirmation_token, = Devise.token_generator.generate(User, :confirmation_token) @@ -120,6 +129,18 @@ def sign_in_live_with_2fa(user = user_with_2fa) user end + def sign_in_live_with_piv_cac(user = user_with_piv_cac) + sign_in_user(user) + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + allow(FeatureManagement).to receive(:development_and_piv_cac_entry_enabled?).and_return(true) + visit login_two_factor_piv_cac_path + stub_piv_cac_service + visit_piv_cac_service( + dn: "C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234", + uuid: user.x509_dn_uuid, + ) + end + def click_submit_default click_button t('forms.buttons.submit.default') end @@ -395,6 +416,32 @@ def stub_twilio_service allow(TwilioService).to receive(:new).and_return(twilio_service) end + def stub_piv_cac_service + allow(Figaro.env).to receive(:identity_pki_disabled).and_return('false') + allow(Figaro.env).to receive(:piv_cac_enabled).and_return('true') + allow(Figaro.env).to receive(:piv_cac_service_url).and_return('http://piv.example.com/') + allow(Figaro.env).to receive(:piv_cac_verify_token_url).and_return('http://piv.example.com/') + stub_request(:post, 'piv.example.com').to_return do |request| + { + status: 200, + body: CGI.unescape(request.body.sub(/^token=/, '')), + } + end + end + + def visit_piv_cac_service(idp_url, token_data) + visit(idp_url + '?token=' + CGI.escape(token_data.to_json)) + end + + def visit_login_two_factor_piv_cac_and_get_nonce + visit login_two_factor_piv_cac_path + get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_mfa.submit'))) + end + + def get_piv_cac_nonce_from_link(link) + CGI.unescape(URI(link['href']).query.sub(/^nonce=/, '')) + end + def link_identity(user, client_id, ial = nil) IdentityLinker.new( user,