[Rt-commit] rt branch, 4.4/custom-role-check-right, created. rt-4.4.2-231-g595288555

[? sunnavy]

The branch, 4.4/custom-role-check-right has been created
        at  595288555ead026b740c1d81bed0ccd83f63d887 (commit)

- Log -----------------------------------------------------------------
commit 9a55859e47ea55ea51a79a34ad05f72bb6418433
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 b9eaec302..854b4d21b 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 261c1c3cbb8d8f60f746f33d7fb4d19ec9634181
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed May 2 05:16:55 2018 +0800

    Hide custom roles from Objects' various Role methods if no rights

diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index 1a88793ae..1748c67e7 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
@@ -276,6 +279,14 @@ sub Roles {
     my $self = shift;
     my %attr = @_;
 
+    my %custom_role;
+    my $check_custom_role;
+    if ( blessed( $self ) && $self->can( 'CustomRoles' ) ) {
+        $check_custom_role = 1;
+        %custom_role =
+          map { 'RT::CustomRole-' . $_->id => 1 } @{ $self->CustomRoles->ItemsArrayRef };
+    }
+
     return   map { $_->[0] }
             sort {   $a->[1]{SortOrder} <=> $b->[1]{SortOrder}
                   or $a->[0] cmp $b->[0] }
@@ -287,7 +298,8 @@ sub Roles {
                 $ok }
             grep { !$_->[1]{AppliesToObjectPredicate}
                  or $_->[1]{AppliesToObjectPredicate}->($self) }
-             map { [ $_, $self->Role($_) ] }
+            grep { !$_->[ 1 ]{UserDefined} or !$check_custom_role or $custom_role{ $_->[ 0 ] } }
+             map { [ $_, $self->_ROLES->{$_} ] }
             keys %{ $self->_ROLES };
 }
 

commit 595288555ead026b740c1d81bed0ccd83f63d887
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed May 2 05:35:15 2018 +0800

    Test custom roles with user with/without rights

diff --git a/t/customroles/basic.t b/t/customroles/basic.t
index d703eee39..429de6a73 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( 'RT::CustomRole-1' ), 'HasRole returns false for users without rights' );
+        ok( !$queue->HasRole( 'RT::CustomRole-2' ), 'HasRole returns false for users without rights' );
+    }
+
+    $alice->PrincipalObj->GrantRight( 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'};

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