Improve CSRF tokens and add to more forms.

9 files changed, 74 insertions, 58 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 4e288556f..72c6baad3 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -215,7 +215,7 @@ sub bodies : Path('bodies') : Args(0) {
         return;
     }
 
-    $c->forward( 'get_token' );
+    $c->forward( '/auth/get_csrf_token' );
 
     my $edit_activity = $c->model('DB::ContactsHistory')->search(
         undef,
@@ -232,7 +232,7 @@ sub bodies : Path('bodies') : Args(0) {
     my $posted = $c->get_param('posted') || '';
     if ( $posted eq 'body' ) {
         $c->forward('check_for_super_user');
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         my $params = $c->forward('body_params');
         my $body = $c->model('DB::Body')->create( $params );
@@ -289,7 +289,7 @@ sub body : Path('body') : Args(1) {
     $c->stash->{body_id} = $body_id;
 
     $c->forward( 'check_for_super_user' );
-    $c->forward( 'get_token' );
+    $c->forward( '/auth/get_csrf_token' );
     $c->forward( 'lookup_body' );
     $c->forward( 'fetch_all_bodies' );
     $c->forward( 'body_form_dropdowns' );
@@ -318,7 +318,7 @@ sub update_contacts : Private {
     my $editor = $c->forward('get_user');
 
     if ( $posted eq 'new' ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         my %errors;
 
@@ -370,7 +370,7 @@ sub update_contacts : Private {
         }
 
     } elsif ( $posted eq 'update' ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         my @categories = $c->get_param_list('confirmed');
 
@@ -393,7 +393,7 @@ sub update_contacts : Private {
         $c->stash->{updated} = _('Values updated');
     } elsif ( $posted eq 'body' ) {
         $c->forward('check_for_super_user');
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         my $params = $c->forward( 'body_params' );
         $c->stash->{body}->update( $params );
@@ -476,7 +476,7 @@ sub category_edit : Path('body') : Args(2) {
 
     $c->stash->{body_id} = $body_id;
 
-    $c->forward( 'get_token' );
+    $c->forward( '/auth/get_csrf_token' );
     $c->forward( 'lookup_body' );
 
     my $contact = $c->stash->{body}->contacts->search( { category => $category } )->first;
@@ -643,7 +643,7 @@ sub report_edit : Path('report_edit') : Args(1) {
 
     $c->stash->{problem} = $problem;
 
-    $c->forward('get_token');
+    $c->forward('/auth/get_csrf_token');
 
     if ( $c->cobrand->moniker eq 'zurich' ) {
         $c->stash->{page} = 'admin';
@@ -689,7 +689,7 @@ sub report_edit : Path('report_edit') : Args(1) {
           ->all ];
 
     if ( $c->get_param('resend') ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         $problem->whensent(undef);
         $problem->update();
@@ -699,7 +699,7 @@ sub report_edit : Path('report_edit') : Args(1) {
         $c->forward( 'log_edit', [ $id, 'problem', 'resend' ] );
     }
     elsif ( $c->get_param('mark_sent') ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
         $problem->whensent(\'current_timestamp');
         $problem->update();
         $c->stash->{status_message} = '<p><em>' . _('That problem has been marked as sent.') . '</em></p>';
@@ -717,7 +717,7 @@ sub report_edit : Path('report_edit') : Args(1) {
         $c->forward('ban_user');
     }
     elsif ( $c->get_param('submit') ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         my $done   = 0;
         my $edited = 0;
@@ -917,7 +917,7 @@ sub users: Path('users') : Args(0) {
         }
 
     } else {
-        $c->forward('get_token');
+        $c->forward('/auth/get_csrf_token');
         $c->forward('fetch_all_bodies');
 
         # Admin users by default
@@ -942,7 +942,7 @@ sub update_edit : Path('update_edit') : Args(1) {
     $c->detach( '/page_error_404_not_found' )
       unless $update;
 
-    $c->forward('get_token');
+    $c->forward('/auth/get_csrf_token');
 
     $c->stash->{update} = $update;
 
@@ -965,7 +965,7 @@ sub update_edit : Path('update_edit') : Args(1) {
         $c->stash->{update}->discard_changes;
     }
     elsif ( $c->get_param('submit') ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         my $old_state = $update->state;
         my $new_state = $c->get_param('state');
@@ -1047,12 +1047,12 @@ sub user_add : Path('user_edit') : Args(0) {
     my ( $self, $c ) = @_;
 
     $c->stash->{template} = 'admin/user_edit.html';
-    $c->forward('get_token');
+    $c->forward('/auth/get_csrf_token');
     $c->forward('fetch_all_bodies');
 
     return unless $c->get_param('submit');
 
-    $c->forward('check_token');
+    $c->forward('/auth/check_csrf_token');
 
     unless ($c->get_param('email')) {
         $c->stash->{field_errors}->{email} = _('Please enter a valid email');
@@ -1084,7 +1084,7 @@ sub user_add : Path('user_edit') : Args(0) {
 sub user_edit : Path('user_edit') : Args(1) {
     my ( $self, $c, $id ) = @_;
 
-    $c->forward('get_token');
+    $c->forward('/auth/get_csrf_token');
 
     my $user = $c->model('DB::User')->find( { id => $id } );
     $c->stash->{user} = $user;
@@ -1092,7 +1092,7 @@ sub user_edit : Path('user_edit') : Args(1) {
     $c->forward('fetch_all_bodies');
 
     if ( $c->get_param('submit') ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
 
         my $edited = 0;
 
@@ -1328,40 +1328,6 @@ sub get_user : Private {
     return $user;
 }
 
-=item get_token
-
-Generate a token based on user and secret
-
-=cut
-
-sub get_token : Private {
-    my ( $self, $c ) = @_;
-
-    my $secret = $c->model('DB::Secret')->get;
-    my $user = $c->forward('get_user');
-    my $token = sha1_hex($user . $secret);
-    $c->stash->{token} = $token;
-
-    return 1;
-}
-
-=item check_token
-
-Check that a token has been set on a request and it's the correct token. If
-not then display 404 page
-
-=cut
-
-sub check_token : Private {
-    my ( $self, $c ) = @_;
-
-    if ( !$c->get_param('token') || $c->get_param('token') ne $c->stash->{token} ) {
-        $c->detach( '/page_error_404_not_found' );
-    }
-
-    return 1;
-}
-
 =item log_edit
 
     $c->forward( 'log_edit', [ $object_id, $object_type, $action_performed ] );
diff --git a/perllib/FixMyStreet/App/Controller/Alert.pm b/perllib/FixMyStreet/App/Controller/Alert.pm
index ddda02abd..b578fbbcc 100644
--- a/perllib/FixMyStreet/App/Controller/Alert.pm
+++ b/perllib/FixMyStreet/App/Controller/Alert.pm
@@ -36,6 +36,8 @@ sub index : Path('') : Args(0) {
 sub list : Path('list') : Args(0) {
     my ( $self, $c ) = @_;
 
+    $c->forward('/auth/get_csrf_token');
+
     return
       unless $c->forward('setup_request')
           && $c->forward('prettify_pc')
@@ -112,6 +114,8 @@ Sign up to email alerts
 sub subscribe_email : Private {
     my ( $self, $c ) = @_;
 
+    $c->forward('/auth/check_csrf_token');
+
     $c->stash->{errors} = [];
     $c->forward('process_user');
 
@@ -146,6 +150,8 @@ sub subscribe_email : Private {
 sub updates : Path('updates') : Args(0) {
     my ( $self, $c ) = @_;
 
+    $c->forward('/auth/get_csrf_token');
+
     $c->stash->{email} = $c->get_param('rznvy');
     $c->stash->{problem_id} = $c->get_param('id');
 }
diff --git a/perllib/FixMyStreet/App/Controller/Around.pm b/perllib/FixMyStreet/App/Controller/Around.pm
index b0340204a..b8f038ce3 100644
--- a/perllib/FixMyStreet/App/Controller/Around.pm
+++ b/perllib/FixMyStreet/App/Controller/Around.pm
@@ -156,6 +156,8 @@ sub display_location : Private {
     # set the template to use
     $c->stash->{template} = 'around/display_location.html';
 
+    $c->forward('/auth/get_csrf_token');
+
     # get the lat,lng
     my $latitude  = $c->stash->{latitude};
     my $longitude = $c->stash->{longitude};
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index 203479253..65533b1d2 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -6,8 +6,9 @@ BEGIN { extends 'Catalyst::Controller'; }
 
 use Email::Valid;
 use Net::Domain::TLD;
-use mySociety::AuthToken;
+use Digest::HMAC_SHA1 qw(hmac_sha1);
 use JSON::MaybeXS;
+use MIME::Base64;
 use Net::Facebook::Oauth2;
 use Net::Twitter::Lite::WithAPIv1_1;
 
@@ -415,12 +416,13 @@ sub change_password : Local {
 
     $c->detach( 'redirect' ) unless $c->user;
 
-    # FIXME - CSRF check here
-    # FIXME - minimum criteria for passwords (length, contain number, etc)
+    $c->forward('get_csrf_token');
 
     # If not a post then no submission
     return unless $c->req->method eq 'POST';
 
+    $c->forward('check_csrf_token');
+
     # get the passwords
     my $new = $c->get_param('new_password') // '';
     my $confirm = $c->get_param('confirm') // '';
@@ -444,6 +446,38 @@ sub change_password : Local {
 
 }
 
+sub get_csrf_token : Private {
+    my ( $self, $c ) = @_;
+
+    my $time = $c->stash->{csrf_time} || time();
+    my $hash = hmac_sha1("$time-" . ($c->sessionid || ""), $c->model('DB::Secret')->get);
+    $hash = encode_base64($hash, "");
+    $hash =~ s/=$//;
+    my $token = "$time-$hash";
+    $c->stash->{csrf_token} = $token unless $c->stash->{csrf_time};
+    return $token;
+}
+
+sub check_csrf_token : Private {
+    my ( $self, $c ) = @_;
+
+    my $token = $c->get_param('token') || "";
+    $token =~ s/ /+/g;
+    my ($time) = $token =~ /^(\d+)-[0-9a-zA-Z+\/]+$/;
+    $c->stash->{csrf_time} = $time;
+    $c->detach('no_csrf_token')
+        unless $time
+            && $time > time() - 3600
+            && $token eq $c->forward('get_csrf_token');
+    delete $c->stash->{csrf_time};
+}
+
+sub no_csrf_token : Private {
+    my ($self, $c) = @_;
+    $c->stash->{message} = _('Unknown error');
+    $c->stash->{template} = 'errors/generic.html';
+}
+
 =head2 sign_out
 
 Log the user out. Tell them we've done so.
diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm
index 77a3346dc..2d23417b9 100644
--- a/perllib/FixMyStreet/App/Controller/Moderate.pm
+++ b/perllib/FixMyStreet/App/Controller/Moderate.pm
@@ -57,6 +57,8 @@ sub report : Chained('moderate') : PathPart('report') : CaptureArgs(1) {
     $c->detach unless $c->user_exists;
     $c->detach unless $c->user->has_permission_to(moderate => $problem->bodies_str);
 
+    $c->forward('/auth/check_csrf_token');
+
     my $original = $problem->find_or_new_related( moderation_original_data => {
         title => $problem->title,
         detail => $problem->detail,
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index b3e546c2c..89df4a52d 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -72,6 +72,7 @@ sub ajax : Path('ajax') : Args(1) {
 sub _display : Private {
     my ( $self, $c, $id ) = @_;
 
+    $c->forward('/auth/get_csrf_token');
     $c->forward( 'load_problem_or_display_error', [ $id ] );
     $c->forward( 'load_updates' );
     $c->forward( 'format_problem_for_display' );
@@ -249,6 +250,8 @@ users too about this change, at which point we can delete:
 sub delete :Local :Args(1) {
     my ( $self, $c, $id ) = @_;
 
+    $c->forward('/auth/check_csrf_token');
+
     $c->forward( 'load_problem_or_display_error', [ $id ] );
     my $p = $c->stash->{problem};
 
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 696234d32..b5d4cba3f 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -81,6 +81,7 @@ sub report_new : Path : Args(0) {
 
     # create the report - loading a partial if available
     $c->forward('initialize_report');
+    $c->forward('/auth/get_csrf_token');
 
     # work out the location for this report and do some checks
     # Also show map if we're just updating the filters
@@ -96,6 +97,7 @@ sub report_new : Path : Args(0) {
     # deal with the user and report and check both are happy
 
     return unless $c->forward('check_form_submitted');
+    $c->forward('/auth/check_csrf_token');
     $c->forward('process_user');
     $c->forward('process_report');
     $c->forward('/photo/process_photo');
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 275a300bd..d03797f56 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -25,6 +25,7 @@ sub report_update : Path : Args(0) {
     $c->forward('check_form_submitted')
       or $c->go( '/report/display', [ $c->stash->{problem}->id ] );
 
+    $c->forward('/auth/check_csrf_token');
     $c->forward('process_update');
     $c->forward('process_user');
     $c->forward('/photo/process_photo');
diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm
index 987f0bb67..2e4a167db 100644
--- a/perllib/FixMyStreet/Cobrand/Zurich.pm
+++ b/perllib/FixMyStreet/Cobrand/Zurich.pm
@@ -541,7 +541,7 @@ sub admin_report_edit {
 
     # If super or dm check that the token is correct before proceeding
     if ( ($type eq 'super' || $type eq 'dm') && $c->get_param('submit') ) {
-        $c->forward('check_token');
+        $c->forward('/auth/check_csrf_token');
     }
 
     # All types of users can add internal notes
@@ -807,7 +807,7 @@ sub admin_report_edit {
             # subdivision, or because the customer was not contactable.
             # We handle these in the same way but with different statuses.
 
-            $c->forward('check_token');
+            $c->forward('/auth/check_csrf_token');
 
             my $not_contactable = $c->get_param('not_contactable');
 
@@ -829,7 +829,7 @@ sub admin_report_edit {
             $self->update_admin_log($c, $problem);
             $c->res->redirect( '/admin/summary' );
         } elsif ($c->get_param('submit')) {
-            $c->forward('check_token');
+            $c->forward('/auth/check_csrf_token');
 
             my $db_update = 0;
             if ( $c->get_param('latitude') != $problem->latitude || $c->get_param('longitude') != $problem->longitude ) {