[Rt-commit] rt branch 5.0/tweak-ticket-search-acl-owner created. rt-5.0.5-248-g5e948dc0f1

[BPS Git Server]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/tweak-ticket-search-acl-owner has been created
        at  5e948dc0f1f996658ff88f40b689dec69cfb2cf2 (commit)

- Log -----------------------------------------------------------------
commit 5e948dc0f1f996658ff88f40b689dec69cfb2cf2
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Fri Apr 12 13:34:29 2024 -0400

    Test ticket searches where Owner has ShowTicket granted

diff --git a/t/ticket/search.t b/t/ticket/search.t
index ba4fb997d4..b27d14883f 100644
--- a/t/ticket/search.t
+++ b/t/ticket/search.t
@@ -392,4 +392,38 @@ $tix = RT::Tickets->new($user);
 $tix->FromSQL('Created > "2018-10-05" and Created < "2018-10-06"');
 is($tix->Count, 0, "We found 0 tickets created on 2018-10-05 but not at 00:00:00 with user in +08:00 timezone");
 
+diag "Test owner in ticket search ACL";
+my $alice    = RT::Test->load_or_create_user( Name => 'alice' );
+my $alice_id = $alice->Id;
+RT::Test->add_rights( { Principal => $alice->PrincipalObj, Right => 'OwnTicket' } );
+RT::Test->add_rights( { Principal => 'Owner', Right => 'ShowTicket' } );
+
+my $alice_current_user = RT::CurrentUser->new( RT->SystemUser );
+$alice_current_user->Load('alice');
+$tix = RT::Tickets->new($alice_current_user);
+$tix->FromSQL('id > 0');
+is( $tix->Count, 0, "User alice doesn't own any tickets" );
+like( $tix->BuildSelectQuery( PreferBind => 0 ), qr/Owner = '$alice_id'/, 'Searched Tickets.Owner in ACL' );
+unlike(
+    $tix->BuildSelectQuery( PreferBind => 0 ),
+    qr/\QLOWER(Groups_1.Name) IN ('owner')\E/i,
+    'Did not search Owner group in ACL'
+);
+
+ok( $t1->SetOwner('alice'), 'Set a ticket owner to alice' );
+$tix->RedoSearch;
+is( $tix->Count,     1,       "We found 1 ticket owned by alice" );
+is( $tix->First->Id, $t1->Id, 'Found expected ticket ' . $t1->Id );
+
+RT::Test->add_rights( { Principal => 'Requestor', Right => 'ShowTicket' } );
+$tix->FromSQL('id > 0');
+is( $tix->Count,     1,       "We found 1 ticket owned by alice" );
+is( $tix->First->Id, $t1->Id, 'Found expected ticket ' . $t1->Id );
+unlike( $tix->BuildSelectQuery( PreferBind => 0 ), qr/Owner = '$alice_id'/, 'Did not search Tickets.Owner in ACL' );
+like(
+    $tix->BuildSelectQuery( PreferBind => 0 ),
+    qr/\QLOWER(Groups_1.Name)\E IN \(('requestor', 'owner'|'owner', 'requestor')\)/i,
+    'Searched Owner group in ACL'
+);
+
 done_testing;

commit a59fd30a2c302fff39e5f65f277097d894a96dc1
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Fri Apr 12 11:14:53 2024 -0400

    Search Owner group along with other watcher groups in ACL for ticket searches
    
    Previously we searched Owner field of Tickets table for ACL check, when
    Owner is granted "ShowTicket". If other watchers like Requestor also had
    "ShowTicket" granted, then generated ACL check SQL would be like:
    
         LOWER(Groups_1.Name) IN ('requestor') )  OR  ( main.Owner = '20363' )
    
    This "OR" thing might cause performance issues on some databases. As Owner
    also has corresponding groups like other watchers, this commit tweaks it to
    be:
    
        LOWER(Groups_1.Name) IN ('requestor', 'owner')

diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 275fe6653a..e379bbbebc 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -3049,16 +3049,18 @@ sub CurrentUserCanSee {
 
         my %queue;
         while ( my ( $role, $queues ) = each %roles ) {
-            next if $role eq 'Owner';
+            next if $role eq 'Owner' && !$join_roles;
             push @{ $queue{ $stringify_queues->($queues) } }, lc $role;
         }
 
         my %queue_applied;
         while ( my ($role, $queues) = each %roles ) {
-            next if $role ne 'Owner' && $queue_applied{ $stringify_queues->($queues) }++;
+            next if ( $role ne 'Owner' || $join_roles ) && $queue_applied{ $stringify_queues->($queues) }++;
 
             $self->SUPER::_OpenParen('ACL');
-            if ( $role eq 'Owner' ) {
+
+            # If there are other roles, then we search owner group to simplify generated SQL.
+            if ( $role eq 'Owner' && !$join_roles ) {
                 $self->Limit(
                     SUBCLAUSE => 'ACL',
                     FIELD           => 'Owner',

-----------------------------------------------------------------------


hooks/post-receive
-- 
rt