fix: enforce inbox limits at model level (#14949)
Fixes https://linear.app/chatwoot/issue/CW-7559/inbox-limit-abuse ## Why The regular inbox API checked limits in the controller, but WhatsApp embedded signup creates inboxes through a service using `Inbox.create!`. That let Enterprise account inbox limits be skipped for embedded signup. ## What this change does - Adds an Inbox create-time validation hook in OSS and implements the limit check in the Enterprise Inbox module. - Removes the duplicate controller/helper limit check so the model is the single enforcement point. - Preserves the existing `402 Payment Required` API response for account inbox limit failures. - Keeps updates to existing inboxes allowed when an account is already at its inbox limit. ## Validation - `bundle exec rspec spec/controllers/api/v1/accounts/inboxes_controller_spec.rb spec/enterprise/models/inbox_spec.rb`
This commit is contained in:
@@ -6,6 +6,7 @@ class Api::V1::Accounts::CallbacksController < Api::V1::Accounts::BaseController
|
||||
page_access_token = params[:page_access_token]
|
||||
page_id = params[:page_id]
|
||||
inbox_name = params[:inbox_name]
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
facebook_channel = Current.account.facebook_pages.create!(
|
||||
page_id: page_id, user_access_token: user_access_token,
|
||||
@@ -15,6 +16,8 @@ class Api::V1::Accounts::CallbacksController < Api::V1::Accounts::BaseController
|
||||
set_instagram_id(page_access_token, facebook_channel)
|
||||
set_avatar(@facebook_inbox, page_id)
|
||||
end
|
||||
rescue CustomExceptions::Inbox::LimitExceeded => e
|
||||
render_error_response(e)
|
||||
rescue StandardError => e
|
||||
ChatwootExceptionTracker.new(e).capture_exception
|
||||
Rails.logger.error "Error in register_facebook_page: #{e.message}"
|
||||
|
||||
@@ -6,6 +6,8 @@ class Api::V1::Accounts::Channels::TwilioChannelsController < Api::V1::Accounts:
|
||||
|
||||
def create
|
||||
process_create
|
||||
rescue CustomExceptions::Inbox::LimitExceeded => e
|
||||
render_error_response(e)
|
||||
rescue StandardError => e
|
||||
render_could_not_create_error(e.message)
|
||||
end
|
||||
|
||||
@@ -2,7 +2,6 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController
|
||||
include Api::V1::InboxesHelper
|
||||
before_action :fetch_inbox, except: [:index, :create]
|
||||
before_action :fetch_agent_bot, only: [:set_agent_bot]
|
||||
before_action :validate_limit, only: [:create]
|
||||
# we are already handling the authorization in fetch inbox
|
||||
before_action :check_authorization, except: [:show]
|
||||
|
||||
|
||||
@@ -8,8 +8,10 @@ class Api::V1::Accounts::Whatsapp::AuthorizationsController < Api::V1::Accounts:
|
||||
validate_embedded_signup_params!
|
||||
channel = process_embedded_signup
|
||||
render_success_response(channel.inbox)
|
||||
rescue StandardError => e
|
||||
rescue CustomExceptions::Inbox::LimitExceeded => e
|
||||
render_error_response(e)
|
||||
rescue StandardError => e
|
||||
render_embedded_signup_error(e)
|
||||
end
|
||||
|
||||
private
|
||||
@@ -55,7 +57,7 @@ class Api::V1::Accounts::Whatsapp::AuthorizationsController < Api::V1::Accounts:
|
||||
render json: response
|
||||
end
|
||||
|
||||
def render_error_response(error)
|
||||
def render_embedded_signup_error(error)
|
||||
Rails.logger.error "[WHATSAPP AUTHORIZATION] Embedded signup error: #{error.message}"
|
||||
Rails.logger.error error.backtrace.join("\n")
|
||||
render json: {
|
||||
|
||||
@@ -9,6 +9,7 @@ module RequestExceptionHandler
|
||||
|
||||
included do
|
||||
rescue_from ActiveRecord::RecordInvalid, with: :render_record_invalid
|
||||
rescue_from CustomExceptions::Inbox::LimitExceeded, with: :render_error_response
|
||||
end
|
||||
|
||||
private
|
||||
@@ -40,8 +41,8 @@ module RequestExceptionHandler
|
||||
render json: { error: message }, status: :not_found
|
||||
end
|
||||
|
||||
def render_could_not_create_error(message)
|
||||
render json: { error: sanitized_error_message(message) }, status: :unprocessable_entity
|
||||
def render_could_not_create_error(error)
|
||||
render json: { error: sanitized_error_message(error) }, status: :unprocessable_entity
|
||||
end
|
||||
|
||||
def render_payment_required(message)
|
||||
|
||||
@@ -11,6 +11,8 @@ class Instagram::CallbacksController < ApplicationController
|
||||
end
|
||||
|
||||
process_successful_authorization
|
||||
rescue CustomExceptions::Inbox::LimitExceeded => e
|
||||
handle_limit_error(e)
|
||||
rescue StandardError => e
|
||||
handle_error(e)
|
||||
end
|
||||
@@ -47,6 +49,14 @@ class Instagram::CallbacksController < ApplicationController
|
||||
redirect_to_error_page(error_info)
|
||||
end
|
||||
|
||||
def handle_limit_error(error)
|
||||
redirect_to_error_page(
|
||||
'error_type' => error.class.name,
|
||||
'code' => Rack::Utils.status_code(error.http_status),
|
||||
'error_message' => error.message
|
||||
)
|
||||
end
|
||||
|
||||
# Extract error details from the exception
|
||||
def extract_error_info(error)
|
||||
if error.is_a?(OAuth2::Error)
|
||||
|
||||
@@ -6,6 +6,8 @@ class Tiktok::CallbacksController < ApplicationController
|
||||
return handle_ungranted_scopes_error unless all_scopes_granted?
|
||||
|
||||
process_successful_authorization
|
||||
rescue CustomExceptions::Inbox::LimitExceeded => e
|
||||
handle_limit_error(e)
|
||||
rescue StandardError => e
|
||||
handle_error(e)
|
||||
end
|
||||
@@ -36,6 +38,14 @@ class Tiktok::CallbacksController < ApplicationController
|
||||
redirect_to_error_page(error_type: error.class.name, code: 500, error_message: error.message)
|
||||
end
|
||||
|
||||
def handle_limit_error(error)
|
||||
redirect_to_error_page(
|
||||
error_type: error.class.name,
|
||||
code: Rack::Utils.status_code(error.http_status),
|
||||
error_message: error.message
|
||||
)
|
||||
end
|
||||
|
||||
# Handles the case when a user denies permissions or cancels the authorization flow
|
||||
def handle_authorization_error
|
||||
redirect_to_error_page(
|
||||
|
||||
@@ -114,10 +114,4 @@ module Api::V1::InboxesHelper
|
||||
'sms' => Current.account.sms_channels
|
||||
}[permitted_params[:channel][:type]]
|
||||
end
|
||||
|
||||
def validate_limit
|
||||
return unless Current.account.inboxes.count >= Current.account.usage_limits[:inboxes]
|
||||
|
||||
render_payment_required('Account limit exceeded. Upgrade to a higher plan')
|
||||
end
|
||||
end
|
||||
|
||||
@@ -8,5 +8,11 @@ module Enterprise::Concerns::Inbox
|
||||
class_name: 'Captain::Assistant'
|
||||
has_many :inbox_capacity_limits, dependent: :destroy
|
||||
has_many :calls, dependent: :destroy_async
|
||||
|
||||
before_create :ensure_create_permitted
|
||||
end
|
||||
|
||||
def ensure_create_permitted
|
||||
raise CustomExceptions::Inbox::LimitExceeded.new({}) if account.inboxes.count >= account.usage_limits[:inboxes]
|
||||
end
|
||||
end
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
class CustomExceptions::Inbox::LimitExceeded < CustomExceptions::Base
|
||||
def message
|
||||
'Account limit exceeded. Upgrade to a higher plan'
|
||||
end
|
||||
|
||||
def to_hash
|
||||
{ error: message }
|
||||
end
|
||||
|
||||
def http_status
|
||||
:payment_required
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,34 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe 'Enterprise Callbacks API', type: :request do
|
||||
describe 'POST /api/v1/accounts/{account.id}/callbacks/register_facebook_page' do
|
||||
let(:account) { create(:account, limits: { inboxes: 1 }) }
|
||||
let(:admin) { create(:user, account: account, role: :administrator) }
|
||||
let(:params) do
|
||||
{
|
||||
user_access_token: 'user-token',
|
||||
page_access_token: 'page-token',
|
||||
page_id: '12345',
|
||||
inbox_name: 'Facebook Inbox'
|
||||
}
|
||||
end
|
||||
|
||||
before do
|
||||
create(:inbox, account: account)
|
||||
end
|
||||
|
||||
it 'returns payment required before creating a Facebook channel when account inbox limit is reached' do
|
||||
expect do
|
||||
post "/api/v1/accounts/#{account.id}/callbacks/register_facebook_page",
|
||||
headers: admin.create_new_auth_token,
|
||||
params: params,
|
||||
as: :json
|
||||
end.not_to change(Channel::FacebookPage, :count)
|
||||
|
||||
expect(response).to have_http_status(:payment_required)
|
||||
expect(response.parsed_body['error']).to eq('Account limit exceeded. Upgrade to a higher plan')
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -5,6 +5,13 @@ RSpec.describe Captain::BaseTaskService, type: :model do
|
||||
let(:inbox) { create(:inbox, account: account) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
let(:perform_result) { { message: 'Test response' } }
|
||||
let(:exhausted_usage_limits) do
|
||||
{
|
||||
agents: ChatwootApp.max_limit,
|
||||
inboxes: ChatwootApp.max_limit,
|
||||
captain: { responses: { current_available: 0 } }
|
||||
}
|
||||
end
|
||||
|
||||
# Create a concrete test service class with enterprise module prepended
|
||||
let(:test_service_class) do
|
||||
@@ -38,9 +45,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do
|
||||
context 'when usage limit is exceeded' do
|
||||
before do
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true)
|
||||
allow(account).to receive(:usage_limits).and_return({
|
||||
captain: { responses: { current_available: 0 } }
|
||||
})
|
||||
allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits)
|
||||
end
|
||||
|
||||
it 'returns usage limit exceeded error' do
|
||||
@@ -125,9 +130,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do
|
||||
context 'when the captain_responses quota is exhausted on Cloud' do
|
||||
before do
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true)
|
||||
allow(account).to receive(:usage_limits).and_return({
|
||||
captain: { responses: { current_available: 0 } }
|
||||
})
|
||||
allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits)
|
||||
end
|
||||
|
||||
it 'returns usage limit exceeded error for services that do not opt into BYOK' do
|
||||
@@ -162,9 +165,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do
|
||||
context 'when the captain_responses quota is exhausted on Cloud' do
|
||||
before do
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true)
|
||||
allow(account).to receive(:usage_limits).and_return({
|
||||
captain: { responses: { current_available: 0 } }
|
||||
})
|
||||
allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits)
|
||||
end
|
||||
|
||||
it 'bypasses the 429 gate and returns the underlying result' do
|
||||
@@ -249,9 +250,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do
|
||||
context 'when the captain_responses quota is exhausted on Cloud' do
|
||||
before do
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true)
|
||||
allow(account).to receive(:usage_limits).and_return({
|
||||
captain: { responses: { current_available: 0 } }
|
||||
})
|
||||
allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits)
|
||||
end
|
||||
|
||||
it 'bypasses the 429 gate and returns the underlying result' do
|
||||
|
||||
@@ -166,9 +166,13 @@ RSpec.describe Captain::ConversationCompletionService do
|
||||
|
||||
before do
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true)
|
||||
allow(account).to receive(:usage_limits).and_return({
|
||||
captain: { responses: { current_available: 0 } }
|
||||
})
|
||||
allow(account).to receive(:usage_limits).and_return(
|
||||
{
|
||||
agents: ChatwootApp.max_limit,
|
||||
inboxes: ChatwootApp.max_limit,
|
||||
captain: { responses: { current_available: 0 } }
|
||||
}
|
||||
)
|
||||
create(:message, conversation: conversation, message_type: :incoming, content: 'What are your hours?')
|
||||
create(:message, conversation: conversation, message_type: :outgoing, content: 'We are open 9-5 Monday to Friday.')
|
||||
allow(mock_chat).to receive(:ask).and_return(mock_response)
|
||||
|
||||
@@ -134,6 +134,29 @@ RSpec.describe Inbox do
|
||||
end
|
||||
end
|
||||
|
||||
describe 'validations' do
|
||||
describe 'account inbox limit' do
|
||||
let(:account) { create(:account, limits: { inboxes: 1 }) }
|
||||
|
||||
before do
|
||||
create(:inbox, account: account)
|
||||
end
|
||||
|
||||
it 'prevents saving inboxes beyond the account limit' do
|
||||
new_inbox = build(:inbox, account: account)
|
||||
|
||||
expect { new_inbox.save! }.to raise_error(CustomExceptions::Inbox::LimitExceeded, 'Account limit exceeded. Upgrade to a higher plan')
|
||||
end
|
||||
|
||||
it 'does not block updates to existing inboxes when the account is at the limit' do
|
||||
inbox = account.inboxes.first
|
||||
inbox.name = 'Updated Inbox'
|
||||
|
||||
expect(inbox).to be_valid
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'audit log' do
|
||||
context 'when inbox is created' do
|
||||
it 'has associated audit log created' do
|
||||
|
||||
@@ -12,7 +12,13 @@ RSpec.describe Messages::AudioTranscriptionService, type: :service do
|
||||
InstallationConfig.find_or_create_by!(name: 'CAPTAIN_OPEN_AI_MODEL') { |config| config.value = 'gpt-4o-mini' }
|
||||
|
||||
# Mock usage limits for transcription to be available
|
||||
allow(account).to receive(:usage_limits).and_return({ captain: { responses: { current_available: 100 } } })
|
||||
allow(account).to receive(:usage_limits).and_return(
|
||||
{
|
||||
agents: ChatwootApp.max_limit,
|
||||
inboxes: ChatwootApp.max_limit,
|
||||
captain: { responses: { current_available: 100 } }
|
||||
}
|
||||
)
|
||||
end
|
||||
|
||||
describe '#perform' do
|
||||
|
||||
@@ -385,7 +385,10 @@ RSpec.describe Captain::BaseTaskService do
|
||||
|
||||
describe '#prompt_from_file' do
|
||||
it 'reads prompt from file' do
|
||||
allow(Rails.root).to receive(:join).and_return(instance_double(Pathname, read: 'Test prompt content'))
|
||||
service
|
||||
prompt_path = instance_double(Pathname, read: 'Test prompt content')
|
||||
allow(Rails.root).to receive(:join).with('lib/integrations/openai/openai_prompts', 'test.liquid').and_return(prompt_path)
|
||||
|
||||
expect(service.send(:prompt_from_file, 'test')).to eq('Test prompt content')
|
||||
end
|
||||
end
|
||||
|
||||
@@ -32,6 +32,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do
|
||||
|
||||
before do
|
||||
account.enable_features('crm_integration')
|
||||
allow(GlobalConfig).to receive(:get).and_return({})
|
||||
allow(GlobalConfig).to receive(:get).with('BRAND_NAME').and_return({ 'BRAND_NAME' => 'TestBrand' })
|
||||
end
|
||||
|
||||
|
||||
@@ -60,6 +60,17 @@ describe Whatsapp::ChannelCreationService do
|
||||
expect(inbox.name).to eq('Test Business WhatsApp')
|
||||
expect(inbox.account).to eq(account)
|
||||
end
|
||||
|
||||
it 'does not leave an orphan channel when inbox creation fails' do
|
||||
allow(Inbox).to receive(:create!).and_wrap_original do |method, *args|
|
||||
method.call(*args)
|
||||
raise ActiveRecord::RecordInvalid, Inbox.new
|
||||
end
|
||||
|
||||
expect do
|
||||
expect { service.perform }.to raise_error(ActiveRecord::RecordInvalid)
|
||||
end.not_to change(Channel::Whatsapp, :count)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when channel already exists for the phone number' do
|
||||
|
||||
Reference in New Issue
Block a user