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
19 changes: 19 additions & 0 deletions app/controllers/concerns/mfa_setup_concern.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,25 @@
module MfaSetupConcern
extend ActiveSupport::Concern

def user_next_authentication_setup_path!(final_path = nil)
case user_session[:selected_mfa_options]&.shift
when 'voice', 'sms', 'phone'
phone_setup_url
when 'auth_app'
authenticator_setup_url
when 'piv_cac'
setup_piv_cac_url
when 'webauthn'
webauthn_setup_url
when 'webauthn_platform'
webauthn_setup_url(platform: true)
when 'backup_code'
backup_code_setup_url
else
final_path
end
end

def confirm_user_authenticated_for_2fa_setup
authenticate_user!(force: true)
return if user_fully_authenticated?
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module TwoFactorAuthentication
class OtpVerificationController < ApplicationController
include TwoFactorAuthenticatable
include MfaSetupConcern

before_action :check_sp_required_mfa_bypass
before_action :confirm_multiple_factors_enabled
Expand All @@ -16,7 +17,11 @@ def create
result = OtpVerificationForm.new(current_user, sanitized_otp_code).submit
post_analytics(result)
if result.success?
handle_valid_otp
next_url = nil
if UserSessionContext.confirmation_context?(context)
next_url = user_next_authentication_setup_path!
end
handle_valid_otp(next_url)
else
handle_invalid_otp
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/backup_code_setup_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def edit; end

def continue
flash[:success] = t('notices.backup_codes_configured')
redirect_to after_mfa_setup_path
redirect_to user_next_authentication_setup_path!(after_mfa_setup_path)
end

def download
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ def process_valid_submission
create_user_event(:piv_cac_enabled)
Funnel::Registration::AddMfa.call(current_user.id, 'piv_cac')
session[:needs_to_setup_piv_cac_after_sign_in] = false
redirect_to after_sign_in_path_for(current_user)
final_path = after_sign_in_path_for(current_user)
redirect_to user_next_authentication_setup_path!(final_path)
end

def piv_cac_enabled?
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/totp_setup_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ def process_valid_code
mark_user_as_fully_authenticated
handle_remember_device
flash[:success] = t('notices.totp_configured')
redirect_to after_mfa_setup_path
user_session.delete(:new_totp_secret)
redirect_to user_next_authentication_setup_path!(after_mfa_setup_path)
end

def handle_remember_device
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,8 @@ def two_factor_options_presenter
end

def process_valid_form
case @two_factor_options_form.selection
when 'voice', 'sms', 'phone'
redirect_to phone_setup_url
when 'auth_app'
redirect_to authenticator_setup_url
when 'piv_cac'
redirect_to setup_piv_cac_url
when 'webauthn'
redirect_to webauthn_setup_url
when 'webauthn_platform'
redirect_to webauthn_setup_url(platform: true)
when 'backup_code'
redirect_to backup_code_setup_url
end
user_session[:selected_mfa_options] = @two_factor_options_form.selection
redirect_to user_next_authentication_setup_path!(user_session[:selected_mfa_options].first)
end

def handle_empty_selection
Expand All @@ -73,7 +61,7 @@ def confirm_user_needs_2fa_setup
end

def two_factor_options_form_params
params.require(:two_factor_options_form).permit(:selection)
params.require(:two_factor_options_form).permit(:selection, selection: [])
end
end
end
3 changes: 2 additions & 1 deletion app/controllers/users/webauthn_setup_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ def process_valid_webauthn(form)
flash[:success] = t('notices.webauthn_configured')
end
user_session[:auth_method] = 'webauthn'
redirect_to after_mfa_setup_path

redirect_to user_next_authentication_setup_path!(after_mfa_setup_path)
end

def handle_remember_device
Expand Down
8 changes: 5 additions & 3 deletions app/forms/two_factor_options_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def initialize(user)
end

def submit(params)
self.selection = params[:selection]
self.selection = Array(params[:selection])

success = valid?
update_otp_delivery_preference_for_user if success && user_needs_updating?
Expand All @@ -33,11 +33,13 @@ def extra_analytics_attributes
end

def user_needs_updating?
%w[voice sms].include?(selection) && selection != user.otp_delivery_preference
(%w[voice sms] & selection).present? &&
!selection.include?(user.otp_delivery_preference)
end

def update_otp_delivery_preference_for_user
user_attributes = { otp_delivery_preference: selection }
user_attributes = { otp_delivery_preference:
selection.find { |element| %w[voice sms].include?(element) } }
UpdateUser.new(user: user, attributes: user_attributes).call
end
end
29 changes: 27 additions & 2 deletions spec/controllers/users/backup_code_setup_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
end

it 'creates backup codes' do
user = create(:user, :signed_up)
user = build(:user, :signed_up)
stub_sign_in(user)
expect(PushNotification::HttpPush).to receive(:deliver).
with(PushNotification::RecoveryInformationChangedEvent.new(user: user))
Expand All @@ -24,7 +24,7 @@
end

it 'deletes backup codes' do
user = create(:user, :signed_up)
user = build(:user, :signed_up)
stub_sign_in(user)
expect(PushNotification::HttpPush).to receive(:deliver).
with(PushNotification::RecoveryInformationChangedEvent.new(user: user))
Expand All @@ -34,6 +34,31 @@
expect(user.backup_code_configurations.length).to eq 0
end

context 'when user selects multiple mfas on account creation' do
it 'redirects to phone setup page' do
user = build(:user, :signed_up)
stub_sign_in(user)
codes = BackupCodeGenerator.new(user).create
controller.user_session[:backup_codes] = codes

controller.user_session[:selected_mfa_options] = ['voice']
post :continue

expect(response).to redirect_to(phone_setup_url)
end
end

context 'when user only selects backup code on account creation' do
it 'redirects to account page' do
user = build(:user, :signed_up)
stub_sign_in(user)
codes = BackupCodeGenerator.new(user).create
controller.user_session[:backup_codes] = codes
post :continue
expect(response).to redirect_to(account_url)
end
end

describe '#refreshed' do
render_views

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,26 +94,55 @@
end

context 'when redirected with a good token' do
it 'redirects to account page' do
get :new, params: { token: good_token }
expect(response).to redirect_to(account_url)
context 'with no additional MFAs chosen on setup' do
it 'redirects to account page' do
get :new, params: { token: good_token }
expect(response).to redirect_to(account_url)
end

it 'sets the piv/cac session information' do
get :new, params: { token: good_token }
json = {
'subject' => 'some dn',
'issuer' => nil,
'presented' => true,
}.to_json

expect(subject.user_session[:decrypted_x509]).to eq json
end

it 'sets the session to not require piv setup upon sign-in' do
get :new, params: { token: good_token }

expect(subject.session[:needs_to_setup_piv_cac_after_sign_in]).to eq false
end
end

it 'sets the piv/cac session information' do
get :new, params: { token: good_token }
json = {
'subject' => 'some dn',
'issuer' => nil,
'presented' => true,
}.to_json

expect(subject.user_session[:decrypted_x509]).to eq json
end

it 'sets the session to not require piv setup upon sign-in' do
get :new, params: { token: good_token }

expect(subject.session[:needs_to_setup_piv_cac_after_sign_in]).to eq false
context 'with additional MFAs leftover' do
before do
subject.user_session[:selected_mfa_options] = ['voice']
end
it 'redirects to phone setup page' do
get :new, params: { token: good_token }
expect(response).to redirect_to(phone_setup_url)
end

it 'sets the piv/cac session information' do
get :new, params: { token: good_token }
json = {
'subject' => 'some dn',
'issuer' => nil,
'presented' => true,
}.to_json

expect(subject.user_session[:decrypted_x509]).to eq json
end

it 'sets the session to not require piv setup upon sign-in' do
get :new, params: { token: good_token }

expect(subject.session[:needs_to_setup_piv_cac_after_sign_in]).to eq false
end
end
end

Expand Down
49 changes: 35 additions & 14 deletions spec/controllers/users/totp_setup_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,30 +194,51 @@
end

context 'when user presents correct code' do
let(:selected_mfa_options) { nil }
before do
secret = ROTP::Base32.random_base32
stub_sign_in_before_2fa
stub_analytics
allow(@analytics).to receive(:track_event)
subject.user_session[:new_totp_secret] = secret
subject.user_session[:selected_mfa_options] = selected_mfa_options

patch :confirm, params: { code: generate_totp_code(secret) }
end
context 'when user selected only one method on account creation' do
it 'redirects to account_path with a success message' do
expect(response).to redirect_to(account_path)
expect(subject.user_session[:new_totp_secret]).to be_nil

result = {
success: true,
errors: {},
totp_secret_present: true,
multi_factor_auth_method: 'totp',
auth_app_configuration_id: next_auth_app_id,
}

expect(@analytics).to have_received(:track_event).
with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
end
end

it 'redirects to account_path with a success message' do
expect(response).to redirect_to(account_path)
expect(subject.user_session[:new_totp_secret]).to be_nil

result = {
success: true,
errors: {},
totp_secret_present: true,
multi_factor_auth_method: 'totp',
auth_app_configuration_id: next_auth_app_id,
}

expect(@analytics).to have_received(:track_event).
with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
context 'when user has multiple MFA methods left in user session' do
let(:selected_mfa_options) { ['voice'] }
it 'redirects to phone_setup_path with a success message and still logs analytics' do
expect(response).to redirect_to(phone_setup_path)

result = {
success: true,
errors: {},
totp_secret_present: true,
multi_factor_auth_method: 'totp',
auth_app_configuration_id: next_auth_app_id,
}

expect(@analytics).to have_received(:track_event).
with(Analytics::MULTI_FACTOR_AUTH_SETUP, result)
end
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@
},
}
params = ActionController::Parameters.new(voice_params)
response = FormResponse.new(success: true, errors: {}, extra: { selection: 'voice' })
response = FormResponse.new(success: true, errors: {}, extra: { selection: ['voice'] })

form = instance_double(TwoFactorOptionsForm)
allow(TwoFactorOptionsForm).to receive(:new).with(user).and_return(form)
expect(form).to receive(:submit).
with(params.require(:two_factor_options_form).permit(:selection)).
and_return(response)
expect(form).to receive(:selection).and_return('voice')
expect(form).to receive(:selection).and_return(['voice'])

patch :create, params: voice_params
end
Expand All @@ -81,7 +81,7 @@
stub_analytics

result = {
selection: 'voice',
selection: ['voice'],
success: true,
errors: {},
}
Expand Down Expand Up @@ -110,6 +110,32 @@
end
end

context 'when multi selection with phone first' do
it 'redirects properly' do
stub_sign_in_before_2fa
patch :create, params: {
two_factor_options_form: {
selection: ['phone', 'auth_app'],
},
}

expect(response).to redirect_to phone_setup_url
end
end

context 'when multi selection with auth app first' do
it 'redirects properly' do
stub_sign_in_before_2fa
patch :create, params: {
two_factor_options_form: {
selection: ['auth_app', 'phone', 'webauthn'],
},
}

expect(response).to redirect_to authenticator_setup_url
end
end

context 'when the selection is auth_app' do
it 'redirects to authentication app setup page' do
stub_sign_in_before_2fa
Expand Down
Loading