[Rt-commit] rt branch, 4.4/add-watcher-custom-role, created. rt-4.4.4-61-g0db3c306c

[Jim Brandt]

The branch, 4.4/add-watcher-custom-role has been created
        at  0db3c306c02f09726d46485daea6fce70a4e758c (commit)

- Log -----------------------------------------------------------------
commit 95e7b1bb0b40ba0764cae50c4b51c159c093a313
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu Sep 5 14:06:54 2019 -0400

    Add test showing custom roles can be set without ModifyTicket

diff --git a/t/ticket/add-watchers.t b/t/ticket/add-watchers.t
index 12b0bb3e2..558755c89 100644
--- a/t/ticket/add-watchers.t
+++ b/t/ticket/add-watchers.t
@@ -1,4 +1,4 @@
-use RT::Test nodata => 1, tests => 34;
+use RT::Test nodata => 1, tests => undef;
 
 use strict;
 use warnings;
@@ -23,6 +23,18 @@ my $queue = RT::Queue->new(RT->SystemUser);
 my ($queue_id) = $queue->Create( Name => 'watcher tests '.$$);
 ok( $queue_id, 'queue created for watcher tests' );
 
+# Create custom role on the queue
+my $custom_role = RT::CustomRole->new(RT->SystemUser);
+my ($ok, $msg) = $custom_role->Create(
+    Name      => 'Custom Role',
+    MaxValues => 0,
+);
+ok($ok, "Created custom role: $msg");
+
+($ok, $msg) = $custom_role->AddToObject($queue->Id);
+ok($ok, "Added custom role to queue: $msg");
+my $custom_role_type = $custom_role->GroupType;
+
 # new privileged user to check rights
 my $user = RT::User->new( RT->SystemUser );
 my ($user_id) = $user->Create(
@@ -45,7 +57,8 @@ ok( !$user->HasRight( Right => 'ModifyTicket', Object => $queue ), "user can't m
 ok( !$user->HasRight( Right => 'Watch',        Object => $queue ), "user can't watch queue tickets" );
 
 my $ticket = RT::Ticket->new( RT->SystemUser );
-my ($rv, $msg) = $ticket->Create( Subject => 'watcher tests', Queue => $queue->Name );
+my $rv;
+($rv, $msg) = $ticket->Create( Subject => 'watcher tests', Queue => $queue->Name );
 ok( $ticket->id, "ticket created" );
 
 my $ticket2 = RT::Ticket->new( $cu );
@@ -57,6 +70,9 @@ ok( $ticket2->Subject, "ticket load by user" );
 ok( !$rv, "user can't add self as Cc" );
 ($rv, $msg) = $ticket2->AddWatcher( Type => 'Requestor', PrincipalId => $user->PrincipalId );
 ok( !$rv, "user can't add self as Requestor" );
+($rv, $msg) = $ticket2->AddWatcher( Type => $custom_role_type, PrincipalId => $user->PrincipalId );
+ok( !$rv, "user can't add self to Custom Role $msg" );
+
 $principal->GrantRight( Right => 'Watch'  , Object => $queue );
 ok(  $user->HasRight( Right => 'Watch',        Object => $queue ), "user can watch queue tickets" );
 ($rv, $msg) = $ticket2->AddWatcher( Type => 'Cc', PrincipalId => $user->PrincipalId );
@@ -125,3 +141,4 @@ ok(  $rv, "user can add self as Cc by Email" );
 ($rv, $msg) = $queue2->AddWatcher( Type => 'Requestor', Email => $user->EmailAddress );
 ok(  $rv, "user can add self as Requestor by Email" );
 
+done_testing();

commit 0db3c306c02f09726d46485daea6fce70a4e758c
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu Sep 5 14:29:15 2019 -0400

    Fall through to false on watcher rights check
    
    _HasModifyWatcherRight previously checked known rights cases
    and returned false when the user didn't have rights for the
    corresponding change. If nothing matched, the fall through return
    value was true, which meant that any watcher change that didn't
    match was allowed. All new custom roles won't match the existing
    watcher type checks, so users could by default edit custom roles
    even without ModifyTicket.
    
    Reverse the logic to return true for know rights and types and
    default to 0 for any unknown cases. At this point, ModifyTicket
    is required to modify custom roles.

diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 38708af5f..cda2f65c3 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -621,11 +621,11 @@ sub _HasModifyWatcherRight {
     # If the watcher isn't the current user then the current user has no right
     return 0 unless $self->CurrentUser->PrincipalId == $principal->id;
     # If it's an AdminCc and they don't have 'WatchAsAdminCc', bail
-    return 0 if $type eq 'AdminCc' and not $self->CurrentUserHasRight('WatchAsAdminCc');
+    return 1 if $type eq 'AdminCc' and $self->CurrentUserHasRight('WatchAsAdminCc');
     # If it's a Requestor or Cc and they don't have 'Watch', bail
-    return 0 if ($type eq "Cc" or $type eq 'Requestor')
-        and not $self->CurrentUserHasRight('Watch');
-    return 1;
+    return 1 if ($type eq "Cc" or $type eq 'Requestor')
+        and $self->CurrentUserHasRight('Watch');
+    return 0;
 }
 
 

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