Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bb2d50c
changelog: Internal, Analytics, add new device property to Multi-Fact…
kevinsmaster5 Dec 15, 2023
0fa9d43
update test coverage to account for new device analytics property
kevinsmaster5 Dec 18, 2023
f05856f
remove unneeded changes
kevinsmaster5 Dec 18, 2023
949b612
change device tracking sign in to account for 2FA submit
kevinsmaster5 Dec 19, 2023
b4dc743
change disavawal second sign in to account for 2FA submit
kevinsmaster5 Dec 19, 2023
accc29c
add test for device cookie service
kevinsmaster5 Dec 20, 2023
aa31580
add new_device event to mfa properties hash
kevinsmaster5 Dec 20, 2023
e9ebcef
fix lint
kevinsmaster5 Dec 20, 2023
7e3301f
remove DeviceCookie service. put as function in User controller. upda…
kevinsmaster5 Jan 2, 2024
c446ea6
add #new_device to user spec
kevinsmaster5 Jan 2, 2024
20cdea8
leverage user session to store new device, fix some style inconsisten…
kevinsmaster5 Jan 2, 2024
d52cad7
update tests to include session value for new device
kevinsmaster5 Jan 2, 2024
c4d3967
fix lint
kevinsmaster5 Jan 2, 2024
4b614bb
revert changes to spec features
kevinsmaster5 Jan 2, 2024
ea2f7e5
fix spec label
kevinsmaster5 Jan 3, 2024
5ae8e42
rename keyword for new_device function
kevinsmaster5 Jan 3, 2024
6c782ac
update spec to account for nil device and also false
kevinsmaster5 Jan 3, 2024
2e22cb2
fix lint
kevinsmaster5 Jan 3, 2024
9d85956
add new device false tests
kevinsmaster5 Jan 3, 2024
be65b95
remove unneeded user_session override
kevinsmaster5 Jan 4, 2024
b093dc8
debug otp spec
kevinsmaster5 Jan 4, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ def show
def create
@backup_code_form = BackupCodeVerificationForm.new(current_user)
result = @backup_code_form.submit(backup_code_params)
analytics.track_mfa_submit_event(result.to_h)
analytics.track_mfa_submit_event(
result.to_h.merge(new_device: user_session[:new_device]),
)
irs_attempts_api_tracker.mfa_login_backup_code(success: result.success?)
handle_result(result)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def form_params
end

def post_analytics(result)
properties = result.to_h.merge(analytics_properties)
properties = result.to_h.merge(analytics_properties, new_device: user_session[:new_device])
analytics.multi_factor_auth_setup(**properties) if context == 'confirmation'

analytics.track_mfa_submit_event(properties)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def track_analytics(result)
analytics_hash = result.to_h.merge(
multi_factor_auth_method: 'personal-key',
multi_factor_auth_method_created_at: mfa_created_at&.strftime('%s%L'),
new_device: user_session[:new_device],
)

analytics.track_mfa_submit_event(analytics_hash)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def analytics_properties
context: context,
multi_factor_auth_method: 'piv_cac',
piv_cac_configuration_id: piv_cac_verification_form&.piv_cac_configuration&.id,
new_device: user_session[:new_device],
}
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ def show

def create
result = TotpVerificationForm.new(current_user, params.require(:code).strip).submit

analytics.track_mfa_submit_event(result.to_h)
analytics.track_mfa_submit_event(result.to_h.merge(new_device: user_session[:new_device]))
irs_attempts_api_tracker.mfa_login_totp(success: result.success?)

if result.success?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def confirm
**analytics_properties,
multi_factor_auth_method_created_at:
webauthn_configuration_or_latest.created_at.strftime('%s%L'),
new_device: user_session[:new_device],
)

if analytics_properties[:multi_factor_auth_method] == 'webauthn_platform'
Expand Down
1 change: 1 addition & 0 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ def process_locked_out_user
def handle_valid_authentication
sign_in(resource_name, resource)
cache_profiles(auth_params[:password])
user_session[:new_device] = current_user.new_device?(cookie_uuid: cookies[:device])
create_user_event(:sign_in_before_2fa)
EmailAddress.update_last_sign_in_at_on_user_id_and_email(
user_id: current_user.id,
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,10 @@ def has_devices?
!recent_devices.empty?
end

def new_device?(cookie_uuid:)
!cookie_uuid || !devices.exists?(cookie_uuid:)
end

# Returns the number of times the user has signed in, corresponding to the `sign_in_before_2fa`
# event.
#
Expand Down
3 changes: 3 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3003,6 +3003,7 @@ def logout_initiated(
# @param [Boolean] success Whether authentication was successful
# @param [Hash] errors Authentication error reasons, if unsuccessful
# @param [String] context
# @param [Boolean] new_device
# @param [String] multi_factor_auth_method
# @param [DateTime] multi_factor_auth_method_created_at time auth method was created
# @param [Integer] auth_app_configuration_id
Expand All @@ -3020,6 +3021,7 @@ def multi_factor_auth(
success:,
errors: nil,
context: nil,
new_device: nil,
multi_factor_auth_method: nil,
multi_factor_auth_method_created_at: nil,
auth_app_configuration_id: nil,
Expand All @@ -3040,6 +3042,7 @@ def multi_factor_auth(
success: success,
errors: errors,
context: context,
new_device: new_device,
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
it 'tracks the valid authentication event' do
freeze_time do
sign_in_before_2fa(user)

stub_analytics
stub_attempts_tracker
analytics_hash = {
success: true,
errors: {},
multi_factor_auth_method: 'backup_code',
multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'),
new_device: nil,
}

expect(@analytics).to receive(:track_mfa_submit_event).
Expand Down Expand Up @@ -86,7 +86,6 @@
it 'tracks the valid authentication event when there are exisitng codes' do
freeze_time do
stub_sign_in_before_2fa(user)

stub_analytics
stub_attempts_tracker

Expand All @@ -96,6 +95,7 @@
errors: {},
multi_factor_auth_method: 'backup_code',
multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'),
new_device: nil,
})

expect(@irs_attempts_api_tracker).to receive(:track_event).
Expand All @@ -109,6 +109,40 @@
end
end

context 'with new device session value' do
it 'tracks new device value' do
freeze_time do
sign_in_before_2fa(user)
subject.user_session[:new_device] = false
stub_analytics
stub_attempts_tracker
analytics_hash = {
success: true,
errors: {},
multi_factor_auth_method: 'backup_code',
multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'),
new_device: false,
}

expect(@analytics).to receive(:track_mfa_submit_event).
with(analytics_hash)

expect(@irs_attempts_api_tracker).to receive(:track_event).
with(:mfa_login_backup_code, success: true)

post :create, params: payload

expect(subject.user_session[:auth_events]).to eq(
[
auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE,
at: Time.zone.now,
],
)
expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false
end
end
end

context 'when the backup code field is empty' do
let(:backup_code) { { backup_code: '' } }
let(:payload) { { backup_code_verification_form: backup_code } }
Expand Down Expand Up @@ -156,6 +190,7 @@
errors: {},
multi_factor_auth_method: 'backup_code',
multi_factor_auth_method_created_at: nil,
new_device: nil,
}

stub_analytics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
context: 'authentication',
multi_factor_auth_method: 'sms',
multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
new_device: nil,
phone_configuration_id: controller.current_user.default_phone_configuration.id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
Expand Down Expand Up @@ -209,6 +210,7 @@
context: 'authentication',
multi_factor_auth_method: 'sms',
multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
new_device: nil,
phone_configuration_id: controller.current_user.default_phone_configuration.id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
Expand Down Expand Up @@ -275,6 +277,7 @@
context: 'authentication',
multi_factor_auth_method: 'sms',
multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
new_device: nil,
phone_configuration_id: controller.current_user.default_phone_configuration.id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
Expand Down Expand Up @@ -312,6 +315,40 @@
end
end

context 'with new device session value' do
it 'tracks new device value' do
subject.user_session[:new_device] = false
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',
multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
new_device: false,
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),
enabled_mfa_methods_count: 1,
in_account_creation_flow: false,
}

stub_analytics

expect(@analytics).to receive(:track_mfa_submit_event).
with(properties)

freeze_time do
post :create, params: {
code: subject.current_user.reload.direct_otp,
otp_delivery_preference: 'sms',
}
end
end
end

context "with a leading '#' sign" do
it 'redirects to the profile' do
post :create, params: {
Expand Down Expand Up @@ -462,6 +499,7 @@
context: 'confirmation',
multi_factor_auth_method: 'sms',
multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
new_device: nil,
phone_configuration_id: phone_id,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
Expand Down Expand Up @@ -552,6 +590,7 @@
multi_factor_auth_method: 'sms',
phone_configuration_id: controller.current_user.default_phone_configuration.id,
multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'),
new_device: nil,
area_code: parsed_phone.area_code,
country_code: parsed_phone.country,
phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
Expand Down Expand Up @@ -634,6 +673,7 @@
context: 'confirmation',
multi_factor_auth_method: 'sms',
multi_factor_auth_method_created_at: nil,
new_device: 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 @@ -55,6 +55,7 @@
multi_factor_auth_method: 'personal-key',
multi_factor_auth_method_created_at: user.reload.
encrypted_recovery_code_digest_generated_at.strftime('%s%L'),
new_device: nil,
}

expect(@analytics).to receive(:track_mfa_submit_event).
Expand Down Expand Up @@ -108,6 +109,34 @@
end
end

context 'with new device session value' do
let(:user) { create(:user, :with_phone) }
let(:personal_key) { { personal_key: PersonalKeyGenerator.new(user).create } }
let(:payload) { { personal_key_form: personal_key } }

it 'tracks new device value' do
personal_key
sign_in_before_2fa(user)
stub_analytics
subject.user_session[:new_device] = false
analytics_hash = {
success: true,
errors: {},
multi_factor_auth_method: 'personal-key',
multi_factor_auth_method_created_at: user.reload.
encrypted_recovery_code_digest_generated_at.strftime('%s%L'),
new_device: false,
}

expect(@analytics).to receive(:track_mfa_submit_event).
with(analytics_hash)

freeze_time do
post :create, params: payload
end
end
end

it 'does generate a new personal key after the user signs in with their old one' do
user = create(:user)
raw_key = PersonalKeyGenerator.new(user).create
Expand Down Expand Up @@ -187,6 +216,7 @@
error_details: { personal_key: { personal_key_incorrect: true } },
multi_factor_auth_method: 'personal-key',
multi_factor_auth_method_created_at: personal_key_generated_at.strftime('%s%L'),
new_device: nil,
}

expect(@analytics).to receive(:track_mfa_submit_event).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
attributes = {
context: 'authentication',
multi_factor_auth_method: 'piv_cac',
new_device: nil,
piv_cac_configuration_id: nil,
}

Expand All @@ -117,6 +118,7 @@
errors: {},
context: 'authentication',
multi_factor_auth_method: 'piv_cac',
new_device: nil,
multi_factor_auth_method_created_at: cfg.created_at.strftime('%s%L'),
piv_cac_configuration_id: cfg.id,
}
Expand All @@ -133,6 +135,30 @@

get :show, params: { token: 'good-token' }
end

context 'with new device session value' do
before do
subject.user_session[:new_device] = false
end
it 'tracks new device value' do
stub_analytics
cfg = controller.current_user.piv_cac_configurations.first

submit_attributes = {
success: true,
errors: {},
context: 'authentication',
multi_factor_auth_method: 'piv_cac',
new_device: false,
multi_factor_auth_method_created_at: cfg.created_at.strftime('%s%L'),
piv_cac_configuration_id: cfg.id,
}
expect(@analytics).to receive(:track_mfa_submit_event).
with(submit_attributes)

get :show, params: { token: 'good-token' }
end
end
end

context 'when the user presents an invalid piv/cac' do
Expand Down Expand Up @@ -203,6 +229,7 @@
attributes = {
context: 'authentication',
multi_factor_auth_method: 'piv_cac',
new_device: nil,
piv_cac_configuration_id: nil,
}

Expand All @@ -220,6 +247,7 @@
context: 'authentication',
multi_factor_auth_method: 'piv_cac',
multi_factor_auth_method_created_at: nil,
new_device: nil,
key_id: nil,
piv_cac_configuration_id: nil,
}
Expand Down
Loading