diff options
39 files changed, 172 insertions, 194 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 ) { diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t index 777d733e2..06932f70a 100644 --- a/t/app/controller/alert_new.t +++ b/t/app/controller/alert_new.t @@ -7,6 +7,10 @@ use FixMyStreet::App; my $mech = FixMyStreet::TestMech->new; +$mech->log_in_ok('test@example.com'); +$mech->get_ok('/alert/subscribe?id=1'); +my ($csrf) = $mech->content =~ /name="token" value="([^"]*)"/; + foreach my $test ( { email => 'test@example.com', @@ -71,7 +75,7 @@ foreach my $test ( $mech->delete_user($user); } - $mech->get_ok( $test->{uri} ); + $mech->get_ok( $test->{uri} . "&token=$csrf" ); $mech->content_contains( $test->{content} ); $user = @@ -113,7 +117,7 @@ foreach my $test ( my $existing_id = $alert->id; my $existing_token = $url_token; - $mech->get_ok( $test->{uri} ); + $mech->get_ok( $test->{uri} . "&token=$csrf" ); $email = $mech->get_email; ok $email, 'got a second email'; @@ -165,7 +169,7 @@ foreach my $test ( # clear existing data so we can be sure we're creating it ok $alert->delete() if $alert && !$test->{exist}; - $mech->get_ok( '/alert/subscribe?type=local&rznvy=test-new@example.com&feed=area:1000:A_Location' ); + $mech->get_ok( '/alert/subscribe?type=local&rznvy=test-new@example.com&feed=area:1000:A_Location&token=' . $csrf ); $alert = FixMyStreet::App->model('DB::Alert')->find( { @@ -262,7 +266,7 @@ for my $test ( FixMyStreet::App->model('DB::Abuse') ->find_or_create( { email => $test->{email} } ); - $mech->get_ok( $test->{uri} ); + $mech->get_ok( $test->{uri} . "&token=$csrf" ); $mech->content_contains( $test->{content} ); $user = diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t index 235a3af7e..9b3d9468a 100644 --- a/t/app/controller/auth.t +++ b/t/app/controller/auth.t @@ -128,7 +128,7 @@ $mech->not_logged_in_ok; ok my $form = $mech->form_name('change_password'), "found change password form"; is_deeply [ sort grep { $_ } map { $_->name } $form->inputs ], # - [ 'confirm', 'new_password' ], + [ 'confirm', 'new_password', 'token' ], "check we got expected fields (ie not old_password)"; # check the various ways the form can be wrong diff --git a/t/app/controller/moderate.t b/t/app/controller/moderate.t index b79f50e73..38216c708 100644 --- a/t/app/controller/moderate.t +++ b/t/app/controller/moderate.t @@ -8,6 +8,7 @@ use FixMyStreet::App; use Data::Dumper; my $mech = FixMyStreet::TestMech->new; +$mech->host('www.example.org'); my $BROMLEY_ID = 2482; my $body = $mech->create_body_ok( $BROMLEY_ID, 'Bromley Council' ); @@ -92,11 +93,12 @@ my %problem_prepopulated = ( subtest 'Problem moderation' => sub { subtest 'Post modify title and text' => sub { - $mech->post_ok('/moderate/report/' . $report->id, { + $mech->get_ok($REPORT_URL); + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_title => 'Good good', problem_detail => 'Good good improved', - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $report->discard_changes; @@ -105,11 +107,11 @@ subtest 'Problem moderation' => sub { }; subtest 'Revert title and text' => sub { - $mech->post_ok('/moderate/report/' . $report->id, { + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_revert_title => 1, problem_revert_detail => 1, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $report->discard_changes; @@ -120,18 +122,18 @@ subtest 'Problem moderation' => sub { subtest 'Make anonymous' => sub { $mech->content_lacks('Reported anonymously'); - $mech->post_ok('/moderate/report/' . $report->id, { + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_show_name => 0, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_contains('Reported anonymously'); - $mech->post_ok('/moderate/report/' . $report->id, { + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_show_name => 1, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_lacks('Reported anonymously'); @@ -140,18 +142,18 @@ subtest 'Problem moderation' => sub { subtest 'Hide photo' => sub { $mech->content_contains('Photo of this report'); - $mech->post_ok('/moderate/report/' . $report->id, { + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_show_photo => 0, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_lacks('Photo of this report'); - $mech->post_ok('/moderate/report/' . $report->id, { + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_show_photo => 1, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_contains('Photo of this report'); @@ -160,10 +162,10 @@ subtest 'Problem moderation' => sub { subtest 'Hide report' => sub { $mech->clear_emails_ok; - my $resp = $mech->post('/moderate/report/' . $report->id, { + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_hide => 1, - }); + }}); $mech->base_unlike( qr{/report/}, 'redirected to front page' ); $report->discard_changes; @@ -185,22 +187,23 @@ $mech->content_lacks('Posted anonymously', 'sanity check'); subtest 'Problem 2' => sub { my $REPORT2_URL = '/report/' . $report2->id ; - $mech->post_ok('/moderate/report/' . $report2->id, { + $mech->get_ok($REPORT2_URL); + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_title => 'Good good', problem_detail => 'Good good improved', - }); + }}); $mech->base_like( qr{\Q$REPORT2_URL\E} ); $report2->discard_changes; is $report2->title, 'Good [...] good'; is $report2->detail, 'Good [...] good [...]improved'; - $mech->post_ok('/moderate/report/' . $report2->id, { + $mech->submit_form_ok({ with_fields => { %problem_prepopulated, problem_revert_title => 1, problem_revert_detail => 1, - }); + }}); $mech->base_like( qr{\Q$REPORT2_URL\E} ); $report2->discard_changes; @@ -229,13 +232,12 @@ my $update = create_update(); subtest 'updates' => sub { - my $MODERATE_UPDATE_URL = sprintf '/moderate/report/%d/update/%d', $report->id, $update->id; - subtest 'Update modify text' => sub { - $mech->post_ok( $MODERATE_UPDATE_URL, { + $mech->get_ok($REPORT_URL); + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_detail => 'update good good good', - }) or die $mech->content; + }}) or die $mech->content; $mech->base_like( qr{\Q$REPORT_URL\E} ); $update->discard_changes; @@ -243,10 +245,10 @@ subtest 'updates' => sub { }; subtest 'Revert text' => sub { - $mech->post_ok( $MODERATE_UPDATE_URL, { + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_revert_detail => 1, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $update->discard_changes; @@ -258,18 +260,18 @@ subtest 'updates' => sub { $mech->content_lacks('Posted anonymously') or die sprintf '%d (%d)', $update->id, $report->comments->count; - $mech->post_ok( $MODERATE_UPDATE_URL, { + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_show_name => 0, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_contains('Posted anonymously'); - $mech->post_ok( $MODERATE_UPDATE_URL, { + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_show_name => 1, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_lacks('Posted anonymously'); @@ -283,18 +285,18 @@ subtest 'updates' => sub { $mech->content_contains('Photo of this report') or die $mech->content; - $mech->post_ok( $MODERATE_UPDATE_URL, { + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_show_photo => 0, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_lacks('Photo of this report'); - $mech->post_ok( $MODERATE_UPDATE_URL, { + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_show_photo => 1, - }); + }}); $mech->base_like( qr{\Q$REPORT_URL\E} ); $mech->content_contains('Photo of this report'); @@ -303,10 +305,10 @@ subtest 'updates' => sub { subtest 'Hide comment' => sub { $mech->content_contains('update good good bad good'); - $mech->post_ok( $MODERATE_UPDATE_URL, { + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_hide => 1, - }); + }}); $mech->content_lacks('update good good bad good'); }; @@ -316,11 +318,11 @@ subtest 'updates' => sub { my $update2 = create_update(); subtest 'Update 2' => sub { - my $MODERATE_UPDATE2_URL = sprintf '/moderate/report/%d/update/%d', $report->id, $update2->id; - $mech->post_ok( $MODERATE_UPDATE2_URL, { + $mech->get_ok($REPORT_URL); + $mech->submit_form_ok({ with_fields => { %update_prepopulated, update_detail => 'update good good good', - }) or die $mech->content; + }}) or die $mech->content; $update2->discard_changes; is $update2->text, 'update good good [...] good', diff --git a/t/app/controller/photo.t b/t/app/controller/photo.t index 425e3c4df..4cec82c44 100644 --- a/t/app/controller/photo.t +++ b/t/app/controller/photo.t @@ -40,11 +40,15 @@ subtest "Check multiple upload worked" => sub { # submit the main form # can't post_ok as we lose the Content_Type header # (TODO rewrite with HTTP::Request::Common and request_ok) + $mech->get_ok('/report/new?lat=53.4031156&lon=-2.9840579'); + my ($csrf) = $mech->content =~ /name="token" value="([^"]*)"/; + $mech->post( '/report/new', Content_Type => 'form-data', Content => { submit_problem => 1, + token => $csrf, title => 'Test', lat => 53.4031156, lon => -2.9840579, # in Liverpool pc => 'L1 4LN', @@ -57,9 +61,6 @@ subtest "Check multiple upload worked" => sub { email => 'test@example.com', phone => '', category => 'Street lighting', - #password_sign_in => '', - #password_register => '', - #remember_me => undef, } ); ok $mech->success, 'Made request with multiple photo upload'; diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t index 7b4bf7854..2a3c7c0b3 100644 --- a/t/app/controller/report_updates.t +++ b/t/app/controller/report_updates.t @@ -510,20 +510,14 @@ subtest 'check non authority user cannot change set state' => sub { $user->update; $mech->get_ok("/report/$report_id"); - $mech->post_ok( "/report/update", { - submit_update => 1, - id => $report_id, - name => $user->name, - may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', - update => 'this is a forbidden update', - state => 'fixed - council', + $mech->submit_form_ok( { + form_id => 'form_update_form', + fields => { + may_show_name => 1, + update => 'this is a forbidden update', + state => 'fixed - council', }, - 'submitted with state', - ); + }, 'submitted with state'); is $mech->uri->path, "/report/update", "at /report/update"; @@ -540,20 +534,14 @@ for my $state ( qw/unconfirmed hidden partial/ ) { $user->update; $mech->get_ok("/report/$report_id"); - $mech->post_ok( "/report/update", { - submit_update => 1, - id => $report_id, - name => $user->name, - may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', - update => 'this is a forbidden update', - state => $state, + $mech->submit_form_ok( { + form_id => 'form_update_form', + fields => { + may_show_name => 1, + update => 'this is a forbidden update', + state => $state, }, - 'submitted with state', - ); + }, 'submitted with state'); is $mech->uri->path, "/report/update", "at /report/update"; @@ -570,10 +558,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to investigating', state => 'investigating', }, @@ -584,10 +568,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to in progress', state => 'in progress', }, @@ -598,10 +578,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to fixed', state => 'fixed', }, @@ -612,10 +588,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to action scheduled', state => 'action scheduled', }, @@ -626,10 +598,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to unable to fix', state => 'unable to fix', }, @@ -640,10 +608,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to internal referral', state => 'internal referral', }, @@ -655,10 +619,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to not responsible', state => 'not responsible', }, @@ -670,10 +630,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to duplicate', state => 'duplicate', }, @@ -685,10 +641,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to internal referral', state => 'internal referral', }, @@ -700,10 +652,6 @@ for my $test ( fields => { name => $user->name, may_show_name => 1, - add_alert => undef, - photo1 => '', - photo2 => '', - photo3 => '', update => 'Set state to fixed', state => 'fixed', }, diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t index cf66136e5..a595f48c9 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -810,17 +810,12 @@ subtest "photo must be supplied for categories that require it" => sub { MAPIT_ID_WHITELIST => [ 423017 ], MAP_TYPE => 'Zurich,OSM', }, sub { - $mech->post_ok( '/report/new', { + $mech->get_ok('/report/new?lat=47.381817&lon=8.529156'); + $mech->submit_form_ok({ with_fields => { detail => 'Problem-Bericht', - lat => 47.381817, - lon => 8.529156, email => 'user@example.org', - pc => '', - name => '', category => 'Graffiti - photo required', - photo => '', - submit_problem => 1, - }); + }}); is $mech->res->code, 200, "missing photo shouldn't return anything but 200"; $mech->content_contains(_("Photo is required."), 'response should contain photo error message'); }; diff --git a/templates/web/base/admin/body-form.html b/templates/web/base/admin/body-form.html index 7acfbfdd5..8c4956f7f 100644 --- a/templates/web/base/admin/body-form.html +++ b/templates/web/base/admin/body-form.html @@ -236,7 +236,7 @@ <p> <input type="hidden" name="posted" value="body"> - <input type="hidden" name="token" value="[% token %]"> + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="submit" value="[% body ? loc('Update body') : loc('Add body') %]"> </p> </form> diff --git a/templates/web/base/admin/body.html b/templates/web/base/admin/body.html index d5e575666..15802fc44 100644 --- a/templates/web/base/admin/body.html +++ b/templates/web/base/admin/body.html @@ -97,7 +97,7 @@ <p> <input type="hidden" name="posted" value="update"> - <input type="hidden" name="token" value="[% token %]"> + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="submit" name="Update statuses" value="[% loc('Update statuses') %]"> </p> </form> @@ -202,7 +202,7 @@ <p> <input type="hidden" name="posted" value="new" > - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="submit" name="Create category" value="[% errors ? loc('Save changes') : loc('Create category') %]" > </p> diff --git a/templates/web/base/admin/category_edit.html b/templates/web/base/admin/category_edit.html index c0bd43ef5..6537fe028 100644 --- a/templates/web/base/admin/category_edit.html +++ b/templates/web/base/admin/category_edit.html @@ -22,7 +22,7 @@ <form method="post" action="[% c.uri_for('body', body_id ) %]" enctype="application/x-www-form-urlencoded" accept-charset="utf-8"> <p><strong>[% loc('Category:') %] </strong>[% contact.category | html %] <input type="hidden" name="category" value="[% contact.category | html %]" > - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > [% IF contact.extra %] <p><strong>[% loc('Extra data:') %] </strong> [% USE Dumper %] diff --git a/templates/web/base/admin/report_edit.html b/templates/web/base/admin/report_edit.html index c0cdead84..065c6c2ce 100644 --- a/templates/web/base/admin/report_edit.html +++ b/templates/web/base/admin/report_edit.html @@ -4,7 +4,7 @@ [% status_message %] <form method="post" action="[% c.uri_for( 'report_edit', problem.id ) %]" enctype="application/x-www-form-urlencoded" accept-charset="utf-8"> - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="hidden" name="submit" value="1" > <ul> [%- cobrand_data = problem.cobrand_data; diff --git a/templates/web/base/admin/update_edit.html b/templates/web/base/admin/update_edit.html index a956bb2cb..06bee6010 100644 --- a/templates/web/base/admin/update_edit.html +++ b/templates/web/base/admin/update_edit.html @@ -4,7 +4,7 @@ [% status_message %] <form method="post" action="[% c.uri_for( 'update_edit', update.id ) %]" enctype="application/x-www-form-urlencoded" accept-charset="utf-8"> - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="hidden" name="submit" value="1" > <ul> [%- cobrand_data = update.cobrand_data; diff --git a/templates/web/base/admin/user-form.html b/templates/web/base/admin/user-form.html index 3956e8533..b863bf96a 100644 --- a/templates/web/base/admin/user-form.html +++ b/templates/web/base/admin/user-form.html @@ -1,5 +1,5 @@ <form method="post" action="[% c.uri_for( 'user_edit', user.id ) %]" enctype="application/x-www-form-urlencoded" accept-charset="utf-8"> - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="hidden" name="submit" value="1" > [% IF c.cobrand.moniker == 'zurich' AND field_errors.email %] diff --git a/templates/web/base/alert/_list.html b/templates/web/base/alert/_list.html index 395948248..65bba2fed 100644 --- a/templates/web/base/alert/_list.html +++ b/templates/web/base/alert/_list.html @@ -1,3 +1,4 @@ + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="hidden" name="type" value="local"> <input type="hidden" name="pc" value="[% pc | html %]"> <input type="hidden" name="latitude" value="[% latitude | html %]"> diff --git a/templates/web/base/alert/updates.html b/templates/web/base/alert/updates.html index 104bfa55a..ecaed37ca 100644 --- a/templates/web/base/alert/updates.html +++ b/templates/web/base/alert/updates.html @@ -23,6 +23,7 @@ <input class="green-btn" type="submit" value="[% loc('Subscribe') %]"> </div> + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="hidden" name="id" value="[% problem_id | html %]"> <input type="hidden" name="type" value="updates"> </fieldset> diff --git a/templates/web/base/around/display_location.html b/templates/web/base/around/display_location.html index 7bf62e528..7c54f4b76 100755 --- a/templates/web/base/around/display_location.html +++ b/templates/web/base/around/display_location.html @@ -40,6 +40,7 @@ [% IF allow_creation %] <form action="[% c.uri_for('/report/new') %]" method="post" name="mapForm" id="mapForm" enctype="multipart/form-data" class="validate" novalidate> + <input type="hidden" name="token" value="[% csrf_token %]"> [% IF c.req.params.map_override %] <input type="hidden" name="map_override" value="[% c.req.params.map_override | html %]"> [% END %] diff --git a/templates/web/base/auth/change_password.html b/templates/web/base/auth/change_password.html index b4170c23e..be0dc69b4 100644 --- a/templates/web/base/auth/change_password.html +++ b/templates/web/base/auth/change_password.html @@ -3,11 +3,12 @@ <h1>[% loc('Change password') %]</h1> [% IF password_changed %] - <p id="fixed">[% loc('Your password has been changed') %]</p> + <p class="form-success">[% loc('Your password has been changed') %]</p> [% END %] <form action="[% c.uri_for('change_password') %]" method="post" name="change_password" class="fieldset"> + <input type="hidden" name="token" value="[% csrf_token %]"> [% IF password_error; diff --git a/templates/web/base/report/_main.html b/templates/web/base/report/_main.html index aaa167108..4821b3fa0 100644 --- a/templates/web/base/report/_main.html +++ b/templates/web/base/report/_main.html @@ -5,6 +5,7 @@ [% IF moderating %] [% original = problem_original %] <form method="post" action="/moderate/report/[% problem.id %]"> + <input type="hidden" name="token" value="[% csrf_token %]"> <p class="moderate-display"> <input type="button" class="btn moderate" value="moderate"> </p> diff --git a/templates/web/base/report/display_tools.html b/templates/web/base/report/display_tools.html index c1413d9c7..58e450c84 100644 --- a/templates/web/base/report/display_tools.html +++ b/templates/web/base/report/display_tools.html @@ -2,6 +2,7 @@ <ul id="key-tools"> [% IF c.user_exists AND c.cobrand.users_can_hide AND c.user.belongs_to_body( c.cobrand.council_id ) %] <li><form method="post" action="/report/delete/[% problem.id %]" id="remove-from-site-form"> + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="submit" id="key-tool-report-abuse" class="abuse" value="Remove from site"> </form></li> [% ELSIF c.cobrand.moniker != 'zurich' %] @@ -40,6 +41,7 @@ <input type="email" name="rznvy" id="alert_rznvy" value="[% email | html %]" size="30" placeholder="[% loc('Your email') %]"> <input class="green-btn" type="submit" value="[% loc('Subscribe') %]"> </div> + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="hidden" name="id" value="[% problem.id %]"> <input type="hidden" name="type" value="updates"> </fieldset> diff --git a/templates/web/base/report/new/fill_in_details.html b/templates/web/base/report/new/fill_in_details.html index 55b3a5207..d7e2f1d3a 100644 --- a/templates/web/base/report/new/fill_in_details.html +++ b/templates/web/base/report/new/fill_in_details.html @@ -12,16 +12,15 @@ <input type="hidden" name="map_override" value="[% c.req.params.map_override | html %]"> [% END %] - <input type="hidden" name="pc" value="[% pc | html %]"> - [% ELSE %] <form action="[% c.uri_for('/report/new') %]" method="post" name="mapSkippedForm"[% IF c.cobrand.allow_photo_upload %] enctype="multipart/form-data"[% END %] class="validate"> - <input type="hidden" name="pc" value="[% pc | html %]"> <input type="hidden" name="skipped" value="1"> [% END %] + <input type="hidden" name="token" value="[% csrf_token %]"> + <input type="hidden" name="pc" value="[% pc | html %]"> <input type="hidden" name="latitude" id="fixmystreet.latitude" value="[% latitude | html %]"> <input type="hidden" name="longitude" id="fixmystreet.longitude" value="[% longitude | html %]"> diff --git a/templates/web/base/report/update-form.html b/templates/web/base/report/update-form.html index f6ce265bf..97e0df779 100644 --- a/templates/web/base/report/update-form.html +++ b/templates/web/base/report/update-form.html @@ -15,6 +15,7 @@ [% INCLUDE 'errors.html' %] <form method="post" action="[% c.uri_for( '/report/update' ) %]" id="form_update_form" name="updateForm" class="validate"[% IF c.cobrand.allow_photo_upload %] enctype="multipart/form-data"[% END %]> + <input type="hidden" name="token" value="[% csrf_token %]"> <fieldset> [% IF NOT login_success AND NOT oauth_need_email %] [% INCLUDE 'report/update/form_update.html' %] diff --git a/templates/web/base/report/update.html b/templates/web/base/report/update.html index a09913d39..aaad33b7a 100644 --- a/templates/web/base/report/update.html +++ b/templates/web/base/report/update.html @@ -8,6 +8,7 @@ <li class="item-list__item item-list__item--updates"> [% IF moderating; original_update = update.moderation_original_data %] <form method="post" action="/moderate/report/[% problem.id %]/update/[% update.id %]"> + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="button" class="btn moderate moderate-display" value="moderate"> <div class="moderate-edit"> <input type="checkbox" class="hide-document" name="update_hide"> diff --git a/templates/web/bromley/report/display.html b/templates/web/bromley/report/display.html index da83e005f..27e1e64a9 100644 --- a/templates/web/bromley/report/display.html +++ b/templates/web/bromley/report/display.html @@ -56,6 +56,7 @@ [% INCLUDE 'errors.html' %] <form method="post" action="[% c.uri_for( '/report/update' ) %]" name="updateForm" class="validate"[% IF c.cobrand.allow_photo_upload %] enctype="multipart/form-data"[% END %]> + <input type="hidden" name="token" value="[% csrf_token %]"> <fieldset> <input type="hidden" name="submit_update" value="1"> <input type="hidden" name="id" value="[% problem.id | html %]"> diff --git a/templates/web/eastsussex/report/update-form.html b/templates/web/eastsussex/report/update-form.html index e4fb47a45..b2c67890f 100644 --- a/templates/web/eastsussex/report/update-form.html +++ b/templates/web/eastsussex/report/update-form.html @@ -24,6 +24,7 @@ </p> <form method="post" action="/report/new"> + <input type="hidden" name="token" value="[% csrf_token %]"> <input type="hidden" name="latitude" value="[% problem.latitude %]"> <input type="hidden" name="longitude" value="[% problem.longitude %]"> <input type="submit" class="green-btn" value="CREATE A NEW PROBLEM NEARBY"> @@ -56,6 +57,7 @@ [% INCLUDE 'errors.html' %] <form method="post" action="[% c.uri_for( '/report/update' ) %]" id="form_update_form" name="updateForm" class="validate"[% IF c.cobrand.allow_photo_upload %] enctype="multipart/form-data"[% END %]> + <input type="hidden" name="token" value="[% csrf_token %]"> <fieldset> <input type="hidden" name="submit_update" value="1"> <input type="hidden" name="id" value="[% problem.id | html %]"> diff --git a/templates/web/seesomething/around/display_location.html b/templates/web/seesomething/around/display_location.html index b54311264..692ea22ee 100644 --- a/templates/web/seesomething/around/display_location.html +++ b/templates/web/seesomething/around/display_location.html @@ -20,6 +20,7 @@ %] <form action="[% c.uri_for('/report/new') %]" method="post" name="mapForm" id="mapForm" enctype="multipart/form-data" class="validate" novalidate> + <input type="hidden" name="token" value="[% csrf_token %]"> [% IF c.req.params.map_override %] <input type="hidden" name="map_override" value="[% c.req.params.map_override | html %]"> [% END %] diff --git a/templates/web/zurich/admin/body-form.html b/templates/web/zurich/admin/body-form.html index ac2887159..966bdf799 100644 --- a/templates/web/zurich/admin/body-form.html +++ b/templates/web/zurich/admin/body-form.html @@ -47,7 +47,7 @@ <p> <input type="hidden" name="posted" value="body"> - <input type="hidden" name="token" value="[% token %]"> + <input type="hidden" name="token" value="[% csrf_token %]"> <p> <input type="submit" value="[% body ? loc('Update body') : loc('Add body') %]"> </p> diff --git a/templates/web/zurich/admin/body.html b/templates/web/zurich/admin/body.html index 771f1e537..1a156773d 100644 --- a/templates/web/zurich/admin/body.html +++ b/templates/web/zurich/admin/body.html @@ -55,7 +55,7 @@ <p> <input type="hidden" name="posted" value="new" > - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="submit" name="Create category" value="[% errors ? loc('Save changes') : loc('Create category') %]"> </p> diff --git a/templates/web/zurich/admin/report_edit-sdm.html b/templates/web/zurich/admin/report_edit-sdm.html index 63c6b5e6f..e7602b659 100644 --- a/templates/web/zurich/admin/report_edit-sdm.html +++ b/templates/web/zurich/admin/report_edit-sdm.html @@ -11,7 +11,7 @@ </div> <form method="post" action="[% c.uri_for( 'report_edit', problem.id ) %]" enctype="application/x-www-form-urlencoded" accept-charset="utf-8"> - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="hidden" name="submit" value="1" > <div class="admin-report-edit admin-report-edit--details"> diff --git a/templates/web/zurich/admin/report_edit.html b/templates/web/zurich/admin/report_edit.html index dd876935c..f1ee7a846 100644 --- a/templates/web/zurich/admin/report_edit.html +++ b/templates/web/zurich/admin/report_edit.html @@ -13,7 +13,7 @@ [% pstate = problem.get_extra_metadata('closure_status') || problem.state %] <form id="report_edit" method="post" action="[% c.uri_for( 'report_edit', problem.id ) %]" enctype="application/x-www-form-urlencoded" accept-charset="utf-8"> - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="hidden" name="submit" value="1" > <div class="admin-report-edit admin-report-edit--details"> diff --git a/templates/web/zurich/admin/template_edit.html b/templates/web/zurich/admin/template_edit.html index 1deda6a77..dbad55f08 100644 --- a/templates/web/zurich/admin/template_edit.html +++ b/templates/web/zurich/admin/template_edit.html @@ -25,7 +25,7 @@ <textarea name="text" class="required">[% rt.text |html %]</textarea> </p> <p> - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="submit" name="Edit templates" value="[% rt.id ? loc('Save changes') : loc('Create template') %]" > </p> [% IF rt.id %] diff --git a/templates/web/zurich/admin/update_edit.html b/templates/web/zurich/admin/update_edit.html index fbd96f3a5..adafff3a8 100644 --- a/templates/web/zurich/admin/update_edit.html +++ b/templates/web/zurich/admin/update_edit.html @@ -4,7 +4,7 @@ [% status_message %] <form method="post" action="[% c.uri_for( 'update_edit', update.id ) %]" enctype="application/x-www-form-urlencoded" accept-charset="utf-8"> - <input type="hidden" name="token" value="[% token %]" > + <input type="hidden" name="token" value="[% csrf_token %]" > <input type="hidden" name="submit" value="1" > <ul> <li><a href="[% c.uri_for_email( '/report', update.problem_id ) %]#update_[% update.id %]">[% loc('View report on site' )%]</a></li> |