[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