Mailing List Archive

rt branch, 4.4-trunk, updated. rt-4.4.4-109-ga59bdd9bf2
The branch, 4.4-trunk has been updated
via a59bdd9bf2d9277ddf3adc9a85e6753cc98cda8f (commit)
via ce833d0d0303b53ff8e35a2518bf2f3e794cd346 (commit)
via 926c20ea994c1f0794e02d8c30aafdc4523721c3 (commit)
via 1f67e8fa35e6fa2b8a4803981f47f39ad5bcc0c3 (commit)
via 34860e5c0407e253cff8da555ce41137583dd532 (commit)
from 1b7a58004491ca9f87e93f71465c0faae0c5dd50 (commit)

Summary of changes:
lib/RT/CustomRole.pm | 16 +++++++++-------
lib/RT/Queue.pm | 3 +++
lib/RT/Record/Role/Roles.pm | 7 +++++--
t/customroles/basic.t | 30 ++++++++++++++++++++++++++----
4 files changed, 43 insertions(+), 13 deletions(-)

- Log -----------------------------------------------------------------
commit 34860e5c0407e253cff8da555ce41137583dd532
Author: sunnavy <sunnavy@bestpractical.com>
Date: Fri May 4 02:10:07 2018 +0800

Avoid duplicated items in index.html

Run converter twice added duplicated items to index.html, and undef
contents_file could fix this. The index call($converter->index(0)) can't
fix it BTW.

diff --git a/devel/tools/rt-static-docs b/devel/tools/rt-static-docs
index e5b26d7c69..25350ae7c7 100755
--- a/devel/tools/rt-static-docs
+++ b/devel/tools/rt-static-docs
@@ -186,6 +186,7 @@ system_chmod("+x", $_) for <docs/UPGRADING*>;
$converter->batch_convert( \@dirs, $opts{to} );

# Run it again to make sure local links are linked correctly
+$converter->contents_file(undef);
$converter->batch_convert( \@dirs, $opts{to} );

# Remove execution bit from workaround above

commit 1f67e8fa35e6fa2b8a4803981f47f39ad5bcc0c3
Author: sunnavy <sunnavy@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 caused
issues.

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 926c20ea994c1f0794e02d8c30aafdc4523721c3
Author: sunnavy <sunnavy@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 ce833d0d0303b53ff8e35a2518bf2f3e794cd346
Author: sunnavy <sunnavy@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@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'};

commit a59bdd9bf2d9277ddf3adc9a85e6753cc98cda8f
Merge: 1b7a580044 ce833d0d03
Author: Jim Brandt <jbrandt@bestpractical.com>
Date: Thu Jun 25 14:14:30 2020 -0400

Merge branch '4.4/custom-role-check-right' into 4.4-trunk


-----------------------------------------------------------------------
_______________________________________________
rt-commit mailing list
rt-commit@lists.bestpractical.com
http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-commit