From 0c6ee492160faa94c10266e835e10970fbc9b24e Mon Sep 17 00:00:00 2001 From: Sony Mathew <2040199+sony-mathew@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:48:33 +0530 Subject: [PATCH] fix: enforce email limits for agent invitations Atomically reserve Chatwoot Cloud account email capacity before sending new agent invitations, including bulk creation, and roll back with HTTP 429 when the limit is exhausted. --- app/builders/agent_builder.rb | 17 ++++++++-- .../concerns/request_exception_handler.rb | 4 ++- .../concerns/account_email_rate_limitable.rb | 33 +++++++++++++++++- config/locales/en.yml | 1 + lib/custom_exceptions/account.rb | 14 ++++++++ spec/builders/agent_builder_spec.rb | 31 +++++++++++++++++ .../api/v1/accounts/agents_controller_spec.rb | 32 +++++++++++++++++ .../account_email_rate_limitable_spec.rb | 34 +++++++++++++++++++ 8 files changed, 162 insertions(+), 4 deletions(-) diff --git a/app/builders/agent_builder.rb b/app/builders/agent_builder.rb index d2715011c..5a01c4834 100644 --- a/app/builders/agent_builder.rb +++ b/app/builders/agent_builder.rb @@ -17,7 +17,9 @@ class AgentBuilder ActiveRecord::Base.transaction do @user = find_or_create_user create_account_user + reserve_invitation_email_capacity if user_needs_confirmation? end + @user.send_confirmation_instructions if user_needs_confirmation? @user end @@ -26,18 +28,29 @@ class AgentBuilder # Finds a user by email or creates a new one with a temporary password. # @return [User] the found or created user. def find_or_create_user + @new_user = false user = User.from_email(email) return user if user @name = email.split('@').first if @name.blank? temp_password = "1!aA#{SecureRandom.alphanumeric(12)}" - User.create!(email: email, name: @name, password: temp_password, password_confirmation: temp_password) + User.new(email: email, name: @name, password: temp_password, password_confirmation: temp_password).tap do |new_user| + new_user.skip_confirmation_notification! + new_user.save! + @new_user = true + end end # Checks if the user needs confirmation. # @return [Boolean] true if the user is persisted and not confirmed, false otherwise. def user_needs_confirmation? - @user.persisted? && !@user.confirmed? + @new_user && @user.persisted? && !@user.confirmed? + end + + def reserve_invitation_email_capacity + return if account.reserve_email_send_capacity + + raise CustomExceptions::Account::EmailLimitExceeded.new({}) end # Creates an account user linking the user to the current account. diff --git a/app/controllers/concerns/request_exception_handler.rb b/app/controllers/concerns/request_exception_handler.rb index 43d6edf1f..b132f7eeb 100644 --- a/app/controllers/concerns/request_exception_handler.rb +++ b/app/controllers/concerns/request_exception_handler.rb @@ -9,7 +9,9 @@ module RequestExceptionHandler included do rescue_from ActiveRecord::RecordInvalid, with: :render_record_invalid - rescue_from CustomExceptions::Inbox::LimitExceeded, with: :render_error_response + rescue_from CustomExceptions::Inbox::LimitExceeded, + CustomExceptions::Account::EmailLimitExceeded, + with: :render_error_response end private diff --git a/app/models/concerns/account_email_rate_limitable.rb b/app/models/concerns/account_email_rate_limitable.rb index 5f69e22ee..a07a70771 100644 --- a/app/models/concerns/account_email_rate_limitable.rb +++ b/app/models/concerns/account_email_rate_limitable.rb @@ -20,7 +20,7 @@ module AccountEmailRateLimitable return true unless ChatwootApp.chatwoot_cloud? return true if emails_sent_today < email_rate_limit - Rails.logger.warn("Account #{id} reached daily email rate limit of #{email_rate_limit}. Sent: #{emails_sent_today}") + log_email_limit_reached false end @@ -30,8 +30,39 @@ module AccountEmailRateLimitable end end + def reserve_email_send_capacity(count = 1) + return true unless ChatwootApp.chatwoot_cloud? + + loop do + reservation = attempt_email_capacity_reservation(count) + if reservation == :limit_exceeded + log_email_limit_reached + return false + end + return true if reservation.present? + end + end + private + def attempt_email_capacity_reservation(count) + Redis::Alfred.with do |redis| + redis.watch(email_count_cache_key) do + current_count = redis.get(email_count_cache_key).to_i + next :limit_exceeded if current_count + count > email_rate_limit + + redis.multi do |transaction| + transaction.incrby(email_count_cache_key, count) + transaction.expire(email_count_cache_key, OUTBOUND_EMAIL_TTL) if current_count.zero? + end + end + end + end + + def log_email_limit_reached + Rails.logger.warn("Account #{id} reached daily email rate limit of #{email_rate_limit}. Sent: #{emails_sent_today}") + end + def email_count_cache_key @email_count_cache_key ||= format( Redis::Alfred::ACCOUNT_OUTBOUND_EMAIL_COUNT_KEY, diff --git a/config/locales/en.yml b/config/locales/en.yml index 735d52205..1d6d2a666 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -76,6 +76,7 @@ en: errors: account: not_authorized: You are not authorized to access this account + email_limit_exceeded: The daily email limit for this account has been reached reporting_timezone: invalid: is not a valid timezone support_email: diff --git a/lib/custom_exceptions/account.rb b/lib/custom_exceptions/account.rb index 08c6f0ddb..92d49b282 100644 --- a/lib/custom_exceptions/account.rb +++ b/lib/custom_exceptions/account.rb @@ -42,4 +42,18 @@ module CustomExceptions::Account I18n.t 'errors.plan_upgrade_required.failed' end end + + class EmailLimitExceeded < CustomExceptions::Base + def message + I18n.t('errors.account.email_limit_exceeded') + end + + def to_hash + { error: message } + end + + def http_status + :too_many_requests + end + end end diff --git a/spec/builders/agent_builder_spec.rb b/spec/builders/agent_builder_spec.rb index f140f2f29..6db6ec146 100644 --- a/spec/builders/agent_builder_spec.rb +++ b/spec/builders/agent_builder_spec.rb @@ -24,6 +24,8 @@ RSpec.describe AgentBuilder, type: :model do describe '#perform' do context 'when user does not exist' do + before { clear_enqueued_jobs } + it 'creates a new user' do expect { agent_builder.perform }.to change(User, :count).by(1) end @@ -35,6 +37,28 @@ RSpec.describe AgentBuilder, type: :model do it 'returns a user' do expect(agent_builder.perform).to be_a(User) end + + it 'reserves email capacity and enqueues the invitation' do + allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) + + expect { agent_builder.perform }.to have_enqueued_mail(Devise::Mailer, :confirmation_instructions) + expect(account.emails_sent_today).to eq(1) + end + + context 'when the account email limit is exhausted' do + before do + allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) + account.update!(limits: { 'emails' => 0 }) + end + + it 'does not create the user or enqueue an invitation' do + expect { agent_builder.perform }.to raise_error(CustomExceptions::Account::EmailLimitExceeded) + expect(User.from_email(email)).to be_nil + expect(AccountUser.find_by(account: account, user: User.from_email(email))).to be_nil + mail_jobs = enqueued_jobs.select { |job| job[:job].to_s == 'ActionMailer::MailDeliveryJob' } + expect(mail_jobs).to be_empty + end + end end context 'when user exists' do @@ -49,6 +73,13 @@ RSpec.describe AgentBuilder, type: :model do it 'creates a new account user' do expect { agent_builder.perform }.to change(AccountUser, :count).by(1) end + + it 'does not consume email capacity or enqueue another invitation' do + clear_enqueued_jobs + + expect { agent_builder.perform }.not_to have_enqueued_mail(Devise::Mailer, :confirmation_instructions) + expect(account.emails_sent_today).to eq(0) + end end context 'when only email is provided' do diff --git a/spec/controllers/api/v1/accounts/agents_controller_spec.rb b/spec/controllers/api/v1/accounts/agents_controller_spec.rb index 46b38677a..77ca5bf47 100644 --- a/spec/controllers/api/v1/accounts/agents_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/agents_controller_spec.rb @@ -177,6 +177,22 @@ RSpec.describe 'Agents API', type: :request do expect(response.parsed_body['email']).to eq(params[:email]) expect(account.users.last.name).to eq('NewUser') end + + context 'when the account email limit is exhausted' do + before do + allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) + account.update!(limits: { 'emails' => 0 }) + end + + it 'does not create an agent' do + expect do + post "/api/v1/accounts/#{account.id}/agents", params: params, headers: admin.create_new_auth_token, as: :json + end.not_to change(User, :count) + + expect(response).to have_http_status(:too_many_requests) + expect(response.parsed_body['error']).to eq('The daily email limit for this account has been reached') + end + end end end @@ -211,6 +227,22 @@ RSpec.describe 'Agents API', type: :request do expect(response).to have_http_status(:ok) end + + context 'when the account email limit is exhausted' do + before do + allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) + account.update!(limits: { 'emails' => 0 }) + end + + it 'does not create agents' do + expect do + post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: bulk_create_params, headers: admin.create_new_auth_token + end.not_to change(User, :count) + + expect(response).to have_http_status(:too_many_requests) + expect(response.parsed_body['error']).to eq('The daily email limit for this account has been reached') + end + end end end end diff --git a/spec/models/concerns/account_email_rate_limitable_spec.rb b/spec/models/concerns/account_email_rate_limitable_spec.rb index fb9a86144..2685d707b 100644 --- a/spec/models/concerns/account_email_rate_limitable_spec.rb +++ b/spec/models/concerns/account_email_rate_limitable_spec.rb @@ -83,4 +83,38 @@ RSpec.describe AccountEmailRateLimitable do expect(Redis::Alfred).not_to have_received(:expire) end end + + describe '#reserve_email_send_capacity' do + context 'when chatwoot cloud' do + before do + allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) + account.update!(limits: { 'emails' => 2 }) + end + + it 'atomically reserves capacity without exceeding the limit' do + expect(account.reserve_email_send_capacity).to be true + expect(account.reserve_email_send_capacity).to be true + expect(account.reserve_email_send_capacity).to be false + expect(account.emails_sent_today).to eq(2) + end + + it 'does not partially reserve a batch that exceeds the remaining capacity' do + expect(account.reserve_email_send_capacity(2)).to be true + expect(account.reserve_email_send_capacity(2)).to be false + expect(account.emails_sent_today).to eq(2) + end + end + + context 'when self-hosted' do + before do + allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(false) + account.update!(limits: { 'emails' => 1 }) + end + + it 'does not reserve or track email capacity' do + expect(account.reserve_email_send_capacity(2)).to be true + expect(account.emails_sent_today).to eq(0) + end + end + end end