LG-14673: Add SP info to fraud review events (#11390)

* Update review-reject to attribute events to SP

changelog: Internal, Fraud review, Attribute fraud review analytics events to SPs.

* Update review-pass to attribute events to SP

* Allow logging SP without a request

Update Analytics to always attempt to log SP, even when there is no associated request.

* Don't log extraneous event

If include_missing is specified and there are no missing users, don't log an event for them

* Add test for logging SP without request

* Add parameter documentation for Analytics initializer

* Update analytics spec

Include service_provider in analytics_attributes now that it does not depend on request being non-nil.

app/services/analytics.rb

@@ -6,6 +6,11 @@ class Analytics
 
   attr_reader :user, :request, :sp, :session, :ahoy
 
+  # @param [User] user
+  # @param [ActionDispatch::Request,nil] request
+  # @param [String,nil] sp Service provider issuer string.
+  # @param [Hash] session
+  # @param [Ahoy::Tracker,nil] ahoy
   def initialize(user:, request:, sp:, session:, ahoy: nil)
     @user = user
     @request = request
@@ -21,6 +26,7 @@ def track_event(event, attributes = {})
       event_properties: attributes.except(:user_id).compact,
       new_event: first_event_this_session?,
       path: request&.path,
+      service_provider: sp,
       session_duration: session_duration,
       user_id: attributes[:user_id] || user.uuid,
       locale: I18n.locale,
@@ -58,7 +64,6 @@ def request_attributes
       user_ip: request.remote_ip,
       hostname: request.host,
       pid: Process.pid,
-      service_provider: sp,
       trace_id: request.headers['X-Amzn-Trace-Id'],
     }
 

lib/action_account.rb

@@ -177,6 +177,7 @@ def run(args:, config:)
       messages = []
 
       users.each do |user|
+        profile = nil
         profile_fraud_review_pending_at = nil
         success = false
 
@@ -187,7 +188,6 @@ def run(args:, config:)
         elsif FraudReviewChecker.new(user).fraud_review_eligible?
           profile = user.fraud_review_pending_profile
           profile_fraud_review_pending_at = profile.fraud_review_pending_at
-          profile_age_in_seconds = profile.profile_age_in_seconds
           profile.reject_for_fraud(notify_user: true)
           success = true
 
@@ -211,18 +211,23 @@ def run(args:, config:)
         end
 
         Analytics.new(
-          user: user, request: nil, session: {}, sp: nil,
+          user: user,
+          request: nil,
+          session: {},
+          sp: profile&.initiating_service_provider_issuer,
         ).fraud_review_rejected(
           success:,
           errors: analytics_error_hash,
           exception: nil,
-          profile_fraud_review_pending_at: profile_fraud_review_pending_at,
-          profile_age_in_seconds: profile_age_in_seconds,
+          profile_fraud_review_pending_at:,
+          profile_age_in_seconds: profile&.profile_age_in_seconds,
         )
       end
 
-      if config.include_missing?
-        (uuids - users.map(&:uuid)).each do |missing_uuid|
+      missing_uuids = (uuids - users.map(&:uuid))
+
+      if config.include_missing? && !missing_uuids.empty?
+        missing_uuids.each do |missing_uuid|
           table, messages = log_message(
             uuid: missing_uuid,
             log: log_text[:missing_uuid],
@@ -264,6 +269,7 @@ def run(args:, config:)
 
       messages = []
       users.each do |user|
+        profile = nil
         profile_fraud_review_pending_at = nil
         success = false
 
@@ -273,7 +279,6 @@ def run(args:, config:)
         elsif FraudReviewChecker.new(user).fraud_review_eligible?
           profile = user.fraud_review_pending_profile
           profile_fraud_review_pending_at = profile.fraud_review_pending_at
-          profile_age_in_seconds = profile.profile_age_in_seconds
           profile.activate_after_passing_review
           success = true
 
@@ -305,18 +310,22 @@ def run(args:, config:)
         end
 
         Analytics.new(
-          user: user, request: nil, session: {}, sp: nil,
+          user: user,
+          request: nil,
+          session: {},
+          sp: profile&.initiating_service_provider_issuer,
         ).fraud_review_passed(
           success:,
           errors: analytics_error_hash,
           exception: nil,
           profile_fraud_review_pending_at: profile_fraud_review_pending_at,
-          profile_age_in_seconds: profile_age_in_seconds,
+          profile_age_in_seconds: profile&.profile_age_in_seconds,
         )
       end
 
-      if config.include_missing?
-        (uuids - users.map(&:uuid)).each do |missing_uuid|
+      missing_uuids = (uuids - users.map(&:uuid))
+      if config.include_missing? && !missing_uuids.empty?
+        missing_uuids.each do |missing_uuid|
           table, messages = log_message(
             uuid: missing_uuid,
             log: log_text[:missing_uuid],

spec/lib/action_account_spec.rb

@@ -182,6 +182,26 @@
           errors: { message: 'Error: Could not find user with that UUID' },
         )
       end
+
+      context 'when profile has initiating_service_provider_issuer' do
+        let(:user) do
+          create(
+            :profile,
+            :fraud_review_pending,
+            initiating_service_provider_issuer: 'test-issuer',
+          ).user
+        end
+
+        it 'attributes analytics events to the SP' do
+          expect(Analytics).to receive(:new).
+            with(hash_including(sp: 'test-issuer')).
+            and_return(analytics)
+
+          subtask.run(args:, config:)
+
+          expect(analytics).to have_logged_event('Fraud: Profile review rejected')
+        end
+      end
     end
   end
 
@@ -241,6 +261,26 @@
           errors: { message: 'Error: Could not find user with that UUID' },
         )
       end
+
+      context 'when profile has initiating_service_provider_issuer' do
+        let(:user) do
+          create(
+            :profile,
+            :fraud_review_pending,
+            initiating_service_provider_issuer: 'test-issuer',
+          ).user
+        end
+
+        it 'attributes analytics events to the SP' do
+          expect(Analytics).to receive(:new).
+            with(hash_including(sp: 'test-issuer')).
+            and_return(analytics)
+
+          subtask.run(args:, config:)
+
+          expect(analytics).to have_logged_event('Fraud: Profile review passed')
+        end
+      end
     end
   end
 

spec/services/analytics_spec.rb

@@ -6,6 +6,7 @@
       user_id: current_user.uuid,
       new_event: true,
       path: path,
+      service_provider: 'http://localhost:3000',
       session_duration: nil,
       locale: I18n.locale,
       git_sha: IdentityConfig::GIT_SHA,
@@ -27,7 +28,6 @@
       browser_bot: false,
       hostname: FakeRequest.new.host,
       pid: Process.pid,
-      service_provider: 'http://localhost:3000',
       trace_id: nil,
     }
   end
@@ -194,6 +194,19 @@
         end
       end
     end
+
+    context 'when no request specified' do
+      let(:request) { nil }
+      context 'but an SP was specified via initializer' do
+        it 'logs the SP' do
+          expect(ahoy).to receive(:track).with(
+            'Trackable Event',
+            hash_including(service_provider: 'http://localhost:3000'),
+          )
+          analytics.track_event('Trackable Event')
+        end
+      end
+    end
   end
 
   it 'tracks session duration' do