diff --git a/app/controllers/concerns/mfa_setup_concern.rb b/app/controllers/concerns/mfa_setup_concern.rb index 159c157dc94..1f0a290e660 100644 --- a/app/controllers/concerns/mfa_setup_concern.rb +++ b/app/controllers/concerns/mfa_setup_concern.rb @@ -67,6 +67,11 @@ def mfa_selection_index user_session[:mfa_selection_index] || 0 end + def show_skip_additional_mfa_link? + !(mfa_context.enabled_mfa_methods_count == 1 && + mfa_context.webauthn_platform_configurations.count == 1) + end + private def determine_next_mfa diff --git a/app/controllers/mfa_confirmation_controller.rb b/app/controllers/mfa_confirmation_controller.rb index 8cff30851c6..bda5cda1fae 100644 --- a/app/controllers/mfa_confirmation_controller.rb +++ b/app/controllers/mfa_confirmation_controller.rb @@ -3,7 +3,7 @@ class MfaConfirmationController < ApplicationController before_action :confirm_two_factor_authenticated def show - @content = MfaConfirmationPresenter.new(current_user) + @content = mfa_confirmation_presenter analytics.user_registration_suggest_another_mfa_notice_visited end @@ -39,6 +39,12 @@ def create private + def mfa_confirmation_presenter + MfaConfirmationPresenter.new( + show_skip_additional_mfa_link: show_skip_additional_mfa_link?, + ) + end + def password params.require(:user)[:password] end diff --git a/app/controllers/users/mfa_selection_controller.rb b/app/controllers/users/mfa_selection_controller.rb index 011ee40e8df..41d49521bfa 100644 --- a/app/controllers/users/mfa_selection_controller.rb +++ b/app/controllers/users/mfa_selection_controller.rb @@ -42,6 +42,7 @@ def two_factor_options_presenter user: current_user, phishing_resistant_required: service_provider_mfa_policy.phishing_resistant_required?, piv_cac_required: service_provider_mfa_policy.piv_cac_required?, + show_skip_additional_mfa_link: show_skip_additional_mfa_link?, ) end diff --git a/app/presenters/mfa_confirmation_presenter.rb b/app/presenters/mfa_confirmation_presenter.rb index ba459849fd8..f89d47713b7 100644 --- a/app/presenters/mfa_confirmation_presenter.rb +++ b/app/presenters/mfa_confirmation_presenter.rb @@ -1,6 +1,6 @@ class MfaConfirmationPresenter - def initialize(user) - @user = user + def initialize(show_skip_additional_mfa_link: true) + @show_skip_additional_mfa_link = show_skip_additional_mfa_link end def heading @@ -14,4 +14,8 @@ def info def button I18n.t('mfa.add') end + + def show_skip_additional_mfa_link? + @show_skip_additional_mfa_link + end end diff --git a/app/presenters/two_factor_options_presenter.rb b/app/presenters/two_factor_options_presenter.rb index 897781b2f8a..ef2ee552619 100644 --- a/app/presenters/two_factor_options_presenter.rb +++ b/app/presenters/two_factor_options_presenter.rb @@ -3,12 +3,16 @@ class TwoFactorOptionsPresenter attr_reader :user - def initialize(user_agent:, user: nil, - phishing_resistant_required: false, piv_cac_required: false) + def initialize(user_agent:, + user: nil, + phishing_resistant_required: false, + piv_cac_required: false, + show_skip_additional_mfa_link: true) @user_agent = user_agent @user = user @phishing_resistant_required = phishing_resistant_required @piv_cac_required = piv_cac_required + @show_skip_additional_mfa_link = show_skip_additional_mfa_link end def options @@ -46,6 +50,10 @@ def intro end end + def show_skip_additional_mfa_link? + @show_skip_additional_mfa_link + end + private def piv_cac_option diff --git a/app/views/mfa_confirmation/show.html.erb b/app/views/mfa_confirmation/show.html.erb index bacd62443eb..5466f763230 100644 --- a/app/views/mfa_confirmation/show.html.erb +++ b/app/views/mfa_confirmation/show.html.erb @@ -15,10 +15,12 @@ ) { @content.button } %> -<%= button_to( - auth_method_confirmation_skip_path, - method: :post, - class: 'usa-button usa-button--unstyled', - ) do %> - <%= t('mfa.skip') %> +<% if @content.show_skip_additional_mfa_link? %> + <%= button_to( + auth_method_confirmation_skip_path, + method: :post, + class: 'usa-button usa-button--unstyled', + ) do %> + <%= t('mfa.skip') %> + <% end %> <% end %> \ No newline at end of file diff --git a/app/views/users/mfa_selection/index.html.erb b/app/views/users/mfa_selection/index.html.erb index 705843ebda3..be32cbfd6fa 100644 --- a/app/views/users/mfa_selection/index.html.erb +++ b/app/views/users/mfa_selection/index.html.erb @@ -24,6 +24,8 @@ <%= f.submit t('forms.buttons.continue'), class: 'margin-bottom-1' %> <% end %> -<%= render PageFooterComponent.new do %> - <%= link_to t('mfa.skip'), @after_setup_path, method: :get %> +<% if @presenter.show_skip_additional_mfa_link? %> + <%= render PageFooterComponent.new do %> + <%= link_to t('mfa.skip'), @after_setup_path, method: :get %> + <% end %> <% end %> diff --git a/spec/controllers/concerns/mfa_setup_concern_spec.rb b/spec/controllers/concerns/mfa_setup_concern_spec.rb new file mode 100644 index 00000000000..ecd2000e05b --- /dev/null +++ b/spec/controllers/concerns/mfa_setup_concern_spec.rb @@ -0,0 +1,33 @@ +require 'rails_helper' + +RSpec.describe 'MfaSetupConcern' do + controller ApplicationController do + include MfaSetupConcern + end + + describe '#show_skip_additional_mfa_link?' do + let(:user) { create(:user, :fully_registered) } + + subject(:show_skip_additional_mfa_link?) { controller.show_skip_additional_mfa_link? } + + before do + allow(controller).to receive(:current_user).and_return(user) + end + + it 'returns true' do + expect(show_skip_additional_mfa_link?).to eq(true) + end + + context 'with only webauthn_platform registered' do + let(:user) { create(:user, :with_webauthn_platform) } + + before do + stub_sign_in(user) + end + + it 'returns false' do + expect(show_skip_additional_mfa_link?).to eq(false) + end + end + end +end diff --git a/spec/presenters/mfa_confirmation_presenter_spec.rb b/spec/presenters/mfa_confirmation_presenter_spec.rb index 89649c880f6..ae5477f9ab9 100644 --- a/spec/presenters/mfa_confirmation_presenter_spec.rb +++ b/spec/presenters/mfa_confirmation_presenter_spec.rb @@ -2,7 +2,9 @@ RSpec.describe MfaConfirmationPresenter do let(:user) { create(:user, :with_phone) } - let(:presenter) { described_class.new(user) } + let(:presenter) do + described_class.new + end describe '#heading?' do it 'supplies a message' do @@ -26,4 +28,23 @@ to eq(t('mfa.add')) end end + + describe '#show_skip_additional_mfa_link?' do + it 'returns true' do + expect(presenter.show_skip_additional_mfa_link?).to eq(true) + end + + context 'when show_skip_additional_mfa_link is false' do + let(:show_skip_additional_mfa_link) { false } + let(:presenter) do + described_class.new( + show_skip_additional_mfa_link: show_skip_additional_mfa_link, + ) + end + + it 'returns false' do + expect(presenter.show_skip_additional_mfa_link?).to eq(false) + end + end + end end diff --git a/spec/presenters/two_factor_options_presenter_spec.rb b/spec/presenters/two_factor_options_presenter_spec.rb index 4e561489f94..3d5bfa5609d 100644 --- a/spec/presenters/two_factor_options_presenter_spec.rb +++ b/spec/presenters/two_factor_options_presenter_spec.rb @@ -77,4 +77,24 @@ end end end + + describe '#show_skip_additional_mfa_link?' do + it 'returns true' do + expect(presenter.show_skip_additional_mfa_link?).to eq(true) + end + + context 'when show_skip_additional_mfa_link is false' do + let(:show_skip_additional_mfa_link) { false } + let(:presenter) do + described_class.new( + user_agent: user_agent, + show_skip_additional_mfa_link: show_skip_additional_mfa_link, + ) + end + + it 'returns false' do + expect(presenter.show_skip_additional_mfa_link?).to eq(false) + end + end + end end diff --git a/spec/views/mfa_confirmation/show.html.erb_spec.rb b/spec/views/mfa_confirmation/show.html.erb_spec.rb index 888af368e0f..6351dddba51 100644 --- a/spec/views/mfa_confirmation/show.html.erb_spec.rb +++ b/spec/views/mfa_confirmation/show.html.erb_spec.rb @@ -6,7 +6,7 @@ before do allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:enforce_second_mfa?).and_return(true) - @content = MfaConfirmationPresenter.new(user) + @content = MfaConfirmationPresenter.new end it 'has a localized title' do @@ -38,4 +38,30 @@ text: @content.button, ) end + + it 'has link to skip add mfa' do + render + + expect(rendered).to have_button( + t('mfa.skip'), + ) + end + + context 'when the user only has enabled mfa webauthn platform' do + let(:user) { create(:user, :with_webauthn_platform) } + + before do + @content = MfaConfirmationPresenter.new( + show_skip_additional_mfa_link: false, + ) + end + + it 'does not show link to skip add mfa' do + render + + expect(rendered).not_to have_button( + t('mfa.skip'), + ) + end + end end diff --git a/spec/views/users/mfa_selection/index.html.erb_spec.rb b/spec/views/users/mfa_selection/index.html.erb_spec.rb new file mode 100644 index 00000000000..35f411b2899 --- /dev/null +++ b/spec/views/users/mfa_selection/index.html.erb_spec.rb @@ -0,0 +1,65 @@ +require 'rails_helper' + +RSpec.describe 'users/mfa_selection/index.html.erb' do + let(:user) { create(:user, :fully_registered, :with_personal_key) } + let(:phishing_resistant_required) { true } + let(:piv_cac_required) { true } + let(:user_agent) { '' } + let(:show_skip_additional_mfa_link) { true } + + before do + allow(view).to receive(:current_user).and_return(user) + + @two_factor_options_form ||= TwoFactorOptionsForm.new( + user: user, + phishing_resistant_required: phishing_resistant_required, + piv_cac_required: piv_cac_required, + ) + + @presenter = TwoFactorOptionsPresenter.new( + user_agent: user_agent, + user: user, + phishing_resistant_required: phishing_resistant_required, + piv_cac_required: piv_cac_required, + show_skip_additional_mfa_link: show_skip_additional_mfa_link, + ) + end + + it 'has a localized title' do + expect(view).to receive(:title).with(t('mfa.additional_mfa_required.heading')) + + render + end + + it 'has intro text' do + render + + expect(rendered).to have_content(@presenter.intro) + end + + it 'has link to skip add mfa' do + render + + expect(rendered).to have_link(t('mfa.skip')) + end + + context 'when show_skip_additional_mfa_link is false' do + let(:show_skip_additional_mfa_link) { false } + + before do + @presenter = TwoFactorOptionsPresenter.new( + user_agent: user_agent, + user: user, + phishing_resistant_required: phishing_resistant_required, + piv_cac_required: piv_cac_required, + show_skip_additional_mfa_link: show_skip_additional_mfa_link, + ) + end + + it 'does not show link to skip add mfa' do + render + + expect(rendered).not_to have_link(t('mfa.skip')) + end + end +end