From 6b42ff8d39bfbdfb84f433a43149552b5b8bea94 Mon Sep 17 00:00:00 2001
From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com>
Date: Wed, 13 Aug 2025 20:53:31 +0530
Subject: [PATCH] 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
-
## 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
---
app/models/channel/whatsapp.rb | 42 +---
.../whatsapp/channel_creation_service.rb | 7 +-
.../whatsapp/embedded_signup_service.rb | 36 ++--
.../whatsapp/channel_creation_service_spec.rb | 5 +
.../whatsapp/embedded_signup_service_spec.rb | 184 +++++++++---------
.../whatsapp/webhook_setup_service_spec.rb | 89 ++++++++-
6 files changed, 223 insertions(+), 140 deletions(-)
diff --git a/app/models/channel/whatsapp.rb b/app/models/channel/whatsapp.rb
index 7471cf807..7318cd978 100644
--- a/app/models/channel/whatsapp.rb
+++ b/app/models/channel/whatsapp.rb
@@ -32,7 +32,6 @@ class Channel::Whatsapp < ApplicationRecord
validates :phone_number, presence: true, uniqueness: true
validate :validate_provider_config
- before_save :setup_webhooks
after_create :sync_templates
before_destroy :teardown_webhooks
@@ -60,6 +59,13 @@ class Channel::Whatsapp < ApplicationRecord
delegate :media_url, 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
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?
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
business_account_id = provider_config['business_account_id']
api_key = provider_config['api_key']
@@ -105,12 +83,6 @@ class Channel::Whatsapp < ApplicationRecord
Whatsapp::WebhookSetupService.new(self, business_account_id, api_key).perform
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
Whatsapp::WebhookTeardownService.new(self).perform
end
diff --git a/app/services/whatsapp/channel_creation_service.rb b/app/services/whatsapp/channel_creation_service.rb
index 3039ca003..154f55520 100644
--- a/app/services/whatsapp/channel_creation_service.rb
+++ b/app/services/whatsapp/channel_creation_service.rb
@@ -33,15 +33,14 @@ class Whatsapp::ChannelCreationService
def create_channel_with_inbox
ActiveRecord::Base.transaction do
- channel = create_channel
+ channel = build_channel
create_inbox(channel)
- channel.reload
channel
end
end
- def create_channel
- Channel::Whatsapp.create!(
+ def build_channel
+ Channel::Whatsapp.build(
account: @account,
phone_number: @phone_info[:phone_number],
provider: 'whatsapp_cloud',
diff --git a/app/services/whatsapp/embedded_signup_service.rb b/app/services/whatsapp/embedded_signup_service.rb
index e66506638..1b882b1f1 100644
--- a/app/services/whatsapp/embedded_signup_service.rb
+++ b/app/services/whatsapp/embedded_signup_service.rb
@@ -11,16 +11,34 @@ class Whatsapp::EmbeddedSignupService
def perform
validate_parameters!
- # Exchange code for user access token
- access_token = Whatsapp::TokenExchangeService.new(@code).perform
+ access_token = exchange_code_for_token
+ phone_info = fetch_phone_info(access_token)
+ validate_token_access(access_token)
- # Fetch phone information
- phone_info = Whatsapp::PhoneInfoService.new(@waba_id, @phone_number_id, access_token).perform
+ channel = create_or_reauthorize_channel(access_token, phone_info)
+ 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
+ end
- # Reauthorization flow if inbox_id is present
+ def create_or_reauthorize_channel(access_token, phone_info)
if @inbox_id.present?
Whatsapp::ReauthorizationService.new(
account: @account,
@@ -29,17 +47,11 @@ class Whatsapp::EmbeddedSignupService
business_id: @business_id
).perform(access_token, phone_info)
else
- # Create channel for new authorization
waba_info = { waba_id: @waba_id, business_name: phone_info[:business_name] }
Whatsapp::ChannelCreationService.new(@account, waba_info, phone_info, access_token).perform
end
- rescue StandardError => e
- Rails.logger.error("[WHATSAPP] Embedded signup failed: #{e.message}")
- raise e
end
- private
-
def validate_parameters!
missing_params = []
missing_params << 'code' if @code.blank?
diff --git a/spec/services/whatsapp/channel_creation_service_spec.rb b/spec/services/whatsapp/channel_creation_service_spec.rb
index 1c1f46232..403fb71b8 100644
--- a/spec/services/whatsapp/channel_creation_service_spec.rb
+++ b/spec/services/whatsapp/channel_creation_service_spec.rb
@@ -16,6 +16,11 @@ describe Whatsapp::ChannelCreationService do
describe '#perform' 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
Channel::Whatsapp.destroy_all
diff --git a/spec/services/whatsapp/embedded_signup_service_spec.rb b/spec/services/whatsapp/embedded_signup_service_spec.rb
index 12a4d32df..1db94928e 100644
--- a/spec/services/whatsapp/embedded_signup_service_spec.rb
+++ b/spec/services/whatsapp/embedded_signup_service_spec.rb
@@ -10,121 +10,100 @@ describe Whatsapp::EmbeddedSignupService do
phone_number_id: 'test_phone_number_id'
}
end
- let(:service) do
- described_class.new(
- account: account,
- params: params
- )
+ let(:service) { described_class.new(account: account, params: params) }
+ 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) }
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
allow(GlobalConfig).to receive(:clear_cache)
- allow(Whatsapp::TokenExchangeService).to receive(:new).with(params[:code]).and_return(service_doubles[:token_exchange])
- allow(service_doubles[:token_exchange]).to receive(:perform).and_return(access_token)
+ # Mock service dependencies
+ 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)
- .with(params[:waba_id], params[:phone_number_id], access_token).and_return(service_doubles[:phone_info])
- allow(service_doubles[:phone_info]).to receive(:perform).and_return(phone_info)
+ .with(params[:waba_id], params[:phone_number_id], access_token).and_return(phone_service)
+ allow(phone_service).to receive(:perform).and_return(phone_info)
+ validation_service = instance_double(Whatsapp::TokenValidationService)
allow(Whatsapp::TokenValidationService).to receive(:new)
- .with(access_token, params[:waba_id]).and_return(service_doubles[:token_validation])
- allow(service_doubles[:token_validation]).to receive(:perform)
+ .with(access_token, params[:waba_id]).and_return(validation_service)
+ allow(validation_service).to receive(:perform)
+ channel_creation = instance_double(Whatsapp::ChannelCreationService)
allow(Whatsapp::ChannelCreationService).to receive(:new)
.with(account, { waba_id: params[:waba_id], business_name: 'Test Business' }, phone_info, access_token)
- .and_return(service_doubles[:channel_creation])
- allow(service_doubles[:channel_creation]).to receive(:perform).and_return(channel)
+ .and_return(channel_creation)
+ allow(channel_creation).to receive(:perform).and_return(channel)
- # Webhook setup is now handled in the channel after_create callback
- # 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)
+ allow(channel).to receive(:setup_webhooks)
end
- it 'orchestrates all services in the correct order' do
- expect(service_doubles[:token_exchange]).to receive(:perform).ordered
- 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
+ it 'creates channel and sets up webhooks' do
+ expect(channel).to receive(:setup_webhooks)
result = service.perform
expect(result).to eq(channel)
end
- context 'when required parameters are missing' do
- it 'raises error when code is blank' do
- service = described_class.new(
- account: account,
- 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/)
+ context 'when parameters are invalid' do
+ it 'raises ArgumentError for missing parameters' do
+ invalid_service = described_class.new(account: account, params: { code: '', business_id: '', waba_id: '' })
+ expect { invalid_service.perform }.to raise_error(ArgumentError, /Required parameters are missing/)
end
end
- context 'when any service fails' do
- it 'logs and re-raises the error' do
- allow(service_doubles[:token_exchange]).to receive(:perform).and_raise('Token error')
+ context 'when service fails' do
+ it 'logs and re-raises errors' do
+ 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 { service.perform }.to raise_error('Token error')
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
- context 'when inbox_id is provided (reauthorization flow)' do
+ context 'with reauthorization flow' do
let(:inbox_id) { 123 }
let(:reauth_service) { instance_double(Whatsapp::ReauthorizationService) }
let(:service_with_inbox) do
- described_class.new(
- account: account,
- params: params,
- inbox_id: inbox_id
- )
+ described_class.new(account: account, params: params, inbox_id: inbox_id)
end
before do
@@ -137,16 +116,45 @@ describe Whatsapp::EmbeddedSignupService do
allow(reauth_service).to receive(:perform).with(access_token, phone_info).and_return(channel)
end
- it 'uses ReauthorizationService instead of ChannelCreationService' do
- expect(service_doubles[:token_exchange]).to receive(:perform).ordered
- expect(service_doubles[:phone_info]).to receive(:perform).ordered
- 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)
+ it 'uses ReauthorizationService and sets up webhooks' do
+ expect(reauth_service).to receive(:perform)
+ expect(channel).to receive(:setup_webhooks)
result = service_with_inbox.perform
expect(result).to eq(channel)
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
diff --git a/spec/services/whatsapp/webhook_setup_service_spec.rb b/spec/services/whatsapp/webhook_setup_service_spec.rb
index 7cee115c2..e6a246e5d 100644
--- a/spec/services/whatsapp/webhook_setup_service_spec.rb
+++ b/spec/services/whatsapp/webhook_setup_service_spec.rb
@@ -5,7 +5,7 @@ describe Whatsapp::WebhookSetupService do
create(:channel_whatsapp,
phone_number: '+1234567890',
provider_config: {
- 'phone_number_id' => 'test_phone_id',
+ 'phone_number_id' => '123456789',
'webhook_verify_token' => 'test_verify_token'
},
provider: 'whatsapp_cloud',
@@ -18,9 +18,14 @@ describe Whatsapp::WebhookSetupService do
let(:api_client) { instance_double(Whatsapp::FacebookApiClient) }
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
Channel::Whatsapp.destroy_all
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
describe '#perform' do
@@ -148,5 +153,87 @@ describe Whatsapp::WebhookSetupService do
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