chore: Enforce custom role permissions on conversation access (#12583)
## Summary - ensure conversation lookup uses the permission filter before fetching records - add request specs covering custom role access to unassigned conversations ## Testing - bundle exec rspec spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb ------ https://chatgpt.com/codex/tasks/task_e_68de1f62b9b883268a54882e608a8bb8
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -160,7 +160,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro
|
||||
|
||||
def conversation
|
||||
@conversation ||= Current.account.conversations.find_by!(display_id: params[:id])
|
||||
authorize @conversation.inbox, :show?
|
||||
authorize @conversation, :show?
|
||||
end
|
||||
|
||||
def inbox
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -14,6 +14,7 @@ module AccessTokenAuthHelper
|
||||
ensure_access_token
|
||||
render_unauthorized('Invalid Access Token') && return if @access_token.blank?
|
||||
|
||||
# NOTE: This ensures that current_user is set and available for the rest of the controller actions
|
||||
@resource = @access_token.owner
|
||||
Current.user = @resource if allowed_current_user_type?(@resource)
|
||||
end
|
||||
|
||||
@@ -4,6 +4,44 @@ class ConversationPolicy < ApplicationPolicy
|
||||
end
|
||||
|
||||
def destroy?
|
||||
@account_user&.administrator?
|
||||
administrator?
|
||||
end
|
||||
|
||||
def show?
|
||||
administrator? || agent_bot? || agent_can_view_conversation?
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def agent_can_view_conversation?
|
||||
inbox_access? || team_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 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')
|
||||
|
||||
42
enterprise/app/policies/enterprise/conversation_policy.rb
Normal file
42
enterprise/app/policies/enterprise/conversation_policy.rb
Normal file
@@ -0,0 +1,42 @@
|
||||
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 true if permits_unassigned_manage?(permissions)
|
||||
|
||||
permits_participating?(permissions)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def manage_all_conversations?(permissions)
|
||||
permissions.include?('conversation_manage')
|
||||
end
|
||||
|
||||
def permits_unassigned_manage?(permissions)
|
||||
return false unless permissions.include?('conversation_unassigned_manage')
|
||||
|
||||
unassigned_conversation? || assigned_to_user?
|
||||
end
|
||||
|
||||
def permits_participating?(permissions)
|
||||
return false unless permissions.include?('conversation_participating_manage')
|
||||
|
||||
assigned_to_user? || participant?
|
||||
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
|
||||
@@ -5,6 +5,29 @@ RSpec.describe 'API Base', type: :request do
|
||||
let!(:user) { create(:user, account: account) }
|
||||
|
||||
describe 'request with api_access_token for user' do
|
||||
context 'when accessing an account scoped resource' do
|
||||
let!(:admin) { create(:user, :administrator, account: account) }
|
||||
let!(:conversation) { create(:conversation, account: account) }
|
||||
|
||||
it 'sets Current attributes for the request and then returns the response' do
|
||||
# expect Current.account_user is set to the admin's account_user
|
||||
allow(Current).to receive(:user=).and_call_original
|
||||
allow(Current).to receive(:account=).and_call_original
|
||||
allow(Current).to receive(:account_user=).and_call_original
|
||||
|
||||
get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}",
|
||||
headers: { api_access_token: admin.access_token.token },
|
||||
as: :json
|
||||
|
||||
expect(Current).to have_received(:user=).with(admin).at_least(:once)
|
||||
expect(Current).to have_received(:account=).with(account).at_least(:once)
|
||||
expect(Current).to have_received(:account_user=).with(admin.account_users.first).at_least(:once)
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(response.parsed_body['id']).to eq(conversation.display_id)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when it is an invalid api_access_token' do
|
||||
it 'returns unauthorized' do
|
||||
get '/api/v1/profile',
|
||||
|
||||
@@ -30,5 +30,76 @@ RSpec.describe 'Conversations API', type: :request do
|
||||
expect(response.parsed_body.keys).not_to include('applied_sla')
|
||||
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 has a custom role' 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)
|
||||
end
|
||||
|
||||
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(:unauthorized)
|
||||
end
|
||||
|
||||
it 'returns the conversation when permission allows managing unassigned conversations, including when assigned to agent' do
|
||||
custom_role = create(:custom_role, account: account, permissions: ['conversation_unassigned_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 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
|
||||
|
||||
65
spec/enterprise/policies/conversation_policy_spec.rb
Normal file
65
spec/enterprise/policies/conversation_policy_spec.rb
Normal file
@@ -0,0 +1,65 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe ConversationPolicy, type: :policy do
|
||||
subject { described_class }
|
||||
|
||||
let(:account) { create(:account) }
|
||||
let(:agent) { create(:user, account: account, role: :agent) }
|
||||
let(:inbox) { create(:inbox, account: account) }
|
||||
let(:agent_account_user) { agent.account_users.find_by(account: account) }
|
||||
let(:context) { { user: agent, account: account, account_user: agent_account_user } }
|
||||
|
||||
before do
|
||||
create(:inbox_member, user: agent, inbox: inbox)
|
||||
end
|
||||
|
||||
permissions :show? do
|
||||
context 'when role grants conversation_unassigned_manage' do
|
||||
let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_unassigned_manage']) }
|
||||
|
||||
before do
|
||||
agent_account_user.update!(role: :agent, custom_role: custom_role)
|
||||
end
|
||||
|
||||
it 'allows access to conversations assigned to the agent' do
|
||||
conversation = create(:conversation, account: account, inbox: inbox, assignee: agent)
|
||||
|
||||
expect(subject).to permit(context, conversation)
|
||||
end
|
||||
|
||||
it 'denies access to conversations assigned to someone else' do
|
||||
other_agent = create(:user, account: account, role: :agent)
|
||||
conversation = create(:conversation, account: account, inbox: inbox, assignee: other_agent)
|
||||
|
||||
expect(subject).not_to permit(context, conversation)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when role grants conversation_participating_manage' do
|
||||
let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_participating_manage']) }
|
||||
|
||||
before do
|
||||
agent_account_user.update!(role: :agent, custom_role: custom_role)
|
||||
end
|
||||
|
||||
it 'allows access to conversations assigned to the agent' do
|
||||
conversation = create(:conversation, account: account, inbox: inbox, assignee: agent)
|
||||
|
||||
expect(subject).to permit(context, conversation)
|
||||
end
|
||||
|
||||
it 'allows access to conversations where the agent is a participant' do
|
||||
conversation = create(:conversation, account: account, inbox: inbox, assignee: nil)
|
||||
create(:conversation_participant, conversation: conversation, account: account, user: agent)
|
||||
|
||||
expect(subject).to permit(context, conversation)
|
||||
end
|
||||
|
||||
it 'denies access to unrelated conversations' do
|
||||
conversation = create(:conversation, account: account, inbox: inbox, assignee: nil)
|
||||
|
||||
expect(subject).not_to permit(context, conversation)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -4,11 +4,12 @@ RSpec.describe ConversationPolicy, type: :policy do
|
||||
subject { described_class }
|
||||
|
||||
let(:account) { create(:account) }
|
||||
let(:conversation) { create(:conversation, account: account) }
|
||||
let(:administrator) { create(:user, account: account, role: :administrator) }
|
||||
let(:agent) { create(:user, account: account, role: :agent) }
|
||||
let(:administrator_context) { { user: administrator, account: account, account_user: administrator.account_users.first } }
|
||||
let(:agent_context) { { user: agent, account: account, account_user: agent.account_users.first } }
|
||||
let(:administrator_context) { { user: administrator, account: account, account_user: administrator.account_users.find_by(account: account) } }
|
||||
let(:agent_context) { { user: agent, account: account, account_user: agent.account_users.find_by(account: account) } }
|
||||
|
||||
let(:conversation) { create(:conversation, account: account) }
|
||||
|
||||
permissions :destroy? do
|
||||
context 'when user is an administrator' do
|
||||
@@ -31,4 +32,42 @@ RSpec.describe ConversationPolicy, type: :policy do
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
permissions :show? do
|
||||
context 'when user is an administrator' do
|
||||
it 'allows access' do
|
||||
expect(subject).to permit(administrator_context, conversation)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when agent has inbox access' do
|
||||
let(:inbox) { create(:inbox, account: account) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
|
||||
before { create(:inbox_member, user: agent, inbox: inbox) }
|
||||
|
||||
it 'allows access' do
|
||||
expect(subject).to permit(agent_context, conversation)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when agent has team access' do
|
||||
let(:team) { create(:team, account: account) }
|
||||
let(:conversation) { create(:conversation, :with_team, account: account, team: team) }
|
||||
|
||||
before { create(:team_member, team: team, user: agent) }
|
||||
|
||||
it 'allows access' do
|
||||
expect(subject).to permit(agent_context, conversation)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when agent lacks inbox and team access' do
|
||||
let(:conversation) { create(:conversation, account: account) }
|
||||
|
||||
it 'denies access' do
|
||||
expect(subject).not_to permit(agent_context, conversation)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user