[Rt-commit] rt branch, 4.4/unpriv-lifecycle, created. rt-4.4.1-345-gdb6db05

Shawn Moore shawn at bestpractical.com
Tue May 23 16:26:57 EDT 2017


The branch, 4.4/unpriv-lifecycle has been created
        at  db6db0579a02a1856142cea733bfc0117e11fc97 (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 db6db0579a02a1856142cea733bfc0117e11fc97
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 - those without SeeQueue -
    would be given the default lifecycle instead of the queue's lifecycle.
    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 if
    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.
    
    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