akadmin/chatwoot
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(twilio): verify webhook methods in health check and use api key secret for media downloads

Browse Source
This commit is contained in:
Tanmay Deep Sharma
2026-07-14 18:10:05 +05:30
parent 19ec392f3c
commit b6eb637445
4 changed files with 97 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/services/twilio/health_service.rb
+17 -10
View File
@@ -3,6 +3,9 @@ class Twilio::HealthService
pattr_initialize [:channel!]
# Our Twilio routes are POST-only, so a matching URL on the wrong HTTP method never reaches us.
HTTP_METHOD = 'POST'.freeze
# Compares the webhooks Twilio actually has against the ones Chatwoot expects.
# Errors (bad credentials, unknown number) bubble up to the controller as a 422.
def perform
@@ -18,35 +21,39 @@ class Twilio::HealthService
def messaging_service_webhooks
service = channel.client.messaging.services(channel.messaging_service_sid).fetch
# With use_inbound_webhook_on_number set, Twilio prefers the number's webhook over ours.
configured = service.inbound_method == HTTP_METHOD && !service.use_inbound_webhook_on_number
[webhook('messaging', twilio_callback_index_url, service.inbound_request_url)]
[webhook('messaging', twilio_callback_index_url, service.inbound_request_url, extra: configured)]
end
def phone_number_webhooks
number = channel.client.incoming_phone_numbers.list(phone_number: channel.phone_number).first
raise "Phone number #{channel.phone_number} was not found in the connected Twilio account" if number.nil?
webhooks = [webhook('messaging', twilio_callback_index_url, number.sms_url)]
webhooks = [webhook('messaging', twilio_callback_index_url, number.sms_url, extra: number.sms_method == HTTP_METHOD)]
webhooks += voice_webhooks(number) if channel.voice_enabled?
webhooks
end
def voice_webhooks(number)
[
webhook('voice', channel.voice_call_webhook_url, number.voice_url),
webhook('voice_status', channel.voice_status_webhook_url, number.status_callback),
webhook('voice', channel.voice_call_webhook_url, number.voice_url, extra: number.voice_method == HTTP_METHOD),
webhook('voice_status', channel.voice_status_webhook_url, number.status_callback,
extra: number.status_callback_method == HTTP_METHOD),
# Outbound calls dial through the TwiML app, so a stale voice_url here breaks them silently.
webhook('voice_app', channel.voice_call_webhook_url, twiml_app_voice_url)
twiml_app_webhook
]
end
def twiml_app_voice_url
return if channel.twiml_app_sid.blank?
def twiml_app_webhook
return webhook('voice_app', channel.voice_call_webhook_url, nil) if channel.twiml_app_sid.blank?
channel.client.applications(channel.twiml_app_sid).fetch.voice_url
app = channel.client.applications(channel.twiml_app_sid).fetch
webhook('voice_app', channel.voice_call_webhook_url, app.voice_url, extra: app.voice_method == HTTP_METHOD)
end
def webhook(name, expected, actual)
{ name: name, expected: expected, actual: actual.presence, configured: expected == actual }
def webhook(name, expected, actual, extra: false)
{ name: name, expected: expected, actual: actual.presence, configured: expected == actual && extra }
end
end
app/services/twilio/incoming_message_service.rb
+2 -2
View File
@@ -172,8 +172,8 @@ class Twilio::IncomingMessageService
def download_with_auth(media_url)
auth_credentials = if twilio_channel.api_key_sid.present?
# When using api_key_sid, the auth token should be the api_secret_key
[twilio_channel.api_key_sid, twilio_channel.auth_token]
# Voice channels keep the secret in api_key_secret; SMS channels store it in auth_token.
[twilio_channel.api_key_sid, twilio_channel.api_key_secret.presence || twilio_channel.auth_token]
else
# When using account_sid, the auth token is the account's auth token
[twilio_channel.account_sid, twilio_channel.auth_token]
spec/services/twilio/health_service_spec.rb
+41 -17
View File
@@ -18,12 +18,13 @@ describe Twilio::HealthService do
context 'with a phone number' do
let(:channel) { create(:channel_twilio_sms, :with_phone_number) }
let(:sms_url) { twilio_callback_index_url }
let(:sms_method) { 'POST' }
before do
allow(numbers_list).to receive(:list).and_return([instance_double(NUMBER_INSTANCE, sms_url: sms_url)])
allow(numbers_list).to receive(:list).and_return([instance_double(NUMBER_INSTANCE, sms_url: sms_url, sms_method: sms_method)])
end
it 'reports healthy when the messaging webhook points at chatwoot' do
it 'reports healthy when the messaging webhook points at chatwoot over POST' do
result = described_class.new(channel: channel).perform
expect(result[:status]).to eq('healthy')
@@ -50,12 +51,46 @@ describe Twilio::HealthService do
end
end
context 'with a messaging service' do
let(:channel) { create(:channel_twilio_sms) }
let(:messaging) { instance_double(Twilio::REST::Messaging) }
let(:services) { instance_double(Twilio::REST::Messaging::V1::ServiceContext) }
let(:use_inbound_webhook_on_number) { false }
let(:service) do
instance_double(Twilio::REST::Messaging::V1::ServiceInstance,
inbound_request_url: twilio_callback_index_url, inbound_method: 'POST',
use_inbound_webhook_on_number: use_inbound_webhook_on_number)
end
before do
allow(twilio_client).to receive(:messaging).and_return(messaging)
allow(messaging).to receive(:services).with(channel.messaging_service_sid).and_return(services)
allow(services).to receive(:fetch).and_return(service)
end
it 'checks the inbound request url of the messaging service' do
result = described_class.new(channel: channel).perform
expect(result[:status]).to eq('healthy')
expect(result[:webhooks].first).to include(name: 'messaging', configured: true)
end
context 'when the service still defers to the number webhook' do
let(:use_inbound_webhook_on_number) { true }
it 'reports misconfigured because twilio would bypass our inbound url' do
expect(described_class.new(channel: channel).perform[:status]).to eq('misconfigured')
end
end
end
context 'with voice enabled' do
let(:channel) { create(:channel_twilio_sms, :with_voice) }
let(:sms_url) { nil }
let(:number) do
instance_double(NUMBER_INSTANCE, sms_url: sms_url, voice_url: channel.voice_call_webhook_url,
status_callback: channel.voice_status_webhook_url)
instance_double(NUMBER_INSTANCE, sms_url: sms_url, sms_method: 'POST',
voice_url: channel.voice_call_webhook_url, voice_method: 'POST',
status_callback: channel.voice_status_webhook_url, status_callback_method: 'POST')
end
let(:twiml_app) { instance_double(Twilio::REST::Api::V2010::AccountContext::ApplicationContext) }
@@ -65,7 +100,8 @@ describe Twilio::HealthService do
allow(numbers_list).to receive(:list).and_return([number])
allow(twilio_client).to receive(:applications).and_return(twiml_app)
allow(twiml_app).to receive(:fetch).and_return(
instance_double(Twilio::REST::Api::V2010::AccountContext::ApplicationInstance, voice_url: twiml_app_voice_url)
instance_double(Twilio::REST::Api::V2010::AccountContext::ApplicationInstance,
voice_url: twiml_app_voice_url, voice_method: 'POST')
)
end
@@ -93,18 +129,6 @@ describe Twilio::HealthService do
hash_including(name: 'voice_app', configured: false))
end
end
context 'when the messaging webhook is missing' do
let(:twiml_app_voice_url) { channel.voice_call_webhook_url }
it 'flags messaging while voice stays configured' do
result = described_class.new(channel: channel).perform
expect(result[:status]).to eq('misconfigured')
expect(result[:webhooks]).to include(hash_including(name: 'messaging', configured: false),
hash_including(name: 'voice', configured: true))
end
end
end
end
end
spec/services/twilio/incoming_message_service_spec.rb
+37
View File
@@ -197,6 +197,43 @@ describe Twilio::IncomingMessageService do
end
end
# Voice channels keep the API key secret in api_key_secret, unlike SMS channels which reuse auth_token.
context 'when an attachment arrives on a voice enabled channel' do
before do
stub_request(:get, 'https://chatwoot-assets.local/sample.png')
.to_return(status: 200, body: 'image data', headers: { 'Content-Type' => 'image/png' })
end
let(:voice_channel) do
allow(Twilio::VoiceWebhookSetupService).to receive(:new)
.and_return(instance_double(Twilio::VoiceWebhookSetupService, perform: "AP#{SecureRandom.hex(16)}"))
create(:channel_twilio_sms, :with_voice)
end
let(:params_with_attachment) do
{
SmsSid: 'SMvoice',
From: '+12345',
To: voice_channel.phone_number,
AccountSid: voice_channel.account_sid,
Body: 'mms on a voice inbox',
NumMedia: '1',
MediaContentType0: 'image/jpeg',
MediaUrl0: 'https://chatwoot-assets.local/sample.png'
}
end
it 'downloads the media using the api key secret, not the account auth token' do
allow(Down).to receive(:download).and_call_original
described_class.new(params: params_with_attachment).perform
expect(Down).to have_received(:download).with(
'https://chatwoot-assets.local/sample.png',
http_basic_authentication: [voice_channel.api_key_sid, voice_channel.api_key_secret]
)
end
end
context 'when there is an error downloading the attachment' do
before do
stub_request(:get, 'https://chatwoot-assets.local/sample.png')