LG-12294: Set maximum delay for aggregated email events

app/services/user_alerts/alert_user_about_new_device.rb

@@ -27,7 +27,7 @@ def self.send_alert(user:, disavowal_event:, disavowal_token:)
       return false unless user.sign_in_new_device_at
 
       events = user.events.where(
-        created_at: user.sign_in_new_device_at..,
+        created_at: sign_in_events_start_time(user:)..,
         event_type: [
           'sign_in_before_2fa',
           'sign_in_unsuccessful_2fa',
@@ -49,5 +49,20 @@ def self.send_alert(user:, disavowal_event:, disavowal_token:)
       user.update(sign_in_new_device_at: nil)
       true
     end
+
+    def self.sign_in_events_start_time(user:)
+      # Avoid scenarios where stale events may be reflected in the time since sign in:
+      #
+      # 1. The feature is enabled for a short time in a deployed environment before being disabled
+      # 2. In local development, the server is not always active and the job may not run until later
+      #
+      # Typically, it's guaranteed that even in the worst-case of a sign-in occurring immediately
+      # after a scheduled job run, it should take no longer than twice the scheduled delay. A small
+      # buffer is added to account for delays of the job run or within the job itself.
+      [
+        user.sign_in_new_device_at,
+        (IdentityConfig.store.new_device_alert_delay_in_minutes * 3).minutes.ago,
+      ].max
+    end
   end
 end

spec/services/user_alerts/alert_user_about_new_device_spec.rb

@@ -52,7 +52,7 @@
   end
 
   describe '.send_alert' do
-    let(:sign_in_new_device_at) { 10.minutes.ago }
+    let(:sign_in_new_device_at) { 3.minutes.ago }
     let(:user) { create(:user, :fully_registered, sign_in_new_device_at:) }
     let(:disavowal_event) do
       create(:event, user:, event_type: :sign_in_after_2fa, created_at: 5.minutes.ago)
@@ -62,6 +62,10 @@
       UserAlerts::AlertUserAboutNewDevice.send_alert(user:, disavowal_event:, disavowal_token:)
     end
 
+    before do
+      allow(IdentityConfig.store).to receive(:new_device_alert_delay_in_minutes).and_return(5)
+    end
+
     it 'returns true' do
       expect(result).to eq(true)
     end
@@ -78,33 +82,36 @@
           :event,
           user:,
           event_type: :sign_in_notification_timeframe_expired,
-          created_at: 5.minutes.ago,
+          created_at: Time.zone.now,
         )
       end
 
       it 'sends mailer for authentication events within the time window' do
-        # 1. Exclude events outside the timeframe, e.g. previous sign-in
-        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 11.minutes.ago)
+        # 1. Exclude events outside possible window of time of recurring job run
+        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 16.minutes.ago)
 
-        # 2. Include authentication events inside the timeframe, inclusive
+        # 2. Exclude events outside the timeframe, e.g. previous sign-in
+        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 4.minutes.ago)
 
-        # 2.1 Include sign-in before 2FA
+        # 3. Include authentication events inside the timeframe, inclusive
+
+        # 3.1 Include sign-in before 2FA
         sign_in_before_2fa_event = create(
           :event,
           user:,
           event_type: :sign_in_before_2fa,
           created_at: sign_in_new_device_at,
         )
 
-        # 2.2 Include sign-in unsuccessful 2FA
+        # 3.2 Include sign-in unsuccessful 2FA
         sign_in_unsuccessful_2fa_event = create(
           :event,
           user:,
           event_type: :sign_in_unsuccessful_2fa,
-          created_at: 8.minutes.ago,
+          created_at: 2.minutes.ago,
         )
 
-        # 3. Exclude sign in notification timeframe expired event
+        # 4. Exclude sign in notification timeframe expired event
         disavowal_event
 
         delivery = instance_double(ActionMailer::MessageDelivery)
@@ -125,36 +132,39 @@
 
     context 'with sign in after 2fa disavowal event' do
       let(:disavowal_event) do
-        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 5.minutes.ago)
+        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 1.minute.ago)
       end
 
       it 'sends mailer for authentication events within the time window' do
-        # 1. Exclude events outside the timeframe, e.g. previous sign-in
-        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 11.minutes.ago)
+        # 1. Exclude events outside possible window of time of recurring job run
+        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 16.minutes.ago)
+
+        # 2. Exclude events outside the timeframe, e.g. previous sign-in
+        create(:event, user:, event_type: :sign_in_after_2fa, created_at: 4.minutes.ago)
 
-        # 2. Include authentication events inside the timeframe, inclusive
+        # 3. Include authentication events inside the timeframe, inclusive
 
-        # 2.1 Include sign-in before 2FA
+        # 3.1 Include sign-in before 2FA
         sign_in_before_2fa_event = create(
           :event,
           user:,
           event_type: :sign_in_before_2fa,
           created_at: sign_in_new_device_at,
         )
 
-        # 2.2 Include sign-in unsuccessful 2FA
+        # 3.2 Include sign-in unsuccessful 2FA
         sign_in_unsuccessful_2fa_event = create(
           :event,
           user:,
           event_type: :sign_in_unsuccessful_2fa,
-          created_at: 8.minutes.ago,
+          created_at: 2.minutes.ago,
         )
 
-        # 2.3 Include sign-in after 2FA
+        # 3.3 Include sign-in after 2FA
         sign_in_after_2fa_event = disavowal_event
 
-        # 3. Exclude events not related to authentication, e.g. actions after sign-in
-        create(:event, user:, event_type: :password_changed, created_at: 4.minutes.ago)
+        # 4. Exclude events not related to authentication, e.g. actions after sign-in
+        create(:event, user:, event_type: :password_changed, created_at: 30.seconds.ago)
 
         delivery = instance_double(ActionMailer::MessageDelivery)
         expect(delivery).to receive(:deliver_now_or_later)