diff options
| -rw-r--r-- | conf/httpd.conf | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App.pm | 3 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Tokens.pm | 114 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/View/Web.pm | 15 | ||||
| -rw-r--r-- | t/app/controller/reports_new.t | 143 | ||||
| -rw-r--r-- | t/app/controller/tokens.t | 10 | ||||
| -rw-r--r-- | templates/web/default/tokens/abuse.html | 7 | ||||
| -rw-r--r-- | templates/web/default/tokens/confirm_problem.html | 22 | ||||
| -rw-r--r-- | templates/web/default/tokens/error.html | 9 | ||||
| -rw-r--r-- | templates/web/emptyhomes/tokens/confirm_problem.html | 39 | ||||
| -rwxr-xr-x | web/confirm.cgi | 65 |
11 files changed, 324 insertions, 104 deletions
diff --git a/conf/httpd.conf b/conf/httpd.conf index 72993ee9a..066d2bd08 100644 --- a/conf/httpd.conf +++ b/conf/httpd.conf @@ -54,7 +54,6 @@ RewriteRule ^(.+)/$ $1 [R=permanent] # Confirmation tokens RewriteRule ^/[Aa]/([0-9A-Za-z]{16,18}).*$ /alert.cgi?token=$1 [QSA,L] RewriteRule ^/[Cc]/([0-9A-Za-z]{16,18}).*$ /confirm.cgi?type=update;token=$1 [QSA,L] -RewriteRule ^/[Pp]/([0-9A-Za-z]{16,18}).*$ /confirm.cgi?type=problem;token=$1 [QSA,L] RewriteRule ^/[Qq]/([0-9A-Za-z]{16,18}).*$ /questionnaire.cgi?token=$1 [QSA,L] RewriteRule ^/[Ff]/([0-9A-Za-z]{16,18}).*$ /flickr.cgi?token=$1 [L] RewriteRule ^/[Ll]/([0-9A-Za-z]{16,18}).*$ /index.cgi?partial_token=$1 [L] diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm index d73e721bf..a35ba6c01 100644 --- a/perllib/FixMyStreet/App.pm +++ b/perllib/FixMyStreet/App.pm @@ -76,6 +76,9 @@ __PACKAGE__->config( # Start the application __PACKAGE__->setup(); +# set up DB handle for old code +FixMyStreet->configure_mysociety_dbhandle; + # disable debug logging unless in debaug mode __PACKAGE__->log->disable('debug') # unless __PACKAGE__->debug; diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm new file mode 100644 index 000000000..6feae9e9c --- /dev/null +++ b/perllib/FixMyStreet/App/Controller/Tokens.pm @@ -0,0 +1,114 @@ +package FixMyStreet::App::Controller::Tokens; +use Moose; +use namespace::autoclean; + +BEGIN { extends 'Catalyst::Controller'; } + +use FixMyStreet::Alert; + +=head1 NAME + +FixMyStreet::App::Controller::Tokens - Handle auth tokens + +=head1 DESCRIPTION + +Act on the various tokens that can be submitted. + +=head1 METHODS + +=cut + +=head2 confirm_problem + + /[Pp]/([0-9A-Za-z]{16,18}).*$ + +Confirm a problem - url appears in emails sent to users after they create the +problem but are not logged in. + +=cut + +sub confirm_problem : Path('/P') { + my ( $self, $c, $token_code ) = @_; + + my $auth_token = + $c->forward( 'load_auth_token', [ $token_code, 'problem' ] ); + + # Load the problem + my $problem_id = $auth_token->data; + my $problem = $c->model('DB::Problem')->find( { id => $problem_id } ) + || $c->detach('token_error'); + $c->stash->{problem} = $problem; + + # check that this email or domain are not the cause of abuse. If so hide it. + if ( $problem->is_from_abuser ) { + $problem->update( + { state => 'hidden', lastupdate => \'ms_current_timestamp()' } ); + $c->stash->{template} = 'tokens/abuse.html'; + return; + } + + # We have a problem - confirm it if needed! + $problem->update( + { + state => 'confirmed', + confirmed => \'ms_current_timestamp()', + lastupdate => \'ms_current_timestamp()', + } + ) if $problem->state eq 'unconfirmed'; + + # Subscribe problem reporter to email updates + my $alert_id = + FixMyStreet::Alert::create( $problem->user->email, 'new_updates', + $problem->cobrand, $problem->cobrand_data, $problem_id ); + FixMyStreet::Alert::confirm($alert_id); + + # log the problem creation user in to the site + $c->authenticate( { email => $problem->user->email }, 'no_password' ); + + return 1; +} + +=head2 load_auth_token + + my $auth_token = + $c->forward( 'load_auth_token', [ $token_code, $token_scope ] ); + + +Load the token if possible. If token is not found, or not valid detach to a nice +error message. + +=cut + +sub load_auth_token : Private { + my ( $self, $c, $token_code, $scope ) = @_; + + # clean the token of bad chars (in case of email client issues) + $token_code ||= ''; + $token_code =~ s{[^a-zA-Z0-9]+}{}g; + + # try to load the token + my $token = $c->model('DB::Token')->find( + { + scope => $scope, + token => $token_code, + } + ) || $c->detach('token_error'); + + return $token; +} + +=head2 token_error + +Display an error page saying that there is something wrong with the token. + +=cut + +sub token_error : Private { + my ( $self, $c ) = @_; + $c->stash->{template} = 'tokens/error.html'; + $c->detach; +} + +__PACKAGE__->meta->make_immutable; + +1; diff --git a/perllib/FixMyStreet/App/View/Web.pm b/perllib/FixMyStreet/App/View/Web.pm index e5983ea23..75ca4dd81 100644 --- a/perllib/FixMyStreet/App/View/Web.pm +++ b/perllib/FixMyStreet/App/View/Web.pm @@ -66,5 +66,20 @@ sub tprintf { return sprintf $format, @args; } +=head2 display_crossell_advert + + [% display_crossell_advert( email, name ) %] + +Displays a crosssell advert if permitted by the cobrand. + +=cut + +sub display_crossell_advert { + my ( $self, $c, $email, $name ) = @_; + + return unless $c->cobrand->allow_crosssell_adverts(); + return CrossSell::display_advert( $c->req, $email, $name ); +} + 1; diff --git a/t/app/controller/reports_new.t b/t/app/controller/reports_new.t index dd353a903..aa4920cc5 100644 --- a/t/app/controller/reports_new.t +++ b/t/app/controller/reports_new.t @@ -55,13 +55,15 @@ foreach my $test ( }, ) { - $mech->get_ok('/reports/new'); - $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, - "bad location" ); - is_deeply $mech->form_errors, $test->{errors}, - "expected errors for pc '$test->{pc}'"; - is_deeply $mech->pc_alternatives, $test->{pc_alternatives}, - "expected alternatives for pc '$test->{pc}'"; + subtest "test bad pc value '$test->{pc}'" => sub { + $mech->get_ok('/reports/new'); + $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + "bad location" ); + is_deeply $mech->form_errors, $test->{errors}, + "expected errors for pc '$test->{pc}'"; + is_deeply $mech->pc_alternatives, $test->{pc_alternatives}, + "expected alternatives for pc '$test->{pc}'"; + }; } # check that exact queries result in the correct lat,lng @@ -83,12 +85,14 @@ foreach my $test ( }, ) { - $mech->get_ok('/reports/new'); - $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, - "good location" ); - is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'"; - is_deeply $mech->extract_location, $test, - "got expected location for pc '$test->{pc}'"; + subtest "check lat/lng for '$test->{pc}'" => sub { + $mech->get_ok('/reports/new'); + $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + "good location" ); + is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'"; + is_deeply $mech->extract_location, $test, + "got expected location for pc '$test->{pc}'"; + }; } # test that the various bit of form get filled in and errors correctly @@ -280,37 +284,102 @@ foreach my $test ( }, ) { - pass "--- $test->{msg} ---"; - $mech->get_ok('/reports/new'); + subtest "check form errors where $test->{msg}" => sub { + $mech->get_ok('/reports/new'); + + # submit initial pc form + $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + "submit location" ); + is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'"; + + # submit the main form + $mech->submit_form_ok( { with_fields => $test->{fields} }, + "submit form" ); + + # check that we got the errors expected + is_deeply $mech->form_errors, $test->{errors}, "check errors"; + + # check that fields have changed as expected + my $new_values = { + %{ $test->{fields} }, # values added to form + %{ $test->{changes} }, # changes we expect + }; + is_deeply $mech->visible_form_values, $new_values, + "values correctly changed"; + }; +} + +subtest "test report creation for a user who does not have an account" => sub { + $mech->log_out_ok; + $mech->clear_emails_ok; + + # check that the user does not exist + my $test_email = 'test-1@example.com'; + ok !FixMyStreet::App->model('DB::User')->find( { email => $test_email } ), + "test user does not exist"; # submit initial pc form - $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + $mech->get_ok('/reports/new'); + $mech->submit_form_ok( { with_fields => { pc => 'SW1A 1AA', } }, "submit location" ); - is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'"; - - # submit the main form - $mech->submit_form_ok( { with_fields => $test->{fields} }, "submit form" ); + $mech->submit_form_ok( + { + with_fields => { + title => 'Test Report', + detail => 'Test report details.', + photo => '', + name => 'Joe Bloggs', + may_show_name => '1', + email => 'test-1@example.com', + phone => '07903 123 456', + } + }, + "submit good details" + ); # check that we got the errors expected - is_deeply $mech->form_errors, $test->{errors}, "check errors"; + is_deeply $mech->form_errors, [], "check there were no errors"; - # check that fields have changed as expected - my $new_values = { - %{ $test->{fields} }, # values added to form - %{ $test->{changes} }, # changes we expect - }; - is_deeply $mech->visible_form_values, $new_values, - "values correctly changed"; + # check that the user has been created + my $user = + FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); + ok $user, "created new user"; -} + # find the report + my $report = $user->problems->first; + ok $report, "Found the report"; -#### test report creation for a user who does not have an account -# come to site -# fill in report -# receive token -# confirm token -# report is confirmed -# user is created and logged in + # check that the report is not available yet. + is $report->state, 'unconfirmed', "report not confirmed"; + is $mech->get( '/reports/' . $report->id )->code, 404, "report not found"; + + # receive token + my $email = $mech->get_email; + ok $email, "got an email"; + like $email->body, qr/confirm the problem/i, "confirm the problem"; + + my ($url) = $email->body =~ m{(http://\S+)}; + ok $url, "extracted confirm url '$url'"; + + # confirm token + $mech->get_ok($url); + $report->discard_changes; + is $report->state, 'confirmed', "Report is now confirmed"; + is $report->state, 'confirmed', "report is now confirmed"; + + TODO: { + local $TODO = "'/reports/<<id>>' not handled by catalyst yet"; + $mech->get_ok( '/reports/' . $report->id ); + } + + # user is created and logged in + $mech->logged_in_ok; + + # cleanup + $mech->log_out_ok; + ok $_->delete, "delete problem" for $user->problems; + ok $user->delete, "delete test user"; +}; #### test report creation for a user who has account but is not logged in # come to site @@ -328,8 +397,6 @@ foreach my $test ( #### test completing a partial report (eq flickr upload) -#### test error cases when filling in a report - #### possibly manual testing # create report without using map # create report by clicking on may with javascript off diff --git a/t/app/controller/tokens.t b/t/app/controller/tokens.t new file mode 100644 index 000000000..eec1b0d35 --- /dev/null +++ b/t/app/controller/tokens.t @@ -0,0 +1,10 @@ +use strict; +use warnings; +use Test::More; + + +use Catalyst::Test 'FixMyStreet::App'; +use FixMyStreet::App::Controller::Tokens; + +ok( request('/tokens')->is_success, 'Request should succeed' ); +done_testing(); diff --git a/templates/web/default/tokens/abuse.html b/templates/web/default/tokens/abuse.html new file mode 100644 index 000000000..d1b952621 --- /dev/null +++ b/templates/web/default/tokens/abuse.html @@ -0,0 +1,7 @@ +[% INCLUDE 'header.html', title => loc('Error') %] + +<h1>[% loc('Error') %]</h1> + +<p>[% loc('Sorry, there has been an error confirming your problem.') %]</p> + +[% INCLUDE 'footer.html' %] diff --git a/templates/web/default/tokens/confirm_problem.html b/templates/web/default/tokens/confirm_problem.html new file mode 100644 index 000000000..23d5f52ee --- /dev/null +++ b/templates/web/default/tokens/confirm_problem.html @@ -0,0 +1,22 @@ +[% INCLUDE 'header.html', title => loc('Confirmation') %] + +<h1>[% loc('Confirmation') %]</h1> + +<p class="confirmed"> +[% + loc('You have successfully confirmed your problem'); + + IF problem.council; + loc(' and <strong>we will now send it to the council</strong>'); + END; + + tprintf( + loc( '. You can <a href="%s">view the problem on this site</a>.' ), + c.cobrand.url( '/report/' _ problem.id ) + ); +%] +</p> + +[% display_crossell_advert( problem.email, problem.name ) %] + +[% INCLUDE 'footer.html' %] diff --git a/templates/web/default/tokens/error.html b/templates/web/default/tokens/error.html new file mode 100644 index 000000000..76e22c119 --- /dev/null +++ b/templates/web/default/tokens/error.html @@ -0,0 +1,9 @@ +[% INCLUDE 'header.html', title => loc('Error') %] + +<h1>[% loc('Error') %]</h1> + +[% contact_url = c.cobrand.url('/contact'); %] + +<p>[% tprintf( loc('Thank you for trying to confirm your update or problem. We seem to have an error ourselves though, so <a href="%s">please let us know what went on</a> and we\'ll look into it.'), contact_url ) %]</p> + +[% INCLUDE 'footer.html' %] diff --git a/templates/web/emptyhomes/tokens/confirm_problem.html b/templates/web/emptyhomes/tokens/confirm_problem.html new file mode 100644 index 000000000..501f9234a --- /dev/null +++ b/templates/web/emptyhomes/tokens/confirm_problem.html @@ -0,0 +1,39 @@ +[% INCLUDE 'header.html', title => loc('Confirmation') %] + +<h1>[% loc('Confirmation') %]</h1> + +[% IF problem.council %] + <p>[% + loc( + 'Thank you for reporting an empty property on ReportEmptyHomes.com. We have emailed the lead officer for empty homes in the council responsible with details, and asked them to do whatever they can to get the empty property back into use as soon as possible.' + ) + %]</p> + + <p>[% + loc( + 'It is worth noting however that the process can sometimes be slow, especially if the property is in very poor repair or the owner is unwilling to act. In most cases it can take six months or more before you can expect to see anything change and sometimes there may be considerable barries to a property being brought back into use. This doesn’t mean the council isn’t doing anything. We encourage councils to update the website so you can see what is happening. It may be a long process, but you reporting your concerns about this property to the council is a valuable first step.' + ) + %]</p> + + <p>[% + loc( + 'We may contact you periodically to ask if anything has changed with the property you reported.' + ) + %]</p> + + <p>[% + loc( + 'Thank you for using ReportEmptyHomes.com. Your action is already helping to resolve the UK’s empty homes crisis.' + ) + %]</p> +[% ELSE %] + <p>[% + loc( 'Thank you for reporting an empty property on ReportEmptyHomes.com.' ) + %]</p> +[% END %] + +<p> + <a href="[% c.uri_for( '/reports', problem.id ) | html %]">[% loc('View your report') %]</a>. +</p> + +[% INCLUDE 'footer.html' %] diff --git a/web/confirm.cgi b/web/confirm.cgi index 925b64ed2..8836431ab 100755 --- a/web/confirm.cgi +++ b/web/confirm.cgi @@ -27,8 +27,6 @@ sub main { if ($data) { if ($type eq 'update') { $out = confirm_update($q, $data); - } elsif ($type eq 'problem') { - $out = confirm_problem($q, $data); } elsif ($type eq 'questionnaire') { $out = add_questionnaire($q, $data, $token); } @@ -120,69 +118,6 @@ sub confirm_update { return $out; } -sub confirm_problem { - my ($q, $id) = @_; - my $cobrand = Page::get_cobrand($q); - my ($council, $email, $name, $cobrand_data) = dbh()->selectrow_array("select council, email, name, cobrand_data from problem where id=?", {}, $id); - - (my $domain = $email) =~ s/^.*\@//; - if (dbh()->selectrow_array('select email from abuse where lower(email)=? or lower(email)=?', {}, lc($email), lc($domain))) { - dbh()->do("update problem set state='hidden', lastupdate=ms_current_timestamp() where id=?", {}, $id); - return $q->p(_('Sorry, there has been an error confirming your problem.')); - } else { - dbh()->do("update problem set state='confirmed', confirmed=ms_current_timestamp(), lastupdate=ms_current_timestamp() - where id=? and state='unconfirmed'", {}, $id); - } - my $out; - if ($q->{site} eq 'emptyhomes') { - if ($council) { - $out = $q->p(_('Thank you for reporting an empty property on -ReportEmptyHomes.com. We have emailed the lead officer for empty homes in the council -responsible with details, and asked them to do whatever they can to get the -empty property back into use as soon as possible.')) . -$q->p(_('It is worth noting however that the process can sometimes be slow, -especially if the property is in very poor repair or the owner is unwilling to -act. In most cases it can take six months or more before you can expect to see -anything change and sometimes there may be considerable barries to a property -being brought back into use. This doesn’t mean the council isn’t -doing anything. We encourage councils to update the website so you can -see what is happening. It may be a long process, but you reporting your -concerns about this property to the council is a valuable first step.')) . -$q->p(_('We may contact you periodically to ask if anything has changed -with the property you reported.')) . -$q->p(_('Thank you for using ReportEmptyHomes.com. Your action is already helping -to resolve the UK’s empty homes crisis.')) . -$q->p('<a href="/report/' . $id . '">' . _('View your report') . '</a>.'); - } else { - $out = $q->p(_('Thank you for reporting an empty property on ReportEmptyHomes.com.')) . -$q->p('<a href="/report/' . $id . '">' . _('View your report') . '</a>.'); - } - } else { - my $report_url = Cobrand::url($cobrand, "/report/$id", $q); - $out = $q->p({class => 'confirmed'}, - _('You have successfully confirmed your problem') - . ($council ? _(' and <strong>we will now send it to the council</strong>') : '') - . sprintf(_('. You can <a href="%s">view the problem on this site</a>.'), $report_url) - ); - my $display_advert = Cobrand::allow_crosssell_adverts($cobrand); - if ($display_advert) { - $out .= CrossSell::display_advert($q, $email, $name); - } - my %vars = ( - url_report => $report_url, - url_home => Cobrand::url($cobrand, '/', $q), - ); - my $cobrand_page = Page::template_include('confirmed-problem', $q, Page::template_root($q), %vars); - $out = $cobrand_page if $cobrand_page; - } - - # Subscribe problem reporter to email updates - my $alert_id = FixMyStreet::Alert::create($email, 'new_updates', $cobrand, $cobrand_data, $id); - FixMyStreet::Alert::confirm($alert_id); - - return $out; -} - sub ask_questionnaire { my ($token, $q) = @_; my $cobrand = Page::get_cobrand($q); |