LG-9429: Avoid sending new device notification before fully registered

app/models/user.rb

@@ -66,8 +66,8 @@ def confirmed_email_addresses
     email_addresses.where.not(confirmed_at: nil).order('last_sign_in_at DESC NULLS LAST')
   end
 
-  def need_two_factor_authentication?(_request)
-    MfaPolicy.new(self).two_factor_enabled?
+  def fully_registered?
+    !!registration_log&.registered_at
   end
 
   def confirmed?
@@ -320,7 +320,7 @@ def recent_devices
       map(&:decorate)
   end
 
-  def recent_devices?
+  def has_devices?
     !recent_devices.empty?
   end
 

app/services/user_event_creator.rb

@@ -76,9 +76,7 @@ def build_disavowal_token
 
   # @return [Array(Event, String)] an (event, disavowal_token) tuple
   def create_event_for_new_device(event_type:, user:, disavowal_token:)
-    user_has_multiple_devices = user.recent_devices?
-
-    if user_has_multiple_devices && disavowal_token.nil?
+    if user.fully_registered? && user.has_devices? && disavowal_token.nil?
       device, event, disavowal_token = Device.transaction do
         device = create_device_for_user(user)
         event, disavowal_token = create_user_event_with_disavowal(

config/initializers/devise.rb

@@ -39,6 +39,6 @@
 
   unless bypass_by_cookie
     auth.session(options[:scope])[TwoFactorAuthenticatable::NEED_AUTHENTICATION] =
-      user.need_two_factor_authentication?(auth.request)
+      MfaPolicy.new(user).two_factor_enabled?
   end
 end

spec/factories/users.rb

@@ -177,6 +177,10 @@
 
     trait :signed_up do
       with_phone
+
+      after :create do |user|
+        user.create_registration_log(registered_at: Time.zone.now)
+      end
     end
 
     trait :unconfirmed do

spec/features/new_device_tracking_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 
 describe 'New device tracking' do
+  include SamlAuthHelper
+
   let(:user) { create(:user, :signed_up) }
 
   context 'user has existing devices' do
@@ -37,28 +39,20 @@
     end
   end
 
-  context 'user does not have a phone configured' do
-    let(:user) { create(:user) }
-
-    before do
-      create(:device, user: user)
-    end
-
-    it 'does not send an SMS' do
-      sign_in_user(user)
+  context 'user signs up and confirms email in a different browser' do
+    let(:user) { build(:user) }
 
-      expect(user.reload.devices.length).to eq 2
+    it 'does not send an email' do
+      perform_in_browser(:one) do
+        visit_idp_from_sp_with_ial1(:oidc)
+        sign_up_user_from_sp_without_confirming_email(user.email)
+      end
 
-      device = user.devices.order(created_at: :desc).first
-      expect_delivered_email_count(1)
-      expect_delivered_email(
-        to: [user.email_addresses.first.email],
-        subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME),
-        body: [device.last_used_at.in_time_zone('Eastern Time (US & Canada)').
-              strftime('%B %-d, %Y %H:%M Eastern Time'),
-               'From 127.0.0.1 (IP address potentially located in United States)'],
-      )
-      expect(Telephony::Test::Message.messages.count).to eq 0
+      perform_in_browser(:two) do
+        expect do
+          confirm_email_in_a_different_browser(user.email)
+        end.not_to change { ActionMailer::Base.deliveries.count }
+      end
     end
   end
 end

spec/models/user_spec.rb

@@ -113,27 +113,26 @@
     end
   end
 
-  context '#need_two_factor_authentication?' do
-    let(:request) { ActionController::TestRequest.new }
-
-    it 'is true when two_factor_enabled' do
-      user = build_stubbed(:user)
+  describe '#fully_registered?' do
+    let(:user) { create(:user) }
+    subject(:fully_registered?) { user.fully_registered? }
 
-      mock_mfa = MfaPolicy.new(user)
-      allow(mock_mfa).to receive(:two_factor_enabled?).and_return true
-      allow(MfaPolicy).to receive(:new).with(user).and_return mock_mfa
+    context 'with unconfirmed user' do
+      let(:user) { create(:user, :unconfirmed) }
 
-      expect(user.need_two_factor_authentication?(nil)).to be_truthy
+      it { expect(fully_registered?).to eq(false) }
     end
 
-    it 'is false when not two_factor_enabled' do
-      user = build_stubbed(:user)
+    context 'with confirmed user' do
+      let(:user) { create(:user) }
+
+      it { expect(fully_registered?).to eq(false) }
+    end
 
-      mock_mfa = MfaPolicy.new(user)
-      allow(mock_mfa).to receive(:two_factor_enabled?).and_return false
-      allow(MfaPolicy).to receive(:new).with(user).and_return(mock_mfa)
+    context 'with mfa-enabled user' do
+      let(:user) { create(:user, :signed_up) }
 
-      expect(user.need_two_factor_authentication?(nil)).to be_falsey
+      it { expect(fully_registered?).to eq(true) }
     end
   end
 
@@ -1133,6 +1132,23 @@ def it_should_not_send_survey
     end
   end
 
+  describe '#has_devices?' do
+    let(:user) { create(:user) }
+    subject(:has_devices?) { user.has_devices? }
+
+    context 'with no devices' do
+      it { expect(has_devices?).to eq(false) }
+    end
+
+    context 'with a device' do
+      before do
+        create(:device, user:)
+      end
+
+      it { expect(has_devices?).to eq(true) }
+    end
+  end
+
   describe '#password_reset_profile' do
     let(:user) { create(:user) }
 

spec/services/user_event_creator_spec.rb

@@ -18,7 +18,7 @@
       cookie_jar: cookie_jar,
     )
   end
-  let(:user) { create(:user) }
+  let(:user) { create(:user, :signed_up) }
   let(:device) { create(:device, user: user, cookie_uuid: existing_device_cookie) }
   let(:event_type) { 'account_created' }
 

spec/support/features/session_helper.rb

@@ -196,7 +196,6 @@ def sign_in_before_2fa(user = create(:user))
 
     def sign_in_with_warden(user, auth_method: nil)
       login_as(user, scope: :user, run_callbacks: false)
-      allow(user).to receive(:need_two_factor_authentication?).and_return(false)
 
       Warden.on_next_request do |proxy|
         session = proxy.env['rack.session']