diff --git a/app/controllers/api/v1/accounts/conversations/base_controller.rb b/app/controllers/api/v1/accounts/conversations/base_controller.rb index 500c7772f..223530e27 100644 --- a/app/controllers/api/v1/accounts/conversations/base_controller.rb +++ b/app/controllers/api/v1/accounts/conversations/base_controller.rb @@ -5,6 +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? + authorize @conversation, :show? end end diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 5cf3cafe8..4301eaa4a 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -159,19 +159,8 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro end def conversation - @conversation ||= accessible_conversations.find_by!(display_id: params[:id]) - authorize @conversation.inbox, :show? - end - - def accessible_conversations - scope = Current.account.conversations - return scope unless Current.user.is_a?(User) - - Conversations::PermissionFilterService.new( - scope, - Current.user, - current_account - ).perform + @conversation ||= Current.account.conversations.find_by!(display_id: params[:id]) + authorize @conversation, :show? end def inbox diff --git a/app/controllers/api/v1/accounts/integrations/dyte_controller.rb b/app/controllers/api/v1/accounts/integrations/dyte_controller.rb index c5f795d34..845caab5e 100644 --- a/app/controllers/api/v1/accounts/integrations/dyte_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/dyte_controller.rb @@ -22,7 +22,7 @@ class Api::V1::Accounts::Integrations::DyteController < Api::V1::Accounts::BaseC private def authorize_request - authorize @conversation.inbox, :show? + authorize @conversation, :show? end def render_response(response) diff --git a/app/controllers/concerns/ensure_current_account_helper.rb b/app/controllers/concerns/ensure_current_account_helper.rb index 3baf9ee1e..796d93280 100644 --- a/app/controllers/concerns/ensure_current_account_helper.rb +++ b/app/controllers/concerns/ensure_current_account_helper.rb @@ -10,21 +10,24 @@ module EnsureCurrentAccountHelper account = Account.find(params[:account_id]) render_unauthorized('Account is suspended') and return unless account.active? - if current_user - account_accessible_for_user?(account) - elsif @resource.is_a?(AgentBot) - account_accessible_for_bot?(account) + acting_user = current_user || Current.user + + case acting_user + when User + account_accessible_for_user?(account, acting_user) + when AgentBot + account_accessible_for_bot?(account, acting_user) end account end - def account_accessible_for_user?(account) - @current_account_user = account.account_users.find_by(user_id: current_user.id) + def account_accessible_for_user?(account, user) + @current_account_user = account.account_users.find_by(user_id: user.id) Current.account_user = @current_account_user render_unauthorized('You are not authorized to access this account') unless @current_account_user end - def account_accessible_for_bot?(account) - render_unauthorized('Bot is not authorized to access this account') unless @resource.agent_bot_inboxes.find_by(account_id: account.id) + def account_accessible_for_bot?(account, bot) + render_unauthorized('Bot is not authorized to access this account') unless bot.agent_bot_inboxes.find_by(account_id: account.id) end end diff --git a/app/policies/conversation_policy.rb b/app/policies/conversation_policy.rb index 931e17435..3990f3f71 100644 --- a/app/policies/conversation_policy.rb +++ b/app/policies/conversation_policy.rb @@ -3,7 +3,49 @@ class ConversationPolicy < ApplicationPolicy true end + def show? + administrator? || agent_bot? || agent_can_view_conversation? + end + def destroy? - @account_user&.administrator? + administrator? + end + + private + + def agent_can_view_conversation? + inbox_access? || team_access? || participation_access? + end + + def administrator? + account_user&.administrator? + end + + def agent_bot? + user.is_a?(AgentBot) + end + + def inbox_access? + user.inboxes.where(account_id: account&.id).exists?(id: record.inbox_id) + end + + def team_access? + return false if record.team_id.blank? + + user.teams.where(account_id: account&.id).exists?(id: record.team_id) + end + + def participation_access? + assigned_to_user? || participant? + end + + def assigned_to_user? + record.assignee_id == user.id + end + + def participant? + record.conversation_participants.exists?(user_id: user.id) end end + +ConversationPolicy.prepend_mod_with('ConversationPolicy') diff --git a/enterprise/app/policies/enterprise/conversation_policy.rb b/enterprise/app/policies/enterprise/conversation_policy.rb new file mode 100644 index 000000000..27cd75cb7 --- /dev/null +++ b/enterprise/app/policies/enterprise/conversation_policy.rb @@ -0,0 +1,41 @@ +module Enterprise::ConversationPolicy + def show? + return false unless super + return true unless custom_role_permissions? + + permissions = custom_role_permissions + return true if manage_all_conversations?(permissions) + return permits_unassigned_or_participating?(permissions) if unassigned_conversation? + + permits_participating?(permissions) + end + + private + + def manage_all_conversations?(permissions) + permissions.include?('conversation_manage') + end + + def permits_unassigned_or_participating?(permissions) + return true if permissions.include?('conversation_unassigned_manage') + + permits_participating?(permissions) + end + + def permits_participating?(permissions) + (assigned_to_user? || participant?) && + (permissions.include?('conversation_participating_manage') || permissions.include?('conversation_unassigned_manage')) + end + + def unassigned_conversation? + record.assignee_id.nil? + end + + def custom_role_permissions? + account_user&.custom_role_id.present? + end + + def custom_role_permissions + account_user&.custom_role&.permissions || [] + end +end diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb index ac2306d14..228fe8bd9 100644 --- a/spec/controllers/api/base_controller_spec.rb +++ b/spec/controllers/api/base_controller_spec.rb @@ -5,6 +5,26 @@ RSpec.describe 'API Base', type: :request do let!(:user) { create(:user, account: account) } describe 'request with api_access_token for user' do + context 'when administrator accesses a conversation' do + let!(:admin) { create(:user, :administrator, account: account) } + let!(:conversation) { create(:conversation, account: account) } + + it 'returns the conversation details' do + # Regression: ensure account-scoped access via API tokens populates Current.account_user + allow(Current).to receive(:reset) + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", + headers: { api_access_token: admin.access_token.token }, + as: :json + + expect(response).to have_http_status(:success) + expect(response.parsed_body['id']).to eq(conversation.display_id) + expect(Current.account_user.user_id).to eq(admin.id) + ensure + Current.reset + end + end + context 'when it is an invalid api_access_token' do it 'returns unauthorized' do get '/api/v1/profile', diff --git a/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb index 13a733c1e..3d752aaac 100644 --- a/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -31,6 +31,40 @@ RSpec.describe 'Conversations API', type: :request do expect(response.parsed_body.keys).not_to include('sla_events') end + context 'when agent has team access' do + let(:agent) { create(:user, account: account, role: :agent) } + let(:team) { create(:team, account: account) } + let(:conversation) { create(:conversation, account: account, team: team) } + + before do + create(:team_member, team: team, user: agent) + end + + it 'allows accessing the conversation via team membership' do + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + end + + context 'when agent participates in the conversation' do + let(:agent) { create(:user, account: account, role: :agent) } + let(:conversation) { create(:conversation, account: account) } + + before do + create(:inbox_member, user: agent, inbox: conversation.inbox) + create(:conversation_participant, conversation: conversation, account: account, user: agent) + end + + it 'allows accessing the conversation via participation' do + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + end + context 'when agent has a custom role' do let(:agent) { create(:user, account: account, role: :agent) } let(:conversation) { create(:conversation, account: account) } @@ -39,13 +73,13 @@ RSpec.describe 'Conversations API', type: :request do create(:inbox_member, user: agent, inbox: conversation.inbox) end - it 'returns not found for unassigned conversation without permission' do + it 'returns unauthorized for unassigned conversation without permission' do custom_role = create(:custom_role, account: account, permissions: ['conversation_participating_manage']) account.account_users.find_by(user_id: agent.id).update!(custom_role: custom_role) get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token - expect(response).to have_http_status(:not_found) + expect(response).to have_http_status(:unauthorized) end it 'returns the conversation when permission allows managing unassigned conversations' do @@ -57,6 +91,30 @@ RSpec.describe 'Conversations API', type: :request do expect(response).to have_http_status(:ok) expect(response.parsed_body['id']).to eq(conversation.display_id) end + + it 'returns the conversation when permission allows managing assigned conversations' do + custom_role = create(:custom_role, account: account, permissions: ['conversation_participating_manage']) + account_user = account.account_users.find_by(user_id: agent.id) + account_user.update!(custom_role: custom_role) + conversation.update!(assignee: agent) + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + + it 'returns the conversation when permission allows managing participating conversations' do + custom_role = create(:custom_role, account: account, permissions: ['conversation_participating_manage']) + account_user = account.account_users.find_by(user_id: agent.id) + account_user.update!(custom_role: custom_role) + create(:conversation_participant, conversation: conversation, account: account, user: agent) + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end end end end