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