fix(security): restrict participating scope, agent seat race, and premium feature access

2026-07-22 17:21:33 +05:30
parent 522e3c4d3f
commit 3b7a2c2b0e
19 changed files with 83 additions and 28 deletions
@@ -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
@@ -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
@@ -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) }

@@ -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) }

@@ -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
@@ -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
@@ -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) }

@@ -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) }

@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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