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
2 changes: 2 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class ApplicationController < ActionController::Base
include LocaleHelper
include VerifySpAttributesConcern
include EffectiveUser
include SecondMfaReminderConcern

# Prevent CSRF attacks by raising an exception.
# For APIs, you may want to use :null_session instead.
Expand Down Expand Up @@ -213,6 +214,7 @@ def after_sign_in_path_for(_user)
return fix_broken_personal_key_url if current_user.broken_personal_key?
return user_session.delete(:stored_location) if user_session.key?(:stored_location)
return reactivate_account_url if user_needs_to_reactivate_account?
return second_mfa_reminder_url if user_needs_second_mfa_reminder?
return sp_session_request_url_with_updated_params if sp_session.key?(:request_url)
signed_in_url
end
Expand Down
1 change: 1 addition & 0 deletions app/controllers/concerns/mfa_setup_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def next_setup_path
mfa_method_counts: mfa_context.enabled_two_factor_configuration_counts_hash,
enabled_mfa_methods_count: mfa_context.enabled_mfa_methods_count,
pii_like_keypaths: [[:mfa_method_counts, :phone]],
second_mfa_reminder_conversion: user_session.delete(:second_mfa_reminder_conversion),
success: true,
)
end
Expand Down
30 changes: 30 additions & 0 deletions app/controllers/concerns/second_mfa_reminder_concern.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
module SecondMfaReminderConcern
def user_needs_second_mfa_reminder?
return false unless IdentityConfig.store.second_mfa_reminder_enabled
return false if user_has_dismissed_second_mfa_reminder? || user_has_multiple_mfa_methods?
exceeded_sign_in_count_for_second_mfa_reminder? || exceeded_account_age_for_second_mfa_reminder?
end

private

def user_has_dismissed_second_mfa_reminder?
current_user.second_mfa_reminder_dismissed_at.present?
end

def user_has_multiple_mfa_methods?
MfaContext.new(current_user).enabled_mfa_methods_count > 1
end

def exceeded_sign_in_count_for_second_mfa_reminder?
current_user.sign_in_count(since: second_mfa_reminder_account_age_cutoff) >=
IdentityConfig.store.second_mfa_reminder_sign_in_count
end

def exceeded_account_age_for_second_mfa_reminder?
current_user.created_at.before?(second_mfa_reminder_account_age_cutoff)
end

def second_mfa_reminder_account_age_cutoff
IdentityConfig.store.second_mfa_reminder_account_age_in_days.days.ago
end
end
33 changes: 33 additions & 0 deletions app/controllers/users/second_mfa_reminder_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
module Users
class SecondMfaReminderController < ApplicationController
include SecureHeadersConcern

before_action :confirm_two_factor_authenticated
before_action :apply_secure_headers_override

def new
analytics.second_mfa_reminder_visit
end

def create
analytics.second_mfa_reminder_dismissed(opted_to_add: opted_to_add?)
current_user.update(second_mfa_reminder_dismissed_at: Time.zone.now)
user_session[:second_mfa_reminder_conversion] = true if opted_to_add?
redirect_to dismiss_redirect_path
end

private

def opted_to_add?
params[:add_method].present?
end

def dismiss_redirect_path
if opted_to_add?
authentication_methods_setup_path
else
after_sign_in_path_for(current_user)
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ def index

def create
result = submit_form
analytics_hash = result.to_h
analytics.user_registration_2fa_setup(**analytics_hash)
analytics.user_registration_2fa_setup(**result.to_h)
irs_attempts_api_tracker.mfa_enroll_options_selected(
success: result.success?,
mfa_device_types: @two_factor_options_form.selection,
Expand Down
11 changes: 11 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,17 @@ def has_devices?
!recent_devices.empty?
end

# Returns the number of times the user has signed in, corresponding to the `sign_in_before_2fa`
# event.
#
# A `since` time argument is required, to optimize performance based on database indices for
# querying a user's events.
#
# @param [ActiveSupport::TimeWithZone] since Time window to query user's events
def sign_in_count(since:)
events.where(event_type: :sign_in_before_2fa).where(created_at: since..).count
end

def second_last_signed_in_at
events.where(event_type: 'sign_in_after_2fa').
order(created_at: :desc).limit(2).pluck(:created_at).second
Expand Down
12 changes: 12 additions & 0 deletions app/presenters/two_factor_options_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ def skip_path
after_mfa_setup_path if two_factor_enabled? && show_skip_additional_mfa_link?
end

def skip_label
if user_has_dismissed_second_mfa_reminder?
t('links.cancel')
else
t('mfa.skip')
end
end

private

def piv_cac_option
Expand Down Expand Up @@ -114,4 +122,8 @@ def phishing_resistant_only?
def mfa_policy
@mfa_policy ||= MfaPolicy.new(user)
end

def user_has_dismissed_second_mfa_reminder?
user.second_mfa_reminder_dismissed_at.present?
end
end
14 changes: 14 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3689,6 +3689,17 @@ def saml_auth_request(
)
end

# User dismissed the second MFA reminder page
# @param [Boolean] opted_to_add Whether the user chose to add a method
def second_mfa_reminder_dismissed(opted_to_add:, **extra)
track_event('Second MFA Reminder Dismissed', opted_to_add:, **extra)
end

# User visited the second MFA reminder page
def second_mfa_reminder_visit
track_event('Second MFA Reminder Visited')
end

# Tracks when security event is received
# @param [Boolean] success
# @param [String] error_code
Expand Down Expand Up @@ -4138,13 +4149,15 @@ def user_registration_enter_email_visit
# @param [Boolean] success
# @param [Hash] mfa_method_counts
# @param [Integer] enabled_mfa_methods_count
# @param [Boolean] second_mfa_reminder_conversion Whether it is a result of second MFA reminder.
# @param [Hash] pii_like_keypaths
# Tracks when a user has completed MFA setup
def user_registration_mfa_setup_complete(
success:,
mfa_method_counts:,
enabled_mfa_methods_count:,
pii_like_keypaths:,
second_mfa_reminder_conversion: nil,
**extra
)
track_event(
Expand All @@ -4154,6 +4167,7 @@ def user_registration_mfa_setup_complete(
mfa_method_counts: mfa_method_counts,
enabled_mfa_methods_count: enabled_mfa_methods_count,
pii_like_keypaths: pii_like_keypaths,
second_mfa_reminder_conversion:,
**extra,
}.compact,
)
Expand Down
33 changes: 33 additions & 0 deletions app/views/users/second_mfa_reminder/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<% title t('users.second_mfa_reminder.heading') %>

<%= render StatusPageComponent.new(status: :info, icon: :question) do |c| %>
<% c.with_header { t('users.second_mfa_reminder.heading') } %>

<p><%= t('users.second_mfa_reminder.description') %></p>

<div class="grid-row margin-top-5">
<div class="tablet:grid-col-9">
<%= render ButtonComponent.new(
action: ->(**tag_options, &block) do
button_to(
second_mfa_reminder_path,
**tag_options,
params: { add_method: true },
&block
)
end,
big: true,
full_width: true,
class: 'margin-bottom-2',
).with_content(t('users.second_mfa_reminder.add_method')) %>
<%= render ButtonComponent.new(
action: ->(**tag_options, &block) do
button_to(second_mfa_reminder_path, **tag_options, &block)
end,
outline: true,
big: true,
full_width: true,
).with_content(t('users.second_mfa_reminder.continue', sp_name: decorated_session.sp_name || APP_NAME)) %>
</div>
</div>
<% end %>
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
<% if @presenter.skip_path || !@presenter.two_factor_enabled? %>
<%= render PageFooterComponent.new do %>
<% if @presenter.skip_path %>
<%= link_to t('mfa.skip'), @presenter.skip_path %>
<%= link_to @presenter.skip_label, @presenter.skip_path %>
<% elsif !@presenter.two_factor_enabled? %>
<%= link_to t('links.cancel_account_creation'), sign_up_cancel_path %>
<% end %>
Expand Down
4 changes: 4 additions & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@ rules_of_use_updated_at: '2022-01-19T00:00:00Z' # Production has a newer timesta
s3_public_reports_enabled: false
s3_reports_enabled: false
saml_secret_rotation_enabled: false
second_mfa_reminder_account_age_in_days: 30
second_mfa_reminder_enabled: true
second_mfa_reminder_sign_in_count: 10
seed_agreements_data: true
service_provider_request_ttl_hours: 24
session_check_delay: 30
Expand Down Expand Up @@ -463,6 +466,7 @@ production:
s3_reports_enabled: true
saml_endpoint_configs: '[]'
scrypt_cost: 10000$8$1$
second_mfa_reminder_enabled: false
secret_key_base:
seed_agreements_data: false
session_encryption_key:
Expand Down
6 changes: 6 additions & 0 deletions config/locales/users/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ en:
</ul>
overview_html: We’ve updated our %{link_html}. Please review and check the box
below to continue.
second_mfa_reminder:
add_method: Add an authentication method
continue: Continue to %{sp_name}
description: Your account only has a single authentication method. Avoid being
locked out of your account by adding another authentication method.
heading: Improve your account security
suspended_sign_in_account:
contact_details: We couldn’t sign you in. Please call our contact center at
%{contact_number}.
Expand Down
7 changes: 7 additions & 0 deletions config/locales/users/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ es:
</ul>
overview_html: Actualizamos nuestro %{link_html}. Revise y marque la casilla a
continuación para continuar.
second_mfa_reminder:
add_method: Agregar un método de autenticación
continue: Continúa con %{sp_name}
description: Su cuenta sólo tiene un método de autenticación. Para evitar que
bloqueen su cuenta, puede que necesites usar otro método de
autenticación.
heading: Aumente la seguridad de su cuenta
suspended_sign_in_account:
contact_details: No pudimos iniciar tu sesión. Por favor, llama a nuestro centro
de contacto al %{contact_number}.
Expand Down
7 changes: 7 additions & 0 deletions config/locales/users/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ fr:
</ul>
overview_html: Nous avons mis à jour notre %{link_html}. Veuillez consulter et
cocher la case ci-dessous pour continuer.
second_mfa_reminder:
add_method: Ajouter une méthode d’authentification
continue: Continuer vers %{sp_name}
description: Votre compte ne dispose que d’une seule méthode d’authentification.
Évitez le verrouillage de votre compte en ajoutant une autre méthode
d’authentification.
heading: Améliorer la sécurité de votre compte
suspended_sign_in_account:
contact_details: Nous n’avons pas pu vous connecter. Merci d’appeler notre
centre de contact au %{contact_number}.
Expand Down
3 changes: 3 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@
get '/rules_of_use' => 'users/rules_of_use#new'
post '/rules_of_use' => 'users/rules_of_use#create'

get '/second_mfa_reminder' => 'users/second_mfa_reminder#new'
post '/second_mfa_reminder' => 'users/second_mfa_reminder#create'

get '/piv_cac' => 'users/piv_cac_authentication_setup#new', as: :setup_piv_cac
get '/piv_cac_error' => 'users/piv_cac_authentication_setup#error', as: :setup_piv_cac_error
delete '/piv_cac' => 'users/piv_cac_authentication_setup#delete', as: :disable_piv_cac
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddSignInCountAndSecondMfaReminderDismissedAtToUsers < ActiveRecord::Migration[7.0]
def change
add_column :users, :second_mfa_reminder_dismissed_at, :datetime
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2023_08_14_130423) do
ActiveRecord::Schema[7.0].define(version: 2023_08_31_124437) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_stat_statements"
enable_extension "pgcrypto"
Expand Down Expand Up @@ -608,6 +608,7 @@
t.datetime "reinstated_at"
t.string "encrypted_password_digest_multi_region"
t.string "encrypted_recovery_code_digest_multi_region"
t.datetime "second_mfa_reminder_dismissed_at"
t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
t.index ["uuid"], name: "index_users_on_uuid", unique: true
end
Expand Down
3 changes: 3 additions & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,9 @@ def self.build_store(config_map)
config.add(:saml_endpoint_configs, type: :json, options: { symbolize_names: true })
config.add(:saml_secret_rotation_enabled, type: :boolean)
config.add(:scrypt_cost, type: :string)
config.add(:second_mfa_reminder_account_age_in_days, type: :integer)
config.add(:second_mfa_reminder_enabled, type: :boolean)
config.add(:second_mfa_reminder_sign_in_count, type: :integer)
config.add(:secret_key_base, type: :string)
config.add(:seed_agreements_data, type: :boolean)
config.add(:service_provider_request_ttl_hours, type: :integer)
Expand Down
37 changes: 32 additions & 5 deletions spec/controllers/concerns/mfa_setup_concern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,41 @@
include MfaSetupConcern
end

describe '#show_skip_additional_mfa_link?' do
let(:user) { create(:user, :fully_registered) }
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

describe '#next_setup_path' do
subject(:next_setup_path) { controller.next_setup_path }

context 'when user converts from second mfa reminder' do
let(:user) { create(:user, :fully_registered, :with_phone, :with_backup_code) }

before do
stub_sign_in_before_2fa(user)
stub_analytics
controller.user_session[:second_mfa_reminder_conversion] = true
controller.user_session[:mfa_selections] = []
end

it 'tracks analytics event' do
next_setup_path

before do
allow(controller).to receive(:current_user).and_return(user)
expect(@analytics).to have_logged_event(
'User Registration: MFA Setup Complete',
success: true,
mfa_method_counts: { phone: 1, backup_codes: 10 },
enabled_mfa_methods_count: 2,
second_mfa_reminder_conversion: true,
)
end
end
end

describe '#show_skip_additional_mfa_link?' do
subject(:show_skip_additional_mfa_link?) { controller.show_skip_additional_mfa_link? }

it 'returns true' do
expect(show_skip_additional_mfa_link?).to eq(true)
Expand Down
Loading