[Rt-commit] rt branch, 4.4/role-lifecycle-right-check, updated. rt-4.4.3-29-gc3446c9ca

Craig Kaiser craig at bestpractical.com
Thu Aug 2 17:46:10 EDT 2018


The branch, 4.4/role-lifecycle-right-check has been updated
       via  c3446c9ca7c7c847a7018e328d2e4ee10ebf026a (commit)
       via  8a2bd1520a4c98d155668d15505db26305a7ca33 (commit)
       via  78def29059bb87dcb3b43ad19256c10143cc8eb1 (commit)
       via  c8c21eeb775884367d45ffe8091eeb14c5316f1f (commit)
       via  4a96c5d99e5c820c61de7cb937f4cc5fa6391d40 (commit)
      from  b770ab1c7acced4672d97923937544eea9815f72 (commit)

Summary of changes:
 lib/RT/Catalog.pm               | 24 +++++++++++++++++++++
 lib/RT/Queue.pm                 | 21 ++++++++++--------
 lib/RT/Record/Role/Lifecycle.pm | 10 +++++----
 t/lifecycles/basics.t           | 47 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 88 insertions(+), 14 deletions(-)

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

    Test lifecycle rights at the ticket level

diff --git a/t/lifecycles/basics.t b/t/lifecycles/basics.t
index be3a8579e..d9a43caf4 100644
--- a/t/lifecycles/basics.t
+++ b/t/lifecycles/basics.t
@@ -264,7 +264,7 @@ diag "Role rights are checked for lifecycles at ticket level";
     ($ret, $msg) = $ticket->Load($id);
     ok $ticket->id, 'Loaded ticket in user context';
 
-    is $ticket->QueueObj->Lifecycle($ticket), undef;
+    is $ticket->QueueObj->Lifecycle($ticket), undef, "user_a can\'t see lifecycle without SeeQueue";
 
     ($ret, $msg) = $ticket->AddWatcher(
         Type        => 'AdminCc',
@@ -272,7 +272,7 @@ diag "Role rights are checked for lifecycles at ticket level";
     );
     ok $ret, 'user_a is now AdminCc';
 
-    is $ticket->QueueObj->Lifecycle($ticket), 'default', 'Successfully loaded lifecycle';
+    is $ticket->QueueObj->Lifecycle, 'default', 'Successfully loaded lifecycle';
 
 }
 

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

    Check lifecycle view rights on queue and role levels
    
    If the context object is not checked for 'SeeQueue' then users who have
    their rights from a role may not be able to see the queue lifecycle.
    This will result in users not being able to set statuses that they
    normally should be able to.

diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index a192e4eec..5828597bd 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -786,16 +786,16 @@ sub _Set {
 
 sub Lifecycle {
     my $self    = shift;
-    my $context = shift;
+    my $context_obj = shift;
 
     # Check context level rights if we do not have Queue level rights
-    if ( $context && ! $self->CurrentUserHasRight('SeeQueue') ) {
-        return (undef) unless $context->CurrentUserHasRight('SeeQueue');
-    } elsif ( ! $self->CurrentUserHasRight('SeeQueue') ) {
-        return (undef);
+    if ( $self->CurrentUserHasRight('SeeQueue') ) {
+        return ( $self->__Value('Lifecycle') );
+    } elsif ( $context_obj && $context_obj->CurrentUserHasRight('SeeQueue') ) {
+        return ( $self->__Value('Lifecycle') );
     }
 
-    return ( $self->__Value('Lifecycle') );
+    return ( undef );
 }
 
 sub _Value {
@@ -901,10 +901,13 @@ Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
 =cut
 
 
-=head2 Lifecycle CONTEXT
+=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, provide a context object
-to check at a level other than the Queue level.
 (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 3fa5c9acc..9f7caeb57 100644
--- a/lib/RT/Record/Role/Lifecycle.pm
+++ b/lib/RT/Record/Role/Lifecycle.pm
@@ -83,17 +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 = shift;
+    my $context_obj = shift || undef;
     my $type = $self->LifecycleType;
     my $fallback = $self->_Accessible( Lifecycle => "default" );
 
@@ -101,7 +102,8 @@ sub LifecycleObj {
         return RT::Lifecycle->Load( Type => $type );
     }
 
-    my $name = $self->Lifecycle($context) || $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(

commit 78def29059bb87dcb3b43ad19256c10143cc8eb1
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Thu Aug 2 12:08:30 2018 -0400

    Update lifecycle right test for new context parameter

diff --git a/t/lifecycles/basics.t b/t/lifecycles/basics.t
index d9a43caf4..7ee802a5d 100644
--- a/t/lifecycles/basics.t
+++ b/t/lifecycles/basics.t
@@ -272,6 +272,11 @@ diag "Role rights are checked for lifecycles at ticket level";
     );
     ok $ret, 'user_a is now AdminCc';
 
+    is $ticket->QueueObj->Lifecycle($ticket), 'default', 'Successfully loaded lifecycle';
+
+    $ticket = RT::Ticket->new(RT->SystemUser);
+    ($ret, $msg) = $ticket->Load($id);
+    ok $ticket->id, 'Loaded ticket in user context';
     is $ticket->QueueObj->Lifecycle, 'default', 'Successfully loaded lifecycle';
 
 }

commit 8a2bd1520a4c98d155668d15505db26305a7ca33
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Thu Aug 2 17:38:40 2018 -0400

    Test lifecycle role context rights for assets

diff --git a/t/lifecycles/basics.t b/t/lifecycles/basics.t
index 7ee802a5d..8550e08ce 100644
--- a/t/lifecycles/basics.t
+++ b/t/lifecycles/basics.t
@@ -278,7 +278,47 @@ diag "Role rights are checked for lifecycles at ticket level";
     ($ret, $msg) = $ticket->Load($id);
     ok $ticket->id, 'Loaded ticket in user context';
     is $ticket->QueueObj->Lifecycle, 'default', 'Successfully loaded lifecycle';
-
 }
 
+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)] },
+        { Principal => 'Everyone',  Right => [qw(ModifyAsset WatchAsAdminCc)] },
+    );
+
+    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;
+
+    $asset = RT::Asset->new($user_a);
+    $asset->Load($id);
+    ok $asset->id, 'Loaded asset in user_a context';
+
+    is $asset->CatalogObj->Lifecycle($asset), undef, "user_a can\'t see lifecycle without ShowCatalog";
+    is $asset->CatalogObj->Lifecycle, undef, "user_a can\'t see lifecycle without ShowCatalog";
+
+    $asset = RT::Asset->new(RT->SystemUser);
+    $asset->Load($id);
+    ok $asset->id, 'Loaded asset in user_a context';
+
+    my $group = $asset->RoleGroup('Owner');
+    ok $group->id, "Loaded role group Owner for " . ref($asset);
+
+    ($ret, $msg) = $group->AddMember($user_a->PrincipalId);
+    ok $ret, $msg;
+
+    $asset = RT::Asset->new($user_a);
+    $asset->Load($id);
+    ok $asset->id, 'Loaded asset in user_a context';
+
+    is $asset->CatalogObj->Lifecycle($asset), 'assets', 'Successfully loaded lifecycle';
+
+}
 done_testing;

commit c3446c9ca7c7c847a7018e328d2e4ee10ebf026a
Author: Craig Kaiser <craig at bestpractical.com>
Date:   Thu Aug 2 17:39:14 2018 -0400

    Check role level rights for asset lifecycle

diff --git a/lib/RT/Catalog.pm b/lib/RT/Catalog.pm
index b34f196ec..68c63ec99 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -471,6 +471,30 @@ sub _Set {
     return ($txn_id, scalar $txn->BriefDescription);
 }
 
+=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 ShowCatalog.
+
+(In the database, Lifecycle is stored as varchar(32).)
+=cut
+
+sub Lifecycle {
+    my $self    = shift;
+    my $context_obj = shift;
+
+    # Check context level rights if we do not have Catalog level rights
+    if ( $self->CurrentUserHasRight('ShowCatalog') ) {
+        return ( $self->__Value('Lifecycle') );
+    } elsif ( $context_obj && $context_obj->CurrentUserHasRight('ShowCatalog') ) {
+        return ( $self->__Value('Lifecycle') );
+    }
+
+    return ( undef );
+}
+
 =head2 _Value
 
 Checks L</CurrentUserCanSee> before calling C<SUPER::_Value>.

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


More information about the rt-commit mailing list