diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index aed6d671f5c..2b7570b4501 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -19,6 +19,7 @@ class SessionsController < Devise::SessionsController before_action :check_user_needs_redirect, only: [:new] before_action :apply_secure_headers_override, only: [:new, :create] before_action :clear_session_bad_password_count_if_window_expired, only: [:create] + before_action :set_analytics_user_from_params, only: :create before_action :allow_csp_recaptcha_src, if: :recaptcha_enabled? after_action :add_recaptcha_resource_hints, if: :recaptcha_enabled? @@ -58,6 +59,10 @@ def destroy end end + def analytics_user + @analytics_user || AnonymousUser.new + end + private def clear_session_bad_password_count_if_window_expired @@ -168,6 +173,10 @@ def auth_params params.require(:user).permit(:email, :password) end + def set_analytics_user_from_params + @analytics_user = user_from_params + end + def process_locked_out_user presenter = TwoFactorAuthCode::MaxAttemptsReachedPresenter.new( 'generic_login_attempts', @@ -210,7 +219,6 @@ def track_authentication_attempt analytics.email_and_password_auth( **recaptcha_response, success: success, - user_id: user.uuid, user_locked_out: user_locked_out?(user), rate_limited: rate_limited?, captcha_validation_performed: captcha_validation_performed?, diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 257cb23c201..a87ab5f980f 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -478,7 +478,6 @@ def edit_password_visit(required_password_change: false, **extra) # @param [Boolean] success Whether form validation was successful # @param [Hash] error_details Details for errors that occurred in unsuccessful submission - # @param [String] user_id UUID for user associated with attempted email address # @param [Boolean] user_locked_out if the user is currently locked out of their second factor # @param [Boolean] rate_limited Whether the user has exceeded user IP rate limiting # @param [Boolean] valid_captcha_result Whether user passed the reCAPTCHA check or was exempt @@ -491,7 +490,6 @@ def edit_password_visit(required_password_change: false, **extra) # Tracks authentication attempts at the email/password screen def email_and_password_auth( success:, - user_id:, user_locked_out:, rate_limited:, valid_captcha_result:, @@ -507,7 +505,6 @@ def email_and_password_auth( 'Email and Password Authentication', success:, error_details:, - user_id:, user_locked_out:, rate_limited:, valid_captcha_result:, diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index c0a3f57cf76..9c614032c48 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -87,14 +87,13 @@ end it 'tracks the successful authentication for existing user' do - stub_analytics + stub_analytics(user:) response expect(@analytics).to have_logged_event( 'Email and Password Authentication', success: true, - user_id: user.uuid, user_locked_out: false, rate_limited: false, valid_captcha_result: true, @@ -161,14 +160,13 @@ end it 'tracks as not being from a new device' do - stub_analytics + stub_analytics(user:) response expect(@analytics).to have_logged_event( 'Email and Password Authentication', success: true, - user_id: user.uuid, user_locked_out: false, rate_limited: false, valid_captcha_result: true, @@ -222,8 +220,8 @@ attempt_window_max: 12.hours.in_minutes, }, ) - stub_analytics user = create(:user, :fully_registered) + stub_analytics(user:) travel_to (3.hours + 1.minute).ago do 2.times do @@ -252,7 +250,6 @@ expect(@analytics).to have_logged_event( 'Email and Password Authentication', success: false, - user_id: user.uuid, user_locked_out: false, rate_limited: true, valid_captcha_result: true, @@ -267,7 +264,7 @@ it 'tracks the unsuccessful authentication for existing user' do user = create(:user, :fully_registered) - stub_analytics + stub_analytics(user:) expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original post :create, params: { user: { email: user.email.upcase, password: 'invalid_password' } } @@ -275,7 +272,6 @@ expect(@analytics).to have_logged_event( 'Email and Password Authentication', success: false, - user_id: user.uuid, user_locked_out: false, rate_limited: false, valid_captcha_result: true, @@ -288,7 +284,7 @@ end it 'tracks the authentication attempt for nonexistent user' do - stub_analytics + stub_analytics(user: kind_of(AnonymousUser)) expect(SCrypt::Engine).to receive(:hash_secret).once.and_call_original post :create, params: { user: { email: 'foo@example.com', password: 'password' } } @@ -296,7 +292,6 @@ expect(@analytics).to have_logged_event( 'Email and Password Authentication', success: false, - user_id: 'anonymous-uuid', user_locked_out: false, rate_limited: false, valid_captcha_result: true, @@ -314,14 +309,13 @@ second_factor_locked_at: Time.zone.now, ) - stub_analytics + stub_analytics(user:) 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, rate_limited: false, valid_captcha_result: true, @@ -365,7 +359,7 @@ it 'tracks unsuccessful authentication for failed reCAPTCHA' do user = create(:user, :fully_registered) - stub_analytics + stub_analytics(user:) post :create, params: { user: { email: user.email, password: user.password, score: 0.1 } } @@ -373,7 +367,6 @@ 'Email and Password Authentication', success: false, error_details: { recaptcha_token: { blank: true } }, - user_id: user.uuid, user_locked_out: false, rate_limited: false, valid_captcha_result: false, @@ -400,14 +393,13 @@ :fully_registered, ) - stub_analytics + stub_analytics(user:) 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, rate_limited: false, valid_captcha_result: true, @@ -420,14 +412,13 @@ it 'tracks the presence of SP request_url in session' do subject.session[:sp] = { request_url: mock_valid_site } - stub_analytics + stub_analytics(user: kind_of(AnonymousUser)) 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, rate_limited: false, valid_captcha_result: true, @@ -592,14 +583,13 @@ }.to_json, ) - stub_analytics + stub_analytics(user:) 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, rate_limited: false, valid_captcha_result: true, @@ -718,14 +708,13 @@ expires: 2.days.from_now, } - stub_analytics + stub_analytics(user:) 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, rate_limited: false, valid_captcha_result: true, @@ -746,14 +735,13 @@ value: RememberDeviceCookie.new(user_id: user.id, created_at: 2.days.ago).to_json, } - stub_analytics + stub_analytics(user:) 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, rate_limited: false, valid_captcha_result: true, diff --git a/spec/support/analytics_helper.rb b/spec/support/analytics_helper.rb index 90920f23576..b4da7cc1cdd 100644 --- a/spec/support/analytics_helper.rb +++ b/spec/support/analytics_helper.rb @@ -9,7 +9,7 @@ def stub_analytics(user: nil) end stub.to receive(:analytics).and_wrap_original do |original| - expect(original.call.user).to eq(user) if user + expect(original.call.user).to match(user) if user analytics end diff --git a/spec/support/fake_analytics.rb b/spec/support/fake_analytics.rb index b67674988e2..6f43aaa06e7 100644 --- a/spec/support/fake_analytics.rb +++ b/spec/support/fake_analytics.rb @@ -171,6 +171,10 @@ def track_event(event, attributes = {}) def browser_attributes {} end + + def reset! + @events = Hash.new + end end RSpec.configure do |c| diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index bbe09be0a3e..8136ec3cbc2 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -324,16 +324,34 @@ def user_with_broken_personal_key(scenario) fake_analytics = FakeAnalytics.new allow_any_instance_of(ApplicationController).to receive(:analytics). and_wrap_original do |original| - original_analytics = original.call - if original_analytics.request.params[:controller] == 'users/sessions' && - original_analytics.request.params[:action] == 'create' - expect(original_analytics.user).to eq(user) + if original.receiver.instance_of?(Users::SessionsController) && + original.receiver.action_name == 'create' + expect(original.call.user).to eq(user) asserted_expected_user = true end fake_analytics end + fill_in_credentials_and_submit(user.email, 'wrongpassword') + expect(asserted_expected_user).to eq(true) + expect(fake_analytics).to have_logged_event( + 'reCAPTCHA verify result received', + recaptcha_result: { + assessment_id: kind_of(String), + success: true, + score: 1.0, + errors: [], + reasons: [], + }, + evaluated_as_valid: true, + score_threshold: 0.2, + recaptcha_action: 'sign_in', + form_class: 'RecaptchaMockForm', + ) + asserted_expected_user = false + fake_analytics.reset! + fill_in :user_recaptcha_mock_score, with: '0.1' fill_in_credentials_and_submit(user.email, user.password) expect(asserted_expected_user).to eq(true)