chore: Ensure privilege validations for API endpoints (#2224)

Co-authored-by: Pranav Raj S <pranav@chatwoot.com>

app/controllers/api/base_controller.rb

@@ -16,4 +16,8 @@ def check_authorization(model = nil)
 
     authorize(model)
   end
+
+  def check_admin_authorization?
+    raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
+  end
 end

app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb

@@ -11,6 +11,7 @@ def create
 
   def ensure_inbox
     @inbox = Current.account.inboxes.find(params[:inbox_id])
+    authorize @inbox, :show?
   end
 
   def ensure_contact

app/controllers/api/v1/accounts/contacts/conversations_controller.rb

@@ -8,9 +8,7 @@ def index
   private
 
   def inbox_ids
-    if Current.user.administrator?
-      Current.account.inboxes.pluck(:id)
-    elsif Current.user.agent?
+    if Current.user.administrator? || Current.user.agent?
       Current.user.assigned_inboxes.pluck(:id)
     else
       []

app/controllers/api/v1/accounts/contacts_controller.rb

@@ -48,7 +48,8 @@ def active
   def show; end
 
   def contactable_inboxes
-    @contactable_inboxes = Contacts::ContactableInboxesService.new(contact: @contact).get
+    @all_contactable_inboxes = Contacts::ContactableInboxesService.new(contact: @contact).get
+    @contactable_inboxes = @all_contactable_inboxes.select { |contactable_inbox| policy(contactable_inbox[:inbox]).show? }
   end
 
   def create

app/controllers/api/v1/accounts/conversations/base_controller.rb

@@ -5,5 +5,6 @@ class Api::V1::Accounts::Conversations::BaseController < Api::V1::Accounts::Base
 
   def conversation
     @conversation ||= Current.account.conversations.find_by(display_id: params[:conversation_id])
+    authorize @conversation.inbox, :show?
   end
 end

app/controllers/api/v1/accounts/conversations_controller.rb

@@ -1,7 +1,7 @@
 class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseController
   include Events::Types
 
-  before_action :conversation, except: [:index]
+  before_action :conversation, except: [:index, :meta, :search, :create]
   before_action :contact_inbox, only: [:create]
 
   def index
@@ -79,21 +79,26 @@ def trigger_typing_event(event)
   end
 
   def conversation
-    @conversation ||= Current.account.conversations.find_by(display_id: params[:id])
+    @conversation ||= Current.account.conversations.find_by!(display_id: params[:id])
+    authorize @conversation.inbox, :show?
   end
 
   def contact_inbox
     @contact_inbox = build_contact_inbox
 
     @contact_inbox ||= ::ContactInbox.find_by!(source_id: params[:source_id])
+    authorize @contact_inbox.inbox, :show?
   end
 
   def build_contact_inbox
     return if params[:contact_id].blank? || params[:inbox_id].blank?
 
+    inbox = Current.account.inboxes.find(params[:inbox_id])
+    authorize inbox, :show?
+
     ContactInboxBuilder.new(
       contact_id: params[:contact_id],
-      inbox_id: params[:inbox_id],
+      inbox_id: inbox.id,
       source_id: params[:source_id]
     ).perform
   end

app/controllers/api/v1/accounts/inbox_members_controller.rb

@@ -3,15 +3,19 @@ class Api::V1::Accounts::InboxMembersController < Api::V1::Accounts::BaseControl
   before_action :current_agents_ids, only: [:create]
 
   def create
-    # update also done via same action
-    update_agents_list
-    head :ok
-  rescue StandardError => e
-    Rails.logger.debug "Rescued: #{e.inspect}"
-    render_could_not_create_error('Could not add agents to inbox')
+    authorize @inbox, :create?
+    begin
+      # update also done via same action
+      update_agents_list
+      head :ok
+    rescue StandardError => e
+      Rails.logger.debug "Rescued: #{e.inspect}"
+      render_could_not_create_error('Could not add agents to inbox')
+    end
   end
 
   def show
+    authorize @inbox, :show?
     @agents = Current.account.users.where(id: @inbox.members.select(:user_id))
   end
 

app/controllers/api/v1/accounts/inboxes_controller.rb

@@ -62,6 +62,7 @@ def destroy
 
   def fetch_inbox
     @inbox = Current.account.inboxes.find(params[:id])
+    authorize @inbox, :show?
   end
 
   def fetch_agent_bot

app/controllers/api/v1/accounts/integrations/apps_controller.rb

@@ -1,4 +1,5 @@
 class Api::V1::Accounts::Integrations::AppsController < Api::V1::Accounts::BaseController
+  before_action :check_admin_authorization?
   before_action :fetch_apps, only: [:index]
   before_action :fetch_app, only: [:show]
 

app/controllers/api/v1/accounts/integrations/slack_controller.rb

@@ -1,4 +1,5 @@
 class Api::V1::Accounts::Integrations::SlackController < Api::V1::Accounts::BaseController
+  before_action :check_admin_authorization?
   before_action :fetch_hook, only: [:update, :destroy]
 
   def create

app/controllers/api/v2/accounts/reports_controller.rb

@@ -1,4 +1,6 @@
 class Api::V2::Accounts::ReportsController < Api::V1::Accounts::BaseController
+  before_action :check_authorization
+
   def account
     builder = V2::ReportBuilder.new(Current.account, account_report_params)
     data = builder.build
@@ -23,6 +25,10 @@ def inboxes
 
   private
 
+  def check_authorization
+    raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
+  end
+
   def account_summary_params
     {
       type: :account,

app/finders/conversation_finder.rb

@@ -48,15 +48,11 @@ def perform
   private
 
   def set_inboxes
-    if params[:inbox_id]
-      @inbox_ids = current_account.inboxes.where(id: params[:inbox_id])
-    else
-      if @current_user.administrator?
-        @inbox_ids = current_account.inboxes.pluck(:id)
-      elsif @current_user.agent?
-        @inbox_ids = @current_user.assigned_inboxes.pluck(:id)
-      end
-    end
+    @inbox_ids = if params[:inbox_id]
+                   current_account.inboxes.where(id: params[:inbox_id])
+                 else
+                   @current_user.assigned_inboxes.pluck(:id)
+                 end
   end
 
   def set_assignee_type

app/models/user.rb

@@ -122,7 +122,7 @@ def account
   end
 
   def assigned_inboxes
-    inboxes.where(account_id: Current.account.id)
+    administrator? ? Current.account.inboxes : inboxes.where(account_id: Current.account.id)
   end
 
   def administrator?

app/policies/conversation_policy.rb

@@ -0,0 +1,5 @@
+class ConversationPolicy < ApplicationPolicy
+  def index?
+    true
+  end
+end

app/policies/inbox_policy.rb

@@ -11,18 +11,21 @@ def initialize(user_context, scope)
     end
 
     def resolve
-      if @account_user.administrator?
-        scope.all
-      elsif @account_user.agent?
-        user.assigned_inboxes
-      end
+      user.assigned_inboxes
     end
   end
 
   def index?
     true
   end
 
+  def show?
+    # FIXME: for agent bots, lets bring this validation to policies as well in future
+    return true if @user.blank?
+
+    Current.user.assigned_inboxes.include? record
+  end
+
   def assignable_agents?
     true
   end

spec/controllers/api/v1/accounts/agents_controller_spec.rb

@@ -2,6 +2,8 @@
 
 RSpec.describe 'Agents API', type: :request do
   let(:account) { create(:account) }
+  let(:admin) { create(:user, account: account, role: :administrator) }
+  let(:agent) { create(:user, account: account, role: :agent) }
 
   describe 'GET /api/v1/accounts/{account.id}/agents' do
     context 'when it is an unauthenticated user' do
@@ -38,7 +40,13 @@
     end
 
     context 'when it is an authenticated user' do
-      let(:admin) { create(:user, account: account, role: :administrator) }
+      it 'returns unauthorized for agents' do
+        delete "/api/v1/accounts/#{account.id}/agents/#{other_agent.id}",
+               headers: agent.create_new_auth_token,
+               as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+      end
 
       it 'deletes an agent' do
         delete "/api/v1/accounts/#{account.id}/agents/#{other_agent.id}",
@@ -63,10 +71,17 @@
     end
 
     context 'when it is an authenticated user' do
-      let(:admin) { create(:user, account: account, role: :administrator) }
-
       params = { name: 'TestUser' }
 
+      it 'returns unauthorized for agents' do
+        put "/api/v1/accounts/#{account.id}/agents/#{other_agent.id}",
+            params: params,
+            headers: agent.create_new_auth_token,
+            as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+
       it 'modifies an agent' do
         put "/api/v1/accounts/#{account.id}/agents/#{other_agent.id}",
             params: params,
@@ -91,10 +106,17 @@
     end
 
     context 'when it is an authenticated user' do
-      let(:admin) { create(:user, account: account, role: :administrator) }
-
       params = { name: 'NewUser', email: Faker::Internet.email, role: :agent }
 
+      it 'returns unauthorized for agents' do
+        post "/api/v1/accounts/#{account.id}/agents",
+             params: params,
+             headers: agent.create_new_auth_token,
+             as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+
       it 'creates a new agent' do
         post "/api/v1/accounts/#{account.id}/agents",
              params: params,

spec/controllers/api/v1/accounts/contacts/contact_inboxes_controller_spec.rb

@@ -5,7 +5,7 @@
   let(:contact) { create(:contact, account: account) }
   let(:channel_twilio_sms) { create(:channel_twilio_sms, account: account) }
   let(:channel_api) { create(:channel_api, account: account) }
-  let(:user) { create(:user, account: account) }
+  let(:agent) { create(:user, account: account) }
 
   describe 'GET /api/v1/accounts/{account.id}/contacts/:id/contact_inboxes' do
     context 'when unauthenticated user' do
@@ -15,12 +15,13 @@
       end
     end
 
-    context 'when user is logged in' do
+    context 'when authenticated user with access to inbox' do
       it 'creates a contact inbox' do
+        create(:inbox_member, inbox: channel_api.inbox, user: agent)
         expect do
           post "/api/v1/accounts/#{account.id}/contacts/#{contact.id}/contact_inboxes",
                params: { inbox_id: channel_api.inbox.id },
-               headers: user.create_new_auth_token,
+               headers: agent.create_new_auth_token,
                as: :json
         end.to change(ContactInbox, :count).by(1)
 
@@ -29,10 +30,11 @@
       end
 
       it 'throws error for invalid source id' do
+        create(:inbox_member, inbox: channel_twilio_sms.inbox, user: agent)
         expect do
           post "/api/v1/accounts/#{account.id}/contacts/#{contact.id}/contact_inboxes",
                params: { inbox_id: channel_twilio_sms.inbox.id },
-               headers: user.create_new_auth_token,
+               headers: agent.create_new_auth_token,
                as: :json
         end.to change(ContactInbox, :count).by(0)
 

spec/controllers/api/v1/accounts/contacts_controller_spec.rb

@@ -201,18 +201,29 @@
     end
 
     context 'when it is an authenticated user' do
-      let(:admin) { create(:user, account: account, role: :administrator) }
+      let(:agent) { create(:user, account: account, role: :agent) }
+      let!(:twilio_sms) { create(:channel_twilio_sms, account: account) }
+      let!(:twilio_sms_inbox) { create(:inbox, channel: twilio_sms, account: account) }
+      let!(:twilio_whatsapp) { create(:channel_twilio_sms, medium: :whatsapp, account: account) }
+      let!(:twilio_whatsapp_inbox) { create(:inbox, channel: twilio_whatsapp, account: account) }
+
+      it 'shows the contactable inboxes which the user has access to' do
+        create(:inbox_member, user: agent, inbox: twilio_whatsapp_inbox)
 
-      it 'shows the contact' do
         inbox_service = double
         allow(Contacts::ContactableInboxesService).to receive(:new).and_return(inbox_service)
-        allow(inbox_service).to receive(:get).and_return({})
-        expect(inbox_service).to receive(:get).and_return({})
+        allow(inbox_service).to receive(:get).and_return([
+                                                           { source_id: '1123', inbox: twilio_sms_inbox },
+                                                           { source_id: '1123', inbox: twilio_whatsapp_inbox }
+                                                         ])
+        expect(inbox_service).to receive(:get)
         get "/api/v1/accounts/#{account.id}/contacts/#{contact.id}/contactable_inboxes",
-            headers: admin.create_new_auth_token,
+            headers: agent.create_new_auth_token,
             as: :json
 
         expect(response).to have_http_status(:success)
+        # only the inboxes which agent has access to are shown
+        expect(JSON.parse(response.body)['payload'].pluck('inbox').pluck('id')).to eq([twilio_whatsapp_inbox.id])
       end
     end
   end

spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb

@@ -14,10 +14,14 @@
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated user with access to the inbox' do
       let(:agent) { create(:user, account: account, role: :agent) }
       let(:team) { create(:team, account: account) }
 
+      before do
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
+      end
+
       it 'assigns a user to the conversation' do
         params = { assignee_id: agent.id }
 
@@ -48,6 +52,7 @@
 
       before do
         conversation.update!(assignee: agent)
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
       end
 
       it 'unassigns the assignee from the conversation' do
@@ -69,6 +74,7 @@
 
       before do
         conversation.update!(team: team)
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
       end
 
       it 'unassigns the team from the conversation' do