diff --git a/app/controllers/oauth_callback_controller.rb b/app/controllers/oauth_callback_controller.rb index 4cb02d266..9aa73956a 100644 --- a/app/controllers/oauth_callback_controller.rb +++ b/app/controllers/oauth_callback_controller.rb @@ -71,6 +71,7 @@ class OauthCallbackController < ApplicationController def create_channel_with_inbox ActiveRecord::Base.transaction do channel_email = Channel::Email.create!(email: users_data['email'], account: account) + account.inboxes.create!( account: account, channel: channel_email, diff --git a/app/mailers/agent_notifications/conversation_notifications_mailer.rb b/app/mailers/agent_notifications/conversation_notifications_mailer.rb index a728bbbc6..8bf27220e 100644 --- a/app/mailers/agent_notifications/conversation_notifications_mailer.rb +++ b/app/mailers/agent_notifications/conversation_notifications_mailer.rb @@ -4,7 +4,8 @@ class AgentNotifications::ConversationNotificationsMailer < ApplicationMailer @agent = agent @conversation = conversation - subject = "#{@agent.available_name}, A new conversation [ID - #{@conversation.display_id}] has been created in #{@conversation.inbox&.name}." + inbox_name = @conversation.inbox&.sanitized_name + subject = "#{@agent.available_name}, A new conversation [ID - #{@conversation.display_id}] has been created in #{inbox_name}." @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) send_mail_with_liquid(to: @agent.email, subject: subject) and return end diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index cba95cb69..ba46a5fed 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -104,7 +104,7 @@ class ConversationReplyMailer < ApplicationMailer end def business_name - @inbox.business_name || @inbox.name + @inbox.business_name || @inbox.sanitized_name end def from_email diff --git a/app/models/inbox.rb b/app/models/inbox.rb index f1343a352..69996707b 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -47,8 +47,6 @@ class Inbox < ApplicationRecord # Not allowing characters: validates :name, presence: true - validates :name, if: :check_channel_type?, format: { with: %r{^^\b[^/\\<>@]*\b$}, multiline: true, - message: I18n.t('errors.inboxes.validations.name') } validates :account_id, presence: true validates :timezone, inclusion: { in: TZInfo::Timezone.all_identifiers } validates :out_of_office_message, length: { maximum: Limits::OUT_OF_OFFICE_MESSAGE_MAX_LENGTH } @@ -99,6 +97,16 @@ class Inbox < ApplicationRecord update_account_cache end + # Sanitizes inbox name for balanced email provider compatibility + # ALLOWS: /'._- and Unicode letters/numbers/emojis + # REMOVES: Forbidden chars (\<>@") + spam-trigger symbols (!#$%&*+=?^`{|}~) + def sanitized_name + return default_name_for_blank_name if name.blank? + + sanitized = apply_sanitization_rules(name) + sanitized.blank? && email? ? display_name_from_email : sanitized + end + def sms? channel_type == 'Channel::Sms' end @@ -178,6 +186,22 @@ class Inbox < ApplicationRecord private + def default_name_for_blank_name + email? ? display_name_from_email : '' + end + + def apply_sanitization_rules(name) + name.gsub(/[\\<>@"!#$%&*+=?^`{|}~]/, '') # Remove forbidden chars + .gsub(/[\x00-\x1F\x7F]/, ' ') # Replace control chars with spaces + .gsub(/\A[[:punct:]]+|[[:punct:]]+\z/, '') # Remove leading/trailing punctuation + .gsub(/\s+/, ' ') # Normalize spaces + .strip + end + + def display_name_from_email + channel.email.split('@').first.parameterize.titleize + end + def dispatch_create_event return if ENV['ENABLE_INBOX_EVENTS'].blank? diff --git a/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb index 5661af714..5310e8b10 100644 --- a/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb +++ b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb @@ -18,7 +18,7 @@ RSpec.describe AgentNotifications::ConversationNotificationsMailer do it 'renders the subject' do expect(mail.subject).to eq("#{agent.available_name}, A new conversation [ID - #{conversation - .display_id}] has been created in #{conversation.inbox&.name}.") + .display_id}] has been created in #{conversation.inbox&.sanitized_name}.") end it 'renders the receiver email' do diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index 28bc55385..8485fcf7a 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -87,7 +87,7 @@ RSpec.describe ConversationReplyMailer do let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now } it 'has correct name' do - expect(mail[:from].display_names).to eq(["#{message.sender.available_name} from Inbox"]) + expect(mail[:from].display_names).to eq(["#{message.sender.available_name} from #{message.conversation.inbox.sanitized_name}"]) end end @@ -224,11 +224,11 @@ RSpec.describe ConversationReplyMailer do end context 'when smtp enabled for email channel' do - let(:smtp_email_channel) do + let(:smtp_channel) do create(:channel_email, smtp_enabled: true, smtp_address: 'smtp.gmail.com', smtp_port: 587, smtp_login: 'smtp@gmail.com', smtp_password: 'password', smtp_domain: 'smtp.gmail.com', account: account) end - let(:conversation) { create(:conversation, assignee: agent, inbox: smtp_email_channel.inbox, account: account).reload } + let(:conversation) { create(:conversation, assignee: agent, inbox: smtp_channel.inbox, account: account).reload } let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } it 'use smtp mail server' do @@ -240,19 +240,19 @@ RSpec.describe ConversationReplyMailer do it 'renders sender name in the from address' do mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>" end it 'renders sender name even when assignee is not present' do conversation.update(assignee_id: nil) mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>" end it 'renders assignee name in the from address when sender_name not available' do message.update(sender_id: nil) mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{conversation.assignee.available_name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "#{conversation.assignee.available_name} from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>" end it 'renders inbox name as sender and assignee or business_name not present' do @@ -260,7 +260,7 @@ RSpec.describe ConversationReplyMailer do conversation.update(assignee_id: nil) mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "Notifications from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "Notifications from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>" end context 'when friendly name enabled' do @@ -276,7 +276,7 @@ RSpec.describe ConversationReplyMailer do mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "Notifications from #{conversation.inbox.name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "Notifications from #{conversation.inbox.sanitized_name} <#{smtp_channel.email}>" end it 'renders sender name as sender and assignee nil and business_name present' do @@ -286,7 +286,7 @@ RSpec.describe ConversationReplyMailer do mail = described_class.email_reply(message) expect(mail['from'].value).to eq( - "Notifications from #{conversation.inbox.business_name} <#{smtp_email_channel.email}>" + "Notifications from #{conversation.inbox.business_name} <#{smtp_channel.email}>" ) end @@ -295,7 +295,7 @@ RSpec.describe ConversationReplyMailer do conversation.update(assignee_id: agent.id) mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{agent.available_name} from #{conversation.inbox.business_name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "#{agent.available_name} from #{conversation.inbox.business_name} <#{smtp_channel.email}>" end it 'renders sender name as sender and assignee and business_name present' do @@ -304,7 +304,7 @@ RSpec.describe ConversationReplyMailer do conversation.update(assignee_id: agent.id) mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{agent_2.available_name} from #{conversation.inbox.business_name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "#{agent_2.available_name} from #{conversation.inbox.business_name} <#{smtp_channel.email}>" end end @@ -321,7 +321,7 @@ RSpec.describe ConversationReplyMailer do mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{conversation.inbox.name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "#{conversation.inbox.sanitized_name} <#{smtp_channel.email}>" end it 'renders sender name as business_name present' do @@ -330,17 +330,17 @@ RSpec.describe ConversationReplyMailer do mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{conversation.inbox.business_name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "#{conversation.inbox.business_name} <#{smtp_channel.email}>" end end end context 'when smtp enabled for microsoft email channel' do - let(:ms_smtp_email_channel) do + let(:ms_smtp_channel) do create(:channel_email, imap_login: 'smtp@outlook.com', imap_enabled: true, account: account, provider: 'microsoft', provider_config: { access_token: 'access_token' }) end - let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_email_channel.inbox, account: account).reload } + let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_channel.inbox, account: account).reload } let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } it 'use smtp mail server' do @@ -352,11 +352,11 @@ RSpec.describe ConversationReplyMailer do end context 'when smtp enabled for google email channel' do - let(:ms_smtp_email_channel) do + let(:ms_smtp_channel) do create(:channel_email, imap_login: 'smtp@gmail.com', imap_enabled: true, account: account, provider: 'google', provider_config: { access_token: 'access_token' }) end - let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_email_channel.inbox, account: account).reload } + let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_channel.inbox, account: account).reload } let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } it 'use smtp mail server' do @@ -430,7 +430,7 @@ RSpec.describe ConversationReplyMailer do it 'sets reply to email to be based on the domain' do reply_to_email = "reply+#{message.conversation.uuid}@#{conversation.account.domain}" - reply_to = "#{message.sender.available_name} from #{conversation.inbox.name} <#{reply_to_email}>" + reply_to = "#{message.sender.available_name} from #{conversation.inbox.sanitized_name} <#{reply_to_email}>" expect(mail['REPLY-TO'].value).to eq(reply_to) expect(mail.reply_to).to eq([reply_to_email]) end diff --git a/spec/models/inbox_spec.rb b/spec/models/inbox_spec.rb index 02d68d77a..e25e87e4d 100644 --- a/spec/models/inbox_spec.rb +++ b/spec/models/inbox_spec.rb @@ -164,31 +164,7 @@ RSpec.describe Inbox do let(:inbox) { FactoryBot.create(:inbox) } context 'when validating inbox name' do - it 'does not allow any special character at the end' do - inbox.name = 'this is my inbox name-' - expect(inbox).not_to be_valid - expect(inbox.errors.full_messages).to eq( - ['Name should not start or end with symbols, and it should not have < > / \\ @ characters.'] - ) - end - - it 'does not allow any special character at the start' do - inbox.name = '-this is my inbox name' - expect(inbox).not_to be_valid - expect(inbox.errors.full_messages).to eq( - ['Name should not start or end with symbols, and it should not have < > / \\ @ characters.'] - ) - end - - it 'does not allow chacters like /\@<> in the entire string' do - inbox.name = 'inbox@name' - expect(inbox).not_to be_valid - expect(inbox.errors.full_messages).to eq( - ['Name should not start or end with symbols, and it should not have < > / \\ @ characters.'] - ) - end - - it 'does not empty string' do + it 'does not allow empty string' do inbox.name = '' expect(inbox).not_to be_valid expect(inbox.errors.full_messages[0]).to eq( @@ -282,4 +258,134 @@ RSpec.describe Inbox do inbox.touch # rubocop:disable Rails/SkipsModelValidations end end + + describe '#sanitized_name' do + context 'when inbox name contains forbidden characters' do + it 'removes forbidden and spam-trigger characters' do + inbox = FactoryBot.build(:inbox, name: 'Test/Name\\With@Characters"And\'Quotes!#$%') + expect(inbox.sanitized_name).to eq('Test/NameWithBadCharactersAnd\'Quotes') + end + end + + context 'when inbox name has leading/trailing non-word characters' do + it 'removes leading and trailing symbols' do + inbox = FactoryBot.build(:inbox, name: '!!!Test Name***') + expect(inbox.sanitized_name).to eq('Test Name') + end + + it 'handles mixed leading/trailing characters' do + inbox = FactoryBot.build(:inbox, name: '###@@@Test Inbox Name$$$%%') + expect(inbox.sanitized_name).to eq('Test Inbox Name') + end + end + + context 'when inbox name has multiple spaces' do + it 'normalizes multiple spaces to single space' do + inbox = FactoryBot.build(:inbox, name: 'Test Multiple Spaces') + expect(inbox.sanitized_name).to eq('Test Multiple Spaces') + end + + it 'handles tabs and other whitespace' do + inbox = FactoryBot.build(:inbox, name: "Test\t\nMultiple\r\nSpaces") + expect(inbox.sanitized_name).to eq('Test Multiple Spaces') + end + end + + context 'when inbox name has leading/trailing whitespace' do + it 'strips whitespace' do + inbox = FactoryBot.build(:inbox, name: ' Test Name ') + expect(inbox.sanitized_name).to eq('Test Name') + end + end + + context 'when inbox name becomes empty after sanitization' do + context 'with email channel' do + it 'falls back to email local part' do + email_channel = FactoryBot.build(:channel_email, email: 'support@example.com') + inbox = FactoryBot.build(:inbox, name: '\\<>@"', channel: email_channel) + expect(inbox.sanitized_name).to eq('Support') + end + + it 'handles email with complex local part' do + email_channel = FactoryBot.build(:channel_email, email: 'help-desk_team@example.com') + inbox = FactoryBot.build(:inbox, name: '!!!@@@', channel: email_channel) + expect(inbox.sanitized_name).to eq('Help Desk Team') + end + end + + context 'with non-email channel' do + it 'returns empty string when name becomes blank' do + web_widget_channel = FactoryBot.build(:channel_widget) + inbox = FactoryBot.build(:inbox, name: '\\<>@"', channel: web_widget_channel) + expect(inbox.sanitized_name).to eq('') + end + end + end + + context 'when inbox name is blank initially' do + context 'with email channel' do + it 'uses email local part as fallback' do + email_channel = FactoryBot.build(:channel_email, email: 'customer-care@example.com') + inbox = FactoryBot.build(:inbox, name: '', channel: email_channel) + expect(inbox.sanitized_name).to eq('Customer Care') + end + end + + context 'with non-email channel' do + it 'returns empty string' do + api_channel = FactoryBot.build(:channel_api) + inbox = FactoryBot.build(:inbox, name: '', channel: api_channel) + expect(inbox.sanitized_name).to eq('') + end + end + end + + context 'when inbox name contains valid characters' do + it 'preserves valid characters like hyphens, underscores, and dots' do + inbox = FactoryBot.build(:inbox, name: 'Test-Name_With.Valid-Characters') + expect(inbox.sanitized_name).to eq('Test-Name_With.Valid-Characters') + end + + it 'preserves alphanumeric characters and spaces' do + inbox = FactoryBot.build(:inbox, name: 'Customer Support 123') + expect(inbox.sanitized_name).to eq('Customer Support 123') + end + + it 'preserves balanced safe characters but removes spam-trigger symbols' do + inbox = FactoryBot.build(:inbox, name: "Test!#$%&'*+/=?^_`{|}~-Name") + expect(inbox.sanitized_name).to eq("Test'/_-Name") + end + + it 'keeps commonly used safe characters' do + inbox = FactoryBot.build(:inbox, name: "Support/Help's Team.Desk_2024-Main") + expect(inbox.sanitized_name).to eq("Support/Help's Team.Desk_2024-Main") + end + end + + context 'when inbox name contains problematic characters for email headers' do + it 'preserves Unicode symbols (trademark, etc.)' do + inbox = FactoryBot.build(:inbox, name: 'Test™Name®With©Special™Characters') + expect(inbox.sanitized_name).to eq('Test™Name®With©Special™Characters') + end + end + + context 'with edge cases' do + it 'handles nil name gracefully' do + inbox = FactoryBot.build(:inbox) + allow(inbox).to receive(:name).and_return(nil) + expect { inbox.sanitized_name }.not_to raise_error + end + + it 'handles very long names' do + long_name = 'A' * 1000 + inbox = FactoryBot.build(:inbox, name: long_name) + expect(inbox.sanitized_name).to eq(long_name) + end + + it 'handles unicode characters and preserves emojis' do + inbox = FactoryBot.build(:inbox, name: 'Test Name with émojis 🎉') + expect(inbox.sanitized_name).to eq('Test Name with émojis 🎉') + end + end + end end