[Rt-commit] rt branch, 4.4/add-watcher-custom-role, created. rt-4.4.4-61-gb9828c28e
Jim Brandt
jbrandt at bestpractical.com
Thu Sep 5 15:21:15 EDT 2019
The branch, 4.4/add-watcher-custom-role has been created
at b9828c28eae8110d191eeea6f8b5f61085a2cd7c (commit)
- Log -----------------------------------------------------------------
commit 81e18339bcf71087d3f85986f1aabb01e85a3620
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 Modify rights
Tests custom role watchers on tickets and queues.
diff --git a/t/ticket/add-watchers.t b/t/ticket/add-watchers.t
index 12b0bb3e2..c7cb9196f 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 );
@@ -107,6 +123,9 @@ ok( $rv, "user loaded queue" );
ok( !$rv, "user can't add self as Cc" );
($rv, $msg) = $queue2->AddWatcher( Type => 'Requestor', PrincipalId => $user->PrincipalId );
ok( !$rv, "user can't add self as Requestor" );
+($rv, $msg) = $queue2->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 queues" );
($rv, $msg) = $queue2->AddWatcher( Type => 'Cc', PrincipalId => $user->PrincipalId );
@@ -114,6 +133,11 @@ ok( $rv, "user can add self as Cc by PrincipalId" );
($rv, $msg) = $queue2->AddWatcher( Type => 'Requestor', PrincipalId => $user->PrincipalId );
ok( $rv, "user can add self as Requestor by PrincipalId" );
+$principal->GrantRight( Right => 'ModifyQueueWatchers' , Object => $queue );
+ok( $user->HasRight( Right => 'ModifyQueueWatchers', Object => $queue ), "user can modify all queue watchers" );
+($rv, $msg) = $queue2->AddWatcher( Type => $custom_role_type, PrincipalId => $user->PrincipalId );
+ok( $rv, "user can add self to Custom Role $msg" );
+
# remove user and try adding with Email address
($rv, $msg) = $queue->DeleteWatcher( Type => 'Cc', PrincipalId => $user->PrincipalId );
ok( $rv, "watcher removed by PrincipalId" );
@@ -125,3 +149,5 @@ 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 b9828c28eae8110d191eeea6f8b5f61085a2cd7c
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 or ModifyQueueWatchers.
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 on tickets and
ModifyQueueWatchers on queues.
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index bd486ae9c..a57a4f9ee 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -519,12 +519,14 @@ sub _HasModifyWatcherRight {
return 1 if $self->CurrentUserHasRight('ModifyQueueWatchers');
# 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');
- # 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;
+ # If it's an AdminCc and they have 'WatchAsAdminCc', they can modify
+ return 1 if $type eq 'AdminCc' and $self->CurrentUserHasRight('WatchAsAdminCc');
+ # If it's a Requestor or Cc and they have 'Watch', they can modify
+ return 1 if ($type eq "Cc" or $type eq 'Requestor')
+ and $self->CurrentUserHasRight('Watch');
+
+ # Unknown type, so default to denied.
+ return 0;
}
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 38708af5f..81224831c 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -620,12 +620,14 @@ sub _HasModifyWatcherRight {
return 1 if $self->CurrentUserHasRight('ModifyTicket');
# 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');
- # 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;
+ # If it's an AdminCc and they have 'WatchAsAdminCc', they can modify
+ return 1 if $type eq 'AdminCc' and $self->CurrentUserHasRight('WatchAsAdminCc');
+ # If it's a Requestor or Cc and they have 'Watch', they can modify
+ return 1 if ($type eq "Cc" or $type eq 'Requestor')
+ and $self->CurrentUserHasRight('Watch');
+
+ # Unknown type, so default to denied.
+ return 0;
}
-----------------------------------------------------------------------
More information about the rt-commit
mailing list