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: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def fix_broken_personal_key_url
user_session[:personal_key] = profile.encrypt_recovery_pii(cacher.fetch)
profile.save!

analytics.track_event(Analytics::BROKEN_PERSONAL_KEY_REGENERATED)
analytics.broken_personal_key_regenerated

manage_personal_key_url
else
Expand Down
6 changes: 2 additions & 4 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,14 @@ def alive?
def track_authentication_attempt(email)
user = User.find_with_email(email) || AnonymousUser.new

properties = {
analytics.email_and_password_auth(
success: user_signed_in_and_not_locked_out?(user),
user_id: user.uuid,
user_locked_out: user_locked_out?(user),
stored_location: session['user_return_to'],
sp_request_url_present: sp_session[:request_url].present?,
remember_device: remember_device_cookie.present?,
}

analytics.track_event(Analytics::EMAIL_AND_PASSWORD_AUTH, properties)
)
end

def user_signed_in_and_not_locked_out?(user)
Expand Down
2 changes: 1 addition & 1 deletion app/forms/idv/api_image_upload_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def document_capture_session_uuid
def doc_auth_client
@doc_auth_client ||= DocAuthRouter.client(
vendor_discriminator: document_capture_session_uuid,
warn_notifier: proc { |attrs| track_event(Analytics::DOC_AUTH_WARNING, attrs) },
warn_notifier: proc { |attrs| analytics&.doc_auth_warning(**attrs) },
)
end

Expand Down
2 changes: 1 addition & 1 deletion app/jobs/document_proofing_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def build_analytics(document_capture_session)
def build_doc_auth_client(analytics, document_capture_session)
DocAuthRouter.client(
vendor_discriminator: document_capture_session.uuid,
warn_notifier: proc { |attrs| analytics.track_event(Analytics::DOC_AUTH_WARNING, attrs) },
warn_notifier: proc { |attrs| analytics.doc_auth_warning(**attrs) },
)
end

Expand Down
4 changes: 0 additions & 4 deletions app/services/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,7 @@ def browser_attributes

# rubocop:disable Layout/LineLength
ACCOUNT_RESET_VISIT = 'Account deletion and reset visited'
BROKEN_PERSONAL_KEY_REGENERATED = 'Broken Personal Key: Regenerated'
DOC_AUTH = 'Doc Auth' # visited or submitted is appended
DOC_AUTH_ASYNC = 'Doc Auth Async'
DOC_AUTH_WARNING = 'Doc Auth Warning'
EMAIL_AND_PASSWORD_AUTH = 'Email and Password Authentication'
EMAIL_DELETION_REQUEST = 'Email Deletion Requested'
EMAIL_LANGUAGE_VISITED = 'Email Language: Visited'
EMAIL_LANGUAGE_UPDATED = 'Email Language: Updated'
Expand Down
53 changes: 53 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,59 @@ def banned_user_visited
track_event('Banned User visited')
end

# @identity.idp.event_name Broken Personal Key: Regenerated
# A user that had a broken personal key was routed to a page to regenerate their personal key,
# so that they no longer have a broken one
def broken_personal_key_regenerated
track_event('Broken Personal Key: Regenerated')
end

# @identity.idp.event_name Doc Auth Async
# @param [String, nil] error error message
# @param [String, nil] uuid document capture session uuid
# @param [String. nil] result_id document capture session result id
# When there is an error loading async results during the document authentication flow
def doc_auth_async(error: nil, uuid: nil, result_id: nil, **extra)
track_event('Doc Auth Async', error: error, uuid: uuid, result_id: result_id, **extra)
end

# @identity.idp.event_name Doc Auth Warning
# @param [String] message the warining
# Logged when there is a non-user-facing error in the doc auth process, such as an unrecognized
# field from a vendor
def doc_auth_warning(message: nil, **extra)
track_event('Doc Auth Warning', message: message, **extra)
end

# @identity.idp.event_name Email and Password Authentication
# @param [Boolean] success
# @param [String] user_id
# @param [Boolean] user_locked_out if the user is currently locked out of their second factor
# @param [String] stored_location the URL to return to after signing in
# @param [Boolean] sp_request_url_present if was an SP request URL in the session
# @param [Boolean] remember_device if the remember device cookie was present
# Tracks authentication attempts at the email/password screen
def email_and_password_auth(
success:,
user_id:,
user_locked_out:,
stored_location:,
sp_request_url_present:,
remember_device:,
**extra
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.

Do we need to support extra?

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.

no, but because we got burned by missing it one place, I made us require it for all methods in #6052. Maybe there's some additional documentation param we could no like a @noextra or something when we know but currently there's no exceptions to our lint on requiring that

Copy link
Copy Markdown
Contributor

@aduth aduth Apr 5, 2022

Choose a reason for hiding this comment

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

Gotcha, I thought that sounded familiar, but I swear I had seen another event where we weren't using **extra. Looking again, I don't see it, so must have been imagining things 🤷

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.

You might have seen a zero-param event (those get a free pass) or seen one of the PRs prior to that one 🤷

)
track_event(
'Email and Password Authentication',
success: success,
user_id: user_id,
user_locked_out: user_locked_out,
stored_location: stored_location,
sp_request_url_present: sp_request_url_present,
remember_device: remember_device,
**extra,
)
end

# @identity.idp.event_name IdV: phone confirmation otp submitted
# @param [Boolean] success
# @param [Hash] errors
Expand Down
9 changes: 3 additions & 6 deletions app/services/idv/actions/verify_document_status_action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ def async_state

proofing_job_result = verify_document_capture_session.load_doc_auth_async_result
if proofing_job_result.nil?
@flow.analytics.track_event(
Analytics::DOC_AUTH_ASYNC,
@flow.analytics.doc_auth_async(
error: 'failed to load async result',
uuid: verify_document_capture_session.uuid,
result_id: verify_document_capture_session.result_id,
Expand Down Expand Up @@ -117,12 +116,10 @@ def delete_async
end

def document_capture_analytics(message)
data = {
@flow.analytics.doc_auth_async(
error: message,
uuid: flow_session[verify_document_capture_session_uuid_key],
}

@flow.analytics.track_event(Analytics::DOC_AUTH_ASYNC, data)
)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/services/idv/steps/document_capture_step.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def post_images
result = DocAuthRouter.client(
vendor_discriminator: flow_session[:document_capture_session_uuid],
warn_notifier: proc do |attrs|
@flow.analytics.track_event(Analytics::DOC_AUTH_WARNING, attrs)
@flow.analytics.doc_auth_warning(**attrs)
end,
).post_images(
front_image: front_image.read,
Expand Down
16 changes: 8 additions & 8 deletions spec/controllers/users/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

post :create, params: { user: { email: user.email.upcase, password: user.password } }
end
Expand All @@ -201,7 +201,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

post :create, params: { user: { email: user.email.upcase, password: 'invalid_password' } }
end
Expand All @@ -218,7 +218,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

post :create, params: { user: { email: 'foo@example.com', password: 'password' } }
end
Expand All @@ -241,7 +241,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

post :create, params: { user: { email: user.email.upcase, password: user.password } }
end
Expand All @@ -259,7 +259,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

post :create, params: { user: { email: 'foo@example.com', password: 'password' } }
end
Expand Down Expand Up @@ -328,7 +328,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

profile_encryption_error = {
error: 'Unable to parse encrypted payload',
Expand Down Expand Up @@ -384,7 +384,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

post :create, params: { user: { email: user.email, password: user.password } }
end
Expand All @@ -409,7 +409,7 @@
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)
with('Email and Password Authentication', analytics_hash)

post :create, params: { user: { email: user.email, password: user.password } }
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@
click_idv_continue
end

expect(fake_analytics).to have_logged_event(Analytics::DOC_AUTH_WARNING, {})
expect(fake_analytics).to have_logged_event('Doc Auth Warning', {})
end

it 'proceeds to the next page with valid info and logs analytics info' do
Expand Down
2 changes: 1 addition & 1 deletion spec/forms/idv/api_image_upload_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
it 'logs a doc auth warning' do
form.submit

expect(fake_analytics).to have_logged_event(Analytics::DOC_AUTH_WARNING, {})
expect(fake_analytics).to have_logged_event('Doc Auth Warning', {})
end
end

Expand Down
12 changes: 6 additions & 6 deletions spec/requests/rack_attack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
end

it 'throttles with a custom response' do
analytics = instance_double(Analytics)
analytics = FakeAnalytics.new
allow(Analytics).to receive(:new).and_return(analytics)
allow(analytics).to receive(:track_event)

Expand All @@ -83,7 +83,7 @@
it 'does not throttle if the path is in the allowlist' do
allow(IdentityConfig.store).to receive(:requests_per_ip_path_prefixes_allowlist).
and_return(['/account'])
analytics = instance_double(Analytics)
analytics = FakeAnalytics.new
allow(Analytics).to receive(:new).and_return(analytics)
allow(analytics).to receive(:track_event)

Expand All @@ -99,7 +99,7 @@
end

it 'does not throttle if the ip is in the CIDR block allowlist' do
analytics = instance_double(Analytics)
analytics = FakeAnalytics.new
allow(Analytics).to receive(:new).and_return(analytics)
allow(analytics).to receive(:track_event)

Expand All @@ -121,7 +121,7 @@
end

it 'logs the user UUID' do
analytics = instance_double(Analytics)
analytics = FakeAnalytics.new
allow(Analytics).to receive(:new).and_return(analytics)
allow(analytics).to receive(:track_event)

Expand Down Expand Up @@ -191,7 +191,7 @@
end

it 'throttles with a custom response' do
analytics = instance_double(Analytics)
analytics = FakeAnalytics.new
allow(Analytics).to receive(:new).and_return(analytics)
allow(analytics).to receive(:track_event)

Expand Down Expand Up @@ -254,7 +254,7 @@

context 'when number of logins per email + ip is higher than limit per period' do
it 'throttles with a custom response' do
analytics = instance_double(Analytics)
analytics = FakeAnalytics.new
allow(Analytics).to receive(:new).and_return(analytics)
allow(analytics).to receive(:track_event)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@

expect(analytics).to have_logged_event('Proofing Document Result Missing', {})
expect(analytics).to have_logged_event(
Analytics::DOC_AUTH_ASYNC,
'Doc Auth Async',
error: 'failed to load verify_document_capture_session',
uuid: nil,
)
Expand All @@ -69,7 +69,7 @@

expect(analytics).to have_logged_event('Proofing Document Result Missing', {})
expect(analytics).to have_logged_event(
Analytics::DOC_AUTH_ASYNC,
'Doc Auth Async',
error: 'failed to load async result',
uuid: verify_document_capture_session.uuid,
result_id: verify_document_capture_session.result_id,
Expand Down