From 71fc5f78dc61a4e7c2b0f38a8c7ced3b84f80d67 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 23 May 2022 10:33:55 -0700 Subject: [PATCH 1/7] Migrate PASSWORD_CHANGED event --- app/controllers/users/passwords_controller.rb | 2 +- app/services/analytics.rb | 1 - app/services/analytics_events.rb | 7 +++++++ spec/controllers/users/passwords_controller_spec.rb | 4 ++-- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index a6116b0698f..df32ed2db1e 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -15,7 +15,7 @@ def update result = @update_user_password_form.submit(user_params) - analytics.track_event(Analytics::PASSWORD_CHANGED, result.to_h) + analytics.password_changed(**result.to_h) if result.success? handle_valid_password diff --git a/app/services/analytics.rb b/app/services/analytics.rb index f2ceeb6eff8..b5ccec4b983 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -153,7 +153,6 @@ def session_started_at OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request' OPENID_CONNECT_TOKEN = 'OpenID Connect: token' OTP_DELIVERY_SELECTION = 'OTP: Delivery Selection' - PASSWORD_CHANGED = 'Password Changed' PASSWORD_CREATION = 'Password Creation' PASSWORD_MAX_ATTEMPTS = 'Password Max Attempts Reached' PASSWORD_RESET_EMAIL = 'Password Reset: Email Submitted' diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index d3982602e01..e9bd6249769 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -987,6 +987,13 @@ def multi_factor_auth_enter_webauthn_visit( ) end + # @param [Boolean] success + # @param [Hash] errors + # The user updated their password + def password_changed(success:, errors:, **extra) + track_event('Password Changed', success: success, errors: errors, **extra) + end + # User has visited the page that lets them confirm if they want a new personal key def profile_personal_key_visit track_event('Profile: Visited new personal key') diff --git a/spec/controllers/users/passwords_controller_spec.rb b/spec/controllers/users/passwords_controller_spec.rb index d3be5af633d..bbf18c82d6a 100644 --- a/spec/controllers/users/passwords_controller_spec.rb +++ b/spec/controllers/users/passwords_controller_spec.rb @@ -14,7 +14,7 @@ patch :update, params: { update_user_password_form: params } expect(@analytics).to have_received(:track_event). - with(Analytics::PASSWORD_CHANGED, success: true, errors: {}) + with('Password Changed', success: true, errors: {}) expect(response).to redirect_to account_url expect(flash[:info]).to eq t('notices.password_changed') expect(flash[:personal_key]).to be_nil @@ -73,7 +73,7 @@ patch :update, params: { update_user_password_form: params } expect(@analytics).to have_received(:track_event).with( - Analytics::PASSWORD_CHANGED, + 'Password Changed', success: false, errors: { password: [ From 95cd14916bbfca83408c9b3b272f66ed2f21ff57 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 23 May 2022 10:39:48 -0700 Subject: [PATCH 2/7] Migrate PASSWORD_CREATION event --- app/controllers/sign_up/passwords_controller.rb | 2 +- app/services/analytics.rb | 1 - app/services/analytics_events.rb | 7 +++++++ spec/controllers/sign_up/passwords_controller_spec.rb | 4 ++-- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/controllers/sign_up/passwords_controller.rb b/app/controllers/sign_up/passwords_controller.rb index eda5b6b724b..b5699926e3a 100644 --- a/app/controllers/sign_up/passwords_controller.rb +++ b/app/controllers/sign_up/passwords_controller.rb @@ -13,7 +13,7 @@ def new def create result = password_form.submit(permitted_params) - analytics.track_event(Analytics::PASSWORD_CREATION, result.to_h) + analytics.password_creation(**result.to_h) store_sp_metadata_in_session unless sp_request_id.empty? if result.success? diff --git a/app/services/analytics.rb b/app/services/analytics.rb index b5ccec4b983..154b5f87c2c 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -153,7 +153,6 @@ def session_started_at OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request' OPENID_CONNECT_TOKEN = 'OpenID Connect: token' OTP_DELIVERY_SELECTION = 'OTP: Delivery Selection' - PASSWORD_CREATION = 'Password Creation' PASSWORD_MAX_ATTEMPTS = 'Password Max Attempts Reached' PASSWORD_RESET_EMAIL = 'Password Reset: Email Submitted' PASSWORD_RESET_PASSWORD = 'Password Reset: Password Submitted' diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index e9bd6249769..e8cb003ec2a 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -994,6 +994,13 @@ def password_changed(success:, errors:, **extra) track_event('Password Changed', success: success, errors: errors, **extra) end + # @param [Boolean] success + # @param [Hash] errors + # The user added a password after verifying their email for account creatoin + def password_creation(success:, errors:, **extra) + track_event('Password Creation', success: success, errors: errors, **extra) + end + # User has visited the page that lets them confirm if they want a new personal key def profile_personal_key_visit track_event('Profile: Visited new personal key') diff --git a/spec/controllers/sign_up/passwords_controller_spec.rb b/spec/controllers/sign_up/passwords_controller_spec.rb index f4731083f7d..67dd9dbc8a5 100644 --- a/spec/controllers/sign_up/passwords_controller_spec.rb +++ b/spec/controllers/sign_up/passwords_controller_spec.rb @@ -21,7 +21,7 @@ { errors: {}, success: true, user_id: user.uuid }, ) expect(@analytics).to receive(:track_event). - with(Analytics::PASSWORD_CREATION, analytics_hash) + with('Password Creation', analytics_hash) post :create, params: { password_form: { password: 'NewVal!dPassw0rd' }, @@ -85,7 +85,7 @@ ) expect(@analytics).to receive(:track_event). - with(Analytics::PASSWORD_CREATION, analytics_hash) + with('Password Creation', analytics_hash) post :create, params: { password_form: { password: 'NewVal' }, confirmation_token: token } end From a18df6738e9297410570ad93af67d853d7d838c8 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 23 May 2022 10:42:21 -0700 Subject: [PATCH 3/7] Migrate PASSWORD_MAX_ATTEMPTS event --- app/controllers/mfa_confirmation_controller.rb | 2 +- app/controllers/password_capture_controller.rb | 2 +- app/services/analytics.rb | 1 - app/services/analytics_events.rb | 5 +++++ spec/controllers/mfa_confirmation_controller_spec.rb | 2 +- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/mfa_confirmation_controller.rb b/app/controllers/mfa_confirmation_controller.rb index 0bce0a24559..f5e2b12e853 100644 --- a/app/controllers/mfa_confirmation_controller.rb +++ b/app/controllers/mfa_confirmation_controller.rb @@ -71,7 +71,7 @@ def handle_invalid_password end def handle_max_password_attempts_reached - analytics.track_event(Analytics::PASSWORD_MAX_ATTEMPTS) + analytics.password_max_attempts sign_out redirect_to root_url, flash: { error: t('errors.max_password_attempts_reached') } end diff --git a/app/controllers/password_capture_controller.rb b/app/controllers/password_capture_controller.rb index 94dd31ad2a8..7954917a0a2 100644 --- a/app/controllers/password_capture_controller.rb +++ b/app/controllers/password_capture_controller.rb @@ -54,7 +54,7 @@ def handle_invalid_password end def handle_max_password_attempts_reached - analytics.track_event(Analytics::PASSWORD_MAX_ATTEMPTS) + analytics.password_max_attempts sign_out redirect_to root_url, flash: { error: t('errors.max_password_attempts_reached') } end diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 154b5f87c2c..65afb41e8ce 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -153,7 +153,6 @@ def session_started_at OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request' OPENID_CONNECT_TOKEN = 'OpenID Connect: token' OTP_DELIVERY_SELECTION = 'OTP: Delivery Selection' - PASSWORD_MAX_ATTEMPTS = 'Password Max Attempts Reached' PASSWORD_RESET_EMAIL = 'Password Reset: Email Submitted' PASSWORD_RESET_PASSWORD = 'Password Reset: Password Submitted' PASSWORD_RESET_TOKEN = 'Password Reset: Token Submitted' diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index e8cb003ec2a..34fb5886d56 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1001,6 +1001,11 @@ def password_creation(success:, errors:, **extra) track_event('Password Creation', success: success, errors: errors, **extra) end + # The user got their password incorrect the max number of times, their session was terminated + def password_max_attempts + track_event('Password Max Attempts Reached') + end + # User has visited the page that lets them confirm if they want a new personal key def profile_personal_key_visit track_event('Profile: Visited new personal key') diff --git a/spec/controllers/mfa_confirmation_controller_spec.rb b/spec/controllers/mfa_confirmation_controller_spec.rb index 22453bfc17d..17cf6b8599b 100644 --- a/spec/controllers/mfa_confirmation_controller_spec.rb +++ b/spec/controllers/mfa_confirmation_controller_spec.rb @@ -100,7 +100,7 @@ expect(controller.current_user).to be_nil expect(flash[:error]).to eq t('errors.max_password_attempts_reached') expect(@analytics).to have_received(:track_event). - with(Analytics::PASSWORD_MAX_ATTEMPTS) + with('Password Max Attempts Reached') end end From d560c60445b1fca648aebf1f2815c09a911308f6 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 23 May 2022 10:46:51 -0700 Subject: [PATCH 4/7] Migrate PASSWORD_RESET_EMAIL event --- .../users/reset_passwords_controller.rb | 2 +- app/services/analytics.rb | 1 - app/services/analytics_events.rb | 18 ++++++++++++++++++ .../users/reset_passwords_controller_spec.rb | 10 +++++----- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/controllers/users/reset_passwords_controller.rb b/app/controllers/users/reset_passwords_controller.rb index 94418978cce..f86ea8c2b79 100644 --- a/app/controllers/users/reset_passwords_controller.rb +++ b/app/controllers/users/reset_passwords_controller.rb @@ -9,7 +9,7 @@ def create @password_reset_email_form = PasswordResetEmailForm.new(email) result = @password_reset_email_form.submit - analytics.track_event(Analytics::PASSWORD_RESET_EMAIL, result.to_h) + analytics.password_reset_email(result.to_h) if result.success? handle_valid_email diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 65afb41e8ce..c9ea609bf74 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -153,7 +153,6 @@ def session_started_at OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request' OPENID_CONNECT_TOKEN = 'OpenID Connect: token' OTP_DELIVERY_SELECTION = 'OTP: Delivery Selection' - PASSWORD_RESET_EMAIL = 'Password Reset: Email Submitted' PASSWORD_RESET_PASSWORD = 'Password Reset: Password Submitted' PASSWORD_RESET_TOKEN = 'Password Reset: Token Submitted' PASSWORD_RESET_VISIT = 'Password Reset: Email Form Visited' diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 34fb5886d56..5a0eb3f8262 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1006,6 +1006,24 @@ def password_max_attempts track_event('Password Max Attempts Reached') end + # @param [Boolean] success + # @param [Hash] errors + # @param [Boolean, nil] confirmed if the account the reset is being requested for has a + # confirmed email + # @param [Boolean, nil] active_profile if the account the reset is being requested for has an + # active proofed profile + # The user entered an email address to request a password reset + def password_reset_email(success:, errors:, confirmed:, active_profile:, **extra) + track_event( + 'Password Reset: Email Submitted', + success: success, + errors: errors, + confirmed: confirmed, + active_profile: active_profile, + **extra, + ) + end + # User has visited the page that lets them confirm if they want a new personal key def profile_personal_key_visit track_event('Profile: Visited new personal key') diff --git a/spec/controllers/users/reset_passwords_controller_spec.rb b/spec/controllers/users/reset_passwords_controller_spec.rb index ff39d954638..839b1ac5db2 100644 --- a/spec/controllers/users/reset_passwords_controller_spec.rb +++ b/spec/controllers/users/reset_passwords_controller_spec.rb @@ -322,7 +322,7 @@ } expect(analytics).to have_received(:track_event). - with(Analytics::PASSWORD_RESET_EMAIL, analytics_hash) + with('Password Reset: Email Submitted', analytics_hash) analytics_hash = { success: true, @@ -354,7 +354,7 @@ } expect(@analytics).to receive(:track_event). - with(Analytics::PASSWORD_RESET_EMAIL, analytics_hash) + with('Password Reset: Email Submitted', analytics_hash) expect do put :create, params: { password_reset_email_form: { email: 'Test@example.com' } } @@ -379,7 +379,7 @@ } expect(@analytics).to receive(:track_event). - with(Analytics::PASSWORD_RESET_EMAIL, analytics_hash) + with('Password Reset: Email Submitted', analytics_hash) params = { password_reset_email_form: { email: user.email } } expect { put :create, params: params }. @@ -408,7 +408,7 @@ } expect(@analytics).to receive(:track_event). - with(Analytics::PASSWORD_RESET_EMAIL, analytics_hash) + with('Password Reset: Email Submitted', analytics_hash) params = { password_reset_email_form: { email: user.email } } put :create, params: params @@ -429,7 +429,7 @@ } expect(@analytics).to receive(:track_event). - with(Analytics::PASSWORD_RESET_EMAIL, analytics_hash) + with('Password Reset: Email Submitted', analytics_hash) params = { password_reset_email_form: { email: 'foo' } } expect { put :create, params: params }. From a11dbcc3157b65a80bcfd73c82c51404cc7638c7 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 23 May 2022 10:53:27 -0700 Subject: [PATCH 5/7] Migrate PASSWORD_RESET_PASSWORD event --- app/services/analytics.rb | 1 - app/services/analytics_events.rb | 15 +++++++++++++++ .../users/reset_passwords_controller_spec.rb | 10 +++++----- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/services/analytics.rb b/app/services/analytics.rb index c9ea609bf74..ed234016a42 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -153,7 +153,6 @@ def session_started_at OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request' OPENID_CONNECT_TOKEN = 'OpenID Connect: token' OTP_DELIVERY_SELECTION = 'OTP: Delivery Selection' - PASSWORD_RESET_PASSWORD = 'Password Reset: Password Submitted' PASSWORD_RESET_TOKEN = 'Password Reset: Token Submitted' PASSWORD_RESET_VISIT = 'Password Reset: Email Form Visited' PENDING_ACCOUNT_RESET_CANCELLED = 'Pending account reset cancelled' diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 5a0eb3f8262..699e6849399 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1024,6 +1024,21 @@ def password_reset_email(success:, errors:, confirmed:, active_profile:, **extra ) end + # @param [Boolean] success + # @param [Hash] errors + # @param [Boolean] profile_deactivated if the active profile for the account was deactivated + # (the user will need to use their personal key to reactivate their profile) + # The user changed the password for their account via the paswword reset flow + def password_reset_password(success:, errors:, profile_deactivated:, **extra) + track_event( + 'Password Reset: Password Submitted', + success: success, + errors: errors, + profile_deactivated: profile_deactivated, + **extra, + ) + end + # User has visited the page that lets them confirm if they want a new personal key def profile_personal_key_visit track_event('Profile: Visited new personal key') diff --git a/spec/controllers/users/reset_passwords_controller_spec.rb b/spec/controllers/users/reset_passwords_controller_spec.rb index 839b1ac5db2..4168637764c 100644 --- a/spec/controllers/users/reset_passwords_controller_spec.rb +++ b/spec/controllers/users/reset_passwords_controller_spec.rb @@ -114,7 +114,7 @@ } expect(@analytics).to have_received(:track_event). - with(Analytics::PASSWORD_RESET_PASSWORD, analytics_hash) + with('Password Reset: Password Submitted', analytics_hash) expect(response).to redirect_to new_user_password_path expect(flash[:error]).to eq t('devise.passwords.token_expired') @@ -147,7 +147,7 @@ } expect(@analytics).to receive(:track_event). - with(Analytics::PASSWORD_RESET_PASSWORD, analytics_hash) + with('Password Reset: Password Submitted', analytics_hash) put :update, params: { reset_password_form: form_params } @@ -210,7 +210,7 @@ } expect(@analytics).to have_received(:track_event). - with(Analytics::PASSWORD_RESET_PASSWORD, analytics_hash) + with('Password Reset: Password Submitted', analytics_hash) expect(user.events.password_changed.size).to be 1 @@ -251,7 +251,7 @@ } expect(@analytics).to have_received(:track_event). - with(Analytics::PASSWORD_RESET_PASSWORD, analytics_hash) + with('Password Reset: Password Submitted', analytics_hash) expect(user.active_profile.present?).to eq false @@ -290,7 +290,7 @@ } expect(@analytics).to have_received(:track_event). - with(Analytics::PASSWORD_RESET_PASSWORD, analytics_hash) + with('Password Reset: Password Submitted', analytics_hash) expect(user.reload.confirmed?).to eq true From bf1207df6adb2f2380816e38e786a93c62f7df97 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 23 May 2022 10:58:03 -0700 Subject: [PATCH 6/7] Fix a few stragglers changelog: Internal, Documentation, Document additional analytics events --- .../users/reset_passwords_controller.rb | 4 ++-- .../users/reset_passwords_controller_spec.rb | 14 +++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/app/controllers/users/reset_passwords_controller.rb b/app/controllers/users/reset_passwords_controller.rb index f86ea8c2b79..e78113b52d6 100644 --- a/app/controllers/users/reset_passwords_controller.rb +++ b/app/controllers/users/reset_passwords_controller.rb @@ -9,7 +9,7 @@ def create @password_reset_email_form = PasswordResetEmailForm.new(email) result = @password_reset_email_form.submit - analytics.password_reset_email(result.to_h) + analytics.password_reset_email(**result.to_h) if result.success? handle_valid_email @@ -38,7 +38,7 @@ def update result = @reset_password_form.submit(user_params) - analytics.track_event(Analytics::PASSWORD_RESET_PASSWORD, result.to_h) + analytics.password_reset_password(**result.to_h) if result.success? handle_successful_password_reset diff --git a/spec/controllers/users/reset_passwords_controller_spec.rb b/spec/controllers/users/reset_passwords_controller_spec.rb index 4168637764c..22876ecda4c 100644 --- a/spec/controllers/users/reset_passwords_controller_spec.rb +++ b/spec/controllers/users/reset_passwords_controller_spec.rb @@ -1,9 +1,9 @@ require 'rails_helper' describe Users::ResetPasswordsController, devise: true do - let(:password_error_message) { + let(:password_error_message) do "This password is too short (minimum is #{Devise.password_length.first} characters)" - } + end describe '#edit' do context 'no user matches token' do it 'redirects to page where user enters email for password reset token' do @@ -302,9 +302,7 @@ describe '#create' do context 'no user matches email' do it 'send an email to tell the user they do not have an account yet' do - analytics = instance_double(Analytics) - allow(Analytics).to receive(:new).and_return(analytics) - allow(analytics).to receive(:track_event) + stub_analytics email = 'nonexistent@example.com' expect do @@ -321,8 +319,7 @@ active_profile: false, } - expect(analytics).to have_received(:track_event). - with('Password Reset: Email Submitted', analytics_hash) + expect(@analytics).to have_logged_event('Password Reset: Email Submitted', analytics_hash) analytics_hash = { success: true, @@ -332,8 +329,7 @@ user_id: User.find_with_email(email).uuid, domain_name: 'example.com', } - expect(analytics).to have_received(:track_event). - with(Analytics::USER_REGISTRATION_EMAIL, analytics_hash) + expect(@analytics).to have_logged_event(Analytics::USER_REGISTRATION_EMAIL, analytics_hash) expect(response).to redirect_to forgot_password_path end From 37bd4d1930be9899851fc3a704f4366b634fb697 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Mon, 23 May 2022 11:42:59 -0700 Subject: [PATCH 7/7] Fix typo Co-authored-by: Steve Urciuoli --- app/services/analytics_events.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 699e6849399..0963d24fcf0 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -996,7 +996,7 @@ def password_changed(success:, errors:, **extra) # @param [Boolean] success # @param [Hash] errors - # The user added a password after verifying their email for account creatoin + # The user added a password after verifying their email for account creation def password_creation(success:, errors:, **extra) track_event('Password Creation', success: success, errors: errors, **extra) end