[Rt-commit] rt branch, 4.4/unpriv-lifecycle, created. rt-4.4.1-345-gbe649e7
Shawn Moore
shawn at bestpractical.com
Tue May 23 16:30:09 EDT 2017
The branch, 4.4/unpriv-lifecycle has been created
at be649e73f901a6bb928825aa6613729b960b2c2d (commit)
- Log -----------------------------------------------------------------
commit 9cc97018468a3d7686dc8843560b0520d75ba873
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Tue May 23 20:12:32 2017 +0000
Failing tests for lifecycles without SeeQueue
See I#32799
diff --git a/t/lifecycles/types.t b/t/lifecycles/types.t
index 84cfd86..20ab914 100644
--- a/t/lifecycles/types.t
+++ b/t/lifecycles/types.t
@@ -3,13 +3,13 @@ use warnings;
BEGIN {require './t/lifecycles/utils.pl'};
-is_deeply( [ RT::Lifecycle->ListAll ], [qw/ approvals default delivery /],
+is_deeply( [ RT::Lifecycle->ListAll ], [qw/ approvals default delivery triage /],
"Get the list of all lifecycles (implicitly for for tickets)");
-is_deeply( [ RT::Lifecycle->ListAll('ticket') ], [qw/ approvals default delivery /],
+is_deeply( [ RT::Lifecycle->ListAll('ticket') ], [qw/ approvals default delivery triage /],
"Get the list of all lifecycles for tickets");
-is_deeply( [ RT::Lifecycle->List], [qw/ default delivery /],
+is_deeply( [ RT::Lifecycle->List], [qw/ default delivery triage /],
"Get the list of lifecycles without approvals (implicitly for for tickets)");
-is_deeply( [ RT::Lifecycle->List('ticket') ], [qw/ default delivery /],
+is_deeply( [ RT::Lifecycle->List('ticket') ], [qw/ default delivery triage /],
"Get the list of lifecycles without approvals for tickets");
is_deeply( [ RT::Lifecycle->List('racecar') ], [qw/ racing /],
"Get the list of lifecycles for other types");
@@ -18,7 +18,7 @@ my $tickets = RT::Lifecycle->Load( Name => '', Type => 'ticket' );
ok($tickets, "Got a generalized lifecycle for tickets");
isa_ok( $tickets, "RT::Lifecycle::Ticket", "Is the right subclass" );
is_deeply( [ sort $tickets->Valid ],
- [ sort qw(new open stalled resolved rejected deleted ordered),
+ [ sort qw(new open stalled resolved rejected deleted ordered untriaged ordinary escalated),
'on way', 'delayed', 'delivered' ],
"Only gets ticket statuses" );
diff --git a/t/lifecycles/unprivileged.t b/t/lifecycles/unprivileged.t
new file mode 100644
index 0000000..0a69eb0
--- /dev/null
+++ b/t/lifecycles/unprivileged.t
@@ -0,0 +1,163 @@
+use strict;
+use warnings;
+
+BEGIN {require './t/lifecycles/utils.pl'};
+
+my $triage = RT::Test->load_or_create_queue(
+ Name => 'triage',
+ Lifecycle => 'triage',
+);
+ok $triage && $triage->id, 'loaded or created a queue';
+
+my $user = RT::User->new(RT->SystemUser);
+$user->Create(Name => "SelfService", Password => "password", Privileged => 0);
+
+ok( RT::Test->add_rights( { Principal => 'Everyone', Object => $triage, Right => [ qw(CreateTicket ShowTicket ModifyTicket) ] } ));
+
+# disable autoopen scrip to make tests more straightforward
+# otherwise, RT System will automatically set tickets from "untriaged"
+# to "ordinary". there's little other recourse because unprivileged can
+# only use the reply page to update tickets
+my $scrip = RT::Scrip->new(RT->SystemUser);
+$scrip->LoadByCols(Description => 'On Correspond Open Inactive Tickets');
+my ($ok, $msg) = $scrip->SetDisabled(1);
+ok($ok, $msg);
+
+my $tstatus = sub {
+ DBIx::SearchBuilder::Record::Cachable->FlushCache;
+ my $ticket = RT::Ticket->new( RT->SystemUser );
+ $ticket->Load( $_[0] );
+ return $ticket->Status;
+};
+
+my $txn_creator = sub {
+ DBIx::SearchBuilder::Record::Cachable->FlushCache;
+ my $ticket = RT::Ticket->new( RT->SystemUser );
+ $ticket->Load( $_[0] );
+ my $txns = $ticket->Transactions;
+ $txns->Limit(FIELD => 'Type', VALUE => 'Status');
+ die "Got " . $txns->Count . " transactions; expected 1" if $txns->Count != 1;
+ return $txns->First->Creator;
+};
+
+my ($baseurl) = RT::Test->started_ok;
+my $m = RT::Test::Web->new;
+$m->get_ok("$baseurl/index.html?user=SelfService&pass=password");
+
+my $ticket_id;
+
+diag "create a ticket";
+{
+ $m->get_ok("$baseurl/SelfService/Create.html?Queue=" . $triage->Id);
+ $m->text_contains("Create a ticket in #" . $triage->Id);
+ $m->submit_form_ok({
+ with_fields => {
+ Subject => "can't see queue"
+ },
+ });
+ $m->text_like(qr/Ticket \d+ created in queue 'triage'/);
+ ($ticket_id) = ($m->content =~ /Ticket (\d+) created in queue/);
+ is($tstatus->($ticket_id), 'untriaged', 'used default status');
+}
+
+diag "update a ticket without any special permissions required";
+{
+ $m->follow_link_ok({text => 'Reply'}, "reply to the ticket");
+ $m->text_contains("Update ticket #" . $ticket_id);
+ ok my $form = $m->form_name('TicketUpdate'), 'found form';
+ ok my $input = $form->find_input('Status'), 'found status selector';
+ my @form_values = $input->possible_values;
+ is_deeply(\@form_values, [
+ '',
+ 'untriaged',
+ 'ordinary',
+ ], "possible statuses");
+
+ $m->submit_form_ok({
+ with_fields => {
+ Status => "ordinary",
+ UpdateContent => "hello world",
+ },
+ button => 'SubmitTicket',
+ });
+ $m->text_contains("Correspondence added");
+ $m->text_contains("Status changed from 'untriaged' to 'ordinary'");
+ $m->text_lacks("Permission Denied");
+ is($tstatus->($ticket_id), "ordinary", "updated ticket");
+ is($txn_creator->($ticket_id), $user->Id, "txn creator");
+}
+
+my $ticket2_id;
+
+diag "create a ticket";
+{
+ $m->get_ok("$baseurl/SelfService/Create.html?Queue=" . $triage->Id);
+ $m->text_contains("Create a ticket in #" . $triage->Id);
+ $m->submit_form_ok({
+ with_fields => {
+ Subject => "can't see queue"
+ },
+ });
+ $m->text_like(qr/Ticket \d+ created in queue 'triage'/);
+ ($ticket2_id) = ($m->content =~ /Ticket (\d+) created in queue/);
+ is($tstatus->($ticket2_id), 'untriaged', 'used default status');
+}
+
+diag "update a ticket with necessary special permissions missing";
+{
+ $m->follow_link_ok({text => 'Reply'}, "reply to the ticket");
+ $m->text_contains("Update ticket #" . $ticket2_id);
+ ok my $form = $m->form_name('TicketUpdate'), 'found form';
+ ok my $input = $form->find_input('Status'), 'found status selector';
+ my @form_values = $input->possible_values;
+ is_deeply(\@form_values, [
+ '',
+ 'untriaged',
+ 'ordinary',
+ ], "possible statuses");
+
+ $m->submit_form_ok({
+ with_fields => {
+ Status => "escalated",
+ UpdateContent => "hello world",
+ },
+ button => 'SubmitTicket',
+ });
+ $m->text_contains("Correspondence added");
+ $m->text_contains("Permission Denied");
+ $m->text_lacks("Status changed from 'untriaged' to 'escalated'");
+ is($tstatus->($ticket2_id), "untriaged", "no update");
+}
+
+ok( RT::Test->add_rights( { Principal => 'Everyone', Object => $triage, Right => [ qw(EscalateTicket) ] } ));
+
+diag "update a ticket with necessary special permissions granted";
+{
+ $m->follow_link_ok({text => 'Reply'}, "reply to the ticket");
+ $m->text_contains("Update ticket #" . $ticket2_id);
+ ok my $form = $m->form_name('TicketUpdate'), 'found form';
+ ok my $input = $form->find_input('Status'), 'found status selector';
+ my @form_values = $input->possible_values;
+ is_deeply(\@form_values, [
+ '',
+ 'untriaged',
+ 'ordinary',
+ 'escalated',
+ ], "possible statuses");
+
+ $m->submit_form_ok({
+ with_fields => {
+ Status => "escalated",
+ UpdateContent => "hello world",
+ },
+ button => 'SubmitTicket',
+ });
+ $m->text_contains("Correspondence added");
+ $m->text_contains("Status changed from 'untriaged' to 'escalated'");
+ $m->text_lacks("Permission Denied");
+ is($tstatus->($ticket2_id), "escalated", "now updated");
+ is($txn_creator->($ticket2_id), $user->Id, "txn creator");
+}
+
+undef $m;
+done_testing;
diff --git a/t/lifecycles/utils.pl b/t/lifecycles/utils.pl
index cd167d6..ff32fd4 100644
--- a/t/lifecycles/utils.pl
+++ b/t/lifecycles/utils.pl
@@ -61,6 +61,24 @@ Set(\%Lifecycles,
'delayed -> on way' => {label => 'Put On Way', update => 'Respond'},
},
},
+ triage => {
+ initial => ['untriaged'],
+ active => ['ordinary', 'escalated'],
+ inactive => ['resolved'],
+ defaults => {
+ on_create => 'untriaged',
+ },
+ transitions => {
+ '' => ['untriaged'],
+ untriaged => ['ordinary', 'escalated'],
+ ordinary => ['resolved'],
+ escalated => ['resolved'],
+ resolved => [],
+ },
+ rights => {
+ '* -> escalated' => 'EscalateTicket',
+ },
+ },
racing => {
type => 'racecar',
active => ['on-your-mark', 'get-set', 'go'],
commit be649e73f901a6bb928825aa6613729b960b2c2d
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Tue May 23 20:19:23 2017 +0000
Walk around ACLs when working with lifecycles
This fixes an issue where unprivileged users - essentially anyone
without SeeQueue - would be given the default lifecycle instead of the
queue's lifecycle (because $queue->Lifecycle is guarded by SeeQueue).
This is wholly inappropriate when the queue's lifecycle does not
resemble the default one. The symptom is that users see unexpected
errors like "Status 'custom' isn't a valid status for this ticket" even
when "custom" is a valid status for that ticket's lifecycle, and can be
transitioned to.
So, rather than ignoring the queue's lifecycle, open it up so that when
you're working with a ticket, permissions no longer hide the lifecycle.
This does potentially open up lifecycles that had been previously hidden
by rights, but there are a few mitigating factors. A lifecycle is
unlikely to have private or sensitive information in it. There's also
very little UI for lifecycles beyond the "select status" dropdown.
This mirrors prior art to walk around ACLs to maintain consistency, such
as 68b6a66f.
Fixes: I#32799
diff --git a/lib/RT/Record/Role/Lifecycle.pm b/lib/RT/Record/Role/Lifecycle.pm
index 631c01b..34a3a53 100644
--- a/lib/RT/Record/Role/Lifecycle.pm
+++ b/lib/RT/Record/Role/Lifecycle.pm
@@ -94,18 +94,18 @@ of all lifecycles of the appropriate type.
sub LifecycleObj {
my $self = shift;
my $type = $self->LifecycleType;
- my $fallback = $self->_Accessible( Lifecycle => "default" );
unless (blessed($self) and $self->id) {
return RT::Lifecycle->Load( Type => $type );
}
- my $name = $self->Lifecycle || $fallback;
+ my $name = $self->__Value('Lifecycle');
my $res = RT::Lifecycle->Load( Name => $name, Type => $type );
unless ( $res ) {
RT->Logger->error(
sprintf "Lifecycle '%s' of type %s for %s #%d doesn't exist",
$name, $type, ref($self), $self->id);
+ my $fallback = $self->_Accessible( Lifecycle => "default" );
return RT::Lifecycle->Load( Name => $fallback, Type => $type );
}
return $res;
-----------------------------------------------------------------------
More information about the rt-commit
mailing list