diff --git a/app/controllers/test/piv_cac_authentication_test_subject_controller.rb b/app/controllers/test/piv_cac_authentication_test_subject_controller.rb index eeea57c35a0..60fa7523534 100644 --- a/app/controllers/test/piv_cac_authentication_test_subject_controller.rb +++ b/app/controllers/test/piv_cac_authentication_test_subject_controller.rb @@ -36,7 +36,8 @@ def must_be_in_development end def token_from_params - error, subject = params.slice(:error, :subject) + error = params[:error] + subject = params[:subject] if error.present? with_nonce(error: error).to_json diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 71ae6bbf3fe..b0d0f07070b 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -26,7 +26,7 @@ def create private def confirm_two_factor_enabled - return if confirmation_context? || current_user.two_factor_enabled? + return if confirmation_context? || current_user.phone_enabled? redirect_to phone_setup_url end diff --git a/app/controllers/users/piv_cac_authentication_setup_controller.rb b/app/controllers/users/piv_cac_authentication_setup_controller.rb index 43c85bcd6ab..f145f8e9621 100644 --- a/app/controllers/users/piv_cac_authentication_setup_controller.rb +++ b/app/controllers/users/piv_cac_authentication_setup_controller.rb @@ -3,7 +3,8 @@ class PivCacAuthenticationSetupController < ApplicationController include UserAuthenticator include PivCacConcern - before_action :confirm_two_factor_authenticated + before_action :authenticate_user! + before_action :confirm_two_factor_authenticated, if: :two_factor_enabled? before_action :authorize_piv_cac_setup, only: :new before_action :authorize_piv_cac_disable, only: :delete @@ -30,6 +31,10 @@ def delete private + def two_factor_enabled? + current_user.two_factor_enabled? + end + def process_piv_cac_setup result = user_piv_cac_form.submit analytics.track_event(Analytics::USER_REGISTRATION_PIV_CAC_ENABLED, result.to_h) diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 1947ab7941c..db1a17a40c7 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -7,8 +7,10 @@ class TwoFactorAuthenticationController < ApplicationController def show if current_user.totp_enabled? redirect_to login_two_factor_authenticator_url - elsif current_user.two_factor_enabled? + elsif current_user.phone_enabled? validate_otp_delivery_preference_and_send_code + elsif current_user.piv_cac_enabled? + redirect_to login_two_factor_piv_cac_url else redirect_to two_factor_options_url end diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 9501da62289..07b65001cda 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -7,6 +7,7 @@ class TwoFactorAuthenticationSetupController < ApplicationController def index @two_factor_options_form = TwoFactorOptionsForm.new(current_user) + @presenter = two_factor_options_presenter analytics.track_event(Analytics::USER_REGISTRATION_2FA_SETUP_VISIT) end @@ -18,12 +19,17 @@ def create if result.success? process_valid_form else + @presenter = two_factor_options_presenter render :index end end private + def two_factor_options_presenter + TwoFactorOptionsPresenter.new(current_user, current_sp) + end + def authorize_2fa_setup if user_fully_authenticated? redirect_to account_url @@ -38,6 +44,8 @@ def process_valid_form redirect_to phone_setup_url when 'auth_app' redirect_to authenticator_setup_url + when 'piv_cac' + redirect_to setup_piv_cac_url end end diff --git a/app/forms/two_factor_options_form.rb b/app/forms/two_factor_options_form.rb index 5473549adb1..6ca9a0b8bec 100644 --- a/app/forms/two_factor_options_form.rb +++ b/app/forms/two_factor_options_form.rb @@ -19,6 +19,10 @@ def submit(params) FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) end + def selected?(type) + type == (selection || 'sms') + end + private attr_accessor :user diff --git a/app/models/service_provider.rb b/app/models/service_provider.rb index 2135fef4b11..2c5ead6cdb3 100644 --- a/app/models/service_provider.rb +++ b/app/models/service_provider.rb @@ -47,6 +47,10 @@ def live? active? && approved? end + def piv_cac_available? + PivCacService.piv_cac_available_for_agency?(agency) + end + private def redirect_uris_are_parsable diff --git a/app/models/user.rb b/app/models/user.rb index 467775908e8..5fec213a7a7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -57,22 +57,23 @@ def confirm_piv_cac?(proposed_uuid) end def piv_cac_enabled? - x509_dn_uuid.present? + FeatureManagement.piv_cac_enabled? && x509_dn_uuid.present? end def piv_cac_available? - FeatureManagement.piv_cac_enabled? && ( - piv_cac_enabled? || - identities.any?(&:piv_cac_available?) - ) + piv_cac_enabled? || identities.any?(&:piv_cac_available?) end def need_two_factor_authentication?(_request) two_factor_enabled? end + def phone_enabled? + phone.present? + end + def two_factor_enabled? - phone.present? || totp_enabled? || piv_cac_enabled? + phone_enabled? || totp_enabled? || piv_cac_enabled? end def send_two_factor_authentication_code(_code) diff --git a/app/presenters/two_factor_options_presenter.rb b/app/presenters/two_factor_options_presenter.rb new file mode 100644 index 00000000000..5ef4a5e360a --- /dev/null +++ b/app/presenters/two_factor_options_presenter.rb @@ -0,0 +1,49 @@ +class TwoFactorOptionsPresenter + include ActionView::Helpers::TranslationHelper + + attr_reader :current_user, :service_provider + + def initialize(current_user, sp) + @current_user = current_user + @service_provider = sp + end + + def title + t('titles.two_factor_setup') + end + + def heading + t('devise.two_factor_authentication.two_factor_choice') + end + + def info + t('devise.two_factor_authentication.two_factor_choice_intro') + end + + def label + t('forms.two_factor_choice.legend') + ':' + end + + def options + available_2fa_types.map do |type| + OpenStruct.new( + type: type, + label: t("devise.two_factor_authentication.two_factor_choice_options.#{type}"), + info: t("devise.two_factor_authentication.two_factor_choice_options.#{type}_info"), + selected: type == :sms + ) + end + end + + private + + def available_2fa_types + %w[sms voice auth_app] + piv_cac_if_available + end + + def piv_cac_if_available + return [] if current_user.piv_cac_enabled? + return [] unless current_user.piv_cac_available? || service_provider&.piv_cac_available? + %w[piv_cac] + end +end diff --git a/app/views/users/two_factor_authentication_setup/index.html.slim b/app/views/users/two_factor_authentication_setup/index.html.slim index c490b3e1f8f..85d136b722f 100644 --- a/app/views/users/two_factor_authentication_setup/index.html.slim +++ b/app/views/users/two_factor_authentication_setup/index.html.slim @@ -1,8 +1,7 @@ -- title t('titles.two_factor_setup') +- title @presenter.title -h1.h3.my0 = t('devise.two_factor_authentication.two_factor_choice') -p.mt-tiny.mb3 - = t('devise.two_factor_authentication.two_factor_choice_intro') +h1.h3.my0 = @presenter.heading +p.mt-tiny.mb3 = @presenter.info = simple_form_for(@two_factor_options_form, html: { autocomplete: 'off', role: 'form' }, @@ -10,31 +9,17 @@ p.mt-tiny.mb3 url: two_factor_options_path) do |f| .mb3 fieldset.m0.p0.border-none. - legend.mb1.h4.serif.bold = t('forms.two_factor_choice.legend') + ':' - label.btn-border.col-12.mb1 for="two_factor_options_form_selection_sms" - .radio - = radio_button_tag 'two_factor_options_form[selection]', :sms, true - span.indicator.mt-tiny - span.blue.bold.fs-20p - = t('devise.two_factor_authentication.two_factor_choice_options.sms') - .regular.gray-dark.fs-10p.mb-tiny - = t('devise.two_factor_authentication.two_factor_choice_options.sms_info') - label.btn-border.col-12.mb1 for="two_factor_options_form_selection_voice" - .radio - = radio_button_tag 'two_factor_options_form[selection]', :voice, false - span.indicator.mt-tiny - span.blue.bold.fs-20p - = t('devise.two_factor_authentication.two_factor_choice_options.voice') - .regular.gray-dark.fs-10p.mb-tiny - = t('devise.two_factor_authentication.two_factor_choice_options.voice_info') - label.btn-border.col-12.mb1 for="two_factor_options_form_selection_auth_app" - .radio - = radio_button_tag 'two_factor_options_form[selection]', :auth_app, false - span.indicator.mt-tiny - span.blue.bold.fs-20p - = t('devise.two_factor_authentication.two_factor_choice_options.auth_app') - .regular.gray-dark.fs-10p.mb-tiny - = t('devise.two_factor_authentication.two_factor_choice_options.auth_app_info') + legend.mb1.h4.serif.bold = @presenter.label + - @presenter.options.each do |option| + label.btn-border.col-12.mb1 for="two_factor_options_form_selection_#{option.type}" + .radio + = radio_button_tag('two_factor_options_form[selection]', + option.type, + @two_factor_options_form.selected?(option.type)) + span.indicator.mt-tiny + span.blue.bold.fs-20p = option.label + .regular.gray-dark.fs-10p.mb-tiny = option.info + = f.button :submit, t('forms.buttons.continue') = render 'shared/cancel', link: destroy_user_session_path diff --git a/config/application.yml.example b/config/application.yml.example index f36a5947355..f58993e2ccb 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -144,6 +144,7 @@ development: otp_valid_for: '10' password_pepper: 'f22d4b2cafac9066fe2f4416f5b7a32c6942d82f7e00740c7594a095fa8de8db17c05314be7b18a5d6dd5683e73eadf6cc95aa633e5ad9a701edb95192a6a105' password_strength_enabled: 'true' + piv_cac_agencies: '["Test Government Agency"]' piv_cac_enabled: 'true' piv_cac_service_url: 'https://localhost:8443/' piv_cac_verify_token_url: 'https://localhost:8443/' @@ -366,6 +367,7 @@ test: otp_valid_for: '10' password_pepper: 'f22d4b2cafac9066fe2f4416f5b7a32c6942d82f7e00740c7594a095fa8de8db17c05314be7b18a5d6dd5683e73eadf6cc95aa633e5ad9a701edb95192a6a105' password_strength_enabled: 'false' + piv_cac_agencies: '["Test Government Agency"]' piv_cac_enabled: 'true' piv_cac_service_url: 'https://localhost:8443/' piv_cac_verify_token_url: 'https://localhost:8443/' diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index efb4febbae0..66f716b905c 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -69,6 +69,16 @@ def index end describe '#show' do + context 'when user is piv/cac enabled' do + it 'renders the piv/cac entry screen' do + stub_sign_in_before_2fa(build(:user)) + allow(subject.current_user).to receive(:piv_cac_enabled?).and_return(true) + get :show + + expect(response).to redirect_to login_two_factor_piv_cac_path + end + end + context 'when user is TOTP enabled' do it 'renders the :confirm_totp view' do stub_sign_in_before_2fa(build(:user)) diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index 86159235ad4..63fe066c199 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -127,6 +127,20 @@ end end + context 'when the selection is piv_cac' do + it 'redirects to piv/cac setup page' do + stub_sign_in_before_2fa + + patch :create, params: { + two_factor_options_form: { + selection: 'piv_cac', + }, + } + + expect(response).to redirect_to setup_piv_cac_url + end + end + context 'when the selection is not valid' do it 'renders index page' do stub_sign_in_before_2fa diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 6da1222952d..8e72c903b47 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -163,6 +163,42 @@ expect(page).to have_current_path account_path end + it 'does not allow a user to choose piv/cac as 2FA method during sign up' do + allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(false) + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + begin_sign_up_with_sp_and_loa(loa3: false) + + expect(page).to have_current_path two_factor_options_path + expect(page).not_to have_content( + t('devise.two_factor_authentication.two_factor_choice_options.piv_cac') + ) + end + + context 'when piv/cac is allowed' do + it 'allows a user to choose piv/cac as 2FA method during sign up' do + allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(true) + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + + begin_sign_up_with_sp_and_loa(loa3: false) + + expect(page).to have_current_path two_factor_options_path + expect(page).to have_content( + t('devise.two_factor_authentication.two_factor_choice_options.piv_cac') + ) + end + + it 'directs to the piv/cac setup page' do + allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(true) + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + + begin_sign_up_with_sp_and_loa(loa3: false) + + expect(page).to have_current_path two_factor_options_path + select_2fa_option('piv_cac') + expect(page).to have_current_path setup_piv_cac_path + end + end + it 'does not bypass 2FA when accessing authenticator_setup_path if the user is 2FA enabled' do user = create(:user, :signed_up) sign_in_user(user) diff --git a/spec/models/service_provider_spec.rb b/spec/models/service_provider_spec.rb index 0c3b01aa3ca..d7252f63743 100644 --- a/spec/models/service_provider_spec.rb +++ b/spec/models/service_provider_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe ServiceProvider do + let(:service_provider) { ServiceProvider.from_issuer('http://localhost:3000') } + describe 'validations' do it 'validates that all redirect_uris are absolute, parsable uris' do valid_sp = build(:service_provider, redirect_uris: ['http://foo.com']) @@ -28,26 +30,23 @@ describe '#issuer' do it 'returns the constructor value' do - sp = ServiceProvider.from_issuer('http://localhost:3000') - expect(sp.issuer).to eq 'http://localhost:3000' + expect(service_provider.issuer).to eq 'http://localhost:3000' end end describe '#from_issuer' do context 'the record exists' do it 'fetches the record' do - sp = ServiceProvider.from_issuer('http://localhost:3000') - - expect(sp).to be_a ServiceProvider - expect(sp.persisted?).to eq true + expect(service_provider).to be_a ServiceProvider + expect(service_provider.persisted?).to eq true end end context 'the record does not exist' do - it 'returns NullServiceProvider' do - sp = ServiceProvider.from_issuer('no-such-issuer') + let(:service_provider) { ServiceProvider.from_issuer('no-such-issuer') } - expect(sp).to be_a NullServiceProvider + it 'returns NullServiceProvider' do + expect(service_provider).to be_a NullServiceProvider end end end @@ -55,8 +54,6 @@ describe '#metadata' do context 'when the service provider is defined in the YAML' do it 'returns a hash with symbolized attributes from YAML plus fingerprint' do - service_provider = ServiceProvider.from_issuer('http://localhost:3000') - fingerprint = { fingerprint: '40808e52ef80f92e697149e058af95f898cefd9a54d0dc2416bd607c8f9891fa', } @@ -70,13 +67,35 @@ end end + describe 'piv_cac_available?' do + context 'when the service provider is with an enabled agency' do + it 'is truthy' do + allow(Figaro.env).to receive(:piv_cac_agencies).and_return( + [service_provider.agency].to_json + ) + PivCacService.send(:reset_piv_cac_avaialable_agencies) + + expect(service_provider.piv_cac_available?).to be_truthy + end + end + + context 'when the service provider agency is not enabled' do + it 'is falsey' do + allow(Figaro.env).to receive(:piv_cac_agencies).and_return( + [service_provider.agency + 'X'].to_json + ) + PivCacService.send(:reset_piv_cac_avaialable_agencies) + + expect(service_provider.piv_cac_available?).to be_falsey + end + end + end + describe '#encryption_opts' do context 'when responses are not encrypted' do it 'returns nil' do # block_encryption is set to 'none' for this SP - sp = ServiceProvider.from_issuer('http://localhost:3000') - - expect(sp.encryption_opts).to be_nil + expect(service_provider.encryption_opts).to be_nil end end @@ -113,9 +132,7 @@ context 'when the service provider is included in the list of authorized providers' do it 'returns true' do - sp = ServiceProvider.from_issuer('http://localhost:3000') - - expect(sp.approved?).to be true + expect(service_provider.approved?).to be true end end end @@ -131,27 +148,23 @@ context 'when the service provider is approved but not active' do it 'returns false' do - sp = ServiceProvider.from_issuer('http://localhost:3000') - sp.update(active: false) + service_provider.update(active: false) - expect(sp.live?).to be false + expect(service_provider.live?).to be false end end context 'when the service provider is active and approved' do it 'returns true' do - sp = ServiceProvider.from_issuer('http://localhost:3000') - - expect(sp.live?).to be true + expect(service_provider.live?).to be true end end context 'when the service provider is active but not approved' do it 'returns false' do - sp = ServiceProvider.from_issuer('http://localhost:3000') - sp.update(approved: false) + service_provider.update(approved: false) - expect(sp.live?).to be false + expect(service_provider.live?).to be false end end end