Skip to content

Commit

Permalink
Cache ticket rights for atomic changes
Browse files Browse the repository at this point in the history
Ticket changes can revoke rights from current user, which can block
changes after them because of the revoked rights. E.g. current user can
modify a ticket as the owner, which has "ModifyTicket" right granted.
When current user replies the ticket and also changes ticket owner on
ticket update page, as ticket owner change happens first, previously
he/she wouldn't be able to apply the rest of changes.

This commit caches ticket related rights in advance to ignore rights
revocation inside an atomic change, to get around it.
  • Loading branch information
sunnavy committed Nov 15, 2023
1 parent c9be6f9 commit 31d544f
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
22 changes: 22 additions & 0 deletions lib/RT/Ticket.pm
Original file line number Diff line number Diff line change
Expand Up @@ -1806,6 +1806,28 @@ sub Atomic {
my $context = wantarray;
my @ret;

my $orig_current_user_has_right = $self->can('CurrentUserHasRight');
no warnings 'redefine';

local *CurrentUserHasRight = $orig_current_user_has_right;
if ( $self->{Atomic} == 1 ) {
my %granted;
for my $right ( keys %{ RT::Queue->AvailableRights } ) {
if ( $self->CurrentUserHasRight($right) ) {
$granted{$right} = 1;
}
}

if (%granted) {
*CurrentUserHasRight = sub {
my $self = shift;
my $right = shift;
return 1 if $granted{$right};
return $orig_current_user_has_right->( $self, $right, @_ );
};
}
}

local $@;
eval {
if ($context) {
Expand Down
34 changes: 34 additions & 0 deletions t/web/ticket_owner.t
Original file line number Diff line number Diff line change
Expand Up @@ -509,4 +509,38 @@ diag "user can take/steal ticket with ReassignTicket+OwnTicket right";
ok !($agent_c->find_all_links( text => 'Steal' ))[0], 'no Steal link';
}

ok(
RT::Test->add_rights(
{ Principal => 'Owner', Right => [qw(ModifyTicket)] },
),
'add ModifyTicket to Owner'
);

diag "user can update ticket with owner change when rights are granted via Owner";
{
my $ticket = RT::Ticket->new($user_a);
my ( $id, $txn, $msg ) = $ticket->Create(
Queue => $queue->id,
Subject => 'Test reply with owner change',
Owner => $user_b,
);
ok( $id, 'created a ticket #' . $id ) or diag "error: $msg";
is( $ticket->Owner, $user_b->id, 'correct owner' );

$agent_b->goto_ticket($id);
$agent_b->follow_link_ok( { text => 'Reply' } );
$agent_b->submit_form(
form_name => 'TicketUpdate',
fields => {
Owner => $user_a->id,
UpdateContent => 'Reply content along with owner change',
},
button => 'SubmitTicket',
);
$agent_b->text_contains( 'Owner changed from user_b to user_a', 'got owner change message' );
$agent_b->text_contains( 'Correspondence added', 'got correspondence message' );
$agent_b->text_contains( 'Reply content along with owner change', 'got correspondence' );
ok( !$agent_b->find_link(text => 'Reply'), 'No reply link any more' );
}

done_testing;

0 comments on commit 31d544f

Please sign in to comment.