diff --git a/app/controllers/concerns/sign_in_duration_concern.rb b/app/controllers/concerns/sign_in_duration_concern.rb new file mode 100644 index 00000000000..d90b7f69bb7 --- /dev/null +++ b/app/controllers/concerns/sign_in_duration_concern.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module SignInDurationConcern + extend ActiveSupport::Concern + + def sign_in_duration_seconds + return unless session[:sign_in_page_visited_at] + (Time.zone.now - Time.zone.parse(session[:sign_in_page_visited_at])).seconds.to_f + end +end diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 074d1dbfb82..3862b699ef5 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -10,6 +10,7 @@ class AuthorizationController < ApplicationController include BillableEventTrackable include ForcedReauthenticationConcern include OpenidConnectRedirectConcern + include SignInDurationConcern before_action :build_authorize_form_from_params, only: [:index] before_action :block_biometric_requests_in_production, only: [:index] @@ -215,13 +216,13 @@ def track_events service_provider: @authorize_form.service_provider, user: current_user, ) - analytics.sp_redirect_initiated( ial: event_ial_context.ial, billed_ial: event_ial_context.bill_for_ial_1_or_2, sign_in_flow: session[:sign_in_flow], vtr: sp_session[:vtr], acr_values: sp_session[:acr_values], + sign_in_duration_seconds:, ) track_billing_events end diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index b4dd3343e81..db8368526eb 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -17,6 +17,7 @@ class SamlIdpController < ApplicationController include AuthorizationCountConcern include BillableEventTrackable include SecureHeadersConcern + include SignInDurationConcern prepend_before_action :skip_session_load, only: [:metadata, :remotelogout] prepend_before_action :skip_session_expiration, only: [:metadata, :remotelogout] @@ -187,6 +188,7 @@ def track_events sign_in_flow: session[:sign_in_flow], vtr: sp_session[:vtr], acr_values: sp_session[:acr_values], + sign_in_duration_seconds:, ) track_billing_events end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 7b1555de346..88dde2be88e 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -24,6 +24,7 @@ def new issuer: decorated_sp_session.sp_issuer, ) analytics.sign_in_page_visit(flash: flash[:alert]) + session[:sign_in_page_visited_at] = Time.zone.now.to_s super end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 2a4b3ea8482..b874bbe0812 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -5070,7 +5070,16 @@ def sp_inactive_visit # @param [String, nil] sign_in_flow # @param [String, nil] vtr # @param [String, nil] acr_values - def sp_redirect_initiated(ial:, billed_ial:, sign_in_flow:, vtr:, acr_values:, **extra) + # @param [Integer] sign_in_duration_seconds + def sp_redirect_initiated( + ial:, + billed_ial:, + sign_in_flow:, + vtr:, + acr_values:, + sign_in_duration_seconds:, + **extra + ) track_event( 'SP redirect initiated', ial:, @@ -5078,6 +5087,7 @@ def sp_redirect_initiated(ial:, billed_ial:, sign_in_flow:, vtr:, acr_values:, * sign_in_flow:, vtr: vtr, acr_values: acr_values, + sign_in_duration_seconds:, **extra, ) end diff --git a/spec/controllers/concerns/sign_in_duration_concern_spec.rb b/spec/controllers/concerns/sign_in_duration_concern_spec.rb new file mode 100644 index 00000000000..e8183c30b23 --- /dev/null +++ b/spec/controllers/concerns/sign_in_duration_concern_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +RSpec.describe SignInDurationConcern, type: :controller do + let(:test_class) do + Class.new do + include SignInDurationConcern + + attr_reader :session + + def initialize(session = {}) + @session = session + end + end + end + + let(:instance) { test_class.new } + + describe '#sign_in_duration_seconds' do + let(:sign_in_page_visited_at) {} + around do |example| + freeze_time { example.run } + end + + before do + instance.session[:sign_in_page_visited_at] = sign_in_page_visited_at + end + + context 'when session value is assigned' do + let(:sign_in_page_visited_at) { 6.seconds.ago.to_s } + it 'returns seconds since value' do + expect(instance.sign_in_duration_seconds).to eq(6) + end + end + + context 'when session value is not assigned' do + let(:sign_in_page_visited_at) { nil } + it 'returns nil' do + expect(instance.sign_in_duration_seconds).to be_nil + end + end + end +end diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index 9f27bd1abe1..bf24475ee3e 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -52,6 +52,13 @@ before do stub_sign_in user session[:sign_in_flow] = sign_in_flow + session[:sign_in_page_visited_at] = Time.zone.now.to_s + end + + let(:now) { Time.zone.now } + + around do |ex| + freeze_time { ex.run } end context 'acr with valid params' do @@ -107,6 +114,7 @@ context 'with ial1 requested using acr_values' do it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -145,6 +153,7 @@ 'SP redirect initiated', ial: 1, billed_ial: 1, + sign_in_duration_seconds: 15, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', vtr: nil, @@ -161,6 +170,7 @@ end it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -200,6 +210,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 1, + sign_in_duration_seconds: 15, billed_ial: 1, sign_in_flow:, acr_values: '', @@ -354,6 +365,7 @@ end it 'tracks IAL2 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 2) @@ -395,6 +407,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 2, + sign_in_duration_seconds: 15, billed_ial: 2, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/2', @@ -728,6 +741,7 @@ end it 'tracks IAL2 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 2) @@ -769,6 +783,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 0, + sign_in_duration_seconds: 15, billed_ial: 2, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', @@ -820,6 +835,7 @@ end it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -860,6 +876,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 0, + sign_in_duration_seconds: 15, billed_ial: 1, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', @@ -914,6 +931,7 @@ end it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -954,6 +972,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 0, + sign_in_duration_seconds: 15, billed_ial: 1, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', @@ -1081,6 +1100,7 @@ let(:vtr) { nil } it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) user.identities.last.update!(verified_attributes: %w[given_name family_name birthdate]) @@ -1119,6 +1139,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 1, + sign_in_duration_seconds: 15, billed_ial: 1, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', @@ -1136,6 +1157,7 @@ end it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -1175,6 +1197,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 1, + sign_in_duration_seconds: 15, billed_ial: 1, sign_in_flow:, acr_values: '', @@ -1330,6 +1353,7 @@ end it 'tracks IAL2 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 2) @@ -1371,6 +1395,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 2, + sign_in_duration_seconds: 15, billed_ial: 2, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/2', @@ -1706,6 +1731,7 @@ end it 'tracks IAL2 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 2) @@ -1747,6 +1773,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 0, + sign_in_duration_seconds: 15, billed_ial: 2, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', @@ -1798,6 +1825,7 @@ end it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -1838,6 +1866,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 0, + sign_in_duration_seconds: 15, billed_ial: 1, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', @@ -1892,6 +1921,7 @@ end it 'tracks IAL1 authentication event' do + travel_to now + 15.seconds stub_analytics IdentityLinker.new(user, service_provider).link_identity(ial: 1) @@ -1932,6 +1962,7 @@ expect(@analytics).to have_logged_event( 'SP redirect initiated', ial: 0, + sign_in_duration_seconds: 15, billed_ial: 1, sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/0', diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index ac763552800..ec8a1a73f46 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -817,6 +817,7 @@ def name_id_version(format_urn) expect(@analytics).to receive(:track_event).with( 'SP redirect initiated', ial: Idp::Constants::IAL2, + sign_in_duration_seconds: nil, billed_ial: Idp::Constants::IAL2, sign_in_flow:, acr_values: Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, @@ -965,6 +966,7 @@ def name_id_version(format_urn) expect(@analytics).to receive(:track_event).with( 'SP redirect initiated', ial: 0, + sign_in_duration_seconds: nil, billed_ial: 2, sign_in_flow:, acr_values: Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF, @@ -2434,6 +2436,7 @@ def stub_requested_attributes expect(@analytics).to receive(:track_event).with( 'SP redirect initiated', ial: 1, + sign_in_duration_seconds: nil, billed_ial: 1, sign_in_flow: :sign_in, acr_values: [ @@ -2484,6 +2487,7 @@ def stub_requested_attributes expect(@analytics).to receive(:track_event).with( 'SP redirect initiated', ial: 1, + sign_in_duration_seconds: nil, billed_ial: 1, sign_in_flow: :sign_in, acr_values: [ diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 81ab41fc292..49b782f7303 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -631,6 +631,7 @@ ) get :new + expect(subject.session[:sign_in_page_visited_at]).to_not be(nil) end context 'renders partials' do diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 674321b43f9..e6b18698a01 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -473,21 +473,25 @@ def clipboard_text end it 'logs expected analytics events for end-to-end sign-up' do - analytics = FakeAnalytics.new - allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(analytics) - - visit_idp_from_sp_with_ial1(:oidc) - register_user - click_agree_and_continue - - expect(analytics).to have_logged_event( - 'SP redirect initiated', - ial: 1, - billed_ial: 1, - sign_in_flow: 'create_account', - acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, - vtr: nil, - ) + freeze_time do + analytics = FakeAnalytics.new + allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(analytics) + + visit_idp_from_sp_with_ial1(:oidc) + travel_to Time.zone.now + 15.seconds + register_user + click_agree_and_continue + + expect(analytics).to have_logged_event( + 'SP redirect initiated', + ial: 1, + sign_in_duration_seconds: 15, + billed_ial: 1, + sign_in_flow: 'create_account', + acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + vtr: nil, + ) + end end describe 'visiting the homepage by clicking the logo image' do diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 16548dcb977..ed75dcef0cd 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -32,26 +32,30 @@ end it 'logs expected analytics events' do - user = create(:user, :fully_registered) - analytics = FakeAnalytics.new(user:) - allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(analytics) - - visit_idp_from_sp_with_ial1(sp) - fill_in_credentials_and_submit(user.email, user.password) - fill_in_code_with_last_phone_otp - click_submit_default - click_submit_default if current_path == complete_saml_path - click_agree_and_continue - click_submit_default if current_path == complete_saml_path + freeze_time do + user = create(:user, :fully_registered) + analytics = FakeAnalytics.new(user:) + allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(analytics) - expect(analytics).to have_logged_event( - 'SP redirect initiated', - ial: 1, - billed_ial: 1, - sign_in_flow: 'sign_in', - acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, - vtr: nil, - ) + visit_idp_from_sp_with_ial1(sp) + travel_to Time.zone.now + 15.seconds + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + click_submit_default if current_path == complete_saml_path + click_agree_and_continue + click_submit_default if current_path == complete_saml_path + + expect(analytics).to have_logged_event( + 'SP redirect initiated', + ial: 1, + sign_in_duration_seconds: 15, + billed_ial: 1, + sign_in_flow: 'sign_in', + acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + vtr: nil, + ) + end end end