Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def handle_valid_confirmation_otp
end

def otp_verification_form
OtpVerificationForm.new(current_user, sanitized_otp_code)
OtpVerificationForm.new(current_user, sanitized_otp_code, phone_configuration)
end

def redirect_if_blank_phone
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,23 @@ def show
def create
@personal_key_form = PersonalKeyForm.new(current_user, personal_key_param)
result = @personal_key_form.submit
analytics_hash = result.to_h.merge(multi_factor_auth_method: 'personal-key')

analytics.track_mfa_submit_event(analytics_hash)

track_analytics(result)
handle_result(result)
end

private

def track_analytics(result)
mfa_created_at = current_user.encrypted_recovery_code_digest_generated_at
analytics_hash = result.to_h.merge(
multi_factor_auth_method: 'personal-key',
multi_factor_auth_method_created_at: mfa_created_at,
)

analytics.track_mfa_submit_event(analytics_hash)
end

def check_personal_key_enabled
return if TwoFactorAuthentication::PersonalKeyPolicy.new(current_user).enabled?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def analytics_properties
context: context,
multi_factor_auth_method: auth_method,
webauthn_configuration_id: form&.webauthn_configuration&.id,
multi_factor_auth_method_created_at: form&.webauthn_configuration&.created_at,
}
end

Expand Down
8 changes: 7 additions & 1 deletion app/forms/backup_code_verification_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@ def submit(params)
attr_reader :user, :backup_code

def valid_backup_code?
BackupCodeGenerator.new(@user).verify(backup_code)
backup_code_config.present?
end

def backup_code_config
@backup_code_config ||= BackupCodeGenerator.new(@user).
if_valid_consume_code_return_config(backup_code)
end

def extra_analytics_attributes
{
multi_factor_auth_method: 'backup_code',
multi_factor_auth_method_created_at: backup_code_config.created_at,
}
end
end
8 changes: 6 additions & 2 deletions app/forms/otp_verification_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ class OtpVerificationForm
validate :validate_user_otp_expiration
validate :validate_code_equals_user_otp

def initialize(user, code)
def initialize(user, code, phone_configuration)
@user = user
@code = code
@phone_configuration = phone_configuration
end

def submit
Expand All @@ -28,7 +29,7 @@ def submit

private

attr_reader :code, :user
attr_reader :code, :user, :phone_configuration

def validate_code_length
return if code.blank? || code.size == TwoFactorAuthenticatable::DIRECT_OTP_LENGTH
Expand Down Expand Up @@ -63,8 +64,11 @@ def otp_expired?
end

def extra_analytics_attributes
multi_factor_auth_method_created_at = phone_configuration&.created_at

{
multi_factor_auth_method: 'otp_code',
multi_factor_auth_method_created_at: multi_factor_auth_method_created_at,
}
end
end
7 changes: 4 additions & 3 deletions app/forms/totp_verification_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def submit
cfg = if_valid_totp_code_return_config
FormResponse.new(
success: cfg.present?,
extra: extra_analytics_attributes(cfg&.id),
extra: extra_analytics_attributes(cfg),
)
end

Expand All @@ -29,10 +29,11 @@ def totp_code_length
TwoFactorAuthenticatable::OTP_LENGTH
end

def extra_analytics_attributes(cfg_id)
def extra_analytics_attributes(cfg)
{
multi_factor_auth_method: 'totp',
auth_app_configuration_id: cfg_id,
auth_app_configuration_id: cfg&.id,
multi_factor_auth_method_created_at: cfg&.created_at,
}
end
end
8 changes: 6 additions & 2 deletions app/forms/user_piv_cac_verification_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class UserPivCacVerificationForm
include PivCacFormHelpers

attr_accessor :x509_dn_uuid, :x509_dn, :x509_issuer, :token, :error_type, :nonce, :user, :key_id,
:piv_cac_required, :piv_cac_configuration
:piv_cac_required

validates :token, presence: true
validates :nonce, presence: true
Expand All @@ -20,6 +20,10 @@ def submit
)
end

def piv_cac_configuration
@piv_cac_configuration ||= ::PivCacConfiguration.find_by(x509_dn_uuid: x509_dn_uuid)
end

private

def valid_submission?
Expand All @@ -29,7 +33,6 @@ def valid_submission?
end

def x509_cert_matches
piv_cac_configuration = ::PivCacConfiguration.find_by(x509_dn_uuid: x509_dn_uuid)
if user == piv_cac_configuration&.user
true
else
Expand All @@ -51,6 +54,7 @@ def extra_analytics_attributes
{
multi_factor_auth_method: 'piv_cac',
piv_cac_configuration_id: piv_cac_configuration&.id,
multi_factor_auth_method_created_at: piv_cac_configuration&.created_at,
}
end
end
3 changes: 3 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2302,6 +2302,7 @@ def logout_initiated(
# @param [Hash] errors Authentication error reasons, if unsuccessful
# @param [String] context
# @param [String] multi_factor_auth_method
# @param [DateTime] multi_factor_auth_method_created_at time auth method was created
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For awareness: In trying to verify how we'd query this, I wasn't able to get CloudWatch to produce results for date string filtering. Worst case we might need to convert this to another format like a timestamp number, but hopefully I'm just doing something wrong. Have you been able to devise a query using this type of data?

Slack thread here: https://gsa-tts.slack.com/archives/C5E7EJWF7/p1688563099123109

# @param [Integer] auth_app_configuration_id
# @param [Integer] piv_cac_configuration_id
# @param [Integer] key_id
Expand All @@ -2317,6 +2318,7 @@ def multi_factor_auth(
errors: nil,
context: nil,
multi_factor_auth_method: nil,
multi_factor_auth_method_created_at: nil,
auth_app_configuration_id: nil,
piv_cac_configuration_id: nil,
key_id: nil,
Expand All @@ -2335,6 +2337,7 @@ def multi_factor_auth(
errors: errors,
context: context,
multi_factor_auth_method: multi_factor_auth_method,
multi_factor_auth_method_created_at: multi_factor_auth_method_created_at,
auth_app_configuration_id: auth_app_configuration_id,
piv_cac_configuration_id: piv_cac_configuration_id,
key_id: key_id,
Expand Down
15 changes: 10 additions & 5 deletions app/services/backup_code_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,17 @@ def create

# @return [Boolean]
def verify(plaintext_code)
return false unless plaintext_code.present?
if_valid_consume_code_return_config(plaintext_code).present?
end

# @return [BackupCodeConfiguration, nil]
def if_valid_consume_code_return_config(plaintext_code)
return unless plaintext_code.present?
backup_code = RandomPhrase.normalize(plaintext_code)
code = BackupCodeConfiguration.find_with_code(code: backup_code, user_id: @user.id)
return false unless code_usable?(code)
code.update!(used_at: Time.zone.now)
true
config = BackupCodeConfiguration.find_with_code(code: backup_code, user_id: @user.id)
return unless code_usable?(config)
config.update!(used_at: Time.zone.now)
config
end

def delete_existing_codes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,19 @@
context 'when the user enters an invalid OTP during authentication context' do
before do
sign_in_before_2fa
subject.user_session[:mfa_selections] = ['sms']
expect(subject.current_user.reload.second_factor_attempts_count).to eq 0
controller.user_session[:mfa_selections] = ['sms']
expect(controller.current_user.reload.second_factor_attempts_count).to eq 0
phone_configuration_created_at = controller.current_user.
default_phone_configuration.created_at

properties = {
success: false,
error_details: { code: [:incorrect_length, :incorrect] },
confirmation_for_add_phone: false,
context: 'authentication',
multi_factor_auth_method: 'sms',
phone_configuration_id: subject.current_user.default_phone_configuration.id,
multi_factor_auth_method_created_at: phone_configuration_created_at,
phone_configuration_id: controller.current_user.default_phone_configuration.id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
Expand All @@ -149,7 +152,7 @@
end

it 'increments second_factor_attempts_count' do
expect(subject.current_user.reload.second_factor_attempts_count).to eq 1
expect(controller.current_user.reload.second_factor_attempts_count).to eq 1
end

it 'redirects to the OTP entry screen' do
Expand All @@ -161,8 +164,8 @@
end

it 'does not set auth_method and requires 2FA' do
expect(subject.user_session[:auth_method]).to eq nil
expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
expect(controller.user_session[:auth_method]).to eq nil
expect(controller.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
end
end

Expand All @@ -173,7 +176,7 @@

post :create, params: { code: '12345', otp_delivery_preference: 'sms' }

expect(subject.current_user.reload.second_factor_attempts_count).to eq 1
expect(controller.current_user.reload.second_factor_attempts_count).to eq 1
end
end

Expand All @@ -186,14 +189,18 @@
IdentityConfig.store.login_otp_confirmation_max_attempts - 1,
)
sign_in_before_2fa(user)
subject.user_session[:mfa_selections] = ['sms']
controller.user_session[:mfa_selections] = ['sms']
phone_configuration_created_at = controller.current_user.
default_phone_configuration.created_at

properties = {
success: false,
error_details: { code: [:incorrect_length, :incorrect] },
confirmation_for_add_phone: false,
context: 'authentication',
multi_factor_auth_method: 'sms',
phone_configuration_id: subject.current_user.default_phone_configuration.id,
multi_factor_auth_method_created_at: phone_configuration_created_at,
phone_configuration_id: controller.current_user.default_phone_configuration.id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
Expand All @@ -210,7 +217,7 @@
expect(@analytics).to receive(:track_event).
with('Multi-Factor Authentication: max attempts reached')
expect(PushNotification::HttpPush).to receive(:deliver).
with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user))
with(PushNotification::MfaLimitAccountLockedEvent.new(user: controller.current_user))

expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted).
with({ reauthentication: false, success: false })
Expand Down Expand Up @@ -251,12 +258,15 @@
end

it 'tracks the valid authentication event' do
phone_configuration_created_at = controller.current_user.
default_phone_configuration.created_at
properties = {
success: true,
confirmation_for_add_phone: false,
context: 'authentication',
multi_factor_auth_method: 'sms',
phone_configuration_id: subject.current_user.default_phone_configuration.id,
multi_factor_auth_method_created_at: phone_configuration_created_at,
phone_configuration_id: controller.current_user.default_phone_configuration.id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
Expand Down Expand Up @@ -429,13 +439,16 @@
phone_configuration = MfaContext.new(subject.current_user).phone_configurations.last
phone_id = phone_configuration.id
parsed_phone = Phonelib.parse(phone_configuration.phone)
phone_configuration_created_at = controller.current_user.
default_phone_configuration.created_at

properties = {
success: true,
errors: nil,
confirmation_for_add_phone: true,
context: 'confirmation',
multi_factor_auth_method: 'sms',
multi_factor_auth_method_created_at: phone_configuration_created_at,
phone_configuration_id: phone_id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
Expand Down Expand Up @@ -514,14 +527,18 @@
end

it 'tracks an event' do
phone_configuration_created_at = controller.current_user.
default_phone_configuration.created_at

properties = {
success: false,
errors: nil,
error_details: { code: [:incorrect_length, :incorrect] },
confirmation_for_add_phone: true,
context: 'confirmation',
multi_factor_auth_method: 'sms',
phone_configuration_id: subject.current_user.default_phone_configuration.id,
phone_configuration_id: controller.current_user.default_phone_configuration.id,
multi_factor_auth_method_created_at: phone_configuration_created_at,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
Expand Down Expand Up @@ -603,6 +620,7 @@
errors: nil,
context: 'confirmation',
multi_factor_auth_method: 'sms',
multi_factor_auth_method_created_at: nil,
confirmation_for_add_phone: false,
phone_configuration_id: nil,
area_code: parsed_phone.area_code,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
context 'when the user enters a valid personal key' do
it 'tracks the valid authentication event' do
sign_in_before_2fa(create(:user, :with_webauthn, :with_phone, :with_personal_key))
personal_key_generated_at = controller.current_user.
encrypted_recovery_code_digest_generated_at

form = instance_double(PersonalKeyForm)
response = FormResponse.new(
Expand All @@ -53,7 +55,12 @@
allow(form).to receive(:submit).and_return(response)

stub_analytics
analytics_hash = { success: true, errors: {}, multi_factor_auth_method: 'personal-key' }
analytics_hash = {
success: true,
errors: {},
multi_factor_auth_method: 'personal-key',
multi_factor_auth_method_created_at: personal_key_generated_at,
}

expect(@analytics).to receive(:track_mfa_submit_event).
with(analytics_hash)
Expand Down Expand Up @@ -154,18 +161,21 @@
end

it 'tracks the max attempts event' do
properties = {
success: false,
errors: {},
multi_factor_auth_method: 'personal-key',
}

user.second_factor_attempts_count =
IdentityConfig.store.login_otp_confirmation_max_attempts - 1
user.save
personal_key_generated_at = controller.current_user.
encrypted_recovery_code_digest_generated_at
stub_analytics
stub_attempts_tracker

properties = {
success: false,
errors: {},
multi_factor_auth_method: 'personal-key',
multi_factor_auth_method_created_at: personal_key_generated_at,
}

expect(@analytics).to receive(:track_mfa_submit_event).
with(properties)
expect(@analytics).to receive(:track_event).
Expand Down
Loading