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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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,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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user