From e2d93f02665412113368b17563c033ba53743c5a Mon Sep 17 00:00:00 2001 From: Luis Date: Mon, 1 Aug 2022 14:56:16 -0500 Subject: [PATCH 1/4] LG-7304 Add Attempts API Logout Initiated tracking changelog: Internal, Attempts API, Track additional events --- .../concerns/saml_idp_logout_concern.rb | 3 +++ .../openid_connect/logout_controller.rb | 3 +++ app/controllers/sign_out_controller.rb | 3 +++ .../openid_connect/logout_controller_spec.rb | 18 +++++++++++++++--- spec/controllers/saml_idp_controller_spec.rb | 9 +++++++++ spec/controllers/sign_out_controller_spec.rb | 4 ++++ 6 files changed, 37 insertions(+), 3 deletions(-) diff --git a/app/controllers/concerns/saml_idp_logout_concern.rb b/app/controllers/concerns/saml_idp_logout_concern.rb index c78659c2658..0d2aede6236 100644 --- a/app/controllers/concerns/saml_idp_logout_concern.rb +++ b/app/controllers/concerns/saml_idp_logout_concern.rb @@ -58,6 +58,9 @@ def track_logout_event oidc: false, saml_request_valid: sp_initiated ? valid_saml_request? : true, ) + irs_attempts_api_tracker.logout_initiated( + success: true, + ) end def track_remote_logout_event diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index b25132f0d9a..c614725adbc 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -10,6 +10,9 @@ def index result = @logout_form.submit analytics.logout_initiated(**result.to_h.except(:redirect_uri)) + irs_attempts_api_tracker.logout_initiated( + success: result.success?, + ) if result.success? && (redirect_uri = result.extra[:redirect_uri]) sign_out diff --git a/app/controllers/sign_out_controller.rb b/app/controllers/sign_out_controller.rb index 95c2062b1a5..e483e0a907b 100644 --- a/app/controllers/sign_out_controller.rb +++ b/app/controllers/sign_out_controller.rb @@ -3,6 +3,9 @@ class SignOutController < ApplicationController def destroy analytics.logout_initiated(method: 'cancel link') + irs_attempts_api_tracker.logout_initiated( + success: true, + ) url_after_cancellation = decorated_session.cancel_link_url sign_out flash[:success] = t('devise.sessions.signed_out') diff --git a/spec/controllers/openid_connect/logout_controller_spec.rb b/spec/controllers/openid_connect/logout_controller_spec.rb index 5337baeb0dd..b788d80f3dd 100644 --- a/spec/controllers/openid_connect/logout_controller_spec.rb +++ b/spec/controllers/openid_connect/logout_controller_spec.rb @@ -51,7 +51,7 @@ expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end - it 'tracks analytics' do + it 'tracks events' do stub_analytics expect(@analytics).to receive(:track_event). with( @@ -65,6 +65,10 @@ ), ) + expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated). + with( + success: true, + ) action end end @@ -84,7 +88,7 @@ action end - it 'tracks analytics' do + it 'tracks events' do stub_analytics errors = { @@ -102,6 +106,10 @@ method: nil, saml_request_valid: nil, ) + expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated). + with( + success: false, + ) action end @@ -109,7 +117,7 @@ context 'with a bad id_token_hint' do let(:id_token_hint) { { id_token_hint: 'abc123' } } - it 'tracks analytics' do + it 'tracks events' do stub_analytics errors_keys = [:id_token_hint, :redirect_uri] @@ -125,6 +133,10 @@ method: nil, saml_request_valid: nil, ) + expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated). + with( + success: false, + ) action end diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 9eda5cf5a2f..25cd5906095 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -11,6 +11,9 @@ result = { sp_initiated: false, oidc: false, saml_request_valid: true } expect(@analytics).to receive(:track_event).with('Logout Initiated', hash_including(result)) + expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + success: true, + ) delete :logout end @@ -21,6 +24,9 @@ result = { sp_initiated: true, oidc: false, saml_request_valid: true } expect(@analytics).to receive(:track_event).with('Logout Initiated', hash_including(result)) + expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + success: true, + ) delete :logout, params: { SAMLRequest: 'foo' } end @@ -30,6 +36,9 @@ result = { sp_initiated: true, oidc: false, saml_request_valid: false } expect(@analytics).to receive(:track_event).with('Logout Initiated', hash_including(result)) + expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + success: true, + ) delete :logout, params: { SAMLRequest: 'foo' } end diff --git a/spec/controllers/sign_out_controller_spec.rb b/spec/controllers/sign_out_controller_spec.rb index ca4eafd1d6c..e76b589a5f5 100644 --- a/spec/controllers/sign_out_controller_spec.rb +++ b/spec/controllers/sign_out_controller_spec.rb @@ -27,6 +27,10 @@ expect(@analytics). to receive(:track_event).with('Logout Initiated', hash_including(method: 'cancel link')) + expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + success: true, + ) + get :destroy end end From 97b0a2e2de673d220397e4abeae8223d205534fe Mon Sep 17 00:00:00 2001 From: Luis Date: Mon, 1 Aug 2022 15:03:18 -0500 Subject: [PATCH 2/4] LG-7304 Fix Lint errors --- app/controllers/sign_out_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/sign_out_controller.rb b/app/controllers/sign_out_controller.rb index e483e0a907b..523bb78c3a2 100644 --- a/app/controllers/sign_out_controller.rb +++ b/app/controllers/sign_out_controller.rb @@ -4,8 +4,8 @@ class SignOutController < ApplicationController def destroy analytics.logout_initiated(method: 'cancel link') irs_attempts_api_tracker.logout_initiated( - success: true, - ) + success: true, + ) url_after_cancellation = decorated_session.cancel_link_url sign_out flash[:success] = t('devise.sessions.signed_out') From de476397e3c64eeae20c9605c5d2323e001e952b Mon Sep 17 00:00:00 2001 From: Luis Date: Mon, 1 Aug 2022 16:01:37 -0500 Subject: [PATCH 3/4] LG-7304 Remove unnecessary properties --- app/controllers/users/sessions_controller.rb | 2 -- app/services/irs_attempts_api/tracker_events.rb | 4 +--- spec/controllers/users/sessions_controller_spec.rb | 4 ---- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 9806097fdc8..33edc12d6d3 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -41,8 +41,6 @@ def create def destroy analytics.logout_initiated(sp_initiated: false, oidc: false) irs_attempts_api_tracker.logout_initiated( - user_uuid: current_user.uuid, - unique_session_id: current_user.unique_session_id, success: true, ) super diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index 36d2fe73599..6a916c3e7f3 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -15,11 +15,9 @@ def email_and_password_auth(email:, success:) # @param [String] unique_session_id The unique session id # @param [Boolean] success True if the email and password matched # A user has initiated a logout event - def logout_initiated(user_uuid:, unique_session_id:, success:) + def logout_initiated(success:) track_event( :logout_initiated, - user_uuid: user_uuid, - unique_session_id: unique_session_id, success: success, ) end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 56080339b47..886320c6bcb 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -129,8 +129,6 @@ sign_in_as_user expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( - user_uuid: controller.current_user.uuid, - unique_session_id: controller.current_user.unique_session_id, success: true, ) @@ -153,8 +151,6 @@ sign_in_as_user expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( - user_uuid: controller.current_user.uuid, - unique_session_id: controller.current_user.unique_session_id, success: true, ) From bca8947d54201c5e190c81cdb5d638128be6b269 Mon Sep 17 00:00:00 2001 From: Luis Date: Fri, 5 Aug 2022 10:58:22 -0500 Subject: [PATCH 4/4] LG-7304 Better stubbing --- .../controllers/openid_connect/logout_controller_spec.rb | 9 ++++++--- spec/controllers/saml_idp_controller_spec.rb | 9 ++++++--- spec/controllers/sign_out_controller_spec.rb | 3 ++- spec/controllers/users/sessions_controller_spec.rb | 9 ++++++--- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/spec/controllers/openid_connect/logout_controller_spec.rb b/spec/controllers/openid_connect/logout_controller_spec.rb index b788d80f3dd..6a90665bbd5 100644 --- a/spec/controllers/openid_connect/logout_controller_spec.rb +++ b/spec/controllers/openid_connect/logout_controller_spec.rb @@ -65,7 +65,8 @@ ), ) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated). + stub_attempts_tracker + expect(@irs_attempts_api_tracker).to receive(:logout_initiated). with( success: true, ) @@ -106,7 +107,8 @@ method: nil, saml_request_valid: nil, ) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated). + stub_attempts_tracker + expect(@irs_attempts_api_tracker).to receive(:logout_initiated). with( success: false, ) @@ -133,7 +135,8 @@ method: nil, saml_request_valid: nil, ) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated). + stub_attempts_tracker + expect(@irs_attempts_api_tracker).to receive(:logout_initiated). with( success: false, ) diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 25cd5906095..57b892e30ea 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -8,10 +8,11 @@ describe '/api/saml/logout' do it 'tracks the event when idp initiated' do stub_analytics + stub_attempts_tracker result = { sp_initiated: false, oidc: false, saml_request_valid: true } expect(@analytics).to receive(:track_event).with('Logout Initiated', hash_including(result)) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + expect(@irs_attempts_api_tracker).to receive(:logout_initiated).with( success: true, ) @@ -21,10 +22,11 @@ it 'tracks the event when sp initiated' do allow(controller).to receive(:saml_request).and_return(FakeSamlLogoutRequest.new) stub_analytics + stub_attempts_tracker result = { sp_initiated: true, oidc: false, saml_request_valid: true } expect(@analytics).to receive(:track_event).with('Logout Initiated', hash_including(result)) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + expect(@irs_attempts_api_tracker).to receive(:logout_initiated).with( success: true, ) @@ -33,10 +35,11 @@ it 'tracks the event when the saml request is invalid' do stub_analytics + stub_attempts_tracker result = { sp_initiated: true, oidc: false, saml_request_valid: false } expect(@analytics).to receive(:track_event).with('Logout Initiated', hash_including(result)) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + expect(@irs_attempts_api_tracker).to receive(:logout_initiated).with( success: true, ) diff --git a/spec/controllers/sign_out_controller_spec.rb b/spec/controllers/sign_out_controller_spec.rb index e76b589a5f5..c7563b8b961 100644 --- a/spec/controllers/sign_out_controller_spec.rb +++ b/spec/controllers/sign_out_controller_spec.rb @@ -22,12 +22,13 @@ it 'tracks the event' do stub_sign_in_before_2fa stub_analytics + stub_attempts_tracker allow(controller.decorated_session).to receive(:cancel_link_url).and_return('foo') expect(@analytics). to receive(:track_event).with('Logout Initiated', hash_including(method: 'cancel link')) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + expect(@irs_attempts_api_tracker).to receive(:logout_initiated).with( success: true, ) diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 886320c6bcb..bb8ed35918a 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -118,6 +118,7 @@ describe 'GET /logout' do it 'tracks a logout event' do stub_analytics + stub_attempts_tracker expect(@analytics).to receive(:track_event).with( 'Logout Initiated', hash_including( @@ -128,7 +129,7 @@ sign_in_as_user - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + expect(@irs_attempts_api_tracker).to receive(:logout_initiated).with( success: true, ) @@ -140,6 +141,7 @@ describe 'DELETE /logout' do it 'tracks a logout event' do stub_analytics + stub_attempts_tracker expect(@analytics).to receive(:track_event).with( 'Logout Initiated', hash_including( @@ -150,7 +152,7 @@ sign_in_as_user - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:logout_initiated).with( + expect(@irs_attempts_api_tracker).to receive(:logout_initiated).with( success: true, ) @@ -201,6 +203,7 @@ subject.session['user_return_to'] = 'http://example.com' stub_analytics + stub_attempts_tracker analytics_hash = { success: true, user_id: user.uuid, @@ -213,7 +216,7 @@ expect(@analytics).to receive(:track_event). with('Email and Password Authentication', analytics_hash) - expect_any_instance_of(IrsAttemptsApi::Tracker).to receive(:email_and_password_auth). + expect(@irs_attempts_api_tracker).to receive(:email_and_password_auth). with(email: user.email, success: true) post :create, params: { user: { email: user.email, password: user.password } }