fix: added HMAC validation for Whatsapp and Instagram webhooks (#14280)
## Description * Added Meta webhook HMAC validation in meta_token_verify_concern.rb. * Wired it into instagram_controller.rb and whatsapp_controller.rb. * WhatsApp now verifies X-Hub-Signature-256 with WHATSAPP_APP_SECRET. * Instagram now verifies with either FB_APP_SECRET or INSTAGRAM_APP_SECRET. * Updated request specs so missing/invalid signatures return 401 and valid signatures still enqueue jobs. Fixes # (issue): [CW-6786](https://linear.app/chatwoot/issue/CW-6786/ghsa-7rw7-pc8v-mrr3-unauthenticated-message-injection-via-missing) ## Type of change Please delete options that are not relevant. - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? * Updated the controller specs and ran them successfully. * The original issue is no longer reproducible. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
co-authored by
Muhsin Keloth
parent
70f799ab35
commit
a9ac1c633d
@@ -2,6 +2,10 @@
|
||||
# This concern handles the token verification step.
|
||||
|
||||
module MetaTokenVerifyConcern
|
||||
CHANNEL_APP_SECRET_KEYS = %w[app_secret app_secret_key client_secret api_secret].freeze
|
||||
META_SIGNATURE_HEADER = 'X-Hub-Signature-256'.freeze
|
||||
META_SIGNATURE_PREFIX = 'sha256='.freeze
|
||||
|
||||
def verify
|
||||
service = is_a?(Webhooks::WhatsappController) ? 'whatsapp' : 'instagram'
|
||||
if valid_token?(params['hub.verify_token'])
|
||||
@@ -14,6 +18,53 @@ module MetaTokenVerifyConcern
|
||||
|
||||
private
|
||||
|
||||
def verify_meta_signature!
|
||||
return unless meta_signature_verification_required?
|
||||
return if valid_meta_signature?
|
||||
|
||||
head :unauthorized
|
||||
end
|
||||
|
||||
def valid_meta_signature?
|
||||
signature = request.headers[META_SIGNATURE_HEADER]
|
||||
return false unless signature&.start_with?(META_SIGNATURE_PREFIX)
|
||||
|
||||
meta_app_secrets.any? do |secret|
|
||||
next false if secret.blank?
|
||||
|
||||
expected_signature = "#{META_SIGNATURE_PREFIX}#{OpenSSL::HMAC.hexdigest('SHA256', secret, meta_request_body)}"
|
||||
ActiveSupport::SecurityUtils.secure_compare(expected_signature, signature)
|
||||
end
|
||||
end
|
||||
|
||||
def meta_request_body
|
||||
@meta_request_body ||= request.raw_post
|
||||
end
|
||||
|
||||
def meta_app_secrets
|
||||
raise 'Overwrite this method in your controller'
|
||||
end
|
||||
|
||||
def meta_signature_verification_required?
|
||||
true
|
||||
end
|
||||
|
||||
def channel_meta_app_secrets(channel)
|
||||
return [] if channel.blank?
|
||||
|
||||
secrets = []
|
||||
secrets << channel.app_secret if channel.respond_to?(:app_secret)
|
||||
secrets.concat(provider_config_meta_app_secrets(channel))
|
||||
secrets.compact_blank.uniq
|
||||
end
|
||||
|
||||
def provider_config_meta_app_secrets(channel)
|
||||
return [] unless channel.respond_to?(:provider_config)
|
||||
|
||||
provider_config = channel.provider_config.to_h.with_indifferent_access
|
||||
CHANNEL_APP_SECRET_KEYS.filter_map { |key| provider_config[key].presence }
|
||||
end
|
||||
|
||||
def valid_token?(_token)
|
||||
raise 'Overwrite this method your controller'
|
||||
end
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
class Webhooks::InstagramController < ActionController::API
|
||||
include MetaTokenVerifyConcern
|
||||
|
||||
before_action :verify_meta_signature!, only: :events
|
||||
|
||||
def events
|
||||
Rails.logger.info('Instagram webhook received events')
|
||||
if params['object'].casecmp('instagram').zero?
|
||||
@@ -39,4 +41,38 @@ class Webhooks::InstagramController < ActionController::API
|
||||
token == GlobalConfigService.load('IG_VERIFY_TOKEN', '') ||
|
||||
token == GlobalConfigService.load('INSTAGRAM_VERIFY_TOKEN', '')
|
||||
end
|
||||
|
||||
def meta_app_secrets
|
||||
[
|
||||
*instagram_channel_meta_app_secrets,
|
||||
GlobalConfigService.load('INSTAGRAM_APP_SECRET', nil),
|
||||
GlobalConfigService.load('FB_APP_SECRET', nil)
|
||||
]
|
||||
end
|
||||
|
||||
def instagram_channel_meta_app_secrets
|
||||
instagram_channels_from_payload.flat_map { |channel| channel_meta_app_secrets(channel) }
|
||||
end
|
||||
|
||||
def instagram_channels_from_payload
|
||||
Array(params.to_unsafe_hash[:entry]).flat_map do |entry|
|
||||
instagram_ids_from_entry(entry.with_indifferent_access).flat_map do |instagram_id|
|
||||
[
|
||||
Channel::Instagram.find_by(instagram_id: instagram_id),
|
||||
Channel::FacebookPage.find_by(instagram_id: instagram_id)
|
||||
]
|
||||
end
|
||||
end.compact.uniq
|
||||
end
|
||||
|
||||
def instagram_ids_from_entry(entry)
|
||||
messages = entry[:messaging].presence || entry[:standby] || []
|
||||
messages.filter_map { |messaging| instagram_id_from_messaging(messaging.with_indifferent_access) }
|
||||
end
|
||||
|
||||
def instagram_id_from_messaging(messaging)
|
||||
return messaging.dig(:sender, :id) if messaging.dig(:message, :is_echo).present?
|
||||
|
||||
messaging.dig(:recipient, :id)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
class Webhooks::WhatsappController < ActionController::API
|
||||
include MetaTokenVerifyConcern
|
||||
|
||||
before_action :verify_meta_signature!, only: :process_payload
|
||||
|
||||
def process_payload
|
||||
if inactive_whatsapp_number?
|
||||
Rails.logger.warn("Rejected webhook for inactive WhatsApp number: #{params[:phone_number]}")
|
||||
@@ -20,6 +22,45 @@ class Webhooks::WhatsappController < ActionController::API
|
||||
token == whatsapp_webhook_verify_token if whatsapp_webhook_verify_token.present?
|
||||
end
|
||||
|
||||
def meta_app_secrets
|
||||
[
|
||||
*channel_meta_app_secrets(whatsapp_channel),
|
||||
GlobalConfigService.load('WHATSAPP_APP_SECRET', nil)
|
||||
]
|
||||
end
|
||||
|
||||
def whatsapp_channel
|
||||
@whatsapp_channel ||= whatsapp_business_payload_channel || Channel::Whatsapp.find_by(phone_number: params[:phone_number])
|
||||
end
|
||||
|
||||
def meta_signature_verification_required?
|
||||
return true if whatsapp_channel.blank?
|
||||
return false unless whatsapp_channel.provider == 'whatsapp_cloud'
|
||||
return true if channel_meta_app_secrets(whatsapp_channel).present?
|
||||
|
||||
whatsapp_channel.provider_config['source'] == 'embedded_signup'
|
||||
end
|
||||
|
||||
def whatsapp_business_payload_channel
|
||||
return unless params[:object] == 'whatsapp_business_account'
|
||||
|
||||
metadata = params.dig(:entry, 0, :changes, 0, :value, :metadata)
|
||||
return if metadata.blank?
|
||||
|
||||
phone_number = normalized_phone_number(metadata[:display_phone_number])
|
||||
phone_number_id = metadata[:phone_number_id]
|
||||
channel = Channel::Whatsapp.find_by(phone_number: phone_number)
|
||||
|
||||
return channel if channel && channel.provider_config['phone_number_id'] == phone_number_id
|
||||
end
|
||||
|
||||
def normalized_phone_number(phone_number)
|
||||
return if phone_number.blank?
|
||||
|
||||
phone_number = phone_number.to_s
|
||||
phone_number.start_with?('+') ? phone_number : "+#{phone_number}"
|
||||
end
|
||||
|
||||
def inactive_whatsapp_number?
|
||||
phone_number = params[:phone_number]
|
||||
return false if phone_number.blank?
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
# ref: https://github.com/jgorset/facebook-messenger#make-a-configuration-provider
|
||||
class ChatwootFbProvider < Facebook::Messenger::Configuration::Providers::Base
|
||||
CHANNEL_APP_SECRET_KEYS = %w[app_secret app_secret_key client_secret api_secret].freeze
|
||||
|
||||
def valid_verify_token?(_verify_token)
|
||||
GlobalConfigService.load('FB_VERIFY_TOKEN', '')
|
||||
end
|
||||
|
||||
def app_secret_for(_page_id)
|
||||
GlobalConfigService.load('FB_APP_SECRET', '')
|
||||
def app_secret_for(page_id)
|
||||
channel_app_secret_for(page_id).presence || GlobalConfigService.load('FB_APP_SECRET', '')
|
||||
end
|
||||
|
||||
def access_token_for(page_id)
|
||||
@@ -14,6 +16,27 @@ class ChatwootFbProvider < Facebook::Messenger::Configuration::Providers::Base
|
||||
|
||||
private
|
||||
|
||||
def channel_app_secret_for(page_id)
|
||||
channel = Channel::FacebookPage.where(page_id: page_id).last
|
||||
return if channel.blank?
|
||||
|
||||
channel_app_secret_candidates(channel).first
|
||||
end
|
||||
|
||||
def channel_app_secret_candidates(channel)
|
||||
secrets = []
|
||||
secrets << channel.app_secret if channel.respond_to?(:app_secret)
|
||||
secrets.concat(provider_config_app_secrets(channel))
|
||||
secrets.compact_blank.uniq
|
||||
end
|
||||
|
||||
def provider_config_app_secrets(channel)
|
||||
return [] unless channel.respond_to?(:provider_config)
|
||||
|
||||
provider_config = channel.provider_config.to_h.with_indifferent_access
|
||||
CHANNEL_APP_SECRET_KEYS.filter_map { |key| provider_config[key].presence }
|
||||
end
|
||||
|
||||
def bot
|
||||
Chatwoot::Bot
|
||||
end
|
||||
|
||||
@@ -1,6 +1,25 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe 'Webhooks::InstagramController', type: :request do
|
||||
let(:client_secret) { 'test-instagram-secret' }
|
||||
|
||||
def signature_for(body, secret = client_secret)
|
||||
"sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, body)}"
|
||||
end
|
||||
|
||||
def post_instagram_webhook(body, signature: signature_for(body), env: { INSTAGRAM_APP_SECRET: client_secret })
|
||||
with_modified_env env do
|
||||
post '/webhooks/instagram',
|
||||
params: body,
|
||||
headers: { 'CONTENT_TYPE' => 'application/json', 'X-Hub-Signature-256' => signature }
|
||||
end
|
||||
end
|
||||
|
||||
before do
|
||||
InstallationConfig.where(name: %w[FB_APP_SECRET IG_VERIFY_TOKEN INSTAGRAM_APP_SECRET INSTAGRAM_VERIFY_TOKEN]).delete_all
|
||||
GlobalConfig.clear_cache
|
||||
end
|
||||
|
||||
describe 'GET /webhooks/verify' do
|
||||
it 'returns 401 when valid params are not present' do
|
||||
get '/webhooks/instagram/verify'
|
||||
@@ -24,26 +43,62 @@ RSpec.describe 'Webhooks::InstagramController', type: :request do
|
||||
|
||||
describe 'POST /webhooks/instagram' do
|
||||
let!(:dm_params) { build(:instagram_message_create_event).with_indifferent_access }
|
||||
let(:body) { dm_params.merge(object: 'instagram').to_json }
|
||||
|
||||
it 'call the instagram events job with the params' do
|
||||
it 'calls the instagram events job with the params for a valid signature' do
|
||||
allow(Webhooks::InstagramEventsJob).to receive(:perform_later)
|
||||
expect(Webhooks::InstagramEventsJob).to receive(:perform_later)
|
||||
|
||||
instagram_params = dm_params.merge(object: 'instagram')
|
||||
post '/webhooks/instagram', params: instagram_params
|
||||
post_instagram_webhook(body)
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it 'accepts webhook payloads signed with the Facebook app secret' do
|
||||
allow(Webhooks::InstagramEventsJob).to receive(:perform_later)
|
||||
expect(Webhooks::InstagramEventsJob).to receive(:perform_later)
|
||||
|
||||
facebook_secret = 'test-facebook-secret'
|
||||
post_instagram_webhook(
|
||||
body,
|
||||
signature: signature_for(body, facebook_secret),
|
||||
env: { FB_APP_SECRET: facebook_secret }
|
||||
)
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it 'returns unauthorized when signature is missing' do
|
||||
allow(Webhooks::InstagramEventsJob).to receive(:perform_later)
|
||||
|
||||
with_modified_env INSTAGRAM_APP_SECRET: client_secret do
|
||||
post '/webhooks/instagram',
|
||||
params: body,
|
||||
headers: { 'CONTENT_TYPE' => 'application/json' }
|
||||
end
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(Webhooks::InstagramEventsJob).not_to have_received(:perform_later)
|
||||
end
|
||||
|
||||
it 'returns unauthorized when signature is invalid' do
|
||||
allow(Webhooks::InstagramEventsJob).to receive(:perform_later)
|
||||
|
||||
post_instagram_webhook(body, signature: 'sha256=invalid-signature')
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(Webhooks::InstagramEventsJob).not_to have_received(:perform_later)
|
||||
end
|
||||
|
||||
context 'when processing echo events' do
|
||||
let!(:echo_params) { build(:instagram_story_mention_event_with_echo).with_indifferent_access }
|
||||
let(:echo_body) { echo_params.merge(object: 'instagram').to_json }
|
||||
|
||||
it 'delays processing for echo events by 2 seconds' do
|
||||
job_double = class_double(Webhooks::InstagramEventsJob)
|
||||
allow(Webhooks::InstagramEventsJob).to receive(:set).with(wait: 2.seconds).and_return(job_double)
|
||||
allow(job_double).to receive(:perform_later)
|
||||
|
||||
instagram_params = echo_params.merge(object: 'instagram')
|
||||
post '/webhooks/instagram', params: instagram_params
|
||||
post_instagram_webhook(echo_body)
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(Webhooks::InstagramEventsJob).to have_received(:set).with(wait: 2.seconds)
|
||||
expect(job_double).to have_received(:perform_later)
|
||||
|
||||
@@ -2,6 +2,33 @@ require 'rails_helper'
|
||||
|
||||
RSpec.describe 'Webhooks::WhatsappController', type: :request do
|
||||
let(:channel) { create(:channel_whatsapp, provider: 'whatsapp_cloud', sync_templates: false, validate_provider_config: false) }
|
||||
let(:client_secret) { 'test-whatsapp-secret' }
|
||||
let(:body) { { content: 'hello' }.to_json }
|
||||
|
||||
def signature_for(body, secret = client_secret)
|
||||
"sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, body)}"
|
||||
end
|
||||
|
||||
def post_whatsapp_webhook(path, body, signature: signature_for(body), env: { WHATSAPP_APP_SECRET: client_secret })
|
||||
with_modified_env env do
|
||||
post path,
|
||||
params: body,
|
||||
headers: { 'CONTENT_TYPE' => 'application/json', 'X-Hub-Signature-256' => signature }
|
||||
end
|
||||
end
|
||||
|
||||
def post_unsigned_whatsapp_webhook(path, body, env: { WHATSAPP_APP_SECRET: client_secret })
|
||||
with_modified_env env do
|
||||
post path,
|
||||
params: body,
|
||||
headers: { 'CONTENT_TYPE' => 'application/json' }
|
||||
end
|
||||
end
|
||||
|
||||
before do
|
||||
InstallationConfig.where(name: 'WHATSAPP_APP_SECRET').delete_all
|
||||
GlobalConfig.clear_cache
|
||||
end
|
||||
|
||||
describe 'GET /webhooks/verify' do
|
||||
it 'returns 401 when valid params are not present' do
|
||||
@@ -23,13 +50,103 @@ RSpec.describe 'Webhooks::WhatsappController', type: :request do
|
||||
end
|
||||
|
||||
describe 'POST /webhooks/whatsapp/{:phone_number}' do
|
||||
it 'call the whatsapp events job with the params' do
|
||||
it 'calls the whatsapp events job with the params for a valid signature' do
|
||||
allow(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
expect(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
post '/webhooks/whatsapp/123221321', params: { content: 'hello' }
|
||||
post_whatsapp_webhook('/webhooks/whatsapp/123221321', body)
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it 'accepts webhook payloads signed with the channel app secret' do
|
||||
channel_secret = 'channel-whatsapp-secret'
|
||||
channel.provider_config = channel.provider_config.merge('app_secret' => channel_secret)
|
||||
channel.save!
|
||||
|
||||
allow(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
expect(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
|
||||
channel_body = {
|
||||
object: 'whatsapp_business_account',
|
||||
entry: [{
|
||||
changes: [{
|
||||
value: {
|
||||
metadata: {
|
||||
display_phone_number: channel.phone_number.delete_prefix('+'),
|
||||
phone_number_id: channel.provider_config['phone_number_id']
|
||||
}
|
||||
}
|
||||
}]
|
||||
}]
|
||||
}.to_json
|
||||
|
||||
post_whatsapp_webhook(
|
||||
"/webhooks/whatsapp/#{channel.phone_number}",
|
||||
channel_body,
|
||||
signature: signature_for(channel_body, channel_secret),
|
||||
env: {}
|
||||
)
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it 'skips signature validation for 360dialog channels' do
|
||||
dialog_channel = create(:channel_whatsapp, provider: 'default', sync_templates: false, validate_provider_config: false)
|
||||
allow(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
expect(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
|
||||
post_unsigned_whatsapp_webhook("/webhooks/whatsapp/#{dialog_channel.phone_number}", body)
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it 'skips signature validation for manual whatsapp cloud channels without an app secret' do
|
||||
channel.update!(
|
||||
provider_config: channel.provider_config.except('app_secret', 'app_secret_key', 'api_secret', 'client_secret', 'source')
|
||||
)
|
||||
allow(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
expect(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
|
||||
channel_body = {
|
||||
object: 'whatsapp_business_account',
|
||||
entry: [{
|
||||
changes: [{
|
||||
value: {
|
||||
metadata: {
|
||||
display_phone_number: channel.phone_number.delete_prefix('+'),
|
||||
phone_number_id: channel.provider_config['phone_number_id']
|
||||
}
|
||||
}
|
||||
}]
|
||||
}]
|
||||
}.to_json
|
||||
|
||||
post_unsigned_whatsapp_webhook("/webhooks/whatsapp/#{channel.phone_number}", channel_body)
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it 'returns unauthorized when signature is missing' do
|
||||
allow(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
|
||||
with_modified_env WHATSAPP_APP_SECRET: client_secret do
|
||||
post '/webhooks/whatsapp/123221321',
|
||||
params: body,
|
||||
headers: { 'CONTENT_TYPE' => 'application/json' }
|
||||
end
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(Webhooks::WhatsappEventsJob).not_to have_received(:perform_later)
|
||||
end
|
||||
|
||||
it 'returns unauthorized when signature is invalid' do
|
||||
allow(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
|
||||
post_whatsapp_webhook('/webhooks/whatsapp/123221321', body, signature: 'sha256=invalid-signature')
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(Webhooks::WhatsappEventsJob).not_to have_received(:perform_later)
|
||||
end
|
||||
|
||||
context 'when phone number is in inactive list' do
|
||||
before do
|
||||
allow(GlobalConfig).to receive(:get_value).with('INACTIVE_WHATSAPP_NUMBERS').and_return('+1234567890,+9876543210')
|
||||
@@ -39,7 +156,7 @@ RSpec.describe 'Webhooks::WhatsappController', type: :request do
|
||||
allow(Rails.logger).to receive(:warn)
|
||||
expect(Rails.logger).to receive(:warn).with('Rejected webhook for inactive WhatsApp number: +1234567890')
|
||||
|
||||
post '/webhooks/whatsapp/+1234567890', params: { content: 'hello' }
|
||||
post_whatsapp_webhook('/webhooks/whatsapp/+1234567890', body)
|
||||
expect(response).to have_http_status(:unprocessable_entity)
|
||||
expect(response.parsed_body['error']).to eq('Inactive WhatsApp number')
|
||||
end
|
||||
@@ -54,7 +171,7 @@ RSpec.describe 'Webhooks::WhatsappController', type: :request do
|
||||
allow(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
expect(Webhooks::WhatsappEventsJob).to receive(:perform_later)
|
||||
|
||||
post '/webhooks/whatsapp/+1234567890', params: { content: 'hello' }
|
||||
post_whatsapp_webhook('/webhooks/whatsapp/+1234567890', body)
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user