Skip to content
Permalink
Browse files

Fixes #2982 - Trigger with current_user.id as precondition is execute…

…d correctly
  • Loading branch information
Mantas Masalskis authored and zammad-sync committed Mar 24, 2020
1 parent eabc924 commit 92f7d9307c71145a4e7bcd8556d3ea13a6582e3b
Showing with 124 additions and 7 deletions.
  1. +25 −7 app/models/ticket.rb
  2. +36 −0 spec/models/trigger_spec.rb
  3. +63 −0 test/unit/ticket_selector_test.rb
@@ -423,8 +423,22 @@ def online_notification_seen_state(user_id_check = nil)
get count of tickets and tickets which match on selector
@param [Hash] selectors hash with conditions
@oparam [Hash] options
@option options [String] :access can be 'full', 'read', 'create' or 'ignore' (ignore means a selector over all tickets), defaults to 'full'
@option options [Integer] :limit of tickets to return
@option options [User] :user is a current user
@option options [Integer] :execution_time is a current user
@return [Integer, [<Ticket>]]
@example
ticket_count, tickets = Ticket.selectors(params[:condition], limit: limit, current_user: current_user, access: 'full')
ticket_count # count of found tickets
tickets # tickets
=end

def self.selectors(selectors, options)
@@ -438,7 +452,7 @@ def self.selectors(selectors, options)

ActiveRecord::Base.transaction(requires_new: true) do

if !current_user
if !current_user || access == 'ignore'
ticket_count = Ticket.distinct.where(query, *bind_params).joins(tables).count
tickets = Ticket.distinct.where(query, *bind_params).joins(tables).limit(limit)
return [ticket_count, tickets]
@@ -1118,18 +1132,22 @@ def self.perform_triggers(ticket, article, item, options = {})
}
end

user_id = ticket.updated_by_id
if article
user_id = article.updated_by_id
end

user = if user_id != 1
User.lookup(id: user_id)
end

# verify is condition is matching
ticket_count, tickets = Ticket.selectors(condition, limit: 1, execution_time: true)
ticket_count, tickets = Ticket.selectors(condition, limit: 1, execution_time: true, current_user: user, access: 'ignore')

next if ticket_count.blank?
next if ticket_count.zero?
next if tickets.first.id != ticket.id

user_id = ticket.updated_by_id
if article
user_id = article.updated_by_id
end

if recursive == false && local_options[:loop_count] > 1
message = "Do not execute recursive triggers per default until Zammad 3.0. With Zammad 3.0 and higher the following trigger is executed '#{trigger.name}' on Ticket:#{ticket.id}. Please review your current triggers and change them if needed."
logger.info { message }
@@ -294,4 +294,40 @@
end
end
end

context 'with pre condition current_user.id' do
let(:perform) do
{ 'ticket.title'=>{ 'value'=>'triggered' } }
end

let(:user) do
user = create(:agent_user)
user.roles.first.groups << group
user
end

let(:group) { Group.first }

let(:ticket) do
create(:ticket,
title: 'Test Ticket', group: group,
owner_id: user.id, created_by_id: user.id, updated_by_id: user.id)
end

shared_examples 'successful trigger' do |attribute:|
let(:attribute) { attribute }

let(:condition) do
{ attribute => { operator: 'is', pre_condition: 'current_user.id', value: '', value_completion: '' } }
end

it "for #{attribute}" do
ticket && trigger
expect { Observer::Transaction.commit }.to change { ticket.reload.title }.to('triggered')
end
end

it_behaves_like 'successful trigger', attribute: 'ticket.updated_by_id'
it_behaves_like 'successful trigger', attribute: 'ticket.owner_id'
end
end
@@ -1199,4 +1199,67 @@ class TicketSelectorTest < ActiveSupport::TestCase

end

test 'access: "ignore"' do
Ticket.destroy_all

Ticket.create!(
title: 'some title1',
group: @group,
customer_id: @customer1.id,
owner_id: @agent1.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
created_at: '2015-02-05 16:37:00',
updated_by_id: 1,
created_by_id: 1,
)

Ticket.create!(
title: 'some title2',
group: @group,
customer_id: @customer1.id,
owner_id: @agent1.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
created_at: '2015-02-05 16:37:00',
updated_by_id: @agent2.id,
created_by_id: 1,
)

condition = {
'ticket.title' => {
operator: 'contains',
value: 'some',
},
}

# visible by owner
ticket_count, _tickets = Ticket.selectors(condition, limit: 10, current_user: @agent1)
assert_equal(2, ticket_count)

# not visible by another agent
ticket_count, _tickets = Ticket.selectors(condition, limit: 10, current_user: @agent2)
assert_equal(0, ticket_count)

# visible by another user when access: "ignore". For example, when tickets are performed after action of another user
ticket_count, _tickets = Ticket.selectors(condition, limit: 10, current_user: @agent2, access: 'ignore')
assert_equal(2, ticket_count)

condition2 = {
'ticket.updated_by_id' => {
operator: 'is',
pre_condition: 'current_user.id',
value: '',
value_completion: ''
}
}

# not visible by another agent even if matches current user precondition
ticket_count, _tickets = Ticket.selectors(condition2, limit: 10, current_user: @agent2)
assert_equal(0, ticket_count)

# visible by another user when access: "ignore" if matches current user precondition
ticket_count, _tickets = Ticket.selectors(condition2, limit: 10, current_user: @agent2, access: 'ignore')
assert_equal(1, ticket_count)
end
end

0 comments on commit 92f7d93

Please sign in to comment.
You can’t perform that action at this time.