[Rt-commit] rt branch, 4.0/allow-any-results-param-by-csrf, updated. rt-4.0.10-75-g67cca06

[Thomas Sibley]

The branch, 4.0/allow-any-results-param-by-csrf has been updated
       via  67cca0664f0c38356e6438fc7aa89a5fa78dbf35 (commit)
       via  81c3880f937110d3c4a7d2473758e3bd870f9ffe (commit)
      from  2123fcb6936ea4ada5f76aab1a8d60207b3615c7 (commit)

Summary of changes:
 share/html/Elements/ListActions |  3 +++
 t/web/action-results.t          | 48 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 t/web/action-results.t

- Log -----------------------------------------------------------------
commit 81c3880f937110d3c4a7d2473758e3bd870f9ffe
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Mon Mar 18 17:04:08 2013 -0700

    Ensure action results are deleted from the session upon first use
    
    This removes the inconsistent display of action results after a page
    reload and keeps the session from growing unbounded.  Previously the
    delete didn't update the session store unless a subsequent component
    incremented the session counter.  Quick ticket creation does not, for
    example.

diff --git a/share/html/Elements/ListActions b/share/html/Elements/ListActions
index 98a4fda..805bd2e 100644
--- a/share/html/Elements/ListActions
+++ b/share/html/Elements/ListActions
@@ -59,16 +59,19 @@
 # backward compatibility, don't use array in new code, but use keyed hash
 if ( ref( $session{'Actions'} ) eq 'ARRAY' ) {
     unshift @actions, @{ delete $session{'Actions'} };
+    $session{'i'}++;
 }
 
 if ( ref( $session{'Actions'}{''} ) eq 'ARRAY' ) {
     unshift @actions, @{ delete $session{'Actions'}{''} };
+    $session{'i'}++;
 }
 
 my $actions_pointer = $DECODED_ARGS->{'results'};
 
 if ($actions_pointer &&  ref( $session{'Actions'}->{$actions_pointer} ) eq 'ARRAY' ) {
     unshift @actions, @{ delete $session{'Actions'}->{$actions_pointer} };
+    $session{'i'}++;
 }
 
 # XXX: run callbacks per row really crazy idea

commit 67cca0664f0c38356e6438fc7aa89a5fa78dbf35
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Mon Mar 18 17:09:36 2013 -0700

    Test that action results are only displayed once and don't trigger CSRF

diff --git a/t/web/action-results.t b/t/web/action-results.t
new file mode 100644
index 0000000..db8c26b
--- /dev/null
+++ b/t/web/action-results.t
@@ -0,0 +1,48 @@
+use strict;
+use warnings;
+use RT::Test tests => 'no_declare';
+
+my ($url, $m) = RT::Test->started_ok;
+
+ok $m->login, "Logged in";
+
+# We test two ticket creation paths since one historically doesn't update the
+# session (quick create) and the other does.
+for my $quick (1, 0) {
+    diag $quick ? "Quick ticket creation" : "Normal ticket creation";
+
+    $m->get_ok("/");
+    $m->submit_form_ok({ form_name => 'CreateTicketInQueue' }, "Create new ticket form")
+        unless $quick;
+    $m->submit_form_ok({
+        with_fields => {
+            Subject => "The Plants",
+            Content => "Please water them.",
+        },
+    }, "Submitted new ticket");
+
+    my $id = RT::Test->last_ticket->id;
+
+    like $m->uri, qr/results=[A-Za-z0-9]{32}/, "URI contains results hash";
+    $m->content_contains("Ticket $id created", "Page contains results message");
+    $m->content_contains("#$id: The Plants") unless $quick;
+
+    diag "Reloading without a referer but with a results hash doesn't trigger the CSRF"; {
+        # Mech's API here sucks.  To drop the Referer and simulate a real browser
+        # reload, we need to make a new request which explicitly adds an empty Referer
+        # header (causing it to never be sent) and then deletes the empty Referer
+        # header to let it be automatically managed again.
+        $m->add_header("Referer" => undef);
+        $m->get_ok( $m->uri, "Reloading the results page without a Referer" );
+        $m->delete_header("Referer");
+
+        like $m->uri, qr/results=[A-Za-z0-9]{32}/, "URI contains results hash";
+        $m->content_lacks("cross-site request forgery", "Skipped the CSRF interstitial")
+            or $m->follow_link_ok({ text => "click here to resume your request" }, "Ignoring CSRF warning");
+        $m->content_lacks("Ticket $id created", "Page lacks results message");
+        $m->content_contains("#$id: The Plants") unless $quick;
+    }
+}
+
+undef $m;
+done_testing;

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