fix(twilio): fall back to phone lookup when MessagingServiceSid is unknown
Twilio includes `MessagingServiceSid` on inbound webhooks whenever the number is part of a Messaging Service pool on Twilio's side (required for US A2P 10DLC), even for Chatwoot channels registered by phone number alone. The signature-verify concern was rejecting those webhooks with 403 because lookup-by-MSS returned nil and did not fall through. Only reject outright when the MSS does resolve to a channel but the AccountSid mismatches — the cross-tenant guard from the earlier review is preserved.
This commit is contained in:
@@ -52,9 +52,11 @@ module TwilioSignatureVerifyConcern
|
||||
def find_twilio_channel
|
||||
if params[:MessagingServiceSid].present?
|
||||
channel = ::Channel::TwilioSms.find_by(messaging_service_sid: params[:MessagingServiceSid])
|
||||
return channel if channel.present? && (params[:AccountSid].blank? || channel.account_sid == params[:AccountSid])
|
||||
if channel.present?
|
||||
return channel if params[:AccountSid].blank? || channel.account_sid == params[:AccountSid]
|
||||
|
||||
return nil
|
||||
return nil
|
||||
end
|
||||
end
|
||||
return if params[:AccountSid].blank?
|
||||
|
||||
|
||||
@@ -121,7 +121,7 @@ RSpec.describe 'Twilio::CallbacksController', type: :request do
|
||||
end
|
||||
end
|
||||
|
||||
context 'when MessagingServiceSid is present but does not match a channel' do
|
||||
context 'when MessagingServiceSid is present but does not match any channel' do
|
||||
let(:params) do
|
||||
{
|
||||
'From' => '+1234567890',
|
||||
@@ -133,10 +133,14 @@ RSpec.describe 'Twilio::CallbacksController', type: :request do
|
||||
}
|
||||
end
|
||||
|
||||
it 'returns forbidden without falling back to phone number lookup' do
|
||||
it 'falls back to phone number lookup and enqueues the job' do
|
||||
url = twilio_callback_index_url
|
||||
post_with_signature(url, params: params)
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
|
||||
expect do
|
||||
post_with_signature(url, params: params)
|
||||
end.to have_enqueued_job(Webhooks::TwilioEventsJob)
|
||||
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user