fix: Do not allow sending messages if merged contact has a duplicate session (#11152)

In this PR https://github.com/chatwoot/chatwoot/pull/11139, if there is
an attempt to create a duplication session for the contact in the same
inbox, we will anonymize the old session.

This PR would prevent sending messages to the older sessions. The
support agents will have to create a new conversation to continue
messages with customer.

app/builders/contact_inbox_builder.rb

@@ -12,50 +12,11 @@ class ContactInboxBuilder
   private
 
   def generate_source_id
-    case @inbox.channel_type
-    when 'Channel::TwilioSms'
-      twilio_source_id
-    when 'Channel::Whatsapp'
-      wa_source_id
-    when 'Channel::Email'
-      email_source_id
-    when 'Channel::Sms'
-      phone_source_id
-    when 'Channel::Api', 'Channel::WebWidget'
-      SecureRandom.uuid
-    else
-      raise "Unsupported operation for this channel: #{@inbox.channel_type}"
-    end
-  end
-
-  def email_source_id
-    raise ActionController::ParameterMissing, 'contact email' unless @contact.email
-
-    @contact.email
-  end
-
-  def phone_source_id
-    raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number
-
-    @contact.phone_number
-  end
-
-  def wa_source_id
-    raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number
-
-    # whatsapp doesn't want the + in e164 format
-    @contact.phone_number.delete('+').to_s
-  end
-
-  def twilio_source_id
-    raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number
-
-    case @inbox.channel.medium
-    when 'sms'
-      @contact.phone_number
-    when 'whatsapp'
-      "whatsapp:#{@contact.phone_number}"
-    end
+    ContactInbox::SourceIdService.new(
+      contact: @contact,
+      channel_type: @inbox.channel_type,
+      medium: @inbox.channel.try(:medium)
+    ).generate
   end
 
   def create_contact_inbox
@@ -91,7 +52,7 @@ class ContactInboxBuilder
 
   def new_source_id
     if @inbox.whatsapp? || @inbox.sms? || @inbox.twilio?
-      "whatsapp:#{@source_id}#{rand(100)}"
+      "#{@source_id}#{rand(100)}"
     else
       "#{rand(10)}#{@source_id}"
     end

app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb

@@ -9,6 +9,8 @@ class Api::V1::Accounts::Contacts::ContactInboxesController < Api::V1::Accounts:
       source_id: params[:source_id],
       hmac_verified: hmac_verified?
     ).perform
+  rescue ArgumentError => e
+    render json: { error: e.message }, status: :unprocessable_entity
   end
 
   private

app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js

@@ -25,6 +25,11 @@ export const generateLabelForContactableInboxesList = ({
     channelType === INBOX_TYPES.TWILIO ||
     channelType === INBOX_TYPES.WHATSAPP
   ) {
+    // Handled separately for Twilio Inbox where phone number is  not mandatory.
+    // You can send message to a contact with Messaging Service Id.
+    if (!phoneNumber) {
+      return name;
+    }
     return `${name} (${phoneNumber})`;
   }
   return name;

app/javascript/dashboard/components-next/NewConversation/helpers/specs/composeConversationHelper.spec.js

@@ -8,8 +8,8 @@ vi.mock('dashboard/api/contacts');
 describe('composeConversationHelper', () => {
   describe('generateLabelForContactableInboxesList', () => {
     const contact = {
-      name: 'John Doe',
-      email: 'john@example.com',
+      name: 'Priority Inbox',
+      email: 'hello@example.com',
       phoneNumber: '+1234567890',
     };
 
@@ -19,7 +19,7 @@ describe('composeConversationHelper', () => {
           ...contact,
           channelType: INBOX_TYPES.EMAIL,
         })
-      ).toBe('John Doe (john@example.com)');
+      ).toBe('Priority Inbox (hello@example.com)');
     });
 
     it('generates label for twilio inbox', () => {
@@ -28,7 +28,14 @@ describe('composeConversationHelper', () => {
           ...contact,
           channelType: INBOX_TYPES.TWILIO,
         })
-      ).toBe('John Doe (+1234567890)');
+      ).toBe('Priority Inbox (+1234567890)');
+
+      expect(
+        helpers.generateLabelForContactableInboxesList({
+          name: 'Priority Inbox',
+          channelType: INBOX_TYPES.TWILIO,
+        })
+      ).toBe('Priority Inbox');
     });
 
     it('generates label for whatsapp inbox', () => {
@@ -37,7 +44,7 @@ describe('composeConversationHelper', () => {
           ...contact,
           channelType: INBOX_TYPES.WHATSAPP,
         })
-      ).toBe('John Doe (+1234567890)');
+      ).toBe('Priority Inbox (+1234567890)');
     });
 
     it('generates label for other inbox types', () => {
@@ -46,7 +53,7 @@ describe('composeConversationHelper', () => {
           ...contact,
           channelType: 'Channel::Api',
         })
-      ).toBe('John Doe');
+      ).toBe('Priority Inbox');
     });
   });
 

app/services/base/send_on_channel_service.rb

@@ -12,8 +12,10 @@ class Base::SendOnChannelService
 
   def perform
     validate_target_channel
+
     return unless outgoing_message?
     return if invalid_message?
+    return if invalid_source_id?
 
     perform_reply
   end
@@ -49,6 +51,29 @@ class Base::SendOnChannelService
     message.private? || outgoing_message_originated_from_channel?
   end
 
+  def invalid_source_id?
+    return false unless channels_to_validate?
+
+    return false if contact_inbox.source_id == expected_source_id
+
+    message.update!(status: :failed, external_error: I18n.t('errors.channel_service.invalid_source_id'))
+    true
+  end
+
+  def expected_source_id
+    ContactInbox::SourceIdService.new(
+      contact: contact,
+      channel_type: inbox.channel_type,
+      medium: inbox.channel.try(:medium)
+    ).generate
+  rescue ArgumentError
+    nil
+  end
+
+  def channels_to_validate?
+    inbox.sms? || inbox.whatsapp? || inbox.email? || inbox.twilio?
+  end
+
   def validate_target_channel
     raise 'Invalid channel service was called' if inbox.channel.class != channel_class
   end

app/services/contact_inbox/source_id_service.rb

@@ -0,0 +1,55 @@
+class ContactInbox::SourceIdService
+  pattr_initialize [:contact!, :channel_type!, { medium: nil }]
+
+  def generate
+    case channel_type
+    when 'Channel::TwilioSms'
+      twilio_source_id
+    when 'Channel::Whatsapp'
+      wa_source_id
+    when 'Channel::Email'
+      email_source_id
+    when 'Channel::Sms'
+      phone_source_id
+    when 'Channel::Api', 'Channel::WebWidget'
+      SecureRandom.uuid
+    else
+      raise ArgumentError, "Unsupported operation for this channel: #{channel_type}"
+    end
+  end
+
+  private
+
+  def email_source_id
+    raise ArgumentError, 'contact email required' unless contact.email
+
+    contact.email
+  end
+
+  def phone_source_id
+    raise ArgumentError, 'contact phone number required' unless contact.phone_number
+
+    contact.phone_number
+  end
+
+  def wa_source_id
+    raise ArgumentError, 'contact phone number required' unless contact.phone_number
+
+    # whatsapp doesn't want the + in e164 format
+    contact.phone_number.delete('+').to_s
+  end
+
+  def twilio_source_id
+    raise ArgumentError, 'contact phone number required' unless contact.phone_number
+    raise ArgumentError, 'medium required for Twilio channel' if medium.blank?
+
+    case medium
+    when 'sms'
+      contact.phone_number
+    when 'whatsapp'
+      "whatsapp:#{contact.phone_number}"
+    else
+      raise ArgumentError, "Unsupported Twilio medium: #{medium}"
+    end
+  end
+end

config/locales/en.yml

@@ -67,6 +67,9 @@ en:
       invalid_message_type: 'Invalid message type. Action not permitted'
     slack:
       invalid_channel_id: 'Invalid slack channel. Please try again'
+    channel_service:
+      invalid_source_id: "This conversation may have originally belonged to a different contact but is now showing here due to a merge or update. You won't be able to continue this conversation. Please create a new conversation to proceed."
+
     inboxes:
       imap:
         socket_error: Please check the network connection, IMAP address and try again.

spec/builders/contact_inbox_builder_spec.rb

@@ -59,7 +59,7 @@ describe ContactInboxBuilder do
             contact: contact,
             inbox: twilio_inbox
           ).perform
-        end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number')
+        end.to raise_error(ArgumentError, 'contact phone number required')
       end
     end
 
@@ -117,7 +117,7 @@ describe ContactInboxBuilder do
             contact: contact,
             inbox: twilio_inbox
           ).perform
-        end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number')
+        end.to raise_error(ArgumentError, 'contact phone number required')
       end
     end
 
@@ -174,7 +174,7 @@ describe ContactInboxBuilder do
             contact: contact,
             inbox: whatsapp_inbox
           ).perform
-        end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number')
+        end.to raise_error(ArgumentError, 'contact phone number required')
       end
     end
 
@@ -232,7 +232,7 @@ describe ContactInboxBuilder do
             contact: contact,
             inbox: sms_inbox
           ).perform
-        end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number')
+        end.to raise_error(ArgumentError, 'contact phone number required')
       end
     end
 
@@ -290,7 +290,7 @@ describe ContactInboxBuilder do
             contact: contact,
             inbox: email_inbox
           ).perform
-        end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact email')
+        end.to raise_error(ArgumentError, 'contact email required')
       end
     end
 

spec/services/contact_inbox/source_id_service_spec.rb

@@ -0,0 +1,117 @@
+require 'rails_helper'
+
+RSpec.describe ContactInbox::SourceIdService do
+  let(:contact) { create(:contact, email: 'test@example.com', phone_number: '+1234567890') }
+
+  describe '#generate' do
+    context 'when channel is TwilioSms' do
+      let(:channel_type) { 'Channel::TwilioSms' }
+
+      context 'with SMS medium' do
+        subject { described_class.new(contact: contact, channel_type: channel_type, medium: 'sms') }
+
+        it 'returns phone number as source id' do
+          expect(subject.generate).to eq(contact.phone_number)
+        end
+      end
+
+      context 'with WhatsApp medium' do
+        subject { described_class.new(contact: contact, channel_type: channel_type, medium: 'whatsapp') }
+
+        it 'returns whatsapp prefixed phone number as source id' do
+          expect(subject.generate).to eq("whatsapp:#{contact.phone_number}")
+        end
+      end
+
+      context 'with invalid medium' do
+        subject { described_class.new(contact: contact, channel_type: channel_type, medium: 'invalid') }
+
+        it 'raises ArgumentError' do
+          expect { subject.generate }.to raise_error(ArgumentError, 'Unsupported Twilio medium: invalid')
+        end
+      end
+
+      context 'without medium' do
+        subject { described_class.new(contact: contact, channel_type: channel_type) }
+
+        it 'raises ArgumentError' do
+          expect { subject.generate }.to raise_error(ArgumentError, 'medium required for Twilio channel')
+        end
+      end
+    end
+
+    context 'when channel is Whatsapp' do
+      subject { described_class.new(contact: contact, channel_type: 'Channel::Whatsapp') }
+
+      it 'returns phone number without + as source id' do
+        expect(subject.generate).to eq(contact.phone_number.delete('+'))
+      end
+
+      context 'when contact has no phone number' do
+        let(:contact) { create(:contact, phone_number: nil) }
+
+        it 'raises ArgumentError' do
+          expect { subject.generate }.to raise_error(ArgumentError, 'contact phone number required')
+        end
+      end
+    end
+
+    context 'when channel is Email' do
+      subject { described_class.new(contact: contact, channel_type: 'Channel::Email') }
+
+      it 'returns email as source id' do
+        expect(subject.generate).to eq(contact.email)
+      end
+
+      context 'when contact has no email' do
+        let(:contact) { create(:contact, email: nil) }
+
+        it 'raises ArgumentError' do
+          expect { subject.generate }.to raise_error(ArgumentError, 'contact email required')
+        end
+      end
+    end
+
+    context 'when channel is SMS' do
+      subject { described_class.new(contact: contact, channel_type: 'Channel::Sms') }
+
+      it 'returns phone number as source id' do
+        expect(subject.generate).to eq(contact.phone_number)
+      end
+
+      context 'when contact has no phone number' do
+        let(:contact) { create(:contact, phone_number: nil) }
+
+        it 'raises ArgumentError' do
+          expect { subject.generate }.to raise_error(ArgumentError, 'contact phone number required')
+        end
+      end
+    end
+
+    context 'when channel is Api' do
+      subject { described_class.new(contact: contact, channel_type: 'Channel::Api') }
+
+      it 'returns a UUID as source id' do
+        allow(SecureRandom).to receive(:uuid).and_return('uuid-123')
+        expect(subject.generate).to eq('uuid-123')
+      end
+    end
+
+    context 'when channel is WebWidget' do
+      subject { described_class.new(contact: contact, channel_type: 'Channel::WebWidget') }
+
+      it 'returns a UUID as source id' do
+        allow(SecureRandom).to receive(:uuid).and_return('uuid-123')
+        expect(subject.generate).to eq('uuid-123')
+      end
+    end
+
+    context 'when channel is unsupported' do
+      subject { described_class.new(contact: contact, channel_type: 'Channel::Unknown') }
+
+      it 'raises ArgumentError' do
+        expect { subject.generate }.to raise_error(ArgumentError, 'Unsupported operation for this channel: Channel::Unknown')
+      end
+    end
+  end
+end

spec/services/sms/send_on_sms_service_spec.rb

@@ -5,8 +5,9 @@ describe Sms::SendOnSmsService do
     context 'when a valid message' do
       let(:sms_request) { double }
       let!(:sms_channel) { create(:channel_sms) }
-      let!(:contact_inbox) { create(:contact_inbox, inbox: sms_channel.inbox, source_id: '+123456789') }
-      let!(:conversation) { create(:conversation, contact_inbox: contact_inbox, inbox: sms_channel.inbox) }
+      let!(:contact) { create(:contact, phone_number: '+123456789') }
+      let!(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: sms_channel.inbox, source_id: '+123456789') }
+      let!(:conversation) { create(:conversation, contact_inbox: contact_inbox, inbox: sms_channel.inbox, contact: contact) }
 
       it 'calls channel.send_message' do
         message = create(:message, message_type: :outgoing, content: 'test',

spec/services/twilio/send_on_twilio_service_spec.rb

@@ -13,9 +13,11 @@ describe Twilio::SendOnTwilioService do
   let!(:twilio_whatsapp) { create(:channel_twilio_sms, medium: :whatsapp, account: account) }
   let!(:twilio_inbox) { create(:inbox, channel: twilio_sms, account: account) }
   let!(:twilio_whatsapp_inbox) { create(:inbox, channel: twilio_whatsapp, account: account) }
-  let!(:contact) { create(:contact, account: account) }
-  let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: twilio_inbox) }
+  let!(:contact) { create(:contact, account: account, phone_number: '+123456789') }
+  let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: '+123456789') }
+  let(:whatsapp_contact_inbox) { create(:contact_inbox, contact: contact, inbox: twilio_whatsapp_inbox, source_id: 'whatsapp:+123456789') }
   let(:conversation) { create(:conversation, contact: contact, inbox: twilio_inbox, contact_inbox: contact_inbox) }
+  let(:whatsapp_conversation) { create(:conversation, contact: contact, inbox: twilio_whatsapp_inbox, contact_inbox: whatsapp_contact_inbox) }
 
   before do
     allow(Twilio::REST::Client).to receive(:new).and_return(twilio_client)
@@ -71,7 +73,7 @@ describe Twilio::SendOnTwilioService do
       allow(message_record_double).to receive(:sid).and_return('1234')
 
       message = build(
-        :message, message_type: 'outgoing', inbox: twilio_whatsapp_inbox, account: account, conversation: conversation
+        :message, message_type: 'outgoing', inbox: twilio_whatsapp_inbox, account: account, conversation: whatsapp_conversation
       )
       attachment = message.attachments.new(account_id: message.account_id, file_type: :image)
       attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')

spec/services/whatsapp/send_on_whatsapp_service_spec.rb

@@ -18,8 +18,9 @@ describe Whatsapp::SendOnWhatsappService do
     context 'when a valid message' do
       let(:whatsapp_request) { instance_double(HTTParty::Response) }
       let!(:whatsapp_channel) { create(:channel_whatsapp, sync_templates: false) }
-      let!(:contact_inbox) { create(:contact_inbox, inbox: whatsapp_channel.inbox, source_id: '123456789') }
-      let!(:conversation) { create(:conversation, contact_inbox: contact_inbox, inbox: whatsapp_channel.inbox) }
+      let!(:contact) { create(:contact, phone_number: '+123456789') }
+      let!(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: whatsapp_channel.inbox, source_id: '123456789') }
+      let!(:conversation) { create(:conversation, contact: contact, contact_inbox: contact_inbox, inbox: whatsapp_channel.inbox) }
       let(:api_key) { 'test_key' }
       let(:headers) { { 'D360-API-KEY' => api_key, 'Content-Type' => 'application/json' } }
       let(:template_body) do
@@ -35,14 +36,12 @@ describe Whatsapp::SendOnWhatsappService do
         }
       end
 
-      let(:success_response) { { 'messages' => [{ 'id' => '123456789' }] }.to_json }
+      let(:success_response) { { 'messages' => [{ 'id' => 'message-123456789' }] }.to_json }
 
       it 'calls channel.send_message when with in 24 hour limit' do
         # to handle the case of 24 hour window limit.
-        create(:message, message_type: :incoming, content: 'test',
-                         conversation: conversation)
-        message = create(:message, message_type: :outgoing, content: 'test',
-                                   conversation: conversation)
+        create(:message, message_type: :incoming, content: 'test', conversation: conversation)
+        message = create(:message, message_type: :outgoing, content: 'test', conversation: conversation)
 
         stub_request(:post, 'https://waba.360dialog.io/v1/messages')
           .with(
@@ -52,7 +51,7 @@ describe Whatsapp::SendOnWhatsappService do
           .to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' })
 
         described_class.new(message: message).perform
-        expect(message.reload.source_id).to eq('123456789')
+        expect(message.reload.source_id).to eq('message-123456789')
       end
 
       it 'calls channel.send_template when after 24 hour limit' do
@@ -66,7 +65,7 @@ describe Whatsapp::SendOnWhatsappService do
           ).to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' })
 
         described_class.new(message: message).perform
-        expect(message.reload.source_id).to eq('123456789')
+        expect(message.reload.source_id).to eq('message-123456789')
       end
 
       it 'calls channel.send_template if template_params are present' do
@@ -79,7 +78,7 @@ describe Whatsapp::SendOnWhatsappService do
           ).to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' })
 
         described_class.new(message: message).perform
-        expect(message.reload.source_id).to eq('123456789')
+        expect(message.reload.source_id).to eq('message-123456789')
       end
 
       it 'calls channel.send_template when template has regexp characters' do
@@ -106,7 +105,18 @@ describe Whatsapp::SendOnWhatsappService do
           ).to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' })
 
         described_class.new(message: message).perform
-        expect(message.reload.source_id).to eq('123456789')
+        expect(message.reload.source_id).to eq('message-123456789')
+      end
+
+      context 'when source_id validation is required' do
+        let(:message) { create(:message, conversation: conversation, message_type: :outgoing) }
+
+        it 'marks message as failed when source_ids do not match' do
+          contact_inbox.update!(source_id: '1234567890')
+          described_class.new(message: message).perform
+          expect(message.reload.status).to eq('failed')
+          expect(message.external_error).to include('This conversation may have originally belonged to a different contact')
+        end
       end
     end
   end