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

Craig Kaiser craig at bestpractical.com
Thu Aug 9 16:18:12 EDT 2018


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

- Log -----------------------------------------------------------------
commit 095f0d387ee2ecd6c60156153195c66323bc7203
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..703f2007d 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(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";
+
+    my $group = $asset->RoleGroup('Owner');
+    ok $group->id, "Loaded role group Owner for " . ref($asset);
+
+    $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) = $group->AddMember($user_a->PrincipalId);
+    ok $ret, $msg;
+
+    is $asset->CatalogObj->Lifecycle, 'assets', 'Successfully loaded lifecycle with rights check at role level';
+}
+
 done_testing;

commit 6c6bd8e225887a8f9bf1f4140d9ab82385ed46bf
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..5ccff51b1 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -471,6 +471,26 @@ 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;
+
+    return ( $self->__Value('Lifecycle') ) unless ! $context_obj || $context_obj->CatalogObj->Id ne $self->Id
+        || ! ( $context_obj->CurrentUserHasRight('ShowCatalog') and $context_obj->CurrentUserHasRight('AdminCatalog') );
+
+    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..68f227389 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -784,7 +784,15 @@ sub _Set {
     return ( $ret, $msg );
 }
 
+sub Lifecycle {
+    my $self        = shift;
+    my $context_obj = shift;
 
+    return ( $self->__Value('Lifecycle') ) unless ! $context_obj || ($context_obj->QueueObj->Id ne $self->Id)
+        || ! $context_obj->CurrentUserHasRight('SeeQueue');
+
+    return ( $self->_Value('Lifecycle') );
+}
 
 sub _Value {
     my $self = shift;
@@ -889,9 +897,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..9f7caeb57 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,8 @@ sub LifecycleObj {
         return RT::Lifecycle->Load( Type => $type );
     }
 
-    my $name = $self->Lifecycle || $fallback;
+    my $name = $self->Lifecycle($context_obj) || $fallback;
+    RT::Logger->debug('Failing back to default lifecycle value') unless $self->Lifecycle($context_obj);
     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 ac50136c32d5169f623c8c869917f99eb4754d4e
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 703f2007d..c70293ce5 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";
@@ -296,11 +296,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) = $group->AddMember($user_a->PrincipalId);
     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($asset);
+    is $lifecycle->Name, 'assets', 'Test LifecycleObj method';
+
+    $lifecycle = $asset->CatalogObj->LifecycleObj;
+    is $lifecycle->Name, 'assets', 'Test LifecycleObj method';
 }
 
 done_testing;

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


More information about the rt-commit mailing list