From 1577288843cdbaf9dd93b28b88aeb624a2b4fa1f Mon Sep 17 00:00:00 2001
From: Muhsin Keloth <muhsinkeramam@gmail.com>
Date: Fri, 12 Jan 2024 06:53:31 +0530
Subject: [PATCH] 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.
---
 .../twilio/incoming_message_service.rb        | 30 ++++++++--
 .../twilio/incoming_message_service_spec.rb   | 60 +++++++++++++++++++
 2 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/app/services/twilio/incoming_message_service.rb b/app/services/twilio/incoming_message_service.rb
index abb6eb9f9..3887ff380 100644
--- a/app/services/twilio/incoming_message_service.rb
+++ b/app/services/twilio/incoming_message_service.rb
@@ -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
diff --git a/spec/services/twilio/incoming_message_service_spec.rb b/spec/services/twilio/incoming_message_service_spec.rb
index 02ec8013d..5f4dcec69 100644
--- a/spec/services/twilio/incoming_message_service_spec.rb
+++ b/spec/services/twilio/incoming_message_service_spec.rb
@@ -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