diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 159e784baf2..d8a1df59251 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -72,7 +72,7 @@ def increment_session_bad_password_count end def process_locked_out_session - warden.logout(:user) + sign_out(:user) warden.lock! flash[:error] = t( @@ -100,7 +100,7 @@ def valid_captcha_result? def process_failed_captcha flash[:error] = t('errors.messages.invalid_recaptcha_token') - warden.logout(:user) + sign_out(:user) warden.lock! redirect_to root_url end @@ -176,6 +176,7 @@ def track_authentication_attempt(email) bad_password_count: session[:bad_password_count].to_i, sp_request_url_present: sp_session[:request_url].present?, remember_device: remember_device_cookie.present?, + new_device: success ? new_device? : nil, ) end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 8bea717ba07..7baab74bdf4 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -404,6 +404,8 @@ def edit_password_visit # @param [String] bad_password_count represents number of prior login failures # @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 + # @param [Boolean, nil] new_device Whether the user is authenticating from a new device. Nil if + # there is the attempt was unsuccessful, since it cannot be known whether it's a new device. # Tracks authentication attempts at the email/password screen def email_and_password_auth( success:, @@ -413,6 +415,7 @@ def email_and_password_auth( bad_password_count:, sp_request_url_present:, remember_device:, + new_device:, **extra ) track_event( @@ -424,6 +427,7 @@ def email_and_password_auth( bad_password_count:, sp_request_url_present:, remember_device:, + new_device:, **extra, ) end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index c22af06d3bd..ad235ebe656 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -57,6 +57,7 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: false, + new_device: true, ) end @@ -113,6 +114,24 @@ response end + + it 'tracks as not being from a new device' do + stub_analytics + + response + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', + success: true, + user_id: user.uuid, + user_locked_out: false, + valid_captcha_result: true, + bad_password_count: 0, + sp_request_url_present: false, + remember_device: false, + new_device: false, + ) + end end end @@ -150,7 +169,12 @@ user = create(:user, :fully_registered) stub_analytics - analytics_hash = { + expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original + + post :create, params: { user: { email: user.email.upcase, password: 'invalid_password' } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: user.uuid, user_locked_out: false, @@ -158,19 +182,19 @@ bad_password_count: 1, sp_request_url_present: false, remember_device: false, - } - expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email.upcase, password: 'invalid_password' } } + new_device: nil, + ) expect(subject.session[:sign_in_flow]).to eq(:sign_in) end it 'tracks the authentication attempt for nonexistent user' do stub_analytics - analytics_hash = { + expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original + + post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: 'anonymous-uuid', user_locked_out: false, @@ -178,13 +202,8 @@ bad_password_count: 1, sp_request_url_present: false, remember_device: false, - } - expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + new_device: nil, + ) end it 'tracks unsuccessful authentication for locked out user' do @@ -195,7 +214,11 @@ ) stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email.upcase, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: user.uuid, user_locked_out: true, @@ -203,12 +226,8 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: false, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email.upcase, password: user.password } } + new_device: nil, + ) end it 'tracks unsuccessful authentication for failed reCAPTCHA' do @@ -229,6 +248,7 @@ valid_captcha_result: false, bad_password_count: 0, remember_device: false, + new_device: nil, sp_request_url_present: false, ) end @@ -241,7 +261,10 @@ stub_analytics - analytics_hash = { + post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: user.uuid, user_locked_out: false, @@ -249,18 +272,18 @@ bad_password_count: 2, sp_request_url_present: false, remember_device: false, - } - - post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - post :create, params: { user: { email: user.email.upcase, password: 'invalid' } } + new_device: nil, + ) end it 'tracks the presence of SP request_url in session' do subject.session[:sp] = { request_url: mock_valid_site } stub_analytics - analytics_hash = { + + post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: false, user_id: 'anonymous-uuid', user_locked_out: false, @@ -268,12 +291,8 @@ bad_password_count: 1, sp_request_url_present: true, remember_device: false, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: 'foo@example.com', password: 'password' } } + new_device: nil, + ) end context 'IAL1 user' do @@ -431,7 +450,11 @@ ) stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: true, user_id: user.uuid, user_locked_out: false, @@ -439,19 +462,12 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: false, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - profile_encryption_error = { + new_device: true, + ) + expect(@analytics).to have_logged_event( + 'Profile Encryption: Invalid', error: 'Unable to parse encrypted payload', - } - expect(@analytics).to receive(:track_event). - with('Profile Encryption: Invalid', profile_encryption_error) - - post :create, params: { user: { email: user.email, password: user.password } } - + ) expect(controller.user_session[:encrypted_profiles]).to be_nil expect(profile.reload).to_not be_active end @@ -558,7 +574,11 @@ } stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: true, user_id: user.uuid, user_locked_out: false, @@ -566,12 +586,8 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: true, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email, password: user.password } } + new_device: true, + ) end end @@ -584,7 +600,11 @@ } stub_analytics - analytics_hash = { + + post :create, params: { user: { email: user.email, password: user.password } } + + expect(@analytics).to have_logged_event( + 'Email and Password Authentication', success: true, user_id: user.uuid, user_locked_out: false, @@ -592,12 +612,8 @@ bad_password_count: 0, sp_request_url_present: false, remember_device: true, - } - - expect(@analytics).to receive(:track_event). - with('Email and Password Authentication', analytics_hash) - - post :create, params: { user: { email: user.email, password: user.password } } + new_device: true, + ) end end