Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions app/controllers/concerns/mfa_setup_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module MfaSetupConcern
extend ActiveSupport::Concern

def next_setup_path
if user_needs_confirmation_screen?
if suggest_second_mfa?
auth_method_confirmation_url
elsif next_setup_choice
confirmation_path
Expand Down Expand Up @@ -45,11 +45,6 @@ def confirm_user_authenticated_for_2fa_setup
redirect_to user_two_factor_authentication_url
end

def user_needs_confirmation_screen?
suggest_second_mfa? &&
IdentityConfig.store.select_multiple_mfa_options
end

def in_multi_mfa_selection_flow?
return false unless user_session[:mfa_selections].present?
mfa_selection_index < mfa_selection_count
Expand Down
6 changes: 0 additions & 6 deletions app/controllers/users/mfa_selection_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ class MfaSelectionController < ApplicationController

before_action :authenticate_user
before_action :confirm_user_authenticated_for_2fa_setup
before_action :multiple_factors_enabled?

def index
two_factor_options_form
Expand Down Expand Up @@ -70,10 +69,5 @@ def process_valid_form
def two_factor_options_form_params
params.require(:two_factor_options_form).permit(:selection, selection: [])
end

def multiple_factors_enabled?
return if IdentityConfig.store.select_multiple_mfa_options
redirect_to after_mfa_setup_path
end
end
end
11 changes: 6 additions & 5 deletions app/forms/two_factor_options_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class TwoFactorOptionsForm
backup_code] }

validates :selection, length: { minimum: 1 }, if: :has_no_mfa_or_in_required_flow?
validates :selection, length: { minimum: 2, message: 'phone' }, if: :phone_validations?
validates :selection, length: { minimum: 2, message: 'phone' }, if: :phone_valid?

def initialize(user:, phishing_resistant_required:, piv_cac_required:)
self.user = user
Expand Down Expand Up @@ -74,9 +74,10 @@ def phone_alternative_enabled?
count >= 2 || (count == 1 && MfaContext.new(user).phone_configurations.none?)
end

def phone_validations?
IdentityConfig.store.select_multiple_mfa_options &&
phone_selected? && has_no_configured_mfa? &&
!phone_alternative_enabled? && kantara_2fa_phone_restricted?
def phone_valid?
phone_selected? &&
has_no_configured_mfa? &&
!phone_alternative_enabled? &&
kantara_2fa_phone_restricted?
end
end
6 changes: 4 additions & 2 deletions app/policies/mfa_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ def multiple_factors_enabled?
end

def multiple_non_restricted_factors_enabled?
IdentityConfig.store.select_multiple_mfa_options ?
mfa_user.enabled_non_restricted_mfa_methods_count > 1 :
if IdentityConfig.store.kantara_2fa_phone_restricted
mfa_user.enabled_non_restricted_mfa_methods_count > 1
else
multiple_factors_enabled?
end
end

def unphishable?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,8 @@ def setup_info(type)
t('two_factor_authentication.two_factor_choice_options.auth_app_info')
when 'backup_code'
t('two_factor_authentication.two_factor_choice_options.backup_code_info')
when 'phone'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is unreachable, since PhoneSelectionPresenter overrides the info method and therefore setup_info would not be called for those methods. The code here is incorrect anyways, since it's showing the Kantara messaging based on select_multiple_mfa_options (should only be shown with kantara_2fa_phone_restricted).

def info
IdentityConfig.store.kantara_2fa_phone_restricted &&
MfaContext.new(user).enabled_mfa_methods_count == 0 ?
t('two_factor_authentication.two_factor_choice_options.phone_info_html') :
t('two_factor_authentication.two_factor_choice_options.phone_info')
end

IdentityConfig.store.select_multiple_mfa_options ?
t('two_factor_authentication.two_factor_choice_options.phone_info_html') :
t('two_factor_authentication.two_factor_choice_options.phone_info')
when 'piv_cac'
t('two_factor_authentication.two_factor_choice_options.piv_cac_info')
when 'sms'
t('two_factor_authentication.two_factor_choice_options.sms_info')
when 'voice'
t('two_factor_authentication.two_factor_choice_options.voice_info')
when 'webauthn'
t('two_factor_authentication.two_factor_choice_options.webauthn_info')
when 'webauthn_platform'
Expand Down
4 changes: 1 addition & 3 deletions app/presenters/two_factor_options_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ def intro
t('two_factor_authentication.two_factor_hspd12_choice_intro')
elsif phishing_resistant_only?
t('two_factor_authentication.two_factor_aal3_choice_intro')
elsif IdentityConfig.store.select_multiple_mfa_options
t('mfa.info')
else
t('two_factor_authentication.two_factor_choice_intro')
t('mfa.info')
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/sign_up/completions/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<%= render 'sign_up/completions/requested_attributes', pii: @presenter.pii %>
</div>

<% if !@multiple_factors_enabled && IdentityConfig.store.select_multiple_mfa_options %>
<% if !@multiple_factors_enabled %>
<%= render(AlertComponent.new(type: :warning, class: 'margin-bottom-4')) do %>
<%= link_to(
t('mfa.second_method_warning.link'),
Expand Down
29 changes: 3 additions & 26 deletions app/views/users/two_factor_authentication_setup/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,11 @@
<div class="margin-bottom-4">
<fieldset class="margin-0 padding-0 border-0">
<legend class="margin-bottom-2 usa-sr-only"><%= @presenter.intro %></legend>
<% if IdentityConfig.store.select_multiple_mfa_options %>
<%= hidden_field_tag 'two_factor_options_form[selection][]', nil %>
<% end %>
<%= hidden_field_tag 'two_factor_options_form[selection][]', nil %>
<% @presenter.options.each do |option| %>
<div id="<%= "select_#{option.type}" %>" class="<%= option.html_class %>">
<% if IdentityConfig.store.select_multiple_mfa_options %>
<%= render partial: 'partials/multi_factor_authentication/mfa_selection',
locals: { form: f, option: option } %>
<% else %>
<%= radio_button_tag(
'two_factor_options_form[selection]',
option.type,
false,
disabled: option.disabled?,
class: 'usa-radio__input usa-radio__input--tile',
) %>
<%= label_tag(
"two_factor_options_form_selection_#{option.type}",
class: 'usa-radio__label usa-radio__label--illustrated',
) do %>
<%= image_tag(asset_url("mfa-options/#{option.type}.svg"), alt: "#{option.label} icon", class: 'usa-radio__image') %>
<div class="usa-radio__label--text"><%= option.label %>
<span class="usa-radio__label-description">
<%= option.info %>
</span>
</div>
<% end %>
<% end %>
<%= render partial: 'partials/multi_factor_authentication/mfa_selection',
locals: { form: f, option: option } %>
</div>
<% end %>
</fieldset>
Expand Down
3 changes: 0 additions & 3 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ s3_reports_enabled: false
saml_internal_post: false
saml_secret_rotation_enabled: false
seed_agreements_data: true
select_multiple_mfa_options: false
service_provider_request_ttl_hours: 24
session_check_delay: 30
session_check_frequency: 30
Expand Down Expand Up @@ -389,7 +388,6 @@ development:
saml_endpoint_configs: '[{"suffix":"2021","secret_key_passphrase":"trust-but-verify"},{"suffix":"2022","secret_key_passphrase":"trust-but-verify"}]'
scrypt_cost: 10000$8$1$
secret_key_base: development_secret_key_base
select_multiple_mfa_options: true
session_encryption_key: 27bad3c25711099429c1afdfd1890910f3b59f5a4faec1c85e945cb8b02b02f261ba501d99cfbb4fab394e0102de6fecf8ffe260f322f610db3e96b2a775c120
show_account_recovery_recovery_options: true
skip_encryption_allowed_list: '["urn:gov:gsa:SAML:2.0.profiles:sp:sso:localhost"]'
Expand Down Expand Up @@ -564,7 +562,6 @@ test:
saml_endpoint_configs: '[{"suffix":"2022","secret_key_passphrase":"trust-but-verify"}]'
saml_internal_post: true
scrypt_cost: 800$8$1$
select_multiple_mfa_options: false
secret_key_base: test_secret_key_base
session_encryption_key: 27bad3c25711099429c1afdfd1890910f3b59f5a4faec1c85e945cb8b02b02f261ba501d99cfbb4fab394e0102de6fecf8ffe260f322f610db3e96b2a775c120
skip_encryption_allowed_list: '[]'
Expand Down
2 changes: 0 additions & 2 deletions config/locales/two_factor_authentication/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ en:
need to verify your identity using a physical device such as a security
key or government employee ID (PIV or CAC) to access your information.
two_factor_choice: Authentication method setup
two_factor_choice_intro: Add another layer of security by using one of the
multi-factor authentication options below.
two_factor_choice_options:
auth_app: Authentication application
auth_app_info: Download or use an authentication app of your choice to generate
Expand Down
2 changes: 0 additions & 2 deletions config/locales/two_factor_authentication/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@ es:
llave de seguridad o una identificación de empleado del Gobierno (PIV o
CAC) para acceder a su información.
two_factor_choice: Configuración del método de autenticación
two_factor_choice_intro: Agregue un nivel adicional de seguridad usando una de
las siguientes opciones de autenticación de múltiples factores.
two_factor_choice_options:
auth_app: Aplicación de autenticación
auth_app_info: Descargue o use la aplicación de autenticación de su preferencia
Expand Down
2 changes: 0 additions & 2 deletions config/locales/two_factor_authentication/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ fr:
physique tel qu’une clé de sécurité ou un badge d’employé du gouvernement
(PIV ou CAC) pour accéder à vos informations.
two_factor_choice: Configuration de la méthode d’authentification
two_factor_choice_intro: Ajoutez une autre étape de sécurité en utilisant l’une
des options d’authentification multifactorielle ci-dessous.
two_factor_choice_options:
auth_app: Demande d’authentification
auth_app_info: Téléchargez ou utilisez une application d’authentification de
Expand Down
1 change: 0 additions & 1 deletion lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ def self.build_store(config_map)
config.add(:scrypt_cost, type: :string)
config.add(:secret_key_base, type: :string)
config.add(:seed_agreements_data, type: :boolean)
config.add(:select_multiple_mfa_options, type: :boolean)
config.add(:service_provider_request_ttl_hours, type: :integer)
config.add(:saml_internal_post, type: :boolean)
config.add(:session_check_delay, type: :integer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -572,11 +572,10 @@
end
end

context 'Feature flag #select_multiple_mfa_options is true' do
describe 'multiple MFA handling' do
let(:mfa_selections) { ['sms', 'backup_code'] }
before do
subject.user_session[:mfa_selections] = mfa_selections
allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return true

post(
:create,
Expand Down
3 changes: 1 addition & 2 deletions spec/controllers/users/backup_code_setup_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,12 @@
expect(user.backup_code_configurations.length).to eq 10
end

context 'with multiple MFA selection on' do
describe 'multiple MFA handling' do
let(:mfa_selections) { ['backup_code', 'voice'] }
before do
@user = build(:user)
stub_sign_in(@user)
controller.user_session[:mfa_selections] = mfa_selections
allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return true
end

context 'when user selects multiple mfas on account creation' do
Expand Down
6 changes: 1 addition & 5 deletions spec/controllers/users/mfa_selection_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

describe Users::MfaSelectionController do
let(:current_sp) { create(:service_provider) }
before do
allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return(true)
end

describe '#index' do
before do
Expand Down Expand Up @@ -47,9 +44,8 @@
patch :update, params: voice_params
end

context 'when the selection is only phone and multi mfa is enabled' do
context 'when the selection is only phone and kantara phone restriction is enabled' do
before do
allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return(true)
allow(IdentityConfig.store).to receive(:kantara_2fa_phone_restricted).and_return(true)
stub_sign_in_before_2fa

Expand Down
Loading