fixup! Redirect to confirmation message after POST when making report

perllib/FixMyStreet/App/Controller/Report.pm

@@ -6,8 +6,6 @@ use namespace::autoclean;
 use JSON::MaybeXS;
 use List::MoreUtils qw(any);
 use Utils;
-use Digest::HMAC_SHA1 qw(hmac_sha1);
-use MIME::Base64;
 
 BEGIN { extends 'Catalyst::Controller'; }
 
@@ -673,17 +671,16 @@ sub confirmation : Path('confirmation') : Args(1) {
 
     # Now verify the token we've been given is correct
     my $token_param = $c->get_param('token') || "";
-    my $token = $c->forward('/report/get_confirmation_token', [ $report ]);
-    unless ( $token eq $token_param ) {
+    unless ( $token_param eq $report->confirmation_token ) {
         $c->detach( '/page_error_404_not_found', [] );
     }
 
     # If the token is valid but expired then may as well be helpful and bounce
     # the user to report page rather than 404.
     # (NB the report may still be unconfirmed, but end result is the same - a 404)
-    my $cutoff = DateTime->now()->subtract( minutes => 1 )->epoch;
-    my $timestamp = $report->confirmed ? $report->confirmed : $report->created;
-    if ( $timestamp->epoch < $cutoff ) {
+    my $cutoff = DateTime->now()->subtract( minutes => 1 );
+    my $timestamp = $report->confirmed || $report->created;
+    if ( $timestamp < $cutoff ) {
         return $c->res->redirect($report->url);
     }
 
@@ -700,21 +697,6 @@ sub confirmation : Path('confirmation') : Args(1) {
     }
 }
 
-sub get_confirmation_token : Private {
-    my ( $self, $c, $p ) = @_;
-
-    if (!$p->created) {
-        # Might be an old handle on the DB row, so reload it
-        $p = FixMyStreet::DB->resultset('Problem')->find({ id => $p->id });
-    }
-    my $time = $p->created->epoch;
-    my $hash = hmac_sha1("$time-" . $p->id, $c->model('DB::Secret')->get);
-    $hash = encode_base64($hash, "");
-    $hash =~ s/\W//g; # don't want + / = in URL params
-    return $hash;
-}
-
-
 
 sub nearby_json :PathPart('nearby.json') :Chained('id') :Args(0) {
     my ($self, $c) = @_;

perllib/FixMyStreet/App/Controller/Report/New.pm

@@ -1917,10 +1917,7 @@ sub redirect_or_confirm_creation : Private {
             $c->stash->{created_report} = 'loggedin';
             $c->stash->{template} = 'tokens/confirm_problem.html';
             unless ($no_redirect) {
-                my $redirect = $c->uri_for_action( '/report/confirmation', [ $report->id ] );
-                my $token = $c->forward('/report/get_confirmation_token', [ $report ]);
-                $redirect .= "?token=$token";
-                return $c->res->redirect($redirect);
+                return $c->res->redirect($report->confirmation_url($c));
             }
         }
         return 1;
@@ -1945,9 +1942,7 @@ sub redirect_or_confirm_creation : Private {
         $c->stash->{template}   = 'email_sent.html';
         $c->stash->{email_type} = 'problem';
         unless ($no_redirect) {
-            $redirect = $c->uri_for_action( '/report/confirmation', [ $report->id ] );
-            my $token = $c->forward('/report/get_confirmation_token', [ $report ]);
-            $redirect .= "?token=$token";
+            $redirect = $report->confirmation_url($c);
         }
     } elsif ($report->user->phone_verified) {
         $c->forward( 'send_problem_confirm_text' );

perllib/FixMyStreet/DB/Result/Problem.pm

@@ -266,6 +266,8 @@ use List::Util qw/any uniq/;
 use LWP::Simple qw($ua);
 use URI;
 use URI::QueryParam;
+use Digest::HMAC_SHA1 qw(hmac_sha1);
+use MIME::Base64;
 
 my $IM = eval {
     require Image::Magick;
@@ -629,6 +631,18 @@ sub view_url {
     return "/R/" . $self->view_token->token;
 }
 
+=head2 confirmation_url
+
+Return a URL for this problem report that shows the confirmation page
+with the correct token in the URL parameter.
+
+=cut
+
+sub confirmation_url {
+    my ($self, $c) = @_;
+    return $c->uri_for_action( '/report/confirmation', [ $self->id ] ) . "?token=" . $self->confirmation_token;
+}
+
 =head2 is_hidden
 
 Returns 1 if the problem is in an hidden state otherwise 0.
@@ -1383,4 +1397,19 @@ sub cancel_update_alert {
     }
 };
 
+sub confirmation_token {
+    my $self = shift;
+
+    if (!$self->created) {
+        # Might be an old handle on the DB row, so reload it
+        $self = FixMyStreet::DB->resultset('Problem')->find({ id => $self->id });
+    }
+    my $time = $self->created->epoch;
+    my $hash = hmac_sha1("$time-" . $self->id, FixMyStreet::DB->resultset('Secret')->get);
+    $hash = encode_base64($hash, "");
+    $hash =~ s/\W//g; # don't want + / = in URL params
+    return $hash;
+}
+
+
 1;

t/app/controller/report_new.t

@@ -23,8 +23,6 @@ use Test::Deep;
 use Test::MockModule;
 use Test::MockTime qw(:all);
 use FixMyStreet::TestMech;
-use Digest::HMAC_SHA1 qw(hmac_sha1);
-use MIME::Base64;
 
 # disable info logs for this test run
 FixMyStreet::App->log->disable('info');
@@ -1296,10 +1294,7 @@ subtest "report confirmation page" => sub {
         });
         $report->discard_changes;
 
-        my $time = $report->created->epoch;
-        my $token = hmac_sha1("$time-" . $report->id, FixMyStreet::DB->resultset('Secret')->get);
-        $token = encode_base64($token, "");
-        $token =~ s/\W//g;
+        my $token = $report->confirmation_token;
 
         # check that going to confirmation page with valid token works
         $mech->get_ok("/report/confirmation/" . $report->id . "?token=$token");
@@ -1334,10 +1329,7 @@ subtest "report confirmation page" => sub {
         # make report 10 minutes old and regenerate token
         my $created = $report->created->subtract({ minutes => 10 });
         $report->update({ created => $created, confirmed => $created });
-        $time = $report->created->epoch;
-        $token = hmac_sha1("$time-" . $report->id, FixMyStreet::DB->resultset('Secret')->get);
-        $token = encode_base64($token, "");
-        $token =~ s/\W//g;
+        $token = $report->confirmation_token;
 
         # check that going to confirmation page now redirects to the report page
         $mech->get_ok("/report/confirmation/" . $report->id . "?token=$token");