CV2-4638: fix sentry issue and send ID to SecurityMailer instead of t…

…he object (#1896)

app/mailers/security_mailer.rb

@@ -1,7 +1,9 @@
 class SecurityMailer < ApplicationMailer
   layout nil
 
-  def notify(user, type, activity)
+  def notify(user_id, type, activity_id)
+    user = User.find_by_id(user_id)
+    activity = LoginActivity.find_by_id(activity_id)
     address = []
     Geocoder.configure(language: I18n.locale)
     ip_result = Geocoder.search(activity.ip).first
@@ -16,16 +18,16 @@ def notify(user, type, activity)
     @location = address.compact.join(', ')
     @timestamp = activity.created_at
     @ip = activity.ip
-    @platform = @user_agent.os.split.first
+    @platform = begin @user_agent.os.split.first rescue 'Unknown' end
     subject = I18n.t("mail_security.#{type}_subject",
       app_name: CheckConfig.get('app_name'), browser: @user_agent.browser, platform: @platform)
     mail(to: email, subject: subject)
   end
 
-  def custom_notification(user, subject)
-    @user = user
+  def custom_notification(user_id, subject)
+    @user = User.find_by_id(user_id)
     attachments.inline['signup.png'] = File.read('public/images/signup.png')
-    mail(to: user.email, subject: subject)
+    mail(to: @user.email, subject: subject)
   end
 
 end

app/models/login_activity.rb

@@ -40,10 +40,10 @@ def send_successfull_login_notification(user)
     activities = user.login_activities.where(success: true).last(2)
     ip = activities.map(&:ip).uniq
     if ip.count > 1
-      SecurityMailer.delay.notify(user, 'ip', self)
+      SecurityMailer.delay.notify(user.id, 'ip', self.id)
     else
       user_agent = activities.map(&:user_agent).uniq
-      SecurityMailer.delay.notify(user, 'device', self) if user_agent.count > 1
+      SecurityMailer.delay.notify(user.id, 'device', self.id) if user_agent.count > 1
     end
   end
 
@@ -55,7 +55,7 @@ def send_failed_login_notification(user)
       failed_attempts = LoginActivity.where('identity = ? AND success = ? AND created_at > ?', self.identity, false, last_notification).count
     end
     if failed_attempts >= CheckConfig.get('failed_attempts', 4).to_i
-      SecurityMailer.delay.notify(user, 'failed', self)
+      SecurityMailer.delay.notify(user.id, 'failed', self.id)
       user.set_failed_notifications_time = self.created_at
       user.skip_check_ability = true
       user.save!

lib/tasks/check_new_signup_notification.rake

@@ -5,7 +5,7 @@ namespace :check do
     User.where.not(email: '').find_each do |u|
       unless u.encrypted_password?
         print '.'
-        SecurityMailer.delay.custom_notification(u, subject)
+        SecurityMailer.delay.custom_notification(u.id, subject)
       end
     end
   end

test/mailers/security_mailer_test.rb

@@ -11,33 +11,33 @@ def teardown
   test "should send security notification" do
     user = create_user
     la = create_login_activity user: user, success: true
-    email = SecurityMailer.notify(user, 'ip', la)
+    email = SecurityMailer.notify(user.id, 'ip', la.id)
     assert_emails 1 do
       email.deliver_now
     end
-    email = SecurityMailer.notify(user, 'device', la)
+    email = SecurityMailer.notify(user.id, 'device', la.id)
     assert_emails 1 do
       email.deliver_now
     end
-    email = SecurityMailer.notify(user, 'failed', la)
+    email = SecurityMailer.notify(user.id, 'failed', la.id)
     assert_emails 1 do
       email.deliver_now
     end
     Geocoder.stubs(:search).returns([OpenStruct.new(data: { 'loc' => OpenStruct.new({ city: 'Cairo', country: 'Egypt' }) })])
     la = create_login_activity user: user, success: true
-    email = SecurityMailer.notify(user, 'ip', la)
+    email = SecurityMailer.notify(user.id, 'ip', la.id)
     assert_emails 1 do
       email.deliver_now
     end
     Geocoder.unstub(:search)
     # should not notify if email empty
     user.update_columns(email: '')
-    email = SecurityMailer.notify(user, 'ip', la)
+    email = SecurityMailer.notify(user.id, 'ip', la.id)
     assert_emails 0 do
       email.deliver_now
     end
     user.update_columns(email: nil)
-    email = SecurityMailer.notify(user, 'ip', la)
+    email = SecurityMailer.notify(user.id, 'ip', la.id)
     assert_emails 0 do
       email.deliver_now
     end
@@ -46,7 +46,7 @@ def teardown
   test "should send custom notification" do
     user = create_user
     subject = 'email subject'
-    email = SecurityMailer.custom_notification(user, subject)
+    email = SecurityMailer.custom_notification(user.id, subject)
     assert_emails 1 do
       email.deliver_now
     end