[Rt-commit] rt branch, 4.4/custom-role-check-right, created. rt-4.4.3-122-g59b4401256
? sunnavy
sunnavy at bestpractical.com
Thu Dec 20 10:10:54 EST 2018
The branch, 4.4/custom-role-check-right has been created
at 59b440125640a4bd75f0d05cc00d8909229f3cb8 (commit)
- Log -----------------------------------------------------------------
commit e5d9033c3bf1dd82f6e32e39c5d33449c8f1bc95
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed May 2 05:06:00 2018 +0800
Make sure RT::Queue::CustomRoles returns an empty collection if no rights
Previously it did behave like an empty collection(i.e. when there are no
Limit or UnLimit calls on it), but in mason, we call extra
limits(LimitToSingleValue/LimitToMultipleValue) on it, which breaked it.
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 52b9f10330..a05bb1c83f 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -483,6 +483,9 @@ sub CustomRoles {
$roles->LimitToObjectId( $self->Id );
$roles->ApplySortOrder;
}
+ else {
+ $roles->Limit( FIELD => 'id', VALUE => 0, SUBCLAUSE => 'ACL' );
+ }
return ($roles);
}
commit e7c293b65e20067de41e6ab75ab87136eb6798b3
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed May 2 05:16:55 2018 +0800
Filter queue custom roles by checking current user's right
Previously, "Roles" rerturned all the roles registered for the object
and didn't consider if current user has right to see them or not.
"Role" and "HasRole" have the same issue.
This commit adds the rights check to AppliesToObjectPredicate, which is
called in "Roles". As "HasRole" calls "Roles" internally, it's fixed
automatically. "Role" is tweaked to add the rights check via "HasRole".
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index 8c9b084519..4283eed6bd 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -207,14 +207,16 @@ sub _RegisterAsRole {
return 1;
}
- # custom roles apply to queues, so canonicalize a ticket
- # into its queue
- if ($object->isa('RT::Ticket')) {
- $object = $object->QueueObj;
- }
+ if ( $object->isa('RT::Ticket') || $object->isa('RT::Queue') ) {
+ return 0 unless $object->CurrentUserHasRight('SeeQueue');
- if ($object->isa('RT::Queue')) {
- return $role->IsAdded($object->Id);
+ # custom roles apply to queues, so canonicalize a ticket
+ # into its queue
+ if ( $object->isa('RT::Ticket') ) {
+ $object = $object->QueueObj;
+ }
+
+ return $role->IsAdded( $object->Id );
}
return 0;
diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index 1a88793aec..6f79ea89f9 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -248,7 +248,10 @@ Returns an empty hashref if the role doesn't exist.
=cut
sub Role {
- return \%{ $_[0]->_ROLES->{$_[1]} || {} };
+ my $self = shift;
+ my $type = shift;
+ return {} unless $self->HasRole( $type );
+ return \%{ $self->_ROLES->{$type} };
}
=head2 Roles
@@ -287,7 +290,7 @@ sub Roles {
$ok }
grep { !$_->[1]{AppliesToObjectPredicate}
or $_->[1]{AppliesToObjectPredicate}->($self) }
- map { [ $_, $self->Role($_) ] }
+ map { [ $_, $self->_ROLES->{$_} ] }
keys %{ $self->_ROLES };
}
commit 59b440125640a4bd75f0d05cc00d8909229f3cb8
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed May 2 05:35:15 2018 +0800
Test custom roles for user rights
diff --git a/t/customroles/basic.t b/t/customroles/basic.t
index d703eee395..0ab458483b 100644
--- a/t/customroles/basic.t
+++ b/t/customroles/basic.t
@@ -133,9 +133,30 @@ diag 'roles now added to queues' if $ENV{'TEST_VERBOSE'};
is_deeply([sort RT::Ticket->Roles], ['AdminCc', 'Cc', 'Owner', 'RT::CustomRole-1', 'RT::CustomRole-2', 'Requestor'], 'Ticket->Roles');
is_deeply([sort RT::Queue->ManageableRoleGroupTypes], ['AdminCc', 'Cc', 'RT::CustomRole-2'], 'Queue->ManageableRoleTypes');
+ my $alice = RT::Test->load_or_create_user( EmailAddress => 'alice at example.com' );
+ for my $q ( $general, $inbox, $specs, $development ) {
+ my $queue = RT::Queue->new( $alice );
+ $queue->Load( $q->id );
+ ok( $queue->id, 'Load queue' );
+
+ my $qroles = $queue->CustomRoles;
+ is( $qroles->Count, 0, 'No custom roles for users without rights' );
+ $qroles->LimitToSingleValue;
+ is( $qroles->Count, 0, 'No single custom roles for users without rights' );
+
+ is_deeply( [ sort $queue->Roles ], [ 'AdminCc', 'Cc', 'Owner', 'Requestor' ], 'Roles' );
+ is_deeply( [ sort $queue->ManageableRoleGroupTypes ], [ 'AdminCc', 'Cc' ], 'ManageableRoleTypes' );
+ ok( !$queue->HasRole( $engineer->GroupType ), 'HasRole returns false for users without rights' );
+ ok( !$queue->HasRole( $sales->GroupType ), 'HasRole returns false for users without rights' );
+ }
+
+ RT::Test->set_rights( { Principal => $alice->PrincipalObj, Right => ['SeeQueue'] } );
+
+ my @users = ( RT->SystemUser, $alice );
+ for my $user ( @users ) {
# General
{
- my $roles = RT::CustomRoles->new(RT->SystemUser);
+ my $roles = RT::CustomRoles->new($user);
$roles->LimitToObjectId($general->Id);
is($roles->Count, 0, 'no roles for General');
@@ -152,7 +173,7 @@ diag 'roles now added to queues' if $ENV{'TEST_VERBOSE'};
# Inbox
{
- my $roles = RT::CustomRoles->new(RT->SystemUser);
+ my $roles = RT::CustomRoles->new($user);
$roles->LimitToObjectId($inbox->Id);
is($roles->Count, 1, 'one role for Inbox');
is($roles->Next->Name, 'Sales-' . $$, 'and the one role is Sales');
@@ -171,7 +192,7 @@ diag 'roles now added to queues' if $ENV{'TEST_VERBOSE'};
# Specs
{
- my $roles = RT::CustomRoles->new(RT->SystemUser);
+ my $roles = RT::CustomRoles->new($user);
$roles->LimitToObjectId($specs->Id);
$roles->OrderBy(
FIELD => 'id',
@@ -200,7 +221,7 @@ diag 'roles now added to queues' if $ENV{'TEST_VERBOSE'};
# Development
{
- my $roles = RT::CustomRoles->new(RT->SystemUser);
+ my $roles = RT::CustomRoles->new($user);
$roles->LimitToObjectId($development->Id);
is($roles->Count, 1, 'one role for Development');
is($roles->Next->Name, 'Engineer-' . $$, 'and the one role is sales');
@@ -216,6 +237,7 @@ diag 'roles now added to queues' if $ENV{'TEST_VERBOSE'};
is_deeply([sort $development->ManageableRoleGroupTypes], ['AdminCc', 'Cc'], 'Development->ManageableRoleTypes');
is_deeply([grep { $development->IsManageableRoleGroupType($_) } 'AdminCc', 'Cc', 'Owner', 'RT::CustomRole-1', 'RT::CustomRole-2', 'Requestor', 'Nonexistent'], ['AdminCc', 'Cc'], 'Development IsManageableRoleGroupType');
}
+ }
}
diag 'role names' if $ENV{'TEST_VERBOSE'};
-----------------------------------------------------------------------
More information about the rt-commit
mailing list