From 6928f14a762e5ae40b74d5901fb7685c46ac4ee2 Mon Sep 17 00:00:00 2001 From: Gabor Barany <23645175+gbarany@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:44:25 +0100 Subject: [PATCH 1/7] fix: Validate Twilio webhook signatures (X-Twilio-Signature) (#13638) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-authored-by: Muhsin Keloth Co-authored-by: Sojan Jose --- .../twilio_signature_verify_concern.rb | 88 ++++++++++ app/controllers/twilio/callback_controller.rb | 2 + .../twilio/delivery_status_controller.rb | 6 + .../twilio/callbacks_controller_spec.rb | 151 +++++++++++++++++- .../twilio/delivery_status_controller_spec.rb | 125 ++++++++++++++- 5 files changed, 356 insertions(+), 16 deletions(-) create mode 100644 app/controllers/concerns/twilio_signature_verify_concern.rb diff --git a/app/controllers/concerns/twilio_signature_verify_concern.rb b/app/controllers/concerns/twilio_signature_verify_concern.rb new file mode 100644 index 000000000..b7a4754a4 --- /dev/null +++ b/app/controllers/concerns/twilio_signature_verify_concern.rb @@ -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 diff --git a/app/controllers/twilio/callback_controller.rb b/app/controllers/twilio/callback_controller.rb index d607ba151..9b42cd034 100644 --- a/app/controllers/twilio/callback_controller.rb +++ b/app/controllers/twilio/callback_controller.rb @@ -1,4 +1,6 @@ class Twilio::CallbackController < ApplicationController + include TwilioSignatureVerifyConcern + def create Webhooks::TwilioEventsJob.perform_later(permitted_params.to_unsafe_hash) diff --git a/app/controllers/twilio/delivery_status_controller.rb b/app/controllers/twilio/delivery_status_controller.rb index 1c756a1c2..8e846a737 100644 --- a/app/controllers/twilio/delivery_status_controller.rb +++ b/app/controllers/twilio/delivery_status_controller.rb @@ -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 diff --git a/spec/controllers/twilio/callbacks_controller_spec.rb b/spec/controllers/twilio/callbacks_controller_spec.rb index d16acf229..09b558096 100644 --- a/spec/controllers/twilio/callbacks_controller_spec.rb +++ b/spec/controllers/twilio/callbacks_controller_spec.rb @@ -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 diff --git a/spec/controllers/twilio/delivery_status_controller_spec.rb b/spec/controllers/twilio/delivery_status_controller_spec.rb index fc21f8f94..dc99fdf23 100644 --- a/spec/controllers/twilio/delivery_status_controller_spec.rb +++ b/spec/controllers/twilio/delivery_status_controller_spec.rb @@ -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 From 6a9c44476e2293542c036bffe845c7bb4160ce47 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Tue, 21 Apr 2026 15:55:12 +0400 Subject: [PATCH 2/7] feat(super-admin): Add push diagnostics tool (#14105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We're getting many customer reports saying "I'm not getting notifications." We can't always identify the root cause since there are multiple points of failure. Added a **Push Diagnostics** tool in Super Admin to help us investigate mobile/web push issues. Here's how it works: - Look up a user by email/ID → see all their registered subscriptions with device info (iOS/Android, brand, model), token freshness, and last-updated time - Send a customizable test push and read the raw FCM/web-push/relay response to see if the customer is receiving push notifications—if not, it will show proper errors. - Delete broken subscriptions so the mobile app re-registers on next launch CleanShot 2026-04-20 at 12 56
56@2x Fixes https://linear.app/chatwoot/issue/CW-6892/push-diagnostics-tool --------- Co-authored-by: Muhsin <12408980+muhsin-k@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) --- .../push_diagnostics_controller.rb | 68 +++++++ .../notification/push_test_service.rb | 160 +++++++++++++++ .../application/_navigation.html.erb | 3 +- .../push_diagnostics/show.html.erb | 190 ++++++++++++++++++ config/locales/en.yml | 6 + config/routes.rb | 3 + lib/chatwoot_hub.rb | 8 +- 7 files changed, 435 insertions(+), 3 deletions(-) create mode 100644 app/controllers/super_admin/push_diagnostics_controller.rb create mode 100644 app/services/notification/push_test_service.rb create mode 100644 app/views/super_admin/push_diagnostics/show.html.erb diff --git a/app/controllers/super_admin/push_diagnostics_controller.rb b/app/controllers/super_admin/push_diagnostics_controller.rb new file mode 100644 index 000000000..c33cfdc1e --- /dev/null +++ b/app/controllers/super_admin/push_diagnostics_controller.rb @@ -0,0 +1,68 @@ +class SuperAdmin::PushDiagnosticsController < SuperAdmin::ApplicationController + def show + @query = params[:user_query].to_s.strip + @user = resolve_user(@query) + @subscriptions = @user ? @user.notification_subscriptions.order(:id) : [] + @results = [] + end + + def create + @user = User.find_by(id: params[:user_id]) + return redirect_to super_admin_push_diagnostics_path, alert: I18n.t('super_admin.push_diagnostics.user_not_found') if @user.nil? + + ids = parsed_subscription_ids + if ids.empty? + return redirect_to super_admin_push_diagnostics_path(user_query: @user.id), + alert: I18n.t('super_admin.push_diagnostics.no_subscriptions_to_test') + end + + run_test_and_render(ids) + end + + def destroy_subscriptions + user = User.find_by(id: params[:user_id]) + return redirect_to super_admin_push_diagnostics_path, alert: I18n.t('super_admin.push_diagnostics.user_not_found') if user.nil? + + ids = parsed_subscription_ids + if ids.empty? + return redirect_to super_admin_push_diagnostics_path(user_query: user.id), + alert: I18n.t('super_admin.push_diagnostics.no_subscriptions_to_delete') + end + + deleted_count = user.notification_subscriptions.where(id: ids).destroy_all.size + log_super_admin_action("deleted #{deleted_count} subscriptions for user #{user.id}: #{ids}") + redirect_to super_admin_push_diagnostics_path(user_query: user.id), + notice: I18n.t('super_admin.push_diagnostics.subscriptions_deleted', count: deleted_count) + end + + private + + def run_test_and_render(ids) + @query = @user.id.to_s + @subscriptions = @user.notification_subscriptions.order(:id) + @results = Notification::PushTestService.new( + user: @user, subscription_ids: ids, + title: params[:push_title], body: params[:push_body] + ).perform + + log_super_admin_action("test sent for user #{@user.id} subscriptions #{ids}") + render :show + end + + def log_super_admin_action(message) + Rails.logger.info( + "[SuperAdmin] push diagnostics #{message} " \ + "(actor_id=#{current_super_admin&.id}, actor_email=#{current_super_admin&.email})" + ) + end + + def resolve_user(query) + return if query.blank? + + query.match?(/\A\d+\z/) ? User.find_by(id: query) : User.from_email(query) + end + + def parsed_subscription_ids + Array(params[:subscription_ids]).reject(&:blank?).map(&:to_i) + end +end diff --git a/app/services/notification/push_test_service.rb b/app/services/notification/push_test_service.rb new file mode 100644 index 000000000..4b2bec177 --- /dev/null +++ b/app/services/notification/push_test_service.rb @@ -0,0 +1,160 @@ +class Notification::PushTestService + pattr_initialize [:user!, :subscription_ids!, :title, :body] + + DEFAULT_TITLE = '%s notification test'.freeze + DEFAULT_BODY = 'This is a test from our team to check notification delivery on your device. No action needed.'.freeze + + def self.default_title + format(DEFAULT_TITLE, installation_name: GlobalConfigService.load('INSTALLATION_NAME', 'Chatwoot')) + end + + def self.default_body + DEFAULT_BODY + end + + def perform + selected_subscriptions.map { |subscription| test_send(subscription) } + end + + private + + def resolved_title + title.presence || self.class.default_title + end + + def resolved_body + body.presence || self.class.default_body + end + + def selected_subscriptions + user.notification_subscriptions.where(id: subscription_ids).order(:id) + end + + def test_send(subscription) + if subscription.browser_push? + test_browser_push(subscription) + elsif subscription.fcm? + test_fcm(subscription) + else + result(subscription, subscription.subscription_type.to_s, :skipped, 'Unknown subscription type') + end + end + + def test_browser_push(subscription) + return result(subscription, 'browser_push', :skipped, 'VAPID keys not configured') unless VapidService.public_key + + WebPush.payload_send(**browser_push_payload(subscription)) + result(subscription, 'browser_push', :success, 'Web push accepted by endpoint') + rescue StandardError => e + result(subscription, 'browser_push', :failure, "#{e.class.name}: #{e.message}") + end + + def test_fcm(subscription) + if firebase_credentials_present? + test_fcm_direct(subscription) + elsif chatwoot_hub_enabled? + test_fcm_via_hub(subscription) + else + result(subscription, 'fcm', :skipped, 'No Firebase credentials and push relay disabled') + end + end + + def test_fcm_direct(subscription) + fcm_service = Notification::FcmService.new( + GlobalConfigService.load('FIREBASE_PROJECT_ID', nil), + GlobalConfigService.load('FIREBASE_CREDENTIALS', nil) + ) + response = fcm_service.fcm_client.send_v1(fcm_options(subscription)) + status_code = response[:status_code].to_i + status = status_code.between?(200, 299) ? :success : :failure + result(subscription, 'fcm', status, "HTTP #{status_code} — #{response[:body]}") + rescue StandardError => e + result(subscription, 'fcm', :failure, "#{e.class.name}: #{e.message}") + end + + def test_fcm_via_hub(subscription) + response = ChatwootHub.send_push_with_response(fcm_options(subscription)) + result(subscription, 'fcm_via_hub', :success, "HTTP #{response.code} — #{response.body}") + rescue RestClient::ExceptionWithResponse => e + result(subscription, 'fcm_via_hub', :failure, "HTTP #{e.response&.code} — #{e.response&.body}") + rescue StandardError => e + result(subscription, 'fcm_via_hub', :failure, "#{e.class.name}: #{e.message}") + end + + def firebase_credentials_present? + GlobalConfigService.load('FIREBASE_PROJECT_ID', nil) && GlobalConfigService.load('FIREBASE_CREDENTIALS', nil) + end + + def chatwoot_hub_enabled? + ActiveModel::Type::Boolean.new.cast(ENV.fetch('ENABLE_PUSH_RELAY_SERVER', true)) + end + + def browser_push_payload(subscription) + { + message: JSON.generate( + title: resolved_title, + tag: "super_admin_test_#{Time.zone.now.to_i}", + url: ENV.fetch('FRONTEND_URL', 'https://app.chatwoot.com') + ), + endpoint: subscription.subscription_attributes['endpoint'], + p256dh: subscription.subscription_attributes['p256dh'], + auth: subscription.subscription_attributes['auth'], + vapid: { + subject: ENV.fetch('FRONTEND_URL', 'https://app.chatwoot.com'), + public_key: VapidService.public_key, + private_key: VapidService.private_key + }, + ssl_timeout: 5, + open_timeout: 5, + read_timeout: 5 + } + end + + def fcm_options(subscription) + { + 'token': subscription.subscription_attributes['push_token'], + 'data': { payload: { data: { notification: { type: 'test' } } }.to_json }, + 'notification': { title: resolved_title, body: resolved_body }, + 'android': { priority: 'high' }, + 'apns': { payload: { aps: { sound: 'default', category: Time.zone.now.to_i.to_s } } }, + 'fcm_options': { analytics_label: 'SuperAdminTest' } + } + end + + def result(subscription, type, status, message) + attrs = subscription.subscription_attributes || {} + { + id: subscription.id, + type: type.to_s, + device: device_label(subscription, attrs), + token_tail: token_tail(subscription, attrs), + status: status, + message: message + } + end + + def device_label(subscription, attrs) + if subscription.browser_push? + endpoint_host(attrs['endpoint'].to_s) + else + attrs['device_id'].present? ? "…#{attrs['device_id'].to_s.last(6)}" : '—' + end + end + + def endpoint_host(endpoint) + return '—' if endpoint.blank? + + URI.parse(endpoint).host.presence || endpoint + rescue URI::InvalidURIError + endpoint + end + + def token_tail(subscription, attrs) + if subscription.browser_push? + endpoint = attrs['endpoint'].to_s + endpoint.present? ? "…#{endpoint.last(6)}" : '—' + else + attrs['push_token'].present? ? "…#{attrs['push_token'].to_s.last(6)}" : '—' + end + end +end diff --git a/app/views/super_admin/application/_navigation.html.erb b/app/views/super_admin/application/_navigation.html.erb index 787576d33..da3a024f8 100644 --- a/app/views/super_admin/application/_navigation.html.erb +++ b/app/views/super_admin/application/_navigation.html.erb @@ -32,7 +32,7 @@ as defined by the routes in the `admin/` namespace
    <%= render partial: "nav_item", locals: { icon: 'icon-grid-line', url: super_admin_root_url, label: 'Dashboard' } %> <% Administrate::Namespace.new(namespace).resources.each do |resource| %> - <% next if ["account_users", "access_tokens", "installation_configs", "dashboard", "devise/sessions", "app_configs", "instance_statuses", "settings"].include? resource.resource %> + <% next if ["account_users", "access_tokens", "installation_configs", "dashboard", "devise/sessions", "app_configs", "instance_statuses", "settings", "push_diagnostics"].include? resource.resource %> <%= render partial: "nav_item", locals: { icon: sidebar_icons[resource.resource.to_sym], url: resource_index_route(resource), @@ -48,6 +48,7 @@ as defined by the routes in the `admin/` namespace
      <%= render partial: "nav_item", locals: { icon: 'icon-mist-fill', url: sidekiq_web_url, label: 'Sidekiq Dashboard' } %> <%= render partial: "nav_item", locals: { icon: 'icon-health-book-line', url: super_admin_instance_status_url, label: 'Instance Health' } %> + <%= render partial: "nav_item", locals: { icon: 'icon-mail-send-fill', url: super_admin_push_diagnostics_url, label: 'Push Diagnostics' } %> <%= render partial: "nav_item", locals: { icon: 'icon-dashboard-line', url: '/', label: 'Agent Dashboard' } %> <%= render partial: "nav_item", locals: { icon: 'icon-logout-circle-r-line', url: super_admin_logout_url, label: 'Logout' } %>
    diff --git a/app/views/super_admin/push_diagnostics/show.html.erb b/app/views/super_admin/push_diagnostics/show.html.erb new file mode 100644 index 000000000..736a77cd8 --- /dev/null +++ b/app/views/super_admin/push_diagnostics/show.html.erb @@ -0,0 +1,190 @@ +<% content_for(:title) do %>Push Diagnostics<% end %> + + + +
    +

    + Send a test push notification to a specific user's registered devices to diagnose delivery issues. + Results show the raw FCM / Web Push / relay response so you can see exactly what failed. +

    + +
    +

    1. Look up user

    + <%= form_with url: super_admin_push_diagnostics_path, method: :get, local: true, class: 'flex gap-2 items-center' do |f| %> + <%= f.text_field :user_query, + value: @query, + placeholder: 'user@example.com or numeric user ID', + class: 'border border-slate-100 p-1.5 rounded-md w-80' %> + <%= f.submit 'Look up', class: 'border border-slate-200 bg-slate-50 px-3 py-1.5 rounded-md cursor-pointer' %> + <% end %> + <% if @query.present? && @user.nil? %> +

    No user found for "<%= @query %>".

    + <% end %> +
    + + <% if @user %> +
    +

    + <%= @user.name %> + · <%= @user.email %> + · ID <%= @user.id %> +

    + <% if @user.accounts.any? %> +

    + Accounts: + <% @user.accounts.each_with_index do |account, index| %> + <%= ', ' if index.positive? %> + <%= account.name %> (ID <%= account.id %>) + <% end %> +

    + <% end %> +
    + +
    +

    2. Select subscriptions (<%= @subscriptions.count %>)

    +

    + ⚠️ This sends a real push to every selected device. Use only when diagnosing a reported issue. +

    + + <% if @subscriptions.empty? %> +

    This user has no push subscriptions registered.

    + <% else %> + <%= form_with url: super_admin_push_diagnostics_path, method: :post, local: true do |f| %> + <%= f.hidden_field :user_id, value: @user.id %> +
    +
    + <%= label_tag :push_title, 'Title', class: 'block text-xs font-medium text-slate-600 mb-1' %> + <%= text_field_tag :push_title, + params[:push_title].presence || Notification::PushTestService.default_title, + class: 'border border-slate-100 p-1.5 rounded-md w-full text-sm' %> +
    +
    + <%= label_tag :push_body, 'Body', class: 'block text-xs font-medium text-slate-600 mb-1' %> + <%= text_area_tag :push_body, + params[:push_body].presence || Notification::PushTestService.default_body, + rows: 2, + class: 'border border-slate-100 p-1.5 rounded-md w-full text-sm' %> +
    +

    Customers will see this text as a real push notification on their device.

    +
    + + + + + + + + + + + + + + + <% @subscriptions.each do |sub| %> + <% attrs = (sub.subscription_attributes || {}).stringify_keys %> + <% if sub.browser_push? %> + <% endpoint = attrs['endpoint'].to_s %> + <% host = (begin; URI.parse(endpoint).host; rescue URI::InvalidURIError; nil; end) %> + <% device_display = host.presence || endpoint.presence || '—' %> + <% token_display = endpoint.present? ? "…#{endpoint.last(6)}" : '—' %> + <% else %> + <% device_display = attrs['device_id'].present? ? "…#{attrs['device_id'].to_s.last(6)}" : '—' %> + <% token_display = attrs['push_token'].present? ? "…#{attrs['push_token'].to_s.last(6)}" : '—' %> + <% end %> + <% extra_attrs = attrs.except('endpoint', 'p256dh', 'auth', 'push_token', 'device_id') %> + + + + + + + + + + + <% end %> + +
    IDTypeDevice / endpointPush tokenDevice detailsCreatedLast updated
    + <%= check_box_tag 'subscription_ids[]', sub.id, false, class: 'subscription-checkbox' %> + <%= sub.id %><%= sub.subscription_type %><%= device_display %><%= token_display %> + <% if extra_attrs.present? %> + <% extra_attrs.each do |k, v| %> +
    <%= k %>: <%= v.to_s.truncate(40) %>
    + <% end %> + <% else %> + + <% end %> +
    <%= sub.created_at.strftime('%Y-%m-%d %H:%M') %> + <%= sub.updated_at.strftime('%Y-%m-%d %H:%M') %> + (<%= time_ago_in_words(sub.updated_at) %> ago) +
    +
    + <%= f.submit 'Send Test Push to Selected', + class: 'border border-slate-200 bg-slate-50 px-3 py-1.5 rounded-md cursor-pointer font-medium' %> + <%= submit_tag 'Delete Selected Subscriptions', + formaction: destroy_subscriptions_super_admin_push_diagnostics_path, + formmethod: 'post', + data: { confirm: "Delete the selected subscription(s)? The user won't receive pushes on those devices until their app re-registers." }, + class: 'border border-red-200 bg-red-50 text-red-700 px-3 py-1.5 rounded-md cursor-pointer font-medium' %> +
    + <% end %> + <% end %> +
    + + <% if @results.present? %> +
    +

    3. Results

    + + + + + + + + + + + + + <% @results.each do |r| %> + + + + + + + + + <% end %> + +
    Sub IDTypeDevicePush tokenStatusDetails
    <%= r[:id] %><%= r[:type] %><%= r[:device] %><%= r[:token_tail] %> + <% + color = { + success: 'bg-green-100 text-green-800', + failure: 'bg-red-100 text-red-800', + skipped: 'bg-slate-100 text-slate-600' + }[r[:status]] + %> + <%= r[:status] %> + <%= r[:message] %>
    +
    + <% end %> + <% end %> +
    + +<% content_for :javascript do %> + +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 057f41b81..1841db332 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -496,3 +496,9 @@ en: subject: 'Finish setting up %{custom_domain}' ssl_status: custom_domain_not_configured: 'Custom domain is not configured' + super_admin: + push_diagnostics: + user_not_found: 'User not found.' + no_subscriptions_to_test: 'Select at least one subscription to test.' + no_subscriptions_to_delete: 'Select at least one subscription to delete.' + subscriptions_deleted: "Deleted %{count} subscription(s). The user's device(s) will re-register on next app launch." diff --git a/config/routes.rb b/config/routes.rb index 31453b157..2461539ad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -625,6 +625,9 @@ Rails.application.routes.draw do root to: 'dashboard#index' resource :app_config, only: [:show, :create] + resource :push_diagnostics, only: [:show, :create] do + post :destroy_subscriptions, on: :collection + end # order of resources affect the order of sidebar navigation in super admin resources :accounts, only: [:index, :new, :create, :show, :edit, :update, :destroy] do diff --git a/lib/chatwoot_hub.rb b/lib/chatwoot_hub.rb index be5a07e05..95679ed73 100644 --- a/lib/chatwoot_hub.rb +++ b/lib/chatwoot_hub.rb @@ -106,14 +106,18 @@ class ChatwootHub end def self.send_push(fcm_options) - info = { fcm_options: fcm_options } - RestClient.post(push_notification_url, info.merge(instance_config).to_json, { content_type: :json, accept: :json }) + send_push_with_response(fcm_options) rescue *ExceptionList::REST_CLIENT_EXCEPTIONS => e Rails.logger.error "Exception: #{e.message}" rescue StandardError => e ChatwootExceptionTracker.new(e).capture_exception end + def self.send_push_with_response(fcm_options) + info = { fcm_options: fcm_options } + RestClient.post(push_notification_url, info.merge(instance_config).to_json, { content_type: :json, accept: :json }) + end + def self.emit_event(event_name, event_data) return if ENV['DISABLE_TELEMETRY'] From ca66218cb9f94869138d1b23fb00e5bf8c079d40 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Tue, 21 Apr 2026 19:16:08 +0400 Subject: [PATCH 3/7] Revert: Validate Twilio webhook signatures (X-Twilio-Signature) (#14125) Reverts chatwoot/chatwoot#13638 --- .../twilio_signature_verify_concern.rb | 88 ----------- app/controllers/twilio/callback_controller.rb | 2 - .../twilio/delivery_status_controller.rb | 6 - .../twilio/callbacks_controller_spec.rb | 149 +----------------- .../twilio/delivery_status_controller_spec.rb | 123 +-------------- 5 files changed, 14 insertions(+), 354 deletions(-) delete mode 100644 app/controllers/concerns/twilio_signature_verify_concern.rb diff --git a/app/controllers/concerns/twilio_signature_verify_concern.rb b/app/controllers/concerns/twilio_signature_verify_concern.rb deleted file mode 100644 index b7a4754a4..000000000 --- a/app/controllers/concerns/twilio_signature_verify_concern.rb +++ /dev/null @@ -1,88 +0,0 @@ -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 diff --git a/app/controllers/twilio/callback_controller.rb b/app/controllers/twilio/callback_controller.rb index 9b42cd034..d607ba151 100644 --- a/app/controllers/twilio/callback_controller.rb +++ b/app/controllers/twilio/callback_controller.rb @@ -1,6 +1,4 @@ class Twilio::CallbackController < ApplicationController - include TwilioSignatureVerifyConcern - def create Webhooks::TwilioEventsJob.perform_later(permitted_params.to_unsafe_hash) diff --git a/app/controllers/twilio/delivery_status_controller.rb b/app/controllers/twilio/delivery_status_controller.rb index 8e846a737..1c756a1c2 100644 --- a/app/controllers/twilio/delivery_status_controller.rb +++ b/app/controllers/twilio/delivery_status_controller.rb @@ -1,6 +1,4 @@ class Twilio::DeliveryStatusController < ApplicationController - include TwilioSignatureVerifyConcern - def create Webhooks::TwilioDeliveryStatusJob.perform_later(permitted_params.to_unsafe_hash) @@ -20,8 +18,4 @@ class Twilio::DeliveryStatusController < ApplicationController :ErrorMessage ) end - - def channel_lookup_phone_numbers - [params[:From]].compact_blank - end end diff --git a/spec/controllers/twilio/callbacks_controller_spec.rb b/spec/controllers/twilio/callbacks_controller_spec.rb index 09b558096..d16acf229 100644 --- a/spec/controllers/twilio/callbacks_controller_spec.rb +++ b/spec/controllers/twilio/callbacks_controller_spec.rb @@ -4,160 +4,25 @@ 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' => twilio_channel.phone_number, + 'To' => '+0987654321', 'Body' => 'Test message', 'AccountSid' => 'AC123', 'SmsSid' => 'SM123' } end - 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 - - 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 + it 'enqueues the Twilio events job' do + expect do post twilio_callback_index_url, params: params - expect(response).to have_http_status(:forbidden) - end + end.to have_enqueued_job(Webhooks::TwilioEventsJob).with(params) 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 + it 'returns no content status' do + post twilio_callback_index_url, params: params + expect(response).to have_http_status(:no_content) end end end diff --git a/spec/controllers/twilio/delivery_status_controller_spec.rb b/spec/controllers/twilio/delivery_status_controller_spec.rb index dc99fdf23..fc21f8f94 100644 --- a/spec/controllers/twilio/delivery_status_controller_spec.rb +++ b/spec/controllers/twilio/delivery_status_controller_spec.rb @@ -4,132 +4,23 @@ 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', - 'From' => twilio_channel.phone_number + 'AccountSid' => 'AC123' } end - 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 - - 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 + it 'enqueues the Twilio delivery status job' do + expect do post twilio_delivery_status_index_url, params: params - expect(response).to have_http_status(:forbidden) - end + end.to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob).with(params) 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 + it 'returns no content status' do + post twilio_delivery_status_index_url, params: params + expect(response).to have_http_status(:no_content) end end end From f12118a3c07d8de30a710357b84f18dddacbed0b Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Wed, 22 Apr 2026 11:21:26 +0700 Subject: [PATCH 4/7] fix: void topup invoice when card payment fails (#14104) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a credit top-up's card charge fails, the finalized invoice was left in an open state with no hook back into our fulfillment service. If that invoice was later paid (manually via the hosted invoice page, or by a future dunning flow), the account would never receive its credits — silent revenue loss. This change voids the invoice the moment the charge fails, so a failed top-up cannot turn into a paid-but-unfulfilled invoice. ## Closes ## How to test 1. On a Business-plan account with a Stripe customer, attach a test card that will decline on charge (e.g. `4000 0000 0000 0002`). 2. From the dashboard, open the credit top-up flow and purchase a credit pack. 3. Observe the API returns an error and the account's captain credits are unchanged. 4. In the Stripe dashboard, confirm the corresponding invoice is in `void` status (not `open`). 5. Repeat with a good card (`4242 4242 4242 4242`) and confirm the happy path still fulfills credits. ## What changed - `finalize_and_pay` in `Enterprise::Billing::TopupCheckoutService` now rescues `Stripe::CardError`, voids the open invoice via `Stripe::Invoice.void_invoice`, and re-raises so the controller surfaces the original decline error to the client. - Rescue is intentionally narrow to `Stripe::CardError` (declines, insufficient funds, SCA `authentication_required`). Transient errors like `APIConnectionError` / `RateLimitError` are left to propagate — the charge may have actually succeeded and voiding could be wrong. - Invoices are created with `auto_advance: false`, so Stripe's Smart Retries won't collect on an open invoice; voiding is the correct terminal state. Co-authored-by: Claude Opus 4.7 (1M context) --- .../services/enterprise/billing/topup_checkout_service.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/enterprise/app/services/enterprise/billing/topup_checkout_service.rb b/enterprise/app/services/enterprise/billing/topup_checkout_service.rb index d0ec3e372..00e2b1646 100644 --- a/enterprise/app/services/enterprise/billing/topup_checkout_service.rb +++ b/enterprise/app/services/enterprise/billing/topup_checkout_service.rb @@ -79,7 +79,12 @@ class Enterprise::Billing::TopupCheckoutService def finalize_and_pay(invoice_id) Stripe::Invoice.finalize_invoice(invoice_id, { auto_advance: false }) invoice = Stripe::Invoice.retrieve(invoice_id) - Stripe::Invoice.pay(invoice_id) unless invoice.status == 'paid' + return if invoice.status == 'paid' + + Stripe::Invoice.pay(invoice_id) + rescue Stripe::CardError + Stripe::Invoice.void_invoice(invoice_id) + raise end def fulfill_credits(credits, topup_option) From 475db8531817ab23e56a8d1e38cad38d60f15fee Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Wed, 22 Apr 2026 17:13:26 +0530 Subject: [PATCH 5/7] fix: prevent template picker dropdown cut-off in compose form (#14129) --- .../NewConversation/ComposeConversation.vue | 1 + .../components/ActionButtons.vue | 2 +- .../components/ContentTemplateForm.vue | 4 +- .../components/ContentTemplateSelector.vue | 120 ++++++++++-------- .../components/WhatsAppOptions.vue | 111 ++++++++-------- .../components/WhatsappTemplate.vue | 4 +- .../components-next/popover/Popover.vue | 28 +++- .../modules/contact/ContactDeleteModal.vue | 4 +- .../modules/contact/ContactMergeModal.vue | 4 +- 9 files changed, 154 insertions(+), 124 deletions(-) diff --git a/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue b/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue index 1ab370901..02a00c703 100644 --- a/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue +++ b/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue @@ -233,6 +233,7 @@ onMounted(() => resetContacts()); diff --git a/app/javascript/dashboard/components-next/NewConversation/components/ActionButtons.vue b/app/javascript/dashboard/components-next/NewConversation/components/ActionButtons.vue index aec05a717..e8f484997 100644 --- a/app/javascript/dashboard/components-next/NewConversation/components/ActionButtons.vue +++ b/app/javascript/dashboard/components-next/NewConversation/components/ActionButtons.vue @@ -20,6 +20,7 @@ const props = defineProps({ isEmailOrWebWidgetInbox: { type: Boolean, default: false }, isTwilioSmsInbox: { type: Boolean, default: false }, isTwilioWhatsAppInbox: { type: Boolean, default: false }, + // eslint-disable-next-line vue/no-unused-properties messageTemplates: { type: Array, default: () => [] }, channelType: { type: String, default: '' }, isLoading: { type: Boolean, default: false }, @@ -198,7 +199,6 @@ useEventListener(document, 'paste', onPaste); {