diff --git a/app/controllers/concerns/meta_token_verify_concern.rb b/app/controllers/concerns/meta_token_verify_concern.rb index b3f920644..42fe918cc 100644 --- a/app/controllers/concerns/meta_token_verify_concern.rb +++ b/app/controllers/concerns/meta_token_verify_concern.rb @@ -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 diff --git a/app/controllers/webhooks/instagram_controller.rb b/app/controllers/webhooks/instagram_controller.rb index 569c2524b..6e5168634 100644 --- a/app/controllers/webhooks/instagram_controller.rb +++ b/app/controllers/webhooks/instagram_controller.rb @@ -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 diff --git a/app/controllers/webhooks/whatsapp_controller.rb b/app/controllers/webhooks/whatsapp_controller.rb index c4c376e5c..ee71f3c92 100644 --- a/app/controllers/webhooks/whatsapp_controller.rb +++ b/app/controllers/webhooks/whatsapp_controller.rb @@ -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? diff --git a/config/initializers/facebook_messenger.rb b/config/initializers/facebook_messenger.rb index f93829e65..047715cfb 100644 --- a/config/initializers/facebook_messenger.rb +++ b/config/initializers/facebook_messenger.rb @@ -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 diff --git a/spec/controllers/webhooks/instagram_controller_spec.rb b/spec/controllers/webhooks/instagram_controller_spec.rb index c31d65a2b..043d41b93 100644 --- a/spec/controllers/webhooks/instagram_controller_spec.rb +++ b/spec/controllers/webhooks/instagram_controller_spec.rb @@ -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) diff --git a/spec/controllers/webhooks/whatsapp_controller_spec.rb b/spec/controllers/webhooks/whatsapp_controller_spec.rb index e5fa392b0..05816094b 100644 --- a/spec/controllers/webhooks/whatsapp_controller_spec.rb +++ b/spec/controllers/webhooks/whatsapp_controller_spec.rb @@ -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