[Rt-commit] rt branch, 4.4/role-lifecycle-right-check, created. rt-4.4.3-26-g98a358961

Craig Kaiser craig at bestpractical.com
Fri Aug 10 16:07:12 EDT 2018


The branch, 4.4/role-lifecycle-right-check has been created
        at  98a35896167271bee692721c30e1376dff3f578f (commit)

- Log -----------------------------------------------------------------
commit eee86e6cc17b1c998797dce7914f75a2523a1ea4
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Wed Aug 1 16:42:09 2018 -0400

    Test lifecycle rights check at context object level

diff --git a/t/lifecycles/basics.t b/t/lifecycles/basics.t
index f53f077cc..2927af67f 100644
--- a/t/lifecycles/basics.t
+++ b/t/lifecycles/basics.t
@@ -242,4 +242,65 @@ diag "'!inactive -> inactive' actions are shown even if ticket has unresolved de
     );
 }
 
+diag "Role rights are checked for lifecycles at ticket level";
+{
+
+    my $user_a = RT::Test->load_or_create_user(
+        Name => 'user_a', Password => 'password',
+    );
+    ok $user_a && $user_a->id, 'loaded or created user';
+
+    RT::Test->set_rights(
+        { Principal => 'AdminCc',  Right => [qw(SeeQueue)] },
+        { Principal => 'Everyone', Right => [qw(WatchAsAdminCc)] },
+    );
+
+    my $ticket = RT::Test->create_ticket(Queue => 'General');
+    ok $ticket->id, 'Created new ticket';
+    my $id = $ticket->id;
+
+    is $ticket->QueueObj->Lifecycle, 'default', 'Successfully loaded lifecycle';
+    $ticket->AddWatcher(Type => 'AdminCc', PrincipalId => $user_a->PrincipalId);
+
+    $ticket = RT::Ticket->new($user_a);
+    my ($ret, $msg) = $ticket->Load($id);
+    ok $ticket->id, 'Loaded ticket in user context';
+
+    is $ticket->QueueObj->Lifecycle, 'default', "Rights check at ticket level passes";
+}
+
+diag "Role rights are checked for lifecycles at asset level";
+{
+    my $user_a = RT::Test->load_or_create_user(
+        Name => 'user_a', Password => 'password',
+    );
+    ok $user_a && $user_a->id, 'loaded or created user';
+
+    RT::Test->set_rights(
+        { Principal => 'Owner',  Right => [qw(ShowCatalog AdminCatalog)] },
+        { Principal => 'Everyone',  Right => [qw(ShowAsset ModifyAsset)] },
+    );
+
+    my $asset = RT::Asset->new(RT->SystemUser);
+    my ($ret, $msg) = $asset->Create(Catalog => 'General assets');
+    ok $asset->id, 'Created new asset';
+    my $id = $asset->id;
+
+    is $asset->CatalogObj->Lifecycle, 'assets', "System user can load asset without context object";
+
+    $asset = RT::Asset->new($user_a);
+    $asset->Load($id);
+    ok $asset->id, 'Loaded asset in user_a context';
+
+    is $asset->CatalogObj->Lifecycle, undef, "user_a can\'t see lifecycle without ShowCatalog and AdminCatalog";
+
+    ($ret, $msg) = $asset->AddRoleMember(Type => 'Owner', User => $user_a);
+    ok $ret, $msg;
+
+    is $asset->CatalogObj->Lifecycle, 'assets', 'Successfully loaded lifecycle with rights check at role level';
+
+    my $lifecycle = $asset->CatalogObj->LifecycleObj;
+    is $lifecycle->Name, 'assets', 'Test LifecycleObj method';
+}
+
 done_testing;

commit e4071599e6642f7ac35578bcc74652f5de047a8e
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Wed Aug 1 14:53:32 2018 -0400

    Add method for lifecycle rights check
    
    If the context object is not checked for relevant rights then users who have
    their rights from a role may not be able to see the object's lifecycle.
    This will result in users not being able to set statuses that they
    normally should be able to.

diff --git a/lib/RT/Catalog.pm b/lib/RT/Catalog.pm
index b34f196ec..12ed0435e 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -471,6 +471,28 @@ sub _Set {
     return ($txn_id, scalar $txn->BriefDescription);
 }
 
+=head2 Lifecycle [CONTEXT_OBJ]
+
+Returns the current value of Lifecycle.
+
+Provide an optional asset object as context to check role-level rights
+in addition to catalog-level rights for ShowCatalog and AdminCatalog.
+
+(In the database, Lifecycle is stored as varchar(32).)
+=cut
+
+sub Lifecycle {
+    my $self    = shift;
+    my $context_obj = shift;
+
+    if ( $context_obj && $context_obj->CatalogObj->Id eq $self->Id &&
+        ( $context_obj->CurrentUserHasRight('ShowCatalog') or $context_obj->CurrentUserHasRight('AdminCatalog') ) ) {
+        return ( $self->__Value('Lifecycle') );
+    }
+
+    return ( $self->_Value('Lifecycle') );
+}
+
 =head2 _Value
 
 Checks L</CurrentUserCanSee> before calling C<SUPER::_Value>.
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index b9eaec302..52b9f1033 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -784,7 +784,16 @@ sub _Set {
     return ( $ret, $msg );
 }
 
+sub Lifecycle {
+    my $self        = shift;
+    my $context_obj = shift;
 
+    if ( $context_obj && $context_obj->QueueObj->Id eq $self->Id && $context_obj->CurrentUserHasRight('SeeQueue') ) {
+        return ( $self->__Value('Lifecycle') );
+    }
+
+    return ( $self->_Value('Lifecycle') );
+}
 
 sub _Value {
     my $self = shift;
@@ -889,9 +898,13 @@ Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
 =cut
 
 
-=head2 Lifecycle
+=head2 Lifecycle [CONTEXT_OBJ]
+
+Returns the current value of Lifecycle.
+
+Provide an optional ticket object as context to check role-level rights
+in addition to queue-level rights for SeeQueue.
 
-Returns the current value of Lifecycle. 
 (In the database, Lifecycle is stored as varchar(32).)
 
 
diff --git a/lib/RT/Record/Role/Lifecycle.pm b/lib/RT/Record/Role/Lifecycle.pm
index 6ca07a6ff..014e0746d 100644
--- a/lib/RT/Record/Role/Lifecycle.pm
+++ b/lib/RT/Record/Role/Lifecycle.pm
@@ -83,16 +83,18 @@ requires 'LifecycleType';
 
 =head1 PROVIDES
 
-=head2 LifecycleObj
+=head2 LifecycleObj [CONTEXT_OBJ]
 
 Returns an L<RT::Lifecycle> object for this record's C<Lifecycle>.  If called
 as a class method, returns an L<RT::Lifecycle> object which is an aggregation
-of all lifecycles of the appropriate type.
+of all lifecycles of the appropriate type. Pass an additional L<CONTEXT_OBJ>
+to check rights at the objects context.
 
 =cut
 
 sub LifecycleObj {
     my $self = shift;
+    my $context_obj = shift || undef;
     my $type = $self->LifecycleType;
     my $fallback = $self->_Accessible( Lifecycle => "default" );
 
@@ -100,7 +102,11 @@ sub LifecycleObj {
         return RT::Lifecycle->Load( Type => $type );
     }
 
-    my $name = $self->Lifecycle || $fallback;
+    my $name = $self->Lifecycle($context_obj);
+    if ( !$name ) {
+        RT::Logger->debug('Failing back to default lifecycle value');
+        $name = $fallback;
+    }
     my $res  = RT::Lifecycle->Load( Name => $name, Type => $type );
     unless ( $res ) {
         RT->Logger->error(
diff --git a/lib/RT/Record/Role/Status.pm b/lib/RT/Record/Role/Status.pm
index 7555b886d..3ee90c2f1 100644
--- a/lib/RT/Record/Role/Status.pm
+++ b/lib/RT/Record/Role/Status.pm
@@ -116,7 +116,7 @@ of all lifecycles of the appropriate type.
 sub LifecycleObj {
     my $self = shift;
     my $obj  = $self->LifecycleColumn . "Obj";
-    return $self->$obj->LifecycleObj;
+    return $self->$obj->LifecycleObj($self);
 }
 
 =head2 Lifecycle

commit 53d766b5b0601c2138a65d6da090a8386edf51c0
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Wed Aug 1 16:42:09 2018 -0400

    Test lifecycle rights by passing context object

diff --git a/t/lifecycles/basics.t b/t/lifecycles/basics.t
index 2927af67f..00ba375db 100644
--- a/t/lifecycles/basics.t
+++ b/t/lifecycles/basics.t
@@ -266,7 +266,7 @@ diag "Role rights are checked for lifecycles at ticket level";
     my ($ret, $msg) = $ticket->Load($id);
     ok $ticket->id, 'Loaded ticket in user context';
 
-    is $ticket->QueueObj->Lifecycle, 'default', "Rights check at ticket level passes";
+    is $ticket->QueueObj->Lifecycle($ticket), 'default', "Rights check for role at ticket level passes";
 }
 
 diag "Role rights are checked for lifecycles at asset level";
@@ -293,14 +293,18 @@ diag "Role rights are checked for lifecycles at asset level";
     ok $asset->id, 'Loaded asset in user_a context';
 
     is $asset->CatalogObj->Lifecycle, undef, "user_a can\'t see lifecycle without ShowCatalog and AdminCatalog";
+    is $asset->CatalogObj->Lifecycle($asset), undef, "user_a can\'t see lifecycle without ShowCatalog and AdminCatalog";
 
     ($ret, $msg) = $asset->AddRoleMember(Type => 'Owner', User => $user_a);
     ok $ret, $msg;
 
-    is $asset->CatalogObj->Lifecycle, 'assets', 'Successfully loaded lifecycle with rights check at role level';
+    is $asset->CatalogObj->Lifecycle($asset), 'assets', 'Successfully loaded lifecycle with rights check at role level';
 
     my $lifecycle = $asset->CatalogObj->LifecycleObj;
     is $lifecycle->Name, 'assets', 'Test LifecycleObj method';
+
+    $lifecycle = $asset->CatalogObj->LifecycleObj($asset);
+    is $lifecycle->Name, 'assets', 'Test LifecycleObj method';
 }
 
 done_testing;

commit 98a35896167271bee692721c30e1376dff3f578f
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Fri Aug 10 14:59:38 2018 -0400

    Test web UI for ticket status when rights granted at role level

diff --git a/t/web/lifecycle_rights.t b/t/web/lifecycle_rights.t
new file mode 100644
index 000000000..efa9c93fa
--- /dev/null
+++ b/t/web/lifecycle_rights.t
@@ -0,0 +1,57 @@
+use strict;
+use warnings;
+
+BEGIN {require './t/lifecycles/utils.pl'};
+
+diag 'Test web UI for ticket status when rights granted at role level';
+{
+    my ( $url, $agent ) = RT::Test->started_ok;
+
+    my $delivery = RT::Test->load_or_create_queue(
+        Name => 'delivery',
+        Lifecycle => 'delivery',
+    );
+    ok $delivery && $delivery->id, 'loaded or created a queue';
+
+    my $ticket = RT::Test->create_ticket(Queue => 'Delivery');
+    ok $ticket && $ticket->Id;
+
+    my $user_a = RT::Test->load_or_create_user(
+        Name => 'user_a', Password => 'password', Privileged => 1,
+    );
+    ok $user_a && $user_a->id, 'loaded or created user';
+
+    RT::Test->set_rights(
+        { Principal => 'AdminCc',  Right  => [qw(SeeQueue)] },
+        { Principal => 'Everyone',  Right => [qw(ModifyTicket ShowTicket)] },
+    );
+
+    ok( $agent->login( 'user_a' , 'password' ), 'logged in as user_a');
+
+    $agent->get_ok($url . '/Ticket/Modify.html?id=' . $ticket->Id);
+    $agent->form_name('TicketModify');
+
+    my ($inputs) = $agent->find_all_inputs(
+        type       => 'option',
+        name       => 'Status',
+    );
+
+    # Greater equal to 2 because you can change to current status and current status (Unchanged) without 'SeeQueu'
+    ok $inputs->value_names eq 2, 'We are unable to transition to other statuses without role rights';
+
+    ok $ticket->AddRoleMember(Type => 'AdminCc', User => $user_a);
+
+    $agent->get_ok($url . '/Ticket/Modify.html?id=' . $ticket->Id);
+    $agent->form_name('TicketModify');
+
+    # Refresh page after rights update
+    ($inputs) = $agent->find_all_inputs(
+        type       => 'option',
+        name       => 'Status',
+    );
+
+    ok $inputs->value_names > 2, 'We are able to transition to other statuses with role rights';
+
+}
+
+done_testing;

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


More information about the rt-commit mailing list