Skip to content

Commit

Permalink
Version 1.5.2.
Browse files Browse the repository at this point in the history
Includes:
* [UK] Don't show topic form field when reporting abuse.
* Use token in moderation response URL to prevent hidden report leak.
* Make sure successful submission page is full width.
  • Loading branch information
dracos committed Dec 17, 2014
1 parent e57f715 commit 0009017
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 48 deletions.
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ RSS alerts of problems in their area.

It was created in 2007 by [mySociety](https://www.mysociety.org/) for reporting
problems to UK councils and has been copied around the world. The FixMyStreet
Platform is now at version 1.5.1.
Platform is now at version 1.5.2.

## Installation

Expand All @@ -38,6 +38,12 @@ We've extracted all of the mobile apps from this repository into the

## Releases

* v1.5.2 (17th December 2014)
- Hide unneeded heading on default footer.
- Suppress 'Argument "" isn't numeric' warning on admin report edit page.
- [UK] Don't show topic form field when reporting abuse.
- Use token in moderation response URL to prevent hidden report leak.

* v1.5.1 (12th December 2014)
- Bugfixes
- Use correct cobrand signature in SendReport emails. #960
Expand Down
2 changes: 1 addition & 1 deletion bin/site-specific-install.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/sh

# Set this to the version we want to check out
VERSION=${VERSION_OVERRIDE:-v1.5.1}
VERSION=${VERSION_OVERRIDE:-v1.5.2}

PARENT_SCRIPT_URL=https://github.com/mysociety/commonlib/blob/master/bin/install-site.sh

Expand Down
36 changes: 13 additions & 23 deletions perllib/FixMyStreet/App/Controller/Contact.pm
Original file line number Diff line number Diff line change
Expand Up @@ -59,33 +59,24 @@ generic contact request and set up things accordingly
sub determine_contact_type : Private {
my ( $self, $c ) = @_;

my $id = $c->req->param('id');
my $id = $c->req->param('id');
my $update_id = $c->req->param('update_id');
my $token = $c->req->param('m');
$id = undef unless $id && $id =~ /^[1-9]\d*$/;
$update_id = undef unless $update_id && $update_id =~ /^[1-9]\d*$/;

if ($id) {

# if we're moderating, then we don't show errors in every case, e.g.
# for hidden reports
if ($c->req->param('m')) {
my $problem
= ( !$id || $id =~ m{\D} ) # is id non-numeric?
? undef # ...don't even search
: $c->cobrand->problems->find( { id => $id } );

if ($problem) {
$c->stash->{problem} = $problem;
$c->stash->{moderation_complaint} = 1;
}
else {
$c->forward( '/report/load_problem_or_display_error', [ $id ] );
}
}
else {
if ($token) {
my $token_obj = $c->forward('/tokens/load_auth_token', [ $token, 'moderation' ]);
my $problem = $c->cobrand->problems->find( { id => $token_obj->data->{id} } );
if ($problem) {
$c->stash->{problem} = $problem;
$c->stash->{moderation_complaint} = $token;
} else {
$c->forward( '/report/load_problem_or_display_error', [ $id ] );
}

} elsif ($id) {
$c->forward( '/report/load_problem_or_display_error', [ $id ] );
if ($update_id) {
my $update = $c->model('DB::Comment')->find(
{ id => $update_id }
Expand Down Expand Up @@ -132,9 +123,8 @@ sub validate : Private {
);

push @errors, _('Illegal ID')
if $c->req->param('id') && $c->req->param('id') !~ /^[1-9]\d*$/
or $c->req->param('update_id')
&& $c->req->param('update_id') !~ /^[1-9]\d*$/;
if $c->req->param('id') && !$c->stash->{problem}
or $c->req->param('update_id') && !$c->stash->{update};

push @errors, _('There was a problem showing this page. Please try again later.')
if $c->req->params->{message} && $c->req->params->{message} =~ /\[url=|<a/;
Expand Down
7 changes: 6 additions & 1 deletion perllib/FixMyStreet/App/Controller/Moderate.pm
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ sub report_moderate_audit : Private {
my $sender = FixMyStreet->config('DO_NOT_REPLY_EMAIL');
my $sender_name = _($cobrand->contact_name);

my $token = $c->model("DB::Token")->create({
scope => 'moderation',
data => { id => $problem->id }
});

$c->send_email( 'problem-moderated.txt', {

to => [ [ $user->email, $user->name ] ],
Expand All @@ -113,7 +118,7 @@ sub report_moderate_audit : Private {
user => $user,
problem => $problem,
report_uri => $c->stash->{report_uri},
report_complain_uri => $c->stash->{cobrand_base} . '/contact?m=1&id=' . $problem->id,
report_complain_uri => $c->stash->{cobrand_base} . '/contact?m=' . $token->token,
});
}

Expand Down
3 changes: 3 additions & 0 deletions perllib/FixMyStreet/Cobrand/FixMyStreet.pm
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ sub extra_contact_validation {
my $self = shift;
my $c = shift;

# Don't care about dest if reporting abuse
return () if $c->stash->{problem};

my %errors;

$c->stash->{dest} = $c->req->param('dest');
Expand Down
12 changes: 12 additions & 0 deletions t/app/controller/moderate.t
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ subtest 'Auth' => sub {

$mech->get_ok($REPORT_URL);
$mech->content_lacks('Moderat');

$mech->get_ok('/contact?m=1&id=' . $report->id);
$mech->content_lacks('Good bad bad bad');
};

subtest 'Affiliated and permissioned user can see moderation' => sub {
Expand Down Expand Up @@ -158,6 +161,8 @@ subtest 'Problem moderation' => sub {
};

subtest 'Hide report' => sub {
$mech->clear_emails_ok;

my $resp = $mech->post('/moderate/report/' . $report->id, {
%problem_prepopulated,
problem_hide => 1,
Expand All @@ -167,6 +172,13 @@ subtest 'Problem moderation' => sub {
$report->discard_changes;
is $report->state, 'hidden', 'Is hidden';

my $email = $mech->get_email;
my ($url) = $email->body =~ m{(http://\S+)};
ok $url, "extracted complain url '$url'";

$mech->get_ok($url);
$mech->content_contains('Good bad bad bad');

# reset
$report->update({ state => 'confirmed' });
};
Expand Down
1 change: 1 addition & 0 deletions templates/web/base/contact/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ <h1>[% loc('Contact the team') %]</h1>
[% ELSIF problem %]
<p>
[% IF moderation_complaint %]
<input type="hidden" name="m" value="[% moderation_complaint %]">
[% loc('You are complaining that this problem report was unnecessarily moderated:') %]
[% ELSE %]
[% loc('You are reporting the following problem report for being abusive, containing personal information, or similar:') %]
Expand Down
2 changes: 1 addition & 1 deletion templates/web/base/contact/submit.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[% INCLUDE 'header.html', title = loc('Contact Us') %]
[% INCLUDE 'header.html', title = loc('Contact Us'), bodyclass = 'fullwidthpage' %]

<h1>[% loc('Contact the team') %]</h1>

Expand Down
30 changes: 11 additions & 19 deletions templates/web/base/report/_main.html
Original file line number Diff line number Diff line change
Expand Up @@ -66,28 +66,20 @@ <h1><input type="text" name="problem_title" value="[% problem.title | html %]"><
</em></p>
[% INCLUDE 'report/_support.html' %]

[% IF c.cobrand.moniker != 'southampton' %]
[% INCLUDE 'report/photo.html' object=problem %]
[% END %]
[% INCLUDE 'report/photo.html' object=problem %]
<div class="moderate-display">
[% add_links( problem.detail ) | html_para %]
</div>

<div class="moderate-display">
[% add_links( problem.detail ) | html_para %]
</div>
[% IF moderating %]
<div class="moderate-edit">
[% IF problem.detail != original.detail %]
<input type="checkbox" name="problem_revert_detail" class="revert-textarea">
<label for="problem_revert_detail">Revert to original text</label>
[% END %]
<textarea name="problem_detail">[% add_links( problem.detail ) %]</textarea>
</div>
[% IF moderating %]
<div class="moderate-edit">
[% IF problem.detail != original.detail %]
<input type="checkbox" name="problem_revert_detail" class="revert-textarea">
<label for="problem_revert_detail">Revert to original text</label>
[% END %]
<textarea name="problem_detail">[% add_links( problem.detail ) %]</textarea>
</div>

[% IF c.cobrand.moniker == 'southampton' %]
[% INCLUDE 'report/photo.html' object=problem %]
[% END %]

[% IF moderating %]
<div class="moderate-edit">
<label for="moderation_reason">Moderation reason:</label>
<input type="text" name="moderation_reason" placeholder="Describe why you are moderating this">
Expand Down
6 changes: 4 additions & 2 deletions templates/web/fixmystreet.com/contact/who.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[% IF NOT problem %]
<h4>Topic:</h4>

[% IF field_errors.dest %]
Expand Down Expand Up @@ -55,8 +56,8 @@ <h4>Topic:</h4>
</div>

<div class="checkbox-group">
<input name="dest" id="dest_feeback" type="radio" value="feeback" class="required"[% IF dest AND dest == 'feeback' %] checked[% END %]>
<label class="inline" for="dest_feeback">I have feedback about the site</label>
<input name="dest" id="dest_feedback" type="radio" value="feedback" class="required"[% IF dest AND dest == 'feedback' %] checked[% END %]>
<label class="inline" for="dest_feedback">I have feedback about the site</label>
</div>

<div class="checkbox-group">
Expand All @@ -73,3 +74,4 @@ <h4>Topic:</h4>
<input name="dest" id="dest_update" type="radio" value="update"[% IF dest AND dest == 'update' %] checked[% END %]>
<label class="inline" for="dest_update">My street problem hasn't been fixed</label>
</div>
[% END %]
1 change: 1 addition & 0 deletions templates/web/fixmystreet/contact/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ <h1>[% loc('Contact the team') %]</h1>
[% ELSIF problem %]
<p>
[% IF moderation_complaint %]
<input type="hidden" name="m" value="[% moderation_complaint %]">
[% loc('You are complaining that this problem report was unnecessarily moderated:') %]
[% ELSE %]
[% loc('You are reporting the following problem report for being abusive, containing personal information, or similar:') %]
Expand Down

0 comments on commit 0009017

Please sign in to comment.