[Rt-commit] rt branch, 4.4-trunk, updated. rt-4.4.4-109-ga59bdd9bf2

Jim Brandt jbrandt at bestpractical.com
Thu Jun 25 14:32:05 EDT 2020


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 at 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 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 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 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 ce833d0d0303b53ff8e35a2518bf2f3e794cd346
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'};

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

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


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


More information about the rt-commit mailing list