From 3b7a2c2b0ecd6135be74fa46189ec48263b2cac0 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Wed, 22 Jul 2026 17:21:33 +0530 Subject: [PATCH] fix(security): restrict participating scope, agent seat race, and premium feature access --- .../api/v1/accounts/agents_controller.rb | 58 ++++++++++++------- app/finders/conversation_finder.rb | 3 +- .../captain/assistant_responses_controller.rb | 2 +- .../accounts/captain/assistants_controller.rb | 2 +- .../v1/accounts/captain/base_controller.rb | 13 +++++ .../captain/bulk_actions_controller.rb | 2 +- .../accounts/captain/documents_controller.rb | 2 +- .../v1/accounts/captain/inboxes_controller.rb | 2 +- .../accounts/captain/scenarios_controller.rb | 2 +- .../v1/accounts/custom_roles_controller.rb | 5 ++ .../v1/accounts/sla_policies_controller.rb | 5 ++ .../assistant_responses_controller_spec.rb | 2 + .../captain/assistants_controller_spec.rb | 2 + .../captain/bulk_actions_controller_spec.rb | 2 + .../captain/documents_controller_spec.rb | 2 + .../captain/inboxes_controller_spec.rb | 2 + .../captain/scenarios_controller_spec.rb | 2 + .../accounts/custom_roles_controller_spec.rb | 2 + .../accounts/sla_policies_controller_spec.rb | 1 + 19 files changed, 83 insertions(+), 28 deletions(-) create mode 100644 enterprise/app/controllers/api/v1/accounts/captain/base_controller.rb diff --git a/app/controllers/api/v1/accounts/agents_controller.rb b/app/controllers/api/v1/accounts/agents_controller.rb index 438944f04..b8907ff2e 100644 --- a/app/controllers/api/v1/accounts/agents_controller.rb +++ b/app/controllers/api/v1/accounts/agents_controller.rb @@ -9,17 +9,22 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController end def create - builder = AgentBuilder.new( - email: new_agent_params['email'], - name: new_agent_params['name'], - role: new_agent_params['role'], - availability: new_agent_params['availability'], - auto_offline: new_agent_params['auto_offline'], - inviter: current_user, - account: Current.account - ) + # Lock the account row so concurrent invites can't both pass the seat-limit check (TOCTOU). + Current.account.with_lock do + next unless can_add_agent? - @agent = builder.perform + @agent = AgentBuilder.new( + email: new_agent_params['email'], + name: new_agent_params['name'], + role: new_agent_params['role'], + availability: new_agent_params['availability'], + auto_offline: new_agent_params['auto_offline'], + inviter: current_user, + account: Current.account + ).perform + end + + render_payment_required('Account limit exceeded. Please purchase more licenses') if @agent.blank? end def update @@ -36,19 +41,17 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController def bulk_create emails = params[:emails] - emails.each do |email| - builder = AgentBuilder.new( - email: email, - name: email.split('@').first, - inviter: current_user, - account: Current.account - ) - begin - builder.perform - rescue ActiveRecord::RecordInvalid => e - Rails.logger.info "[Agent#bulk_create] ignoring email #{email}, errors: #{e.record.errors}" + # Lock the account row so concurrent bulk invites can't collectively exceed the seat limit (TOCTOU). + limit_exceeded = false + Current.account.with_lock do + if emails.count > available_agent_count + limit_exceeded = true + next end + + invite_agents(emails) end + return render_payment_required('Account limit exceeded. Please purchase more licenses') if limit_exceeded # This endpoint is used to bulk create agents during onboarding # onboarding_step key in present in Current account custom attributes, since this is a one time operation @@ -59,6 +62,19 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController private + def invite_agents(emails) + emails.each do |email| + AgentBuilder.new( + email: email, + name: email.split('@').first, + inviter: current_user, + account: Current.account + ).perform + rescue ActiveRecord::RecordInvalid => e + Rails.logger.info "[Agent#bulk_create] ignoring email #{email}, errors: #{e.record.errors}" + end + end + def check_authorization super(User) end diff --git a/app/finders/conversation_finder.rb b/app/finders/conversation_finder.rb index 74bf903f5..6d94fcf77 100644 --- a/app/finders/conversation_finder.rb +++ b/app/finders/conversation_finder.rb @@ -142,7 +142,8 @@ class ConversationFinder conversation_ids = current_account.mentions.where(user: current_user).pluck(:conversation_id) @conversations = @conversations.where(id: conversation_ids) when 'participating' - @conversations = current_user.participating_conversations.where(account_id: current_account.id) + participating_ids = current_user.participating_conversations.where(account_id: current_account.id).select(:id) + @conversations = @conversations.where(id: participating_ids) when 'unattended' @conversations = @conversations.unattended end diff --git a/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb index 151cf279c..6beb29c31 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/assistant_responses_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Captain::AssistantResponsesController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Captain::AssistantResponsesController < Api::V1::Accounts::Captain::BaseController before_action :current_account before_action -> { check_authorization(Captain::Assistant) } diff --git a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb index 4fbb93d20..ecc5d7623 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::Captain::BaseController before_action :current_account before_action -> { check_authorization(Captain::Assistant) } diff --git a/enterprise/app/controllers/api/v1/accounts/captain/base_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/base_controller.rb new file mode 100644 index 000000000..9b45dbdb1 --- /dev/null +++ b/enterprise/app/controllers/api/v1/accounts/captain/base_controller.rb @@ -0,0 +1,13 @@ +class Api::V1::Accounts::Captain::BaseController < Api::V1::Accounts::BaseController + before_action :ensure_captain_enabled + + private + + # Captain is a premium feature; block API access when neither the v1 nor v2 flag is enabled for the account. + # `current_account` resolves and memoizes the account, so this does not depend on before_action ordering. + def ensure_captain_enabled + return if current_account.feature_enabled?('captain_integration') || current_account.feature_enabled?('captain_integration_v2') + + render json: { error: 'Captain is not enabled for this account' }, status: :forbidden + end +end diff --git a/enterprise/app/controllers/api/v1/accounts/captain/bulk_actions_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/bulk_actions_controller.rb index 7e2817f69..9d9676635 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/bulk_actions_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/bulk_actions_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Captain::BulkActionsController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Captain::BulkActionsController < Api::V1::Accounts::Captain::BaseController before_action :current_account before_action -> { check_authorization(Captain::Assistant) } before_action :validate_params diff --git a/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb index d88cc6b48..c45a5d404 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Captain::DocumentsController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Captain::DocumentsController < Api::V1::Accounts::Captain::BaseController before_action :current_account before_action -> { check_authorization(Captain::Assistant) } diff --git a/enterprise/app/controllers/api/v1/accounts/captain/inboxes_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/inboxes_controller.rb index f4ec303b6..2ae5e9e29 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/inboxes_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/inboxes_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Captain::InboxesController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Captain::InboxesController < Api::V1::Accounts::Captain::BaseController before_action :current_account before_action -> { check_authorization(Captain::Assistant) } diff --git a/enterprise/app/controllers/api/v1/accounts/captain/scenarios_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/scenarios_controller.rb index 376cee0b3..7fa3dd955 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/scenarios_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/scenarios_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Captain::ScenariosController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Captain::ScenariosController < Api::V1::Accounts::Captain::BaseController before_action :current_account before_action -> { check_authorization(Captain::Scenario) } before_action :set_assistant diff --git a/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb b/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb index 2d6823b90..a45fba1d9 100644 --- a/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb @@ -1,4 +1,5 @@ class Api::V1::Accounts::CustomRolesController < Api::V1::Accounts::EnterpriseAccountsController + before_action :ensure_custom_roles_feature_enabled before_action :fetch_custom_role, only: [:show, :update, :destroy] before_action :check_authorization @@ -28,4 +29,8 @@ class Api::V1::Accounts::CustomRolesController < Api::V1::Accounts::EnterpriseAc def fetch_custom_role @custom_role = Current.account.custom_roles.find_by(id: params[:id]) end + + def ensure_custom_roles_feature_enabled + raise Pundit::NotAuthorizedError unless Current.account.feature_enabled?('custom_roles') + end end diff --git a/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb b/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb index b3b0f52d4..7789a1b59 100644 --- a/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb @@ -1,4 +1,5 @@ class Api::V1::Accounts::SlaPoliciesController < Api::V1::Accounts::EnterpriseAccountsController + before_action :ensure_sla_feature_enabled before_action :fetch_sla, only: [:show, :update, :destroy] before_action :check_authorization @@ -29,4 +30,8 @@ class Api::V1::Accounts::SlaPoliciesController < Api::V1::Accounts::EnterpriseAc def fetch_sla @sla_policy = Current.account.sla_policies.find_by(id: params[:id]) end + + def ensure_sla_feature_enabled + raise Pundit::NotAuthorizedError unless Current.account.feature_enabled?('sla') + end end diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb index 038ee5e8d..7b727c24a 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/assistant_responses_controller_spec.rb @@ -9,6 +9,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::AssistantResponses', type: :request let(:another_assistant) { create(:captain_assistant, account: account) } let(:another_document) { create(:captain_document, account: account, assistant: assistant) } + before { account.enable_features!('captain_integration') } + def json_response JSON.parse(response.body, symbolize_names: true) end diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb index afb6aa2de..79013ef18 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb @@ -5,6 +5,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do let(:admin) { create(:user, account: account, role: :administrator) } let(:agent) { create(:user, account: account, role: :agent) } + before { account.enable_features!('captain_integration') } + def json_response JSON.parse(response.body, symbolize_names: true) end diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/bulk_actions_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/bulk_actions_controller_spec.rb index 31a0bc36f..0f25254e0 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/bulk_actions_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/bulk_actions_controller_spec.rb @@ -24,6 +24,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::BulkActions', type: :request do ) end + before { account.enable_features!('captain_integration') } + def json_response JSON.parse(response.body, symbolize_names: true) end diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb index 4d4b10fcb..c4dfe6227 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb @@ -13,6 +13,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Documents', type: :request do }.with_indifferent_access end + before { account.enable_features!('captain_integration') } + def json_response JSON.parse(response.body, symbolize_names: true) end diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/inboxes_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/inboxes_controller_spec.rb index 85d561755..e5c6d07d0 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/inboxes_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/inboxes_controller_spec.rb @@ -9,6 +9,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Inboxes', type: :request do let(:admin) { create(:user, account: account, role: :administrator) } let(:agent) { create(:user, account: account, role: :agent) } + before { account.enable_features!('captain_integration') } + def json_response JSON.parse(response.body, symbolize_names: true) end diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/scenarios_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/scenarios_controller_spec.rb index 3e68c9e5e..9d12739ca 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/scenarios_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/scenarios_controller_spec.rb @@ -6,6 +6,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Scenarios', type: :request do let(:agent) { create(:user, account: account, role: :agent) } let(:assistant) { create(:captain_assistant, account: account) } + before { account.enable_features!('captain_integration') } + def json_response JSON.parse(response.body, symbolize_names: true) end diff --git a/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb index 5c6d96a7b..3c771ce2b 100644 --- a/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb @@ -6,6 +6,8 @@ RSpec.describe 'Custom Roles API', type: :request do let!(:agent) { create(:user, account: account, role: :agent) } let!(:custom_role) { create(:custom_role, account: account, name: 'Manager') } + before { account.enable_features!('custom_roles') } + describe 'GET #index' do context 'when it is an authenticated administrator' do it 'returns all custom roles in the account' do diff --git a/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb index e1a4fa538..077b586b1 100644 --- a/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb @@ -6,6 +6,7 @@ RSpec.describe 'Enterprise SLA API', type: :request do let(:agent) { create(:user, account: account, role: :agent) } before do + account.enable_features!('sla') create(:sla_policy, account: account, name: 'SLA 1') end