From 534acfbf96778610b82146cb653b78fdc2a22d5b Mon Sep 17 00:00:00 2001
From: Sojan Jose <sojan@pepalo.com>
Date: Fri, 11 Jun 2021 11:44:31 +0530
Subject: [PATCH] chore: Ensure privilege validations for API endpoints (#2224)

Co-authored-by: Pranav Raj S <pranav@chatwoot.com>
---
 app/controllers/api/base_controller.rb        |   4 +
 .../contacts/contact_inboxes_controller.rb    |   1 +
 .../contacts/conversations_controller.rb      |   4 +-
 .../api/v1/accounts/contacts_controller.rb    |   3 +-
 .../accounts/conversations/base_controller.rb |   1 +
 .../v1/accounts/conversations_controller.rb   |  11 +-
 .../v1/accounts/inbox_members_controller.rb   |  16 ++-
 .../api/v1/accounts/inboxes_controller.rb     |   1 +
 .../accounts/integrations/apps_controller.rb  |   1 +
 .../accounts/integrations/slack_controller.rb |   1 +
 .../api/v2/accounts/reports_controller.rb     |   6 +
 app/finders/conversation_finder.rb            |  14 +-
 app/models/user.rb                            |   2 +-
 app/policies/conversation_policy.rb           |   5 +
 app/policies/inbox_policy.rb                  |  13 +-
 .../api/v1/accounts/agents_controller_spec.rb |  32 ++++-
 .../contact_inboxes_controller_spec.rb        |  10 +-
 .../v1/accounts/contacts_controller_spec.rb   |  21 ++-
 .../assignments_controller_spec.rb            |   8 +-
 .../conversations/labels_controller_spec.rb   |  13 +-
 .../conversations/messages_controller_spec.rb |  22 ++-
 .../accounts/conversations_controller_spec.rb | 135 +++++++++++++-----
 .../accounts/inbox_members_controller_spec.rb |  46 +++++-
 .../v1/accounts/inboxes_controller_spec.rb    |   4 +
 .../integrations/apps_controller_spec.rb      |   4 +-
 .../api/v2/accounts/report_controller_spec.rb |  74 +++++++---
 .../integrations/slack_request_spec.rb        |   2 +-
 27 files changed, 335 insertions(+), 119 deletions(-)
 create mode 100644 app/policies/conversation_policy.rb

diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb
index 0cfd7205b..937cd5502 100644
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@@ -16,4 +16,8 @@ class Api::BaseController < ApplicationController
 
     authorize(model)
   end
+
+  def check_admin_authorization?
+    raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
+  end
 end
diff --git a/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb b/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb
index 16686dbfe..fa5b271bf 100644
--- a/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb
+++ b/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb
@@ -11,6 +11,7 @@ class Api::V1::Accounts::Contacts::ContactInboxesController < Api::V1::Accounts:
 
   def ensure_inbox
     @inbox = Current.account.inboxes.find(params[:inbox_id])
+    authorize @inbox, :show?
   end
 
   def ensure_contact
diff --git a/app/controllers/api/v1/accounts/contacts/conversations_controller.rb b/app/controllers/api/v1/accounts/contacts/conversations_controller.rb
index 576fbaa2f..4ccc14729 100644
--- a/app/controllers/api/v1/accounts/contacts/conversations_controller.rb
+++ b/app/controllers/api/v1/accounts/contacts/conversations_controller.rb
@@ -8,9 +8,7 @@ class Api::V1::Accounts::Contacts::ConversationsController < Api::V1::Accounts::
   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
       []
diff --git a/app/controllers/api/v1/accounts/contacts_controller.rb b/app/controllers/api/v1/accounts/contacts_controller.rb
index ac27314fc..46f6b83c7 100644
--- a/app/controllers/api/v1/accounts/contacts_controller.rb
+++ b/app/controllers/api/v1/accounts/contacts_controller.rb
@@ -48,7 +48,8 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
   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
diff --git a/app/controllers/api/v1/accounts/conversations/base_controller.rb b/app/controllers/api/v1/accounts/conversations/base_controller.rb
index 2dae59abd..f521719ae 100644
--- a/app/controllers/api/v1/accounts/conversations/base_controller.rb
+++ b/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
diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb
index 91381095d..9f4f0b45d 100644
--- a/app/controllers/api/v1/accounts/conversations_controller.rb
+++ b/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 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro
   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
diff --git a/app/controllers/api/v1/accounts/inbox_members_controller.rb b/app/controllers/api/v1/accounts/inbox_members_controller.rb
index f715c7040..27e31c855 100644
--- a/app/controllers/api/v1/accounts/inbox_members_controller.rb
+++ b/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
 
diff --git a/app/controllers/api/v1/accounts/inboxes_controller.rb b/app/controllers/api/v1/accounts/inboxes_controller.rb
index 12c7a29e6..a51681617 100644
--- a/app/controllers/api/v1/accounts/inboxes_controller.rb
+++ b/app/controllers/api/v1/accounts/inboxes_controller.rb
@@ -62,6 +62,7 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController
 
   def fetch_inbox
     @inbox = Current.account.inboxes.find(params[:id])
+    authorize @inbox, :show?
   end
 
   def fetch_agent_bot
diff --git a/app/controllers/api/v1/accounts/integrations/apps_controller.rb b/app/controllers/api/v1/accounts/integrations/apps_controller.rb
index f88cc7c5f..7ec5cc5ec 100644
--- a/app/controllers/api/v1/accounts/integrations/apps_controller.rb
+++ b/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]
 
diff --git a/app/controllers/api/v1/accounts/integrations/slack_controller.rb b/app/controllers/api/v1/accounts/integrations/slack_controller.rb
index 5ded9f512..537ddd688 100644
--- a/app/controllers/api/v1/accounts/integrations/slack_controller.rb
+++ b/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
diff --git a/app/controllers/api/v2/accounts/reports_controller.rb b/app/controllers/api/v2/accounts/reports_controller.rb
index 3203f34e9..8fc980255 100644
--- a/app/controllers/api/v2/accounts/reports_controller.rb
+++ b/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 @@ class Api::V2::Accounts::ReportsController < Api::V1::Accounts::BaseController
 
   private
 
+  def check_authorization
+    raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
+  end
+
   def account_summary_params
     {
       type: :account,
diff --git a/app/finders/conversation_finder.rb b/app/finders/conversation_finder.rb
index 4336aff11..8791da5b9 100644
--- a/app/finders/conversation_finder.rb
+++ b/app/finders/conversation_finder.rb
@@ -48,15 +48,11 @@ class ConversationFinder
   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
diff --git a/app/models/user.rb b/app/models/user.rb
index fefe8669d..460608178 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -122,7 +122,7 @@ class User < ApplicationRecord
   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?
diff --git a/app/policies/conversation_policy.rb b/app/policies/conversation_policy.rb
new file mode 100644
index 000000000..133cb3b02
--- /dev/null
+++ b/app/policies/conversation_policy.rb
@@ -0,0 +1,5 @@
+class ConversationPolicy < ApplicationPolicy
+  def index?
+    true
+  end
+end
diff --git a/app/policies/inbox_policy.rb b/app/policies/inbox_policy.rb
index 0bfe6fe40..dd3aa9305 100644
--- a/app/policies/inbox_policy.rb
+++ b/app/policies/inbox_policy.rb
@@ -11,11 +11,7 @@ class InboxPolicy < ApplicationPolicy
     end
 
     def resolve
-      if @account_user.administrator?
-        scope.all
-      elsif @account_user.agent?
-        user.assigned_inboxes
-      end
+      user.assigned_inboxes
     end
   end
 
@@ -23,6 +19,13 @@ class InboxPolicy < ApplicationPolicy
     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
diff --git a/spec/controllers/api/v1/accounts/agents_controller_spec.rb b/spec/controllers/api/v1/accounts/agents_controller_spec.rb
index f0efad1e1..832840b34 100644
--- a/spec/controllers/api/v1/accounts/agents_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/agents_controller_spec.rb
@@ -2,6 +2,8 @@ require 'rails_helper'
 
 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 @@ RSpec.describe 'Agents API', type: :request do
     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 @@ RSpec.describe 'Agents API', type: :request do
     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 @@ RSpec.describe 'Agents API', type: :request do
     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,
diff --git a/spec/controllers/api/v1/accounts/contacts/contact_inboxes_controller_spec.rb b/spec/controllers/api/v1/accounts/contacts/contact_inboxes_controller_spec.rb
index 6bf741347..80de09c40 100644
--- a/spec/controllers/api/v1/accounts/contacts/contact_inboxes_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/contacts/contact_inboxes_controller_spec.rb
@@ -5,7 +5,7 @@ RSpec.describe '/api/v1/accounts/{account.id}/contacts/:id/contact_inboxes', typ
   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 @@ RSpec.describe '/api/v1/accounts/{account.id}/contacts/:id/contact_inboxes', typ
       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 @@ RSpec.describe '/api/v1/accounts/{account.id}/contacts/:id/contact_inboxes', typ
       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)
 
diff --git a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb
index 8bc4e5724..12b58c90f 100644
--- a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb
@@ -201,18 +201,29 @@ RSpec.describe 'Contacts API', type: :request do
     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
diff --git a/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb
index ad79a04ff..4e91b8d03 100644
--- a/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb
@@ -14,10 +14,14 @@ RSpec.describe 'Conversation Assignment API', type: :request do
       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 @@ RSpec.describe 'Conversation Assignment API', type: :request do
 
       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 @@ RSpec.describe 'Conversation Assignment API', type: :request do
 
       before do
         conversation.update!(team: team)
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
       end
 
       it 'unassigns the team from the conversation' do
diff --git a/spec/controllers/api/v1/accounts/conversations/labels_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/labels_controller_spec.rb
index fad91ffbd..f91b3e3a1 100644
--- a/spec/controllers/api/v1/accounts/conversations/labels_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/conversations/labels_controller_spec.rb
@@ -17,9 +17,13 @@ RSpec.describe 'Conversation Label API', type: :request do
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated user with access to the conversation' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
+      end
+
       it 'returns all the labels for the conversation' do
         get api_v1_account_conversation_labels_url(account_id: account.id, conversation_id: conversation.display_id),
             headers: agent.create_new_auth_token,
@@ -49,9 +53,14 @@ RSpec.describe 'Conversation Label API', type: :request do
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated user with access to the conversation' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        conversation.update_labels('label1, label2')
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
+      end
+
       it 'creates labels for the conversation' do
         post api_v1_account_conversation_labels_url(account_id: account.id, conversation_id: conversation.display_id),
              params: { labels: %w[label3 label4] },
diff --git a/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb
index 93283dbb5..d32608ba2 100644
--- a/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb
@@ -15,9 +15,13 @@ RSpec.describe 'Conversation Messages API', type: :request do
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated user with access to conversation' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
+      end
+
       it 'creates a new outgoing message' do
         params = { content: 'test-message', private: true }
 
@@ -124,9 +128,13 @@ RSpec.describe 'Conversation Messages API', type: :request do
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated user with access to conversation' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
+      end
+
       it 'shows the conversation' do
         get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/messages",
             headers: agent.create_new_auth_token,
@@ -149,9 +157,13 @@ RSpec.describe 'Conversation Messages API', type: :request do
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated user with access to conversation' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
+      end
+
       it 'deletes the message' do
         delete "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/messages/#{message.id}",
                headers: agent.create_new_auth_token,
@@ -166,6 +178,10 @@ RSpec.describe 'Conversation Messages API', type: :request do
     context 'when the message id is invalid' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, inbox: conversation.inbox, user: agent)
+      end
+
       it 'returns not found error' do
         delete "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/messages/99999",
                headers: agent.create_new_auth_token,
diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb
index 2ddd3f31a..6a0c419a2 100644
--- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb
@@ -119,8 +119,27 @@ RSpec.describe 'Conversations API', type: :request do
 
     context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
+      let(:administrator) { create(:user, account: account, role: :administrator) }
 
-      it 'shows the conversation' do
+      it 'does not shows the conversation if you do not have access to it' do
+        get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}",
+            headers: agent.create_new_auth_token,
+            as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+
+      it 'shows the conversation if you are an administrator' do
+        get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}",
+            headers: administrator.create_new_auth_token,
+            as: :json
+
+        expect(response).to have_http_status(:success)
+        expect(JSON.parse(response.body, symbolize_names: true)[:id]).to eq(conversation.display_id)
+      end
+
+      it 'shows the conversation if you are an agent with access to inbox' do
+        create(:inbox_member, user: agent, inbox: conversation.inbox)
         get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}",
             headers: agent.create_new_auth_token,
             as: :json
@@ -149,57 +168,71 @@ RSpec.describe 'Conversations API', type: :request do
     context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
-      it 'creates a new conversation' do
+      it 'will not create a new conversation if agent does not have access to inbox' do
         allow(Rails.configuration.dispatcher).to receive(:dispatch)
         additional_attributes = { test: 'test' }
         post "/api/v1/accounts/#{account.id}/conversations",
              headers: agent.create_new_auth_token,
              params: { source_id: contact_inbox.source_id, additional_attributes: additional_attributes },
              as: :json
-
-        expect(response).to have_http_status(:success)
-        response_data = JSON.parse(response.body, symbolize_names: true)
-        expect(response_data[:additional_attributes]).to eq(additional_attributes)
+        expect(response).to have_http_status(:unauthorized)
       end
 
-      it 'creates a conversation in specificed status' do
-        allow(Rails.configuration.dispatcher).to receive(:dispatch)
-        post "/api/v1/accounts/#{account.id}/conversations",
-             headers: agent.create_new_auth_token,
-             params: { source_id: contact_inbox.source_id, status: 'bot' },
-             as: :json
+      context 'when it is an authenticated user who has access to the inbox' do
+        before do
+          create(:inbox_member, user: agent, inbox: inbox)
+        end
 
-        expect(response).to have_http_status(:success)
-        response_data = JSON.parse(response.body, symbolize_names: true)
-        expect(response_data[:status]).to eq('bot')
-      end
+        it 'creates a new conversation' do
+          allow(Rails.configuration.dispatcher).to receive(:dispatch)
+          additional_attributes = { test: 'test' }
+          post "/api/v1/accounts/#{account.id}/conversations",
+               headers: agent.create_new_auth_token,
+               params: { source_id: contact_inbox.source_id, additional_attributes: additional_attributes },
+               as: :json
 
-      it 'creates a new conversation with message when message is passed' do
-        allow(Rails.configuration.dispatcher).to receive(:dispatch)
-        post "/api/v1/accounts/#{account.id}/conversations",
-             headers: agent.create_new_auth_token,
-             params: { source_id: contact_inbox.source_id, message: { content: 'hi' } },
-             as: :json
+          expect(response).to have_http_status(:success)
+          response_data = JSON.parse(response.body, symbolize_names: true)
+          expect(response_data[:additional_attributes]).to eq(additional_attributes)
+        end
 
-        expect(response).to have_http_status(:success)
-        response_data = JSON.parse(response.body, symbolize_names: true)
-        expect(response_data[:additional_attributes]).to eq({})
-        expect(account.conversations.find_by(display_id: response_data[:id]).messages.first.content).to eq 'hi'
-      end
+        it 'creates a conversation in specificed status' do
+          allow(Rails.configuration.dispatcher).to receive(:dispatch)
+          post "/api/v1/accounts/#{account.id}/conversations",
+               headers: agent.create_new_auth_token,
+               params: { source_id: contact_inbox.source_id, status: 'bot' },
+               as: :json
 
-      it 'calls contact inbox builder if contact_id and inbox_id is present' do
-        builder = double
-        contact = create(:contact, account: account)
-        inbox = create(:inbox, account: account)
-        allow(Rails.configuration.dispatcher).to receive(:dispatch)
-        allow(ContactInboxBuilder).to receive(:new).and_return(builder)
-        allow(builder).to receive(:perform)
-        expect(builder).to receive(:perform)
+          expect(response).to have_http_status(:success)
+          response_data = JSON.parse(response.body, symbolize_names: true)
+          expect(response_data[:status]).to eq('bot')
+        end
 
-        post "/api/v1/accounts/#{account.id}/conversations",
-             headers: agent.create_new_auth_token,
-             params: { contact_id: contact.id, inbox_id: inbox.id },
-             as: :json
+        it 'creates a new conversation with message when message is passed' do
+          allow(Rails.configuration.dispatcher).to receive(:dispatch)
+          post "/api/v1/accounts/#{account.id}/conversations",
+               headers: agent.create_new_auth_token,
+               params: { source_id: contact_inbox.source_id, message: { content: 'hi' } },
+               as: :json
+
+          expect(response).to have_http_status(:success)
+          response_data = JSON.parse(response.body, symbolize_names: true)
+          expect(response_data[:additional_attributes]).to eq({})
+          expect(account.conversations.find_by(display_id: response_data[:id]).messages.outgoing.first.content).to eq 'hi'
+        end
+
+        it 'calls contact inbox builder if contact_id and inbox_id is present' do
+          builder = double
+          allow(Rails.configuration.dispatcher).to receive(:dispatch)
+          allow(ContactInboxBuilder).to receive(:new).and_return(builder)
+          allow(builder).to receive(:perform)
+          expect(builder).to receive(:perform)
+
+          post "/api/v1/accounts/#{account.id}/conversations",
+               headers: agent.create_new_auth_token,
+               params: { contact_id: contact.id, inbox_id: inbox.id },
+               as: :json
+        end
       end
     end
   end
@@ -218,6 +251,10 @@ RSpec.describe 'Conversations API', type: :request do
     context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, user: agent, inbox: conversation.inbox)
+      end
+
       it 'toggles the conversation status' do
         expect(conversation.status).to eq('open')
 
@@ -268,6 +305,10 @@ RSpec.describe 'Conversations API', type: :request do
     context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, user: agent, inbox: conversation.inbox)
+      end
+
       it 'toggles the conversation status' do
         allow(Rails.configuration.dispatcher).to receive(:dispatch)
         post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/toggle_typing_status",
@@ -296,6 +337,10 @@ RSpec.describe 'Conversations API', type: :request do
     context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, user: agent, inbox: conversation.inbox)
+      end
+
       it 'updates last seen' do
         post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/update_last_seen",
              headers: agent.create_new_auth_token,
@@ -321,6 +366,10 @@ RSpec.describe 'Conversations API', type: :request do
     context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, user: agent, inbox: conversation.inbox)
+      end
+
       it 'mutes conversation' do
         post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/mute",
              headers: agent.create_new_auth_token,
@@ -347,6 +396,10 @@ RSpec.describe 'Conversations API', type: :request do
     context 'when it is an authenticated user' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
+      before do
+        create(:inbox_member, user: agent, inbox: conversation.inbox)
+      end
+
       it 'unmutes conversation' do
         post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/unmute",
              headers: agent.create_new_auth_token,
@@ -373,6 +426,10 @@ RSpec.describe 'Conversations API', type: :request do
       let(:agent) { create(:user, account: account, role: :agent) }
       let(:params) { { email: 'test@test.com' } }
 
+      before do
+        create(:inbox_member, user: agent, inbox: conversation.inbox)
+      end
+
       it 'mutes conversation' do
         mailer = double
         allow(ConversationReplyMailer).to receive(:with).and_return(mailer)
diff --git a/spec/controllers/api/v1/accounts/inbox_members_controller_spec.rb b/spec/controllers/api/v1/accounts/inbox_members_controller_spec.rb
index 565b0277f..1361f34b6 100644
--- a/spec/controllers/api/v1/accounts/inbox_members_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/inbox_members_controller_spec.rb
@@ -13,14 +13,34 @@ RSpec.describe 'Inbox Member API', type: :request do
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated agent' do
+      let(:agent) { create(:user, account: account, role: :agent) }
+
+      before do
+        create(:inbox_member, user: agent, inbox: inbox)
+      end
+
+      it 'returns unauthorized' do
+        params = { inbox_id: inbox.id, user_ids: [agent.id] }
+
+        post "/api/v1/accounts/#{account.id}/inbox_members",
+             headers: agent.create_new_auth_token,
+             params: params,
+             as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'when it is an authenticated user with access to inbox' do
+      let(:administrator) { create(:user, account: account, role: :administrator) }
       let(:agent) { create(:user, account: account, role: :agent) }
 
       it 'modifies inbox members' do
         params = { inbox_id: inbox.id, user_ids: [agent.id] }
 
         post "/api/v1/accounts/#{account.id}/inbox_members",
-             headers: agent.create_new_auth_token,
+             headers: administrator.create_new_auth_token,
              params: params,
              as: :json
 
@@ -33,7 +53,7 @@ RSpec.describe 'Inbox Member API', type: :request do
         params = { inbox_id: nil, user_ids: [agent.id] }
 
         post "/api/v1/accounts/#{account.id}/inbox_members",
-             headers: agent.create_new_auth_token,
+             headers: administrator.create_new_auth_token,
              params: params,
              as: :json
 
@@ -44,7 +64,7 @@ RSpec.describe 'Inbox Member API', type: :request do
         params = { inbox_id: inbox.id, user_ids: ['invalid'] }
 
         post "/api/v1/accounts/#{account.id}/inbox_members",
-             headers: agent.create_new_auth_token,
+             headers: administrator.create_new_auth_token,
              params: params,
              as: :json
 
@@ -65,16 +85,30 @@ RSpec.describe 'Inbox Member API', type: :request do
       end
     end
 
-    context 'when it is an authenticated user' do
+    context 'when it is an authenticated user with out access to inbox' do
       let(:agent) { create(:user, account: account, role: :agent) }
 
       it 'returns inbox member' do
+        get "/api/v1/accounts/#{account.id}/inbox_members/#{inbox.id}",
+            headers: agent.create_new_auth_token,
+            as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'when it is an authenticated user with access to inbox' do
+      let(:agent) { create(:user, account: account, role: :agent) }
+
+      it 'returns inbox member' do
+        create(:inbox_member, user: agent, inbox: inbox)
+
         get "/api/v1/accounts/#{account.id}/inbox_members/#{inbox.id}",
             headers: agent.create_new_auth_token,
             as: :json
 
         expect(response).to have_http_status(:success)
-        expect(JSON.parse(response.body)).to eq({ payload: inbox.inbox_members }.as_json)
+        expect(JSON.parse(response.body)['payload'].pluck('id')).to eq(inbox.inbox_members.pluck(:user_id))
       end
     end
   end
diff --git a/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb b/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb
index a4143df25..1a5c31088 100644
--- a/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/inboxes_controller_spec.rb
@@ -267,6 +267,10 @@ RSpec.describe 'Inboxes API', type: :request do
   describe 'GET /api/v1/accounts/{account.id}/inboxes/{inbox.id}/agent_bot' do
     let(:inbox) { create(:inbox, account: account) }
 
+    before do
+      create(:inbox_member, user: agent, inbox: inbox)
+    end
+
     context 'when it is an unauthenticated user' do
       it 'returns unauthorized' do
         get "/api/v1/accounts/#{account.id}/inboxes/#{inbox.id}/agent_bot"
diff --git a/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb
index c86163ccf..8f1716c32 100644
--- a/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb
@@ -12,7 +12,7 @@ RSpec.describe 'Integration Apps API', type: :request do
     end
 
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
+      let(:agent) { create(:user, account: account, role: :administrator) }
 
       it 'returns all active apps' do
         first_app = Integrations::App.all.find(&:active?)
@@ -52,7 +52,7 @@ RSpec.describe 'Integration Apps API', type: :request do
     end
 
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
+      let(:agent) { create(:user, account: account, role: :administrator) }
 
       it 'returns details of the app' do
         get api_v1_account_integrations_app_url(account_id: account.id, id: 'slack'),
diff --git a/spec/controllers/api/v2/accounts/report_controller_spec.rb b/spec/controllers/api/v2/accounts/report_controller_spec.rb
index f72b6639a..e6152dc8b 100644
--- a/spec/controllers/api/v2/accounts/report_controller_spec.rb
+++ b/spec/controllers/api/v2/accounts/report_controller_spec.rb
@@ -2,6 +2,8 @@ require 'rails_helper'
 
 RSpec.describe 'Reports API', type: :request do
   let(:account) { create(:account) }
+  let(:admin) { create(:user, account: account, role: :administrator) }
+  let(:agent) { create(:user, account: account, role: :agent) }
   let!(:user) { create(:user, account: account) }
   let!(:inbox) { create(:inbox, account: account) }
   let(:inbox_member) { create(:inbox_member, user: user, inbox: inbox) }
@@ -21,21 +23,28 @@ RSpec.describe 'Reports API', type: :request do
     end
 
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
-
-      it 'return timeseries metrics' do
-        params = {
-          metric: 'conversations_count',
-          type: :account,
-          since: Time.zone.today.to_time.to_i.to_s,
-          until: Time.zone.today.to_time.to_i.to_s
-        }
+      params = {
+        metric: 'conversations_count',
+        type: :account,
+        since: Time.zone.today.to_time.to_i.to_s,
+        until: Time.zone.today.to_time.to_i.to_s
+      }
 
+      it 'returns unauthorized for agents' do
         get "/api/v2/accounts/#{account.id}/reports/account",
             params: params,
             headers: agent.create_new_auth_token,
             as: :json
 
+        expect(response).to have_http_status(:unauthorized)
+      end
+
+      it 'return timeseries metrics' do
+        get "/api/v2/accounts/#{account.id}/reports/account",
+            params: params,
+            headers: admin.create_new_auth_token,
+            as: :json
+
         expect(response).to have_http_status(:success)
         json_response = JSON.parse(response.body)
 
@@ -56,20 +65,27 @@ RSpec.describe 'Reports API', type: :request do
     end
 
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
-
-      it 'returns summary metrics' do
-        params = {
-          type: :account,
-          since: Time.zone.today.to_time.to_i.to_s,
-          until: Time.zone.today.to_time.to_i.to_s
-        }
+      params = {
+        type: :account,
+        since: Time.zone.today.to_time.to_i.to_s,
+        until: Time.zone.today.to_time.to_i.to_s
+      }
 
+      it 'returns unauthorized for agents' do
         get "/api/v2/accounts/#{account.id}/reports/account_summary",
             params: params,
             headers: agent.create_new_auth_token,
             as: :json
 
+        expect(response).to have_http_status(:unauthorized)
+      end
+
+      it 'returns summary metrics' do
+        get "/api/v2/accounts/#{account.id}/reports/account_summary",
+            params: params,
+            headers: admin.create_new_auth_token,
+            as: :json
+
         expect(response).to have_http_status(:success)
         json_response = JSON.parse(response.body)
 
@@ -88,18 +104,24 @@ RSpec.describe 'Reports API', type: :request do
     end
 
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
-
       params = {
         since: 30.days.ago.to_i.to_s,
         until: Time.zone.today.to_time.to_i.to_s
       }
 
-      it 'returns summary' do
+      it 'returns unauthorized for agents' do
         get "/api/v2/accounts/#{account.id}/reports/agents.csv",
             params: params,
             headers: agent.create_new_auth_token
 
+        expect(response).to have_http_status(:unauthorized)
+      end
+
+      it 'returns summary' do
+        get "/api/v2/accounts/#{account.id}/reports/agents.csv",
+            params: params,
+            headers: admin.create_new_auth_token
+
         expect(response).to have_http_status(:success)
       end
     end
@@ -115,18 +137,24 @@ RSpec.describe 'Reports API', type: :request do
     end
 
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
-
       params = {
         since: 30.days.ago.to_i.to_s,
         until: Time.zone.today.to_time.to_i.to_s
       }
 
-      it 'returns summary' do
+      it 'returns unauthorized for agents' do
         get "/api/v2/accounts/#{account.id}/reports/inboxes",
             params: params,
             headers: agent.create_new_auth_token
 
+        expect(response).to have_http_status(:unauthorized)
+      end
+
+      it 'returns summary' do
+        get "/api/v2/accounts/#{account.id}/reports/inboxes",
+            params: params,
+            headers: admin.create_new_auth_token
+
         expect(response).to have_http_status(:success)
       end
     end
diff --git a/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb b/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb
index c61c8d376..0815c6eba 100644
--- a/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb
+++ b/spec/requests/api/v1/accounts/integrations/slack_request_spec.rb
@@ -2,7 +2,7 @@ require 'rails_helper'
 
 RSpec.describe 'Api::V1::Accounts::Integrations::Slacks', type: :request do
   let(:account) { create(:account) }
-  let(:agent) { create(:user, account: account, role: :agent) }
+  let(:agent) { create(:user, account: account, role: :administrator) }
   let!(:hook) { create(:integrations_hook, account: account) }
 
   describe 'POST /api/v1/accounts/{account.id}/integrations/slack' do