fix: enforce email limits for agent invitations (#15082)
# Pull Request Template ## Description Prevents Chatwoot Cloud accounts from exceeding their daily non-channel email allowance through agent invitations. New-user invitations atomically reserve email capacity before mail is queued; when the budget is exhausted, agent creation rolls back and returns HTTP 429. This covers single and bulk agent creation. Self-hosted installations remain unaffected, and adding an existing user does not consume capacity when no invitation is sent. Related to [CW-7637](https://linear.app/chatwoot/issue/CW-7637/prevent-agent-invitation-email-abuse-after-july-20-incident). ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? Verified single and bulk creation at an exhausted budget, successful invitation enqueueing below the limit, no capacity usage for existing users, and no enforcement on self-hosted installations. A concurrent Redis probe admitted exactly five of twenty simultaneous reservations against a limit of five. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
This commit is contained in:
@@ -25,11 +25,13 @@ class AgentBuilder
|
||||
account.with_lock do
|
||||
raise LimitExceededError unless can_add_agent?
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
ActiveRecord::Base.transaction(requires_new: true) do
|
||||
@user = find_or_create_user
|
||||
create_account_user
|
||||
reserve_invitation_email_capacity if user_needs_confirmation?
|
||||
end
|
||||
end
|
||||
@user.send_confirmation_instructions if user_needs_confirmation?
|
||||
@user
|
||||
end
|
||||
|
||||
@@ -42,18 +44,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.
|
||||
|
||||
@@ -76,11 +76,19 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController
|
||||
end
|
||||
|
||||
def bulk_create_agents(emails)
|
||||
email_limit_error = nil
|
||||
|
||||
Current.account.with_lock do
|
||||
raise AgentBuilder::LimitExceededError if emails.count > available_agent_count
|
||||
|
||||
emails.each { |email| create_agent_from_email(email) }
|
||||
emails.each do |email|
|
||||
create_agent_from_email(email)
|
||||
rescue CustomExceptions::Account::EmailLimitExceeded => e
|
||||
email_limit_error = e
|
||||
end
|
||||
end
|
||||
|
||||
raise email_limit_error if email_limit_error
|
||||
end
|
||||
|
||||
def create_agent_from_email(email)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,42 @@ 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
|
||||
if current_count + count > email_rate_limit
|
||||
redis.unwatch
|
||||
next :limit_exceeded
|
||||
end
|
||||
|
||||
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,
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -30,6 +30,8 @@ RSpec.describe AgentBuilder, type: :model do
|
||||
end
|
||||
|
||||
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
|
||||
@@ -41,6 +43,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
|
||||
@@ -55,6 +79,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
|
||||
|
||||
@@ -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,71 @@ 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 new 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
|
||||
|
||||
it 'adds existing users and continues processing after a rejected invitation' do
|
||||
first_existing_user = create(:user, email: 'first-existing@example.com')
|
||||
second_existing_user = create(:user, email: 'second-existing@example.com')
|
||||
params = {
|
||||
emails: [
|
||||
first_existing_user.email,
|
||||
'rejected-invitation@example.com',
|
||||
second_existing_user.email
|
||||
]
|
||||
}
|
||||
|
||||
expect do
|
||||
post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: params, headers: admin.create_new_auth_token
|
||||
end.not_to change(User, :count)
|
||||
|
||||
expect(response).to have_http_status(:too_many_requests)
|
||||
expect(account.reload.users).to include(first_existing_user, second_existing_user)
|
||||
expect(User.from_email('rejected-invitation@example.com')).to be_nil
|
||||
expect(account.emails_sent_today).to eq(0)
|
||||
end
|
||||
|
||||
it 'keeps the onboarding step when an invitation is rejected' do
|
||||
account.update!(custom_attributes: { onboarding_step: 'completed' })
|
||||
|
||||
post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: bulk_create_params, headers: admin.create_new_auth_token
|
||||
|
||||
expect(response).to have_http_status(:too_many_requests)
|
||||
expect(account.reload.custom_attributes['onboarding_step']).to eq('completed')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the account has capacity for only part of the batch' do
|
||||
before do
|
||||
allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true)
|
||||
account.update!(limits: { 'emails' => 1 })
|
||||
end
|
||||
|
||||
it 'persists the successful invitation without leaking email capacity' do
|
||||
expect do
|
||||
post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: bulk_create_params, headers: admin.create_new_auth_token
|
||||
end.to change(User, :count).by(1)
|
||||
|
||||
expect(response).to have_http_status(:too_many_requests)
|
||||
expect(User.from_email(emails.first)).to be_present
|
||||
expect(User.from_email(emails.second)).to be_nil
|
||||
expect(User.from_email(emails.third)).to be_nil
|
||||
expect(account.emails_sent_today).to eq(1)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -83,4 +83,51 @@ 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
|
||||
|
||||
it 'unwatches the counter when capacity is exhausted' do
|
||||
redis = instance_double(Redis)
|
||||
key = format(Redis::Alfred::ACCOUNT_OUTBOUND_EMAIL_COUNT_KEY, account_id: account.id, date: Time.zone.today.to_s)
|
||||
allow(Redis::Alfred).to receive(:with).and_yield(redis)
|
||||
allow(account).to receive(:emails_sent_today).and_return(2)
|
||||
allow(redis).to receive(:watch).with(key).and_yield
|
||||
allow(redis).to receive(:get).with(key).and_return('2')
|
||||
expect(redis).to receive(:unwatch)
|
||||
expect(redis).not_to receive(:multi)
|
||||
|
||||
expect(account.reserve_email_send_capacity).to be false
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user