fix: setup webhook for create and update should be done after db commit (#12176)

## Reference
https://github.com/chatwoot/chatwoot/pull/12149#issuecomment-3178108388

## Description

setup_webhook was done before the save, and hence the meta webhook
validation might fail because of a race condition where the facebook
validation is done before we saving the entry to the database.

## Type of change

Please delete options that are not relevant.

- [ ] Bug fix (non-breaking change which fixes an issue)

## How Has This Been Tested?

- New inbox creation, webhook validation
- Existing inbox update, webhook validation
- 
<img width="614" height="674" alt="image"
src="https://github.com/user-attachments/assets/be223945-deed-475a-82e5-3ae9c54a13fa"
/>


## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] 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
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] 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:
Tanmay Deep Sharma
2025-08-13 20:53:31 +05:30
committed by GitHub
parent a88fef2e1d
commit 6b42ff8d39
6 changed files with 223 additions and 140 deletions

View File

@@ -32,7 +32,6 @@ class Channel::Whatsapp < ApplicationRecord
validates :phone_number, presence: true, uniqueness: true validates :phone_number, presence: true, uniqueness: true
validate :validate_provider_config validate :validate_provider_config
before_save :setup_webhooks
after_create :sync_templates after_create :sync_templates
before_destroy :teardown_webhooks before_destroy :teardown_webhooks
@@ -60,6 +59,13 @@ class Channel::Whatsapp < ApplicationRecord
delegate :media_url, to: :provider_service delegate :media_url, to: :provider_service
delegate :api_headers, to: :provider_service delegate :api_headers, to: :provider_service
def setup_webhooks
perform_webhook_setup
rescue StandardError => e
Rails.logger.error "[WHATSAPP] Webhook setup failed: #{e.message}"
prompt_reauthorization!
end
private private
def ensure_webhook_verify_token def ensure_webhook_verify_token
@@ -70,34 +76,6 @@ class Channel::Whatsapp < ApplicationRecord
errors.add(:provider_config, 'Invalid Credentials') unless provider_service.validate_provider_config? errors.add(:provider_config, 'Invalid Credentials') unless provider_service.validate_provider_config?
end end
def setup_webhooks
return unless should_setup_webhooks?
perform_webhook_setup
rescue StandardError => e
handle_webhook_setup_error(e)
end
def provider_config_changed?
will_save_change_to_provider_config?
end
def should_setup_webhooks?
whatsapp_cloud_provider? && embedded_signup_source? && webhook_config_present? && provider_config_changed?
end
def whatsapp_cloud_provider?
provider == 'whatsapp_cloud'
end
def embedded_signup_source?
provider_config['source'] == 'embedded_signup'
end
def webhook_config_present?
provider_config['business_account_id'].present? && provider_config['api_key'].present?
end
def perform_webhook_setup def perform_webhook_setup
business_account_id = provider_config['business_account_id'] business_account_id = provider_config['business_account_id']
api_key = provider_config['api_key'] api_key = provider_config['api_key']
@@ -105,12 +83,6 @@ class Channel::Whatsapp < ApplicationRecord
Whatsapp::WebhookSetupService.new(self, business_account_id, api_key).perform Whatsapp::WebhookSetupService.new(self, business_account_id, api_key).perform
end end
def handle_webhook_setup_error(error)
Rails.logger.error "[WHATSAPP] Webhook setup failed: #{error.message}"
# Don't raise the error to prevent channel creation from failing
# Webhooks can be retried later
end
def teardown_webhooks def teardown_webhooks
Whatsapp::WebhookTeardownService.new(self).perform Whatsapp::WebhookTeardownService.new(self).perform
end end

View File

@@ -33,15 +33,14 @@ class Whatsapp::ChannelCreationService
def create_channel_with_inbox def create_channel_with_inbox
ActiveRecord::Base.transaction do ActiveRecord::Base.transaction do
channel = create_channel channel = build_channel
create_inbox(channel) create_inbox(channel)
channel.reload
channel channel
end end
end end
def create_channel def build_channel
Channel::Whatsapp.create!( Channel::Whatsapp.build(
account: @account, account: @account,
phone_number: @phone_info[:phone_number], phone_number: @phone_info[:phone_number],
provider: 'whatsapp_cloud', provider: 'whatsapp_cloud',

View File

@@ -11,16 +11,34 @@ class Whatsapp::EmbeddedSignupService
def perform def perform
validate_parameters! validate_parameters!
# Exchange code for user access token access_token = exchange_code_for_token
access_token = Whatsapp::TokenExchangeService.new(@code).perform phone_info = fetch_phone_info(access_token)
validate_token_access(access_token)
# Fetch phone information channel = create_or_reauthorize_channel(access_token, phone_info)
phone_info = Whatsapp::PhoneInfoService.new(@waba_id, @phone_number_id, access_token).perform channel.setup_webhooks
channel
# Validate token has access to the WABA rescue StandardError => e
Rails.logger.error("[WHATSAPP] Embedded signup failed: #{e.message}")
raise e
end
private
def exchange_code_for_token
Whatsapp::TokenExchangeService.new(@code).perform
end
def fetch_phone_info(access_token)
Whatsapp::PhoneInfoService.new(@waba_id, @phone_number_id, access_token).perform
end
def validate_token_access(access_token)
Whatsapp::TokenValidationService.new(access_token, @waba_id).perform Whatsapp::TokenValidationService.new(access_token, @waba_id).perform
end
# Reauthorization flow if inbox_id is present def create_or_reauthorize_channel(access_token, phone_info)
if @inbox_id.present? if @inbox_id.present?
Whatsapp::ReauthorizationService.new( Whatsapp::ReauthorizationService.new(
account: @account, account: @account,
@@ -29,17 +47,11 @@ class Whatsapp::EmbeddedSignupService
business_id: @business_id business_id: @business_id
).perform(access_token, phone_info) ).perform(access_token, phone_info)
else else
# Create channel for new authorization
waba_info = { waba_id: @waba_id, business_name: phone_info[:business_name] } waba_info = { waba_id: @waba_id, business_name: phone_info[:business_name] }
Whatsapp::ChannelCreationService.new(@account, waba_info, phone_info, access_token).perform Whatsapp::ChannelCreationService.new(@account, waba_info, phone_info, access_token).perform
end end
rescue StandardError => e
Rails.logger.error("[WHATSAPP] Embedded signup failed: #{e.message}")
raise e
end end
private
def validate_parameters! def validate_parameters!
missing_params = [] missing_params = []
missing_params << 'code' if @code.blank? missing_params << 'code' if @code.blank?

View File

@@ -16,6 +16,11 @@ describe Whatsapp::ChannelCreationService do
describe '#perform' do describe '#perform' do
before do before do
# Stub the webhook teardown service to prevent HTTP calls during cleanup
teardown_service = instance_double(Whatsapp::WebhookTeardownService)
allow(Whatsapp::WebhookTeardownService).to receive(:new).and_return(teardown_service)
allow(teardown_service).to receive(:perform)
# Clean up any existing channels to avoid phone number conflicts # Clean up any existing channels to avoid phone number conflicts
Channel::Whatsapp.destroy_all Channel::Whatsapp.destroy_all

View File

@@ -10,121 +10,100 @@ describe Whatsapp::EmbeddedSignupService do
phone_number_id: 'test_phone_number_id' phone_number_id: 'test_phone_number_id'
} }
end end
let(:service) do let(:service) { described_class.new(account: account, params: params) }
described_class.new( let(:access_token) { 'test_access_token' }
account: account, let(:phone_info) do
params: params {
) phone_number_id: params[:phone_number_id],
phone_number: '+1234567890',
verified: true,
business_name: 'Test Business'
}
end end
let(:channel) { instance_double(Channel::Whatsapp) }
describe '#perform' do describe '#perform' do
let(:access_token) { 'test_access_token' }
let(:phone_info) do
{
phone_number_id: params[:phone_number_id],
phone_number: '+1234567890',
verified: true,
business_name: 'Test Business'
}
end
let(:channel) { instance_double(Channel::Whatsapp) }
let(:service_doubles) do
{
token_exchange: instance_double(Whatsapp::TokenExchangeService),
phone_info: instance_double(Whatsapp::PhoneInfoService),
token_validation: instance_double(Whatsapp::TokenValidationService),
channel_creation: instance_double(Whatsapp::ChannelCreationService)
}
end
before do before do
allow(GlobalConfig).to receive(:clear_cache) allow(GlobalConfig).to receive(:clear_cache)
allow(Whatsapp::TokenExchangeService).to receive(:new).with(params[:code]).and_return(service_doubles[:token_exchange]) # Mock service dependencies
allow(service_doubles[:token_exchange]).to receive(:perform).and_return(access_token) token_exchange = instance_double(Whatsapp::TokenExchangeService)
allow(Whatsapp::TokenExchangeService).to receive(:new).with(params[:code]).and_return(token_exchange)
allow(token_exchange).to receive(:perform).and_return(access_token)
phone_service = instance_double(Whatsapp::PhoneInfoService)
allow(Whatsapp::PhoneInfoService).to receive(:new) allow(Whatsapp::PhoneInfoService).to receive(:new)
.with(params[:waba_id], params[:phone_number_id], access_token).and_return(service_doubles[:phone_info]) .with(params[:waba_id], params[:phone_number_id], access_token).and_return(phone_service)
allow(service_doubles[:phone_info]).to receive(:perform).and_return(phone_info) allow(phone_service).to receive(:perform).and_return(phone_info)
validation_service = instance_double(Whatsapp::TokenValidationService)
allow(Whatsapp::TokenValidationService).to receive(:new) allow(Whatsapp::TokenValidationService).to receive(:new)
.with(access_token, params[:waba_id]).and_return(service_doubles[:token_validation]) .with(access_token, params[:waba_id]).and_return(validation_service)
allow(service_doubles[:token_validation]).to receive(:perform) allow(validation_service).to receive(:perform)
channel_creation = instance_double(Whatsapp::ChannelCreationService)
allow(Whatsapp::ChannelCreationService).to receive(:new) allow(Whatsapp::ChannelCreationService).to receive(:new)
.with(account, { waba_id: params[:waba_id], business_name: 'Test Business' }, phone_info, access_token) .with(account, { waba_id: params[:waba_id], business_name: 'Test Business' }, phone_info, access_token)
.and_return(service_doubles[:channel_creation]) .and_return(channel_creation)
allow(service_doubles[:channel_creation]).to receive(:perform).and_return(channel) allow(channel_creation).to receive(:perform).and_return(channel)
# Webhook setup is now handled in the channel after_create callback allow(channel).to receive(:setup_webhooks)
# So we stub it at the model level
webhook_service = instance_double(Whatsapp::WebhookSetupService)
allow(Whatsapp::WebhookSetupService).to receive(:new).and_return(webhook_service)
allow(webhook_service).to receive(:perform)
end end
it 'orchestrates all services in the correct order' do it 'creates channel and sets up webhooks' do
expect(service_doubles[:token_exchange]).to receive(:perform).ordered expect(channel).to receive(:setup_webhooks)
expect(service_doubles[:phone_info]).to receive(:perform).ordered
expect(service_doubles[:token_validation]).to receive(:perform).ordered
expect(service_doubles[:channel_creation]).to receive(:perform).ordered
result = service.perform result = service.perform
expect(result).to eq(channel) expect(result).to eq(channel)
end end
context 'when required parameters are missing' do context 'when parameters are invalid' do
it 'raises error when code is blank' do it 'raises ArgumentError for missing parameters' do
service = described_class.new( invalid_service = described_class.new(account: account, params: { code: '', business_id: '', waba_id: '' })
account: account, expect { invalid_service.perform }.to raise_error(ArgumentError, /Required parameters are missing/)
params: params.merge(code: '')
)
expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: code/)
end
it 'raises error when business_id is blank' do
service = described_class.new(
account: account,
params: params.merge(business_id: '')
)
expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: business_id/)
end
it 'raises error when waba_id is blank' do
service = described_class.new(
account: account,
params: params.merge(waba_id: '')
)
expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: waba_id/)
end
it 'raises error when multiple parameters are blank' do
service = described_class.new(
account: account,
params: params.merge(code: '', business_id: '')
)
expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: code, business_id/)
end end
end end
context 'when any service fails' do context 'when service fails' do
it 'logs and re-raises the error' do it 'logs and re-raises errors' do
allow(service_doubles[:token_exchange]).to receive(:perform).and_raise('Token error') token_exchange = instance_double(Whatsapp::TokenExchangeService)
allow(Whatsapp::TokenExchangeService).to receive(:new).and_return(token_exchange)
allow(token_exchange).to receive(:perform).and_raise('Token error')
expect(Rails.logger).to receive(:error).with('[WHATSAPP] Embedded signup failed: Token error') expect(Rails.logger).to receive(:error).with('[WHATSAPP] Embedded signup failed: Token error')
expect { service.perform }.to raise_error('Token error') expect { service.perform }.to raise_error('Token error')
end end
it 'prompts reauthorization when webhook setup fails' do
# Create a real channel to test the actual webhook failure behavior
real_channel = create(:channel_whatsapp, account: account, phone_number: '+1234567890',
validate_provider_config: false, sync_templates: false)
# Mock the channel creation to return our real channel
channel_creation = instance_double(Whatsapp::ChannelCreationService)
allow(Whatsapp::ChannelCreationService).to receive(:new).and_return(channel_creation)
allow(channel_creation).to receive(:perform).and_return(real_channel)
# Mock webhook setup to fail
allow(real_channel).to receive(:perform_webhook_setup).and_raise('Webhook setup error')
# Verify channel is not marked for reauthorization initially
expect(real_channel.reauthorization_required?).to be false
# The service completes successfully even if webhook fails (webhook error is rescued in setup_webhooks)
result = service.perform
expect(result).to eq(real_channel)
# Verify the channel is now marked for reauthorization
expect(real_channel.reauthorization_required?).to be true
end
end end
context 'when inbox_id is provided (reauthorization flow)' do context 'with reauthorization flow' do
let(:inbox_id) { 123 } let(:inbox_id) { 123 }
let(:reauth_service) { instance_double(Whatsapp::ReauthorizationService) } let(:reauth_service) { instance_double(Whatsapp::ReauthorizationService) }
let(:service_with_inbox) do let(:service_with_inbox) do
described_class.new( described_class.new(account: account, params: params, inbox_id: inbox_id)
account: account,
params: params,
inbox_id: inbox_id
)
end end
before do before do
@@ -137,16 +116,45 @@ describe Whatsapp::EmbeddedSignupService do
allow(reauth_service).to receive(:perform).with(access_token, phone_info).and_return(channel) allow(reauth_service).to receive(:perform).with(access_token, phone_info).and_return(channel)
end end
it 'uses ReauthorizationService instead of ChannelCreationService' do it 'uses ReauthorizationService and sets up webhooks' do
expect(service_doubles[:token_exchange]).to receive(:perform).ordered expect(reauth_service).to receive(:perform)
expect(service_doubles[:phone_info]).to receive(:perform).ordered expect(channel).to receive(:setup_webhooks)
expect(service_doubles[:token_validation]).to receive(:perform).ordered
expect(reauth_service).to receive(:perform).with(access_token, phone_info).ordered
expect(service_doubles[:channel_creation]).not_to receive(:perform)
result = service_with_inbox.perform result = service_with_inbox.perform
expect(result).to eq(channel) expect(result).to eq(channel)
end end
it 'clears reauthorization flag' do
inbox = create(:inbox, account: account)
whatsapp_channel = create(:channel_whatsapp, account: account, phone_number: '+1234567890',
validate_provider_config: false, sync_templates: false)
inbox.update!(channel: whatsapp_channel)
whatsapp_channel.prompt_reauthorization!
service_with_real_inbox = described_class.new(account: account, params: params, inbox_id: inbox.id)
# Mock the ReauthorizationService to return our test channel
reauth_service = instance_double(Whatsapp::ReauthorizationService)
allow(Whatsapp::ReauthorizationService).to receive(:new).with(
account: account,
inbox_id: inbox.id,
phone_number_id: params[:phone_number_id],
business_id: params[:business_id]
).and_return(reauth_service)
# Perform the reauthorization and clear the flag
allow(reauth_service).to receive(:perform) do
whatsapp_channel.reauthorized!
whatsapp_channel
end
allow(whatsapp_channel).to receive(:setup_webhooks).and_return(true)
expect(whatsapp_channel.reauthorization_required?).to be true
result = service_with_real_inbox.perform
expect(result).to eq(whatsapp_channel)
expect(whatsapp_channel.reauthorization_required?).to be false
end
end end
end end
end end

View File

@@ -5,7 +5,7 @@ describe Whatsapp::WebhookSetupService do
create(:channel_whatsapp, create(:channel_whatsapp,
phone_number: '+1234567890', phone_number: '+1234567890',
provider_config: { provider_config: {
'phone_number_id' => 'test_phone_id', 'phone_number_id' => '123456789',
'webhook_verify_token' => 'test_verify_token' 'webhook_verify_token' => 'test_verify_token'
}, },
provider: 'whatsapp_cloud', provider: 'whatsapp_cloud',
@@ -18,9 +18,14 @@ describe Whatsapp::WebhookSetupService do
let(:api_client) { instance_double(Whatsapp::FacebookApiClient) } let(:api_client) { instance_double(Whatsapp::FacebookApiClient) }
before do before do
# Stub webhook teardown to prevent HTTP calls during cleanup
stub_request(:delete, /graph.facebook.com/).to_return(status: 200, body: '{}', headers: {})
# Clean up any existing channels to avoid phone number conflicts # Clean up any existing channels to avoid phone number conflicts
Channel::Whatsapp.destroy_all Channel::Whatsapp.destroy_all
allow(Whatsapp::FacebookApiClient).to receive(:new).and_return(api_client) allow(Whatsapp::FacebookApiClient).to receive(:new).and_return(api_client)
# Default stub for phone_number_verified? with any argument
allow(api_client).to receive(:phone_number_verified?).and_return(false)
end end
describe '#perform' do describe '#perform' do
@@ -148,5 +153,87 @@ describe Whatsapp::WebhookSetupService do
end end
end end
end end
context 'when webhook setup fails and should trigger reauthorization' do
before do
allow(api_client).to receive(:phone_number_verified?).with('123456789').and_return(true)
allow(api_client).to receive(:subscribe_waba_webhook).and_raise('Invalid access token')
end
it 'raises error with webhook setup failure message' do
with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
expect { service.perform }.to raise_error(/Webhook setup failed: Invalid access token/)
end
end
it 'logs the webhook setup failure' do
with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
expect(Rails.logger).to receive(:error).with('[WHATSAPP] Webhook setup failed: Invalid access token')
expect { service.perform }.to raise_error(/Webhook setup failed/)
end
end
end
context 'when used during reauthorization flow' do
let(:existing_channel) do
create(:channel_whatsapp,
phone_number: '+1234567890',
provider_config: {
'phone_number_id' => '123456789',
'webhook_verify_token' => 'existing_verify_token',
'business_id' => 'existing_business_id',
'waba_id' => 'existing_waba_id'
},
provider: 'whatsapp_cloud',
sync_templates: false,
validate_provider_config: false)
end
let(:new_access_token) { 'new_access_token' }
let(:service_reauth) { described_class.new(existing_channel, waba_id, new_access_token) }
before do
allow(api_client).to receive(:phone_number_verified?).with('123456789').and_return(true)
allow(api_client).to receive(:subscribe_waba_webhook)
.with(waba_id, anything, 'existing_verify_token').and_return({ 'success' => true })
end
it 'successfully reauthorizes with new access token' do
with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
expect(api_client).not_to receive(:register_phone_number)
expect(api_client).to receive(:subscribe_waba_webhook)
.with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'existing_verify_token')
service_reauth.perform
end
end
it 'uses the existing webhook verify token during reauthorization' do
with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
expect(api_client).to receive(:subscribe_waba_webhook)
.with(waba_id, anything, 'existing_verify_token')
service_reauth.perform
end
end
end
context 'when webhook setup is successful in creation flow' do
before do
allow(api_client).to receive(:phone_number_verified?).with('123456789').and_return(true)
allow(api_client).to receive(:subscribe_waba_webhook)
.with(waba_id, anything, 'test_verify_token').and_return({ 'success' => true })
end
it 'completes successfully without errors' do
with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
expect { service.perform }.not_to raise_error
end
end
it 'does not log any errors' do
with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
expect(Rails.logger).not_to receive(:error)
service.perform
end
end
end
end end
end end