fix: Twilo attachment download fallback to the step where authorization headers are not passed. (#8681)

In the previous release, we enabled "HTTP Basic Authentication" to secure all attachments requiring HTTP authentication. This is particularly important for media files that may contain sensitive data, as recommended by Twilio. However, some users experienced issues because they did not enable this option despite our alerts prompting them to do so.  If the authenticated attachment download call fails, add another call to download the attachment without authentication.
This commit is contained in:
Muhsin Keloth
2024-01-12 06:53:31 +05:30
committed by GitHub
parent 63b3686746
commit 1577288843
2 changed files with 85 additions and 5 deletions

View File

@@ -107,11 +107,7 @@ class Twilio::IncomingMessageService
def attach_files
return if params[:MediaUrl0].blank?
attachment_file = Down.download(
params[:MediaUrl0],
# https://support.twilio.com/hc/en-us/articles/223183748-Protect-Media-Access-with-HTTP-Basic-Authentication-for-Programmable-Messaging
http_basic_authentication: [twilio_channel.account_sid, twilio_channel.auth_token || twilio_channel.api_key_sid]
)
attachment_file = download_attachment_file
attachment = @message.attachments.new(
account_id: @message.account_id,
@@ -126,4 +122,28 @@ class Twilio::IncomingMessageService
@message.save!
end
def download_attachment_file
download_with_auth
rescue Down::Error => e
handle_download_attachment_error(e)
end
def download_with_auth
Down.download(
params[:MediaUrl0],
# https://support.twilio.com/hc/en-us/articles/223183748-Protect-Media-Access-with-HTTP-Basic-Authentication-for-Programmable-Messaging
http_basic_authentication: [twilio_channel.account_sid, twilio_channel.auth_token || twilio_channel.api_key_sid]
)
end
# This is just a temporary workaround since some users have not yet enabled media protection. We will remove this in the future.
def handle_download_attachment_error(error)
logger.info "Error downloading attachment from Twilio: #{error.message}"
if error.message.include?('401 Unauthorized')
Down.download(params[:MediaUrl0])
else
ChatwootExceptionTracker.new(error, account: @inbox.account).capture_exception
end
end
end

View File

@@ -169,5 +169,65 @@ describe Twilio::IncomingMessageService do
expect(twilio_sms_channel.inbox.conversations.count).to eq(1)
end
end
context 'when a message with an attachment is received' do
before do
stub_request(:get, 'https://chatwoot-assets.local/sample.png')
.to_return(status: 200, body: 'image data', headers: {})
end
let(:params_with_attachment) do
{
SmsSid: 'SMxx',
From: '+12345',
AccountSid: 'ACxxx',
MessagingServiceSid: twilio_channel.messaging_service_sid,
Body: 'testing3',
NumMedia: '1',
MediaContentType0: 'image/jpeg',
MediaUrl0: 'https://chatwoot-assets.local/sample.png'
}
end
it 'creates a new message with media in existing conversation' do
described_class.new(params: params_with_attachment).perform
expect(conversation.reload.messages.last.content).to eq('testing3')
expect(conversation.reload.messages.last.attachments.count).to eq(1)
expect(conversation.reload.messages.last.attachments.first.file_type).to eq('image')
end
end
context 'when there is an error downloading the attachment' do
before do
stub_request(:get, 'https://chatwoot-assets.local/sample.png')
.to_raise(Down::Error.new('Download error'))
stub_request(:get, 'https://chatwoot-assets.local/sample.png')
.to_return(status: 200, body: 'image data', headers: {})
end
let(:params_with_attachment_error) do
{
SmsSid: 'SMxx',
From: '+12345',
AccountSid: 'ACxxx',
MessagingServiceSid: twilio_channel.messaging_service_sid,
Body: 'testing3',
NumMedia: '1',
MediaContentType0: 'image/jpeg',
MediaUrl0: 'https://chatwoot-assets.local/sample.png'
}
end
it 'retries downloading the attachment without a token after an error' do
expect do
described_class.new(params: params_with_attachment_error).perform
end.not_to raise_error
expect(conversation.reload.messages.last.content).to eq('testing3')
expect(conversation.reload.messages.last.attachments.count).to eq(1)
expect(conversation.reload.messages.last.attachments.first.file_type).to eq('image')
end
end
end
end