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

[? sunnavy]

The branch, 4.4/custom-role-check-right has been created
        at  afb2699e4083e1a7b18b9e26327b25e21aaa3ae6 (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 94a947f70308f4eaea05f62deaab10025428cd16
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..b71560f99 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,12 @@ sub Roles {
     my $self = shift;
     my %attr = @_;
 
+    my %custom_role;
+    if ( blessed( $self ) && $self->can( 'CustomRoles' ) ) {
+        %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 +296,8 @@ sub Roles {
                 $ok }
             grep { !$_->[1]{AppliesToObjectPredicate}
                  or $_->[1]{AppliesToObjectPredicate}->($self) }
-             map { [ $_, $self->Role($_) ] }
+            grep { !$_->[ 1 ]{UserDefined} or !%custom_role or $custom_role{ $_->[ 0 ] } }
+             map { [ $_, $self->_ROLES->{$_} ] }
             keys %{ $self->_ROLES };
 }
 

commit afb2699e4083e1a7b18b9e26327b25e21aaa3ae6
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..548224b70 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 );
+        $q->Load( $q->id );
+        ok( $q->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'};

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