fix: Validate Twilio webhook signatures (X-Twilio-Signature) (#13638)
Fixes #13619 ## Summary - Add `TwilioSignatureVerifyConcern` that validates the `X-Twilio-Signature` header using `Twilio::Security::RequestValidator` (already bundled via `twilio-ruby` gem) - Include the concern in `Twilio::CallbackController` and `Twilio::DeliveryStatusController` — both endpoints were previously accepting requests from any source with no authentication - Channels using API key authentication (`api_key_sid` present) skip validation with a warning log, since Twilio signs with the account auth token which isn't stored for those channels ## How it works 1. `before_action` looks up the `Channel::TwilioSms` from request params (`MessagingServiceSid` or `AccountSid` + phone number) 2. Validates the HMAC-SHA1 signature using the channel's auth token 3. Returns `403 Forbidden` if signature is invalid, missing, or channel not found 4. Handles reverse proxy URL reconstruction via `X-Forwarded-Proto` header Follows the same pattern used by `Webhooks::ShopifyController` and `Webhooks::TiktokController`. ## Test plan - [x] Valid signature → 204 No Content, job enqueued - [x] Invalid signature → 403 Forbidden, job not enqueued - [x] Missing signature header → 403 Forbidden - [x] Channel not found → 403 Forbidden - [x] API key channel → skips validation, job enqueued (with warning log) - [x] MessagingServiceSid lookup → validates and enqueues - [x] All existing Twilio service/job specs pass (99 examples, 0 failures) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com> Co-authored-by: Sojan Jose <sojan@pepalo.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
Muhsin Keloth
Sojan Jose
parent
d2625d8544
commit
6928f14a76
@@ -0,0 +1,88 @@
|
||||
module TwilioSignatureVerifyConcern
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
before_action :verify_twilio_signature!
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def verify_twilio_signature!
|
||||
channel = find_twilio_channel
|
||||
return log_and_reject_missing_channel if channel.blank?
|
||||
return if channel.api_key_sid.present? && log_api_key_skip(channel)
|
||||
|
||||
head :forbidden unless valid_signature?(channel)
|
||||
end
|
||||
|
||||
def log_and_reject_missing_channel
|
||||
Rails.logger.warn(
|
||||
'[TWILIO] Channel not found for webhook ' \
|
||||
"account_sid=#{params[:AccountSid]} messaging_service_sid=#{params[:MessagingServiceSid]} " \
|
||||
"to=#{params[:To]} from=#{params[:From]}"
|
||||
)
|
||||
head :forbidden
|
||||
end
|
||||
|
||||
def log_api_key_skip(channel)
|
||||
Rails.logger.warn(
|
||||
'[TWILIO] Signature validation skipped: channel uses API key authentication. ' \
|
||||
"account_sid=#{params[:AccountSid]} channel_id=#{channel.id}"
|
||||
)
|
||||
end
|
||||
|
||||
def valid_signature?(channel)
|
||||
signature = request.headers['X-Twilio-Signature']
|
||||
if signature.blank?
|
||||
Rails.logger.warn("[TWILIO] Missing X-Twilio-Signature header account_sid=#{params[:AccountSid]}")
|
||||
return false
|
||||
end
|
||||
|
||||
validator = Twilio::Security::RequestValidator.new(channel.auth_token)
|
||||
request_url = reconstruct_url
|
||||
return true if validator.validate(request_url, request.request_parameters, signature)
|
||||
|
||||
Rails.logger.warn(
|
||||
'[TWILIO] Signature validation failed ' \
|
||||
"account_sid=#{params[:AccountSid]} channel_id=#{channel.id} url=#{request_url} ip=#{request.remote_ip}"
|
||||
)
|
||||
false
|
||||
end
|
||||
|
||||
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])
|
||||
|
||||
return nil
|
||||
end
|
||||
return if params[:AccountSid].blank?
|
||||
|
||||
find_channel_by_phone_number
|
||||
end
|
||||
|
||||
def find_channel_by_phone_number
|
||||
channel_lookup_phone_numbers.each do |phone|
|
||||
channel = ::Channel::TwilioSms.find_by(account_sid: params[:AccountSid], phone_number: phone)
|
||||
return channel if channel
|
||||
end
|
||||
nil
|
||||
end
|
||||
|
||||
def channel_lookup_phone_numbers
|
||||
[params[:To], params[:From]].compact_blank
|
||||
end
|
||||
|
||||
def reconstruct_url
|
||||
url = request.original_url
|
||||
url = url.sub('http://', 'https://') if url.start_with?('http://') && https_request?
|
||||
url
|
||||
end
|
||||
|
||||
def https_request?
|
||||
return true if request.ssl?
|
||||
|
||||
forwarded_proto = request.headers['X-Forwarded-Proto'].to_s.split(',').map(&:strip).find(&:present?)
|
||||
forwarded_proto&.casecmp?('https')
|
||||
end
|
||||
end
|
||||
@@ -1,4 +1,6 @@
|
||||
class Twilio::CallbackController < ApplicationController
|
||||
include TwilioSignatureVerifyConcern
|
||||
|
||||
def create
|
||||
Webhooks::TwilioEventsJob.perform_later(permitted_params.to_unsafe_hash)
|
||||
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
class Twilio::DeliveryStatusController < ApplicationController
|
||||
include TwilioSignatureVerifyConcern
|
||||
|
||||
def create
|
||||
Webhooks::TwilioDeliveryStatusJob.perform_later(permitted_params.to_unsafe_hash)
|
||||
|
||||
@@ -18,4 +20,8 @@ class Twilio::DeliveryStatusController < ApplicationController
|
||||
:ErrorMessage
|
||||
)
|
||||
end
|
||||
|
||||
def channel_lookup_phone_numbers
|
||||
[params[:From]].compact_blank
|
||||
end
|
||||
end
|
||||
|
||||
@@ -4,25 +4,160 @@ RSpec.describe 'Twilio::CallbacksController', type: :request do
|
||||
include Rails.application.routes.url_helpers
|
||||
|
||||
describe 'POST /twilio/callback' do
|
||||
let(:account) { create(:account) }
|
||||
let(:twilio_channel) { create(:channel_twilio_sms, :with_phone_number, account: account, account_sid: 'AC123') }
|
||||
let(:params) do
|
||||
{
|
||||
'From' => '+1234567890',
|
||||
'To' => '+0987654321',
|
||||
'To' => twilio_channel.phone_number,
|
||||
'Body' => 'Test message',
|
||||
'AccountSid' => 'AC123',
|
||||
'SmsSid' => 'SM123'
|
||||
}
|
||||
end
|
||||
|
||||
it 'enqueues the Twilio events job' do
|
||||
expect do
|
||||
post twilio_callback_index_url, params: params
|
||||
end.to have_enqueued_job(Webhooks::TwilioEventsJob).with(params)
|
||||
def post_with_signature(url, params:)
|
||||
validator = Twilio::Security::RequestValidator.new(twilio_channel.auth_token)
|
||||
signature = validator.build_signature_for(url, params)
|
||||
post url, params: params, headers: { 'X-Twilio-Signature' => signature }
|
||||
end
|
||||
|
||||
it 'returns no content status' do
|
||||
post twilio_callback_index_url, params: params
|
||||
expect(response).to have_http_status(:no_content)
|
||||
context 'with valid signature' do
|
||||
it 'enqueues the Twilio events job' do
|
||||
url = twilio_callback_index_url
|
||||
expect do
|
||||
post_with_signature(url, params: params)
|
||||
end.to have_enqueued_job(Webhooks::TwilioEventsJob)
|
||||
end
|
||||
|
||||
it 'returns no content status' do
|
||||
url = twilio_callback_index_url
|
||||
post_with_signature(url, params: params)
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with invalid signature' do
|
||||
it 'returns forbidden status' do
|
||||
post twilio_callback_index_url, params: params, headers: { 'X-Twilio-Signature' => 'invalid' }
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
|
||||
it 'does not enqueue the job' do
|
||||
expect do
|
||||
post twilio_callback_index_url, params: params, headers: { 'X-Twilio-Signature' => 'invalid' }
|
||||
end.not_to have_enqueued_job(Webhooks::TwilioEventsJob)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with missing signature header' do
|
||||
it 'returns forbidden status' do
|
||||
post twilio_callback_index_url, params: params
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when channel is not found' do
|
||||
it 'returns forbidden status' do
|
||||
post twilio_callback_index_url, params: params.merge('AccountSid' => 'UNKNOWN', 'To' => '+0000000000')
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when channel uses API key authentication' do
|
||||
let(:twilio_channel) do
|
||||
create(:channel_twilio_sms, :with_phone_number, account: account, account_sid: 'AC123', api_key_sid: 'SK123')
|
||||
end
|
||||
|
||||
it 'skips signature validation and enqueues the job' do
|
||||
expect do
|
||||
post twilio_callback_index_url, params: params
|
||||
end.to have_enqueued_job(Webhooks::TwilioEventsJob)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when behind a reverse proxy with X-Forwarded-Proto' do
|
||||
it 'validates signature against the HTTPS URL' do
|
||||
http_url = twilio_callback_index_url
|
||||
https_url = http_url.sub('http://', 'https://')
|
||||
validator = Twilio::Security::RequestValidator.new(twilio_channel.auth_token)
|
||||
signature = validator.build_signature_for(https_url, params)
|
||||
post http_url, params: params, headers: {
|
||||
'X-Twilio-Signature' => signature,
|
||||
'X-Forwarded-Proto' => 'https'
|
||||
}
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
|
||||
it 'validates signature when forwarded proto is a comma-separated chain' do
|
||||
http_url = twilio_callback_index_url
|
||||
https_url = http_url.sub('http://', 'https://')
|
||||
validator = Twilio::Security::RequestValidator.new(twilio_channel.auth_token)
|
||||
signature = validator.build_signature_for(https_url, params)
|
||||
post http_url, params: params, headers: {
|
||||
'X-Twilio-Signature' => signature,
|
||||
'X-Forwarded-Proto' => 'https,http'
|
||||
}
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with MessagingServiceSid lookup' do
|
||||
let(:twilio_channel) { create(:channel_twilio_sms, account: account, account_sid: 'AC123') }
|
||||
let(:params) do
|
||||
{
|
||||
'From' => '+1234567890',
|
||||
'Body' => 'Test message',
|
||||
'AccountSid' => 'AC123',
|
||||
'SmsSid' => 'SM123',
|
||||
'MessagingServiceSid' => twilio_channel.messaging_service_sid
|
||||
}
|
||||
end
|
||||
|
||||
it 'validates and enqueues the job' do
|
||||
url = twilio_callback_index_url
|
||||
post_with_signature(url, params: params)
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when MessagingServiceSid is present but does not match a channel' do
|
||||
let(:params) do
|
||||
{
|
||||
'From' => '+1234567890',
|
||||
'To' => twilio_channel.phone_number,
|
||||
'Body' => 'Test message',
|
||||
'AccountSid' => 'AC123',
|
||||
'SmsSid' => 'SM123',
|
||||
'MessagingServiceSid' => 'MG_UNKNOWN'
|
||||
}
|
||||
end
|
||||
|
||||
it 'returns forbidden without falling back to phone number lookup' do
|
||||
url = twilio_callback_index_url
|
||||
post_with_signature(url, params: params)
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when MessagingServiceSid matches a channel but AccountSid does not' do
|
||||
let(:other_channel) { create(:channel_twilio_sms, account: account, account_sid: 'AC_OTHER') }
|
||||
let(:params) do
|
||||
{
|
||||
'From' => '+1234567890',
|
||||
'To' => twilio_channel.phone_number,
|
||||
'Body' => 'Test message',
|
||||
'AccountSid' => 'AC123',
|
||||
'SmsSid' => 'SM123',
|
||||
'MessagingServiceSid' => other_channel.messaging_service_sid
|
||||
}
|
||||
end
|
||||
|
||||
it 'returns forbidden without falling back to phone number lookup' do
|
||||
url = twilio_callback_index_url
|
||||
post_with_signature(url, params: params)
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -4,23 +4,132 @@ RSpec.describe 'Twilio::DeliveryStatusController', type: :request do
|
||||
include Rails.application.routes.url_helpers
|
||||
|
||||
describe 'POST /twilio/delivery_status' do
|
||||
let(:account) { create(:account) }
|
||||
let(:twilio_channel) { create(:channel_twilio_sms, :with_phone_number, account: account, account_sid: 'AC123') }
|
||||
let(:params) do
|
||||
{
|
||||
'MessageSid' => 'SM123',
|
||||
'MessageStatus' => 'delivered',
|
||||
'AccountSid' => 'AC123'
|
||||
'AccountSid' => 'AC123',
|
||||
'From' => twilio_channel.phone_number
|
||||
}
|
||||
end
|
||||
|
||||
it 'enqueues the Twilio delivery status job' do
|
||||
expect do
|
||||
post twilio_delivery_status_index_url, params: params
|
||||
end.to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob).with(params)
|
||||
def post_with_signature(url, params:, channel: twilio_channel)
|
||||
validator = Twilio::Security::RequestValidator.new(channel.auth_token)
|
||||
signature = validator.build_signature_for(url, params)
|
||||
post url, params: params, headers: { 'X-Twilio-Signature' => signature }
|
||||
end
|
||||
|
||||
it 'returns no content status' do
|
||||
post twilio_delivery_status_index_url, params: params
|
||||
expect(response).to have_http_status(:no_content)
|
||||
context 'with valid signature' do
|
||||
it 'enqueues the delivery status job' do
|
||||
url = twilio_delivery_status_index_url
|
||||
expect do
|
||||
post_with_signature(url, params: params)
|
||||
end.to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob)
|
||||
end
|
||||
|
||||
it 'returns no content status' do
|
||||
url = twilio_delivery_status_index_url
|
||||
post_with_signature(url, params: params)
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with invalid signature' do
|
||||
it 'returns forbidden status' do
|
||||
post twilio_delivery_status_index_url, params: params, headers: { 'X-Twilio-Signature' => 'invalid' }
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
|
||||
it 'does not enqueue the job' do
|
||||
expect do
|
||||
post twilio_delivery_status_index_url, params: params, headers: { 'X-Twilio-Signature' => 'invalid' }
|
||||
end.not_to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with missing signature header' do
|
||||
it 'returns forbidden status' do
|
||||
post twilio_delivery_status_index_url, params: params
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when channel uses API key authentication' do
|
||||
let(:twilio_channel) do
|
||||
create(:channel_twilio_sms, :with_phone_number, account: account, account_sid: 'AC123', api_key_sid: 'SK123')
|
||||
end
|
||||
|
||||
it 'skips signature validation and enqueues the job' do
|
||||
expect do
|
||||
post twilio_delivery_status_index_url, params: params
|
||||
end.to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with MessagingServiceSid lookup' do
|
||||
let(:twilio_channel) { create(:channel_twilio_sms, account: account, account_sid: 'AC123') }
|
||||
let(:params) do
|
||||
{
|
||||
'MessageSid' => 'SM123',
|
||||
'MessageStatus' => 'delivered',
|
||||
'AccountSid' => 'AC123',
|
||||
'MessagingServiceSid' => twilio_channel.messaging_service_sid
|
||||
}
|
||||
end
|
||||
|
||||
it 'validates and enqueues the job' do
|
||||
url = twilio_delivery_status_index_url
|
||||
post_with_signature(url, params: params)
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when To does not map to a channel but From does' do
|
||||
let(:params) do
|
||||
{
|
||||
'MessageSid' => 'SM123',
|
||||
'MessageStatus' => 'delivered',
|
||||
'AccountSid' => 'AC123',
|
||||
'To' => '+19999999999',
|
||||
'From' => twilio_channel.phone_number
|
||||
}
|
||||
end
|
||||
|
||||
it 'falls back to From lookup and enqueues the delivery status job' do
|
||||
url = twilio_delivery_status_index_url
|
||||
|
||||
expect do
|
||||
post_with_signature(url, params: params)
|
||||
end.to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob)
|
||||
|
||||
expect(response).to have_http_status(:no_content)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when To maps to an API-key channel but From maps to a different channel' do
|
||||
let!(:api_key_channel) do
|
||||
create(:channel_twilio_sms, :with_phone_number, account: account, account_sid: 'AC123', api_key_sid: 'SK123')
|
||||
end
|
||||
let!(:from_channel) { create(:channel_twilio_sms, :with_phone_number, account: account, account_sid: 'AC123') }
|
||||
let(:params) do
|
||||
{
|
||||
'MessageSid' => 'SM123',
|
||||
'MessageStatus' => 'delivered',
|
||||
'AccountSid' => 'AC123',
|
||||
'To' => api_key_channel.phone_number,
|
||||
'From' => from_channel.phone_number
|
||||
}
|
||||
end
|
||||
|
||||
it 'rejects invalid signatures instead of skipping verification' do
|
||||
expect do
|
||||
post twilio_delivery_status_index_url, params: params, headers: { 'X-Twilio-Signature' => 'invalid' }
|
||||
end.not_to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob)
|
||||
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user