Maintenance: Missing authorization of the queried object in LinksController#index
Approved-by: Mantas Masalskis <mm@zammad.com>
This commit is contained in:
committed by
Florian Liebe
parent
1129cc4f03
commit
465b2a8f43
@@ -1,7 +1,7 @@
|
||||
# Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
|
||||
|
||||
class LinksController < ApplicationController
|
||||
prepend_before_action -> { authorize! }, only: %i[add remove]
|
||||
prepend_before_action -> { authorize! }, only: %i[index add remove]
|
||||
prepend_before_action :authentication_check
|
||||
|
||||
# GET /api/v1/links
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
# Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
|
||||
|
||||
class Controllers::LinksControllerPolicy < Controllers::ApplicationControllerPolicy
|
||||
def index?
|
||||
object_show?
|
||||
end
|
||||
|
||||
def add?
|
||||
object_target_update? && object_source_show?
|
||||
end
|
||||
@@ -11,23 +15,32 @@ class Controllers::LinksControllerPolicy < Controllers::ApplicationControllerPol
|
||||
|
||||
private
|
||||
|
||||
def object_show?
|
||||
policy = object_policy(record.params[:link_object], id: record.params[:link_object_value])
|
||||
|
||||
object_access?(policy, :agent_read_access?)
|
||||
rescue ActiveRecord::RecordNotFound, NoMatchingPatternError
|
||||
# A missing or unsupported object must deny like an unauthorized one. Letting the
|
||||
# 404 through would turn the endpoint into an existence oracle for every ticket id.
|
||||
false
|
||||
end
|
||||
|
||||
def object_target_update?
|
||||
policy = object_policy(record.params[:link_object_target], id: record.params[:link_object_target_value])
|
||||
|
||||
case policy
|
||||
when TicketPolicy
|
||||
policy.agent_update_access?
|
||||
when KnowledgeBase::AnswerPolicy
|
||||
policy.show?
|
||||
end
|
||||
object_access?(policy, :agent_update_access?)
|
||||
end
|
||||
|
||||
def object_source_show?
|
||||
policy = object_policy(record.params[:link_object_source], number: record.params[:link_object_source_number])
|
||||
|
||||
object_access?(policy, :agent_read_access?)
|
||||
end
|
||||
|
||||
def object_access?(policy, ticket_access)
|
||||
case policy
|
||||
when TicketPolicy
|
||||
policy.agent_read_access?
|
||||
policy.public_send(ticket_access)
|
||||
when KnowledgeBase::AnswerPolicy
|
||||
policy.show?
|
||||
end
|
||||
|
||||
@@ -16,6 +16,97 @@ describe Controllers::LinksControllerPolicy do
|
||||
rec
|
||||
end
|
||||
|
||||
describe '#index' do
|
||||
let(:action_name) { :index }
|
||||
let(:params) do
|
||||
{
|
||||
link_object: link_object,
|
||||
link_object_value: link_object_value,
|
||||
action: action_name.to_s
|
||||
}
|
||||
end
|
||||
|
||||
context 'with ticket' do
|
||||
let(:ticket) { create(:ticket) }
|
||||
let(:link_object) { 'Ticket' }
|
||||
let(:link_object_value) { ticket.id }
|
||||
|
||||
context 'when user has read access on the queried ticket' do
|
||||
let(:user) { create(:agent, groups: [ticket.group]) }
|
||||
|
||||
it { is_expected.to permit_action(action_name) }
|
||||
end
|
||||
|
||||
context 'when user has no access on the queried ticket' do
|
||||
let(:user) { create(:agent) }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
|
||||
context 'when user is the customer of the queried ticket' do
|
||||
let(:user) { create(:customer) }
|
||||
let(:ticket) { create(:ticket, customer: user) }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
|
||||
context 'when user is the customer of an unrelated ticket' do
|
||||
let(:user) { create(:customer) }
|
||||
|
||||
before { create(:link, from: ticket, to: create(:ticket, customer: user)) }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
|
||||
context 'when the queried ticket does not exist' do
|
||||
let(:user) { create(:agent) }
|
||||
let(:link_object_value) { 99_999_999 }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
end
|
||||
|
||||
context 'with knowledge base answer' do
|
||||
let(:link_object) { 'KnowledgeBase::Answer::Translation' }
|
||||
let(:link_object_value) { kb_answer.id }
|
||||
let(:role) { create(:role, permission_names: %w[knowledge_base.reader]) }
|
||||
let(:user) { create(:agent, roles: [role]) }
|
||||
|
||||
context 'when the answer is visible to the user' do
|
||||
let(:kb_answer) { published_answer.translations.first }
|
||||
|
||||
it { is_expected.to permit_action(action_name) }
|
||||
end
|
||||
|
||||
context 'when the answer is not visible to the user' do
|
||||
let(:kb_answer) { archived_answer.translations.first }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
|
||||
context 'when the answer translation does not exist' do
|
||||
let(:link_object_value) { 99_999_999 }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an unsupported link object' do
|
||||
let(:user) { create(:admin) }
|
||||
let(:link_object) { 'Announcement' }
|
||||
let(:link_object_value) { 1 }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
|
||||
context 'without a link object' do
|
||||
let(:user) { create(:admin) }
|
||||
let(:params) { { action: action_name.to_s } }
|
||||
|
||||
it { is_expected.to forbid_action(action_name) }
|
||||
end
|
||||
end
|
||||
|
||||
describe '#add' do
|
||||
context 'with target ticket and source ticket' do
|
||||
let(:ticket_source) { create(:ticket) }
|
||||
|
||||
@@ -45,6 +45,46 @@ RSpec.describe 'Link', type: :request do
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a customer requests links of a foreign Ticket', authenticated_as: -> { customer } do
|
||||
let(:ticket) { create(:ticket) }
|
||||
let(:customer) { create(:customer) }
|
||||
let(:linked) { create(:ticket, customer: customer) }
|
||||
|
||||
before do
|
||||
create(:link, from: ticket, to: linked)
|
||||
|
||||
get '/api/v1/links', params: { link_object: 'Ticket', link_object_value: ticket.id }, as: :json
|
||||
end
|
||||
|
||||
it 'is forbidden' do
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when requesting links of a non-existing object', authenticated_as: -> { agent } do
|
||||
let(:agent) { create(:agent) }
|
||||
|
||||
before do
|
||||
get '/api/v1/links', params: { link_object: 'Ticket', link_object_value: 99_999_999 }, as: :json
|
||||
end
|
||||
|
||||
it 'is forbidden and does not reveal the missing object' do
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when requesting links of an unsupported object', authenticated_as: -> { agent } do
|
||||
let(:agent) { create(:agent) }
|
||||
|
||||
before do
|
||||
get '/api/v1/links', params: { link_object: 'Announcement', link_object_value: 1 }, as: :json
|
||||
end
|
||||
|
||||
it 'is forbidden and does not raise' do
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'POST /api/v1/links/add', authenticated_as: -> { agent } do
|
||||
|
||||
Reference in New Issue
Block a user