fix: downcase email before finding (#8921)

* fix: downcase email when finding

* feat: add `from_email` class

* refactor: use `from_email`

* feat: add rule to disallow find_by email directly

* chore:  remove redundant test

Since the previous imlpmentation didn't do a case-insentive search, a new user would be created, and the error would be raised at the DB layer. With the new changes, this test case is redundant

* refactor: use from_email

.rubocop.yml

@@ -2,6 +2,7 @@ require:
   - rubocop-performance
   - rubocop-rails
   - rubocop-rspec
+  - ./lib/rubocop/user_find_by.rb
 
 Layout/LineLength:
   Max: 150
@@ -140,6 +141,10 @@ RSpec/MultipleExpectations:
 RSpec/MultipleMemoizedHelpers:
   Max: 14
 
+# custom rules
+UseFromEmail:
+  Enabled: true
+
 AllCops:
   NewCops: enable
   Exclude:

app/actions/contact_identify_action.rb

@@ -59,7 +59,7 @@ class ContactIdentifyAction
   def existing_email_contact
     return if params[:email].blank?
 
-    @existing_email_contact ||= account.contacts.find_by(email: params[:email])
+    @existing_email_contact ||= account.contacts.from_email(params[:email])
   end
 
   def existing_phone_number_contact

app/builders/agent_builder.rb

@@ -27,7 +27,7 @@ class AgentBuilder
   # Finds a user by email or creates a new one with a temporary password.
   # @return [User] the found or created user.
   def find_or_create_user
-    user = User.find_by(email: email)
+    user = User.from_email(email)
     return user if user
 
     temp_password = "1!aA#{SecureRandom.alphanumeric(12)}"

app/builders/contact_inbox_with_contact_builder.rb

@@ -75,7 +75,7 @@ class ContactInboxWithContactBuilder
   def find_contact_by_email(email)
     return if email.blank?
 
-    account.contacts.find_by(email: email.downcase)
+    account.contacts.from_email(email)
   end
 
   def find_contact_by_phone_number(phone_number)

app/controllers/devise_overrides/passwords_controller.rb

@@ -5,7 +5,7 @@ class DeviseOverrides::PasswordsController < Devise::PasswordsController
   skip_before_action :authenticate_user!, raise: false
 
   def create
-    @user = User.find_by(email: params[:email])
+    @user = User.from_email(params[:email])
     if @user
       @user.send_reset_password_instructions
       build_response(I18n.t('messages.reset_password_success'), 200)

app/controllers/devise_overrides/sessions_controller.rb

@@ -33,7 +33,7 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController
   def process_sso_auth_token
     return if params[:email].blank?
 
-    user = User.find_by(email: params[:email])
+    user = User.from_email(params[:email])
     @resource = user if user&.valid_sso_auth_token?(params[:sso_auth_token])
   end
 end

app/controllers/platform/api/v1/users_controller.rb

@@ -8,7 +8,7 @@ class Platform::Api::V1::UsersController < PlatformController
   def show; end
 
   def create
-    @resource = (User.find_by(email: user_params[:email]) || User.new(user_params))
+    @resource = (User.from_email(user_params[:email]) || User.new(user_params))
     @resource.skip_confirmation!
     @resource.save!
     @platform_app.platform_app_permissibles.find_or_create_by!(permissible: @resource)

app/mailboxes/imap/imap_mailbox.rb

@@ -91,7 +91,7 @@ class Imap::ImapMailbox
   end
 
   def find_or_create_contact
-    @contact = @inbox.contacts.find_by(email: @processed_mail.original_sender)
+    @contact = @inbox.contacts.from_email(@processed_mail.original_sender)
     if @contact.present?
       @contact_inbox = ContactInbox.find_by(inbox: @inbox, contact: @contact)
     else

app/mailboxes/support_mailbox.rb

@@ -86,7 +86,7 @@ class SupportMailbox < ApplicationMailbox
   end
 
   def find_or_create_contact
-    @contact = @inbox.contacts.find_by(email: original_sender_email)
+    @contact = @inbox.contacts.from_email(original_sender_email)
     if @contact.present?
       @contact_inbox = ContactInbox.find_by(inbox: @inbox, contact: @contact)
     else

app/models/contact.rb

@@ -165,6 +165,12 @@ class Contact < ApplicationRecord
     email_format
   end
 
+  def self.from_email(email)
+    # rubocop:disable UseFromEmail,Migration/DepartmentName
+    find_by(email: email.downcase)
+    # rubocop:enable UseFromEmail,Migration/DepartmentName
+  end
+
   private
 
   def ip_lookup

app/models/user.rb

@@ -156,6 +156,12 @@ class User < ApplicationRecord
     mutations_from_database.changed?('email')
   end
 
+  def self.from_email(email)
+    # rubocop:disable UseFromEmail,Migration/DepartmentName
+    find_by(email: email.downcase)
+    # rubocop:enable UseFromEmail,Migration/DepartmentName
+  end
+
   private
 
   def remove_macros

app/services/data_import/contact_manager.rb

@@ -35,7 +35,7 @@ class DataImport::ContactManager
   def find_contact_by_email(params)
     return unless params[:email]
 
-    @account.contacts.find_by(email: params[:email])
+    @account.contacts.from_email(params[:email])
   end
 
   def find_contact_by_phone_number(params)

lib/integrations/slack/slack_message_helper.rb

@@ -69,7 +69,7 @@ module Integrations::Slack::SlackMessageHelper
 
   def sender
     user_email = slack_client.users_info(user: params[:event][:user])[:user][:profile][:email]
-    conversation.account.users.find_by(email: user_email)
+    conversation.account.users.from_email(user_email)
   end
 
   def private_note?

lib/rubocop/user_find_by.rb

@@ -0,0 +1,16 @@
+require 'rubocop'
+
+# Enforces use of from_email for email attribute lookups
+class UseFromEmail < RuboCop::Cop::Cop
+  MSG = 'Use `from_email` for email lookups to ensure case insensitivity.'.freeze
+
+  def_node_matcher :find_by_email?, <<~PATTERN
+    (send _ :find_by (hash (pair (sym :email) _)))
+  PATTERN
+
+  def on_send(node)
+    return unless find_by_email?(node)
+
+    add_offense(node, message: MSG)
+  end
+end

lib/seeders/account_seeder.rb

@@ -88,7 +88,7 @@ class Seeders::AccountSeeder
   end
 
   def create_conversation(contact_inbox:, conversation_data:)
-    assignee = User.find_by(email: conversation_data['assignee']) if conversation_data['assignee'].present?
+    assignee = User.from_email(conversation_data['assignee']) if conversation_data['assignee'].present?
     conversation = contact_inbox.conversations.create!(account: contact_inbox.inbox.account, contact: contact_inbox.contact,
                                                        inbox: contact_inbox.inbox, assignee: assignee)
     create_messages(conversation: conversation, messages: conversation_data['messages'])
@@ -111,7 +111,7 @@ class Seeders::AccountSeeder
     if message_data['message_type'] == 'incoming'
       conversation.contact
     elsif message_data['sender'].present?
-      User.find_by(email: message_data['sender'])
+      User.from_email(message_data['sender'])
     end
   end
 

spec/controllers/platform/api/v1/users_controller_spec.rb

@@ -115,11 +115,6 @@ RSpec.describe 'Platform Users API', type: :request do
           )
         )
         expect(platform_app.platform_app_permissibles.first.permissible_id).to eq data['id']
-
-        post '/platform/api/v1/users/', params: { name: 'test', email: 'TesT@test.com', password: 'Password1!' },
-                                        headers: { api_access_token: platform_app.access_token.token }, as: :json
-        data = response.parsed_body
-        expect(data['message']).to eq('Email has already been taken')
       end
 
       it 'fetch existing user and creates permissible for the user' do

spec/jobs/data_import_job_spec.rb

@@ -111,7 +111,7 @@ RSpec.describe DataImportJob do
 
           described_class.perform_now(existing_data_import)
           expect(existing_data_import.account.contacts.count).to eq(csv_length)
-          contact = Contact.find_by(email: csv_data[0]['email'])
+          contact = Contact.from_email(csv_data[0]['email'])
           expect(contact).to be_present
           expect(contact.phone_number).to eq("+#{csv_data[0]['phone_number']}")
           expect(contact.name).to eq((csv_data[0]['name']).to_s)