diff options
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth.pm | 5 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/Update.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Tokens.pm | 107 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/ResultSet/AlertType.pm | 7 | ||||
-rw-r--r-- | t/app/controller/alert_new.t | 8 | ||||
-rw-r--r-- | t/app/model/alert_type.t | 2 | ||||
-rw-r--r-- | templates/web/base/report/update-form.html | 2 | ||||
-rw-r--r-- | templates/web/eastsussex/report/update-form.html | 2 | ||||
-rw-r--r-- | templates/web/fixmystreet/report/update-form.html | 2 |
10 files changed, 102 insertions, 41 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index 66cf3979c..63bf91ff5 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -155,6 +155,11 @@ sub token : Path('/M') : Args(1) { return; } + if ( $token_obj->created < DateTime->now->subtract( days => 1 ) ) { + $c->stash->{token_not_found} = 1; + return; + } + # Sign out in case we are another user $c->logout(); diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 97250f859..7b001ee4c 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -151,6 +151,10 @@ sub load_updates : Private { @combined = map { $_->[1] } sort { $a->[0] <=> $b->[0] } @combined; $c->stash->{updates} = \@combined; + if ($c->sessionid && $c->flash->{alert_to_reporter}) { + $c->stash->{alert_to_reporter} = 1; + } + return 1; } diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index 79fed0fd1..17aec2113 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -420,8 +420,8 @@ happen before calling this. sub signup_for_alerts : Private { my ( $self, $c ) = @_; + my $update = $c->stash->{update}; if ( $c->stash->{add_alert} ) { - my $update = $c->stash->{update}; my $options = { user => $update->user, alert_type => 'new_updates', @@ -438,7 +438,7 @@ sub signup_for_alerts : Private { } $alert->confirm(); - } elsif ( $c->user && ( my $alert = $c->user->alert_for_problem($c->stash->{update}->problem_id) ) ) { + } elsif ( my $alert = $update->user->alert_for_problem($update->problem_id) ) { $alert->disable(); } diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm index 96bcedd37..21c269502 100644 --- a/perllib/FixMyStreet/App/Controller/Tokens.pm +++ b/perllib/FixMyStreet/App/Controller/Tokens.pm @@ -43,17 +43,17 @@ sub confirm_problem : Path('/P') { # Load the problem my $data = $auth_token->data; - my $problem_id = ref $data ? $data->{id} : $data; + $data = { id => $data } unless ref $data; + + my $problem_id = $data->{id}; # Look at all problems, not just cobrand, in case am approving something we don't actually show my $problem = $c->model('DB::Problem')->find( { id => $problem_id } ) || $c->detach('token_error'); $c->stash->{report} = $problem; - if ( $problem->state eq 'unconfirmed' && $auth_token->created < DateTime->now->subtract( months => 1 ) ) { - $c->stash->{template} = 'errors/generic.html'; - $c->stash->{message} = _("I'm afraid we couldn't validate that token, as the report was made too long ago."); - return; - } + $c->detach('token_too_old') + if $problem->state eq 'unconfirmed' + && $auth_token->created < DateTime->now->subtract( months => 1 ); # check that this email or domain are not the cause of abuse. If so hide it. if ( $problem->is_from_abuser ) { @@ -71,7 +71,7 @@ sub confirm_problem : Path('/P') { confirmed => \'ms_current_timestamp()', } ); - if ( ref($data) && ( $data->{name} || $data->{password} ) ) { + if ( $data->{name} || $data->{password} ) { $problem->user->name( $data->{name} ) if $data->{name}; $problem->user->phone( $data->{phone} ) if $data->{phone}; $problem->user->update; @@ -80,21 +80,26 @@ sub confirm_problem : Path('/P') { return 1; } - # We have a problem - confirm it if needed! - my $old_state = $problem->state; + if ($problem->state ne 'unconfirmed') { + my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; + $c->res->redirect($report_uri); + return; + } + + # We have an unconfirmed problem $problem->update( { state => 'confirmed', confirmed => \'ms_current_timestamp()', lastupdate => \'ms_current_timestamp()', } - ) if $problem->state eq 'unconfirmed'; + ); # Subscribe problem reporter to email updates $c->forward( '/report/new/create_reporter_alert' ); # log the problem creation user in to the site - if ( ref($data) && ( $data->{name} || $data->{password} ) ) { + if ( $data->{name} || $data->{password} ) { $problem->user->name( $data->{name} ) if $data->{name}; $problem->user->phone( $data->{phone} ) if $data->{phone}; $problem->user->password( $data->{password}, 1 ) if $data->{password}; @@ -104,11 +109,6 @@ sub confirm_problem : Path('/P') { $c->authenticate( { email => $problem->user->email }, 'no_password' ); $c->set_session_cookie_expire(0); - if ( FixMyStreet::DB::Result::Problem->visible_states()->{$old_state} ) { - my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; - $c->res->redirect($report_uri); - } - $c->stash->{created_report} = 'fromemail'; return 1; } @@ -149,21 +149,27 @@ sub confirm_alert : Path('/A') { my $auth_token = $c->forward( 'load_auth_token', [ $token_code, 'alert' ] ); - # Load the problem + # Load the alert my $alert_id = $auth_token->data->{id}; $c->stash->{confirm_type} = $auth_token->data->{type}; my $alert = $c->model('DB::Alert')->find( { id => $alert_id } ) || $c->detach('token_error'); $c->stash->{alert} = $alert; + $c->detach('token_too_old') + if $c->stash->{confirm_type} ne 'unsubscribe' + && $auth_token->created < DateTime->now->subtract( months => 1 ); + # check that this email or domain are not the cause of abuse. If so hide it. if ( $alert->is_from_abuser ) { $c->stash->{template} = 'tokens/abuse.html'; return; } - $c->authenticate( { email => $alert->user->email }, 'no_password' ); - $c->set_session_cookie_expire(0); + if (!$alert->confirmed && $c->stash->{confirm_type} ne 'unsubscribe') { + $c->authenticate( { email => $alert->user->email }, 'no_password' ); + $c->set_session_cookie_expire(0); + } $c->forward('/alert/confirm'); @@ -195,7 +201,7 @@ sub confirm_update : Path('/C') { my $auth_token = $c->forward( 'load_auth_token', [ $token_code, 'comment' ] ); - # Load the problem + # Load the update my $data = $auth_token->data; my $comment_id = $data->{id}; $c->stash->{add_alert} = $data->{add_alert}; @@ -204,26 +210,32 @@ sub confirm_update : Path('/C') { || $c->detach('token_error'); $c->stash->{update} = $comment; + $c->detach('token_too_old') + if $comment->state ne 'confirmed' + && $auth_token->created < DateTime->now->subtract( months => 1 ); + # check that this email or domain are not the cause of abuse. If so hide it. if ( $comment->is_from_abuser ) { $c->stash->{template} = 'tokens/abuse.html'; return; } + if ( $comment->state ne 'unconfirmed' ) { + my $report_uri = $c->cobrand->base_url_for_report( $comment->problem ) . $comment->problem->url; + $c->res->redirect($report_uri); + return; + } + if ( $data->{name} || $data->{password} ) { $comment->user->name( $data->{name} ) if $data->{name}; $comment->user->password( $data->{password}, 1 ) if $data->{password}; $comment->user->update; } + $c->authenticate( { email => $comment->user->email }, 'no_password' ); $c->set_session_cookie_expire(0); - if ( $comment->confirmed ) { - my $report_uri = $c->cobrand->base_url_for_report( $comment->problem ) . $comment->problem->url; - $c->res->redirect($report_uri); - } else { - $c->forward('/report/update/confirm'); - } + $c->forward('/report/update/confirm'); return 1; } @@ -234,6 +246,7 @@ sub load_questionnaire : Private { my $auth_token = $c->forward( 'load_auth_token', [ $token_code, 'questionnaire' ] ); $c->stash->{id} = $auth_token->data; $c->stash->{token} = $token_code; + $c->stash->{token_obj} = $auth_token; my $questionnaire = $c->model('DB::Questionnaire')->find( { id => $c->stash->{id} }, @@ -247,11 +260,43 @@ sub questionnaire : Path('/Q') : Args(1) { my ( $self, $c, $token_code ) = @_; $c->forward( 'load_questionnaire', [ $token_code ] ); - $c->authenticate( { email => $c->stash->{questionnaire}->problem->user->email }, 'no_password' ); - $c->set_session_cookie_expire(0); + $c->detach('token_too_old') if $c->stash->{token_obj}->created < DateTime->now->subtract( months => 1 ); + + my $questionnaire = $c->stash->{questionnaire}; + if (!$questionnaire->whenanswered) { + $c->authenticate( { email => $questionnaire->problem->user->email }, 'no_password' ); + $c->set_session_cookie_expire(0); + } $c->forward( '/questionnaire/show' ); } +=head2 alert_to_reporter + + /R/([0-9A-Za-z]{16,18}).*$ + +A link in an update alert to a problem reporter - show the "reopen report" +tickbox but don't log the person in. + +=cut + +sub alert_to_reporter : Path('/R') { + my ( $self, $c, $token_code ) = @_; + + my $auth_token = + $c->forward( 'load_auth_token', [ $token_code, 'alert_to_reporter' ] ); + my $data = $auth_token->data; + + my $problem_id = $data->{id}; + my $problem = $c->model('DB::Problem')->find( { id => $problem_id } ) + || $c->detach('token_error'); + + $c->detach('token_too_old') if $auth_token->created < DateTime->now->subtract( months => 1 ); + + $c->flash->{alert_to_reporter} = 1; + my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; + $c->res->redirect($report_uri); +} + =head2 load_auth_token my $auth_token = @@ -298,6 +343,12 @@ sub token_error : Private { $c->stash->{template} = 'tokens/error.html'; } +sub token_too_old : Private { + my ( $self, $c ) = @_; + $c->stash->{token_not_found} = 1; + $c->stash->{template} = 'auth/token.html'; +} + __PACKAGE__->meta->make_immutable; 1; diff --git a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm index ad180cbd5..0b430008a 100644 --- a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm +++ b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm @@ -103,13 +103,12 @@ sub email_alerts ($) { } ); $data{alert_email} = $user->email; my $token_obj = FixMyStreet::App->model('DB::Token')->create( { - scope => 'email_sign_in', + scope => 'alert_to_reporter', data => { - email => $user->email, - r => 'report/' . $row->{id}, + id => $row->{id}, } } ); - $data{problem_url} = $url . "/M/" . $token_obj->token; + $data{problem_url} = $url . "/R/" . $token_obj->token; } else { $data{problem_url} = $url . "/report/" . $row->{id}; } diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t index 708a152bc..ac2ec20ac 100644 --- a/t/app/controller/alert_new.t +++ b/t/app/controller/alert_new.t @@ -461,19 +461,21 @@ subtest "Test normal alert signups and that alerts are sent" => sub { like $email->body, qr/Other User/, 'Update name given'; unlike $email->body, qr/Anonymous User/, 'Update name not given'; - # The update alert was to the problem reporter, so has a login update URL + # The update alert was to the problem reporter, so has a special update URL + $mech->log_out_ok; $mech->get_ok( "/report/$report_id" ); $mech->content_lacks( 'has not been fixed' ); - my ($url) = $email->body =~ m{(http://\S+/M/\S+)}; + my ($url) = $email->body =~ m{(http://\S+/R/\S+)}; ok $url, "extracted update url '$url'"; $mech->get_ok( $url ); is $mech->uri->path, "/report/" . $report_id, "redirected to report page"; $mech->content_contains( 'has not been fixed' ); - $mech->logged_in_ok; + $mech->not_logged_in_ok; ($url) = $emails[0]->body =~ m{http://\S+(/A/\S+)}; $mech->get_ok( $url ); $mech->content_contains('alert deleted'); + $mech->not_logged_in_ok; $mech->delete_user($user1); $mech->delete_user($user2); diff --git a/t/app/model/alert_type.t b/t/app/model/alert_type.t index 5b7494a04..528c4d354 100644 --- a/t/app/model/alert_type.t +++ b/t/app/model/alert_type.t @@ -150,7 +150,7 @@ for my $test ( like $body, qr/$msg/, 'email says problem is ' . $test->{state}; if ($to eq $user->email) { - like $body, qr{/M/}, 'contains problem login url'; + like $body, qr{/R/}, 'contains problem login url'; } elsif ($to eq $user3->email) { like $body, qr{/report/$report_id}, 'contains problem url'; } diff --git a/templates/web/base/report/update-form.html b/templates/web/base/report/update-form.html index 50bc2906c..4e762a9a5 100644 --- a/templates/web/base/report/update-form.html +++ b/templates/web/base/report/update-form.html @@ -37,7 +37,7 @@ </select> </div> [% ELSE %] - [% IF problem.is_fixed AND c.user_exists AND c.user.id == problem.user_id %] + [% IF problem.is_fixed AND ((c.user_exists AND c.user.id == problem.user_id) OR alert_to_reporter) %] <div class="checkbox"> <input type="checkbox" name="reopen" id="form_reopen" value="1"[% ' checked' IF update.mark_open %]> <label class="inline" for="form_reopen">[% loc('This problem has not been fixed') %]</label> diff --git a/templates/web/eastsussex/report/update-form.html b/templates/web/eastsussex/report/update-form.html index e4314b81d..af966f417 100644 --- a/templates/web/eastsussex/report/update-form.html +++ b/templates/web/eastsussex/report/update-form.html @@ -82,7 +82,7 @@ [% END %] </select> [% ELSE %] - [% IF problem.is_fixed AND c.user_exists AND c.user.id == problem.user_id %] + [% IF problem.is_fixed AND ((c.user_exists AND c.user.id == problem.user_id) OR alert_to_reporter) %] <input type="checkbox" name="reopen" id="form_reopen" value="1"[% ' checked' IF update.mark_open %]> <label class="inline" for="form_reopen">[% loc('This problem has not been fixed') %]</label> diff --git a/templates/web/fixmystreet/report/update-form.html b/templates/web/fixmystreet/report/update-form.html index bde2c84e2..f5dca4669 100644 --- a/templates/web/fixmystreet/report/update-form.html +++ b/templates/web/fixmystreet/report/update-form.html @@ -34,7 +34,7 @@ [% END %] </select> [% ELSE %] - [% IF problem.is_fixed AND c.user_exists AND c.user.id == problem.user_id %] + [% IF problem.is_fixed AND ((c.user_exists AND c.user.id == problem.user_id) OR alert_to_reporter) %] <input type="checkbox" name="reopen" id="form_reopen" value="1"[% ' checked' IF update.mark_open %]> <label class="inline" for="form_reopen">[% loc('This problem has not been fixed') %]</label> |