aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--perllib/Catalyst/Authentication/Store/FixMyStreetUser.pm54
-rw-r--r--perllib/FixMyStreet/App.pm21
-rw-r--r--perllib/FixMyStreet/App/Controller/Admin.pm4
-rw-r--r--perllib/FixMyStreet/App/Controller/Admin/Users.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Moderate.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Photo.pm33
-rw-r--r--perllib/FixMyStreet/App/Controller/Report.pm5
-rw-r--r--perllib/FixMyStreet/App/Controller/Reports.pm2
-rw-r--r--perllib/FixMyStreet/App/Model/PhotoSet.pm7
-rw-r--r--perllib/FixMyStreet/DB/ResultSet/Problem.pm2
-rw-r--r--perllib/FixMyStreet/Roles/PhotoSet.pm7
-rw-r--r--t/app/controller/photo.t71
-rw-r--r--t/app/controller/token.t4
-rw-r--r--t/app/model/user.t16
-rw-r--r--web/cobrands/fixmystreet-uk-councils/roadworks.js2
-rw-r--r--web/cobrands/lincolnshire/assets.js5
-rw-r--r--web/cobrands/lincolnshire/roadworks.js15
17 files changed, 213 insertions, 39 deletions
diff --git a/perllib/Catalyst/Authentication/Store/FixMyStreetUser.pm b/perllib/Catalyst/Authentication/Store/FixMyStreetUser.pm
new file mode 100644
index 000000000..240f4b1de
--- /dev/null
+++ b/perllib/Catalyst/Authentication/Store/FixMyStreetUser.pm
@@ -0,0 +1,54 @@
+package Catalyst::Authentication::Store::FixMyStreetUser;
+
+use Moose;
+use namespace::autoclean;
+extends 'Catalyst::Authentication::Store::DBIx::Class::User';
+
+use Carp;
+use Try::Tiny;
+
+sub AUTOLOAD {
+ my $self = shift;
+ (my $method) = (our $AUTOLOAD =~ /([^:]+)$/);
+ return if $method eq "DESTROY";
+
+ if (my $code = $self->_user->can($method)) {
+ return $self->_user->$code(@_);
+ }
+ elsif (my $accessor =
+ try { $self->_user->result_source->column_info($method)->{accessor} }) {
+ return $self->_user->$accessor(@_);
+ } else {
+ croak sprintf("Can't locate object method '%s'", $method);
+ }
+}
+
+__PACKAGE__->meta->make_immutable(inline_constructor => 0);
+
+1;
+__END__
+
+=head1 NAME
+
+Catalyst::Authentication::Store::FixMyStreetUser - The backing user
+class for the Catalyst::Authentication::Store::DBIx::Class storage
+module, adjusted to die on unknown lookups.
+
+=head1 DESCRIPTION
+
+The Catalyst::Authentication::Store::FixMyStreetUser class implements user
+storage connected to an underlying DBIx::Class schema object.
+
+=head1 SUBROUTINES / METHODS
+
+=head2 AUTOLOAD
+
+Delegates method calls to the underlying user row.
+Unlike the default, dies if an unknown method is called.
+
+=head1 LICENSE
+
+Copyright (c) 2007-2019. All rights reserved. This program is free software;
+you can redistribute it and/or modify it under the same terms as Perl itself.
+
+=cut
diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm
index 36f736cd2..e9c3fac8d 100644
--- a/perllib/FixMyStreet/App.pm
+++ b/perllib/FixMyStreet/App.pm
@@ -34,6 +34,12 @@ extends 'Catalyst';
our $VERSION = '0.01';
+my $store = { # Catalyst::Authentication::Store::DBIx::Class
+ class => 'DBIx::Class',
+ user_model => 'DB::User',
+ store_user_class => 'Catalyst::Authentication::Store::FixMyStreetUser',
+};
+
__PACKAGE__->config(
# Use REQUEST_URI, not PATH_INFO, to infer path. This fixes an issue
@@ -83,20 +89,14 @@ __PACKAGE__->config(
},
],
},
- store => { # Catalyst::Authentication::Store::DBIx::Class
- class => 'DBIx::Class',
- user_model => 'DB::User',
- },
+ store => $store,
},
no_password => { # use post confirm etc
credential => { # Catalyst::Authentication::Credential::Password
class => 'Password',
password_type => 'none',
},
- store => { # Catalyst::Authentication::Store::DBIx::Class
- class => 'DBIx::Class',
- user_model => 'DB::User',
- },
+ store => $store,
},
access_token => {
use_session => 0,
@@ -106,10 +106,7 @@ __PACKAGE__->config(
# This means the token has to be 18 characters long (as generated by AuthToken)
token_lookup => { like => "%access_token,T18:TOKEN,%" },
},
- store => {
- class => 'DBIx::Class',
- user_model => 'DB::User',
- },
+ store => $store,
},
},
);
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 05ac48c8c..6167a16f5 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -535,8 +535,8 @@ sub report_edit : Path('report_edit') : Args(1) {
$self->remove_photo($c, $problem, $remove_photo_param);
}
- if ($problem->state eq 'hidden') {
- $problem->get_photoset->delete_cached;
+ if ($problem->state eq 'hidden' || $problem->non_public) {
+ $problem->get_photoset->delete_cached(plus_updates => 1);
}
if ( $problem->is_visible() and $old_state eq 'unconfirmed' ) {
diff --git a/perllib/FixMyStreet/App/Controller/Admin/Users.pm b/perllib/FixMyStreet/App/Controller/Admin/Users.pm
index bcbc808ed..6af4ae831 100644
--- a/perllib/FixMyStreet/App/Controller/Admin/Users.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin/Users.pm
@@ -497,7 +497,7 @@ sub user_hide_everywhere : Private {
my ( $self, $c, $user ) = @_;
my $problems = $user->problems->search({ state => { '!=' => 'hidden' } });
while (my $problem = $problems->next) {
- $problem->get_photoset->delete_cached;
+ $problem->get_photoset->delete_cached(plus_updates => 1);
$problem->update({ state => 'hidden' });
}
my $updates = $user->comments->search({ state => { '!=' => 'hidden' } });
diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm
index 22869d531..f71698e84 100644
--- a/perllib/FixMyStreet/App/Controller/Moderate.pm
+++ b/perllib/FixMyStreet/App/Controller/Moderate.pm
@@ -214,7 +214,7 @@ sub report_moderate_hide : Private {
if ($c->get_param('problem_hide')) {
$problem->update({ state => 'hidden' });
- $problem->get_photoset->delete_cached;
+ $problem->get_photoset->delete_cached(plus_updates => 1);
$c->res->redirect( '/' ); # Go directly to front-page
$c->detach( 'report_moderate_audit', ['hide'] ); # break chain here.
diff --git a/perllib/FixMyStreet/App/Controller/Photo.pm b/perllib/FixMyStreet/App/Controller/Photo.pm
index 7b536a292..d7a5b4bb3 100644
--- a/perllib/FixMyStreet/App/Controller/Photo.pm
+++ b/perllib/FixMyStreet/App/Controller/Photo.pm
@@ -39,6 +39,7 @@ sub during :LocalRegex('^(temp|fulltemp)\.([0-9a-f]{40}\.(?:jpeg|png|gif|tiff))$
$size = $size eq 'temp' ? 'default' : 'full';
my $photo = $photoset->get_image_data(size => $size, default => $c->cobrand->default_photo_resize);
+ $c->stash->{non_public} = 0;
$c->forward( 'output', [ $photo ] );
}
@@ -52,10 +53,11 @@ sub index :LocalRegex('^(c/)?([1-9]\d*)(?:\.(\d+))?(?:\.(full|tn|fp))?\.(?:jpeg|
my $item;
if ( $is_update ) {
($item) = $c->model('DB::Comment')->search( {
- id => $id,
- state => 'confirmed',
- photo => { '!=', undef },
- } );
+ 'me.id' => $id,
+ 'me.state' => 'confirmed',
+ 'problem.state' => [ FixMyStreet::DB::Result::Problem->visible_states() ],
+ 'me.photo' => { '!=', undef },
+ }, { prefetch => 'problem' });
} else {
($item) = $c->cobrand->problems->search( {
id => $id,
@@ -68,6 +70,19 @@ sub index :LocalRegex('^(c/)?([1-9]\d*)(?:\.(\d+))?(?:\.(full|tn|fp))?\.(?:jpeg|
$c->detach( 'no_photo' ) unless $c->cobrand->allow_photo_display($item, $photo_number); # Should only be for reports, not updates
+ my $problem = $is_update ? $item->problem : $item;
+ $c->stash->{non_public} = $problem->non_public;
+
+ if ($c->stash->{non_public}) {
+ my $body_ids = $problem->bodies_str_ids;
+ # Check permission
+ $c->detach('no_photo') unless $c->user_exists;
+ $c->detach('no_photo') unless $c->user->is_superuser
+ || $c->user->id == $problem->user->id
+ || $c->user->has_permission_to('report_inspect', $body_ids)
+ || $c->user->has_permission_to('report_mark_private', $body_ids);
+ }
+
my $photo;
$photo = $item->get_photoset
->get_image_data( num => $photo_number, size => $size, default => $c->cobrand->default_photo_resize )
@@ -80,10 +95,12 @@ sub output : Private {
my ( $self, $c, $photo ) = @_;
# Save to file
- path(FixMyStreet->path_to('web', 'photo', 'c'))->mkpath;
- my $out = FixMyStreet->path_to('web', $c->req->path);
- my $symlink_exists = $photo->{symlink} ? symlink($photo->{symlink}, $out) : undef;
- path($out)->spew_raw($photo->{data}) unless $symlink_exists;
+ if (!FixMyStreet->config('LOGIN_REQUIRED') && !$c->stash->{non_public}) {
+ path(FixMyStreet->path_to('web', 'photo', 'c'))->mkpath;
+ my $out = FixMyStreet->path_to('web', $c->req->path);
+ my $symlink_exists = $photo->{symlink} ? symlink($photo->{symlink}, $out) : undef;
+ path($out)->spew_raw($photo->{data}) unless $symlink_exists;
+ }
$c->res->content_type( $photo->{content_type} );
$c->res->body( $photo->{data} );
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 7f798f4f4..f2f411635 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -435,7 +435,7 @@ sub inspect : Private {
$problem->confirmed( \'current_timestamp' );
}
if ( $problem->state eq 'hidden' ) {
- $problem->get_photoset->delete_cached;
+ $problem->get_photoset->delete_cached(plus_updates => 1);
}
if ( $problem->state eq 'duplicate') {
if (my $duplicate_of = $c->get_param('duplicate_of')) {
@@ -477,6 +477,9 @@ sub inspect : Private {
}
$problem->non_public($c->get_param('non_public') ? 1 : 0);
+ if ($problem->non_public) {
+ $problem->get_photoset->delete_cached(plus_updates => 1);
+ }
if ( !$c->forward( '/admin/report_edit_location', [ $problem ] ) ) {
# New lat/lon isn't valid, show an error
diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm
index 49bdce379..8109fda2e 100644
--- a/perllib/FixMyStreet/App/Controller/Reports.pm
+++ b/perllib/FixMyStreet/App/Controller/Reports.pm
@@ -659,7 +659,7 @@ sub check_non_public_reports_permission : Private {
if ( $c->user_exists ) {
my $user_has_permission;
- if ( $c->user->is_super_user ) {
+ if ( $c->user->is_superuser ) {
$user_has_permission = 1;
} else {
my $body = $c->stash->{body};
diff --git a/perllib/FixMyStreet/App/Model/PhotoSet.pm b/perllib/FixMyStreet/App/Model/PhotoSet.pm
index 58b352c73..8621286b0 100644
--- a/perllib/FixMyStreet/App/Model/PhotoSet.pm
+++ b/perllib/FixMyStreet/App/Model/PhotoSet.pm
@@ -235,7 +235,7 @@ sub get_image_data {
}
sub delete_cached {
- my ($self) = @_;
+ my ($self, %params) = @_;
my $object = $self->object or return;
my $id = $object->id or return;
@@ -256,6 +256,11 @@ sub delete_cached {
unlink FixMyStreet->path_to(@dirs, "$id.$i$size.$type");
}
}
+
+ # Loop through all the updates as well if requested
+ if ($params{plus_updates}) {
+ $_->get_photoset->delete_cached() foreach $object->comments->all;
+ }
}
sub remove_images {
diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
index 37fc34057..9e45ac35e 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
@@ -158,7 +158,7 @@ sub _recent {
# Need to reattach schema so that confirmed column gets reinflated.
$probs->[0]->result_source->schema( $rs->result_source->schema ) if $probs->[0];
# Catch any cached ones since hidden
- $probs = [ grep { $_->photo && ! $_->is_hidden } @$probs ];
+ $probs = [ grep { $_->photo && ! $_->is_hidden && !$_->non_public } @$probs ];
} else {
$probs = [ $rs->search( $query, $attrs )->all ];
Memcached::set($key, $probs, _cache_timeout());
diff --git a/perllib/FixMyStreet/Roles/PhotoSet.pm b/perllib/FixMyStreet/Roles/PhotoSet.pm
index 4a40ef3f9..27a63bad5 100644
--- a/perllib/FixMyStreet/Roles/PhotoSet.pm
+++ b/perllib/FixMyStreet/Roles/PhotoSet.pm
@@ -38,15 +38,18 @@ sub photos {
my $id = $self->id;
my $typ = $self->result_source->name eq 'comment' ? 'c/' : '';
+ my $non_public = $self->result_source->name eq 'comment'
+ ? $self->problem->non_public : $self->non_public;
+
my @photos = map {
my $cachebust = substr($_, 0, 8);
# Some Varnish configurations (e.g. on mySociety infra) strip cookies from
# images, which means image requests will be redirected to the login page
- # if LOGIN_REQUIRED is set. To stop this happening, Varnish should be
+ # if e.g. LOGIN_REQUIRED is set. To stop this happening, Varnish should be
# configured to not strip cookies if the cookie_passthrough param is
# present, which this line ensures will be if LOGIN_REQUIRED is set.
my $extra = '';
- if (FixMyStreet->config('LOGIN_REQUIRED')) {
+ if (FixMyStreet->config('LOGIN_REQUIRED') || $non_public) {
$cachebust .= '&cookie_passthrough=1';
$extra = '?cookie_passthrough=1';
}
diff --git a/t/app/controller/photo.t b/t/app/controller/photo.t
index 842daa0dc..e1bf35fcf 100644
--- a/t/app/controller/photo.t
+++ b/t/app/controller/photo.t
@@ -13,7 +13,7 @@ my $mech = FixMyStreet::TestMech->new;
my $sample_file = path(__FILE__)->parent->child("sample.jpg");
ok $sample_file->exists, "sample file $sample_file exists";
-my $westminster = $mech->create_body_ok(2527, 'Liverpool City Council');
+my $body = $mech->create_body_ok(2527, 'Liverpool City Council');
subtest "Check multiple upload worked" => sub {
$mech->get_ok('/around');
@@ -112,4 +112,73 @@ subtest "Check photo uploading URL and endpoints work" => sub {
};
};
+subtest "Check no access to update photos on hidden reports" => sub {
+ my $UPLOAD_DIR = tempdir( CLEANUP => 1 );
+
+ my ($report) = $mech->create_problems_for_body(1, $body->id, 'Title');
+ my $update = $mech->create_comment_for_problem($report, $report->user, $report->name, 'Text', $report->anonymous, 'confirmed', 'confirmed', { photo => $report->photo });
+
+ FixMyStreet::override_config {
+ PHOTO_STORAGE_BACKEND => 'FileSystem',
+ PHOTO_STORAGE_OPTIONS => {
+ UPLOAD_DIR => $UPLOAD_DIR,
+ },
+ }, sub {
+ my $image_path = path('t/app/controller/sample.jpg');
+ $image_path->copy( path($UPLOAD_DIR, '74e3362283b6ef0c48686fb0e161da4043bbcc97.jpeg') );
+
+ $mech->get_ok('/photo/c/' . $update->id . '.0.jpeg');
+
+ $report->update({ state => 'hidden' });
+ $report->get_photoset->delete_cached(plus_updates => 1);
+
+ my $res = $mech->get('/photo/c/' . $update->id . '.0.jpeg');
+ is $res->code, 404, 'got 404';
+ };
+};
+
+subtest 'non_public photos only viewable by correct people' => sub {
+ my $UPLOAD_DIR = tempdir( CLEANUP => 1 );
+ path(FixMyStreet->path_to('web/photo'))->remove_tree({ keep_root => 1 });
+
+ my ($report) = $mech->create_problems_for_body(1, $body->id, 'Title', {
+ non_public => 1,
+ });
+
+ FixMyStreet::override_config {
+ PHOTO_STORAGE_BACKEND => 'FileSystem',
+ PHOTO_STORAGE_OPTIONS => {
+ UPLOAD_DIR => $UPLOAD_DIR,
+ },
+ }, sub {
+ my $image_path = path('t/app/controller/sample.jpg');
+ $image_path->copy( path($UPLOAD_DIR, '74e3362283b6ef0c48686fb0e161da4043bbcc97.jpeg') );
+
+ $mech->log_out_ok;
+ my $i = '/photo/' . $report->id . '.0.jpeg';
+ my $res = $mech->get($i);
+ is $res->code, 404, 'got 404';
+
+ $mech->log_in_ok('test@example.com');
+ $i = '/photo/' . $report->id . '.0.jpeg';
+ $mech->get_ok($i);
+ my $image_file = FixMyStreet->path_to("web$i");
+ ok !-e $image_file, 'File not cached out';
+
+ my $user = $mech->log_in_ok('someoneelse@example.com');
+ $i = '/photo/' . $report->id . '.0.jpeg';
+ $res = $mech->get($i);
+ is $res->code, 404, 'got 404';
+
+ $user->update({ from_body => $body });
+ $user->user_body_permissions->create({ body => $body, permission_type => 'report_inspect' });
+ $i = '/photo/' . $report->id . '.0.jpeg';
+ $mech->get_ok($i);
+
+ $user->update({ from_body => undef, is_superuser => 1 });
+ $i = '/photo/' . $report->id . '.0.jpeg';
+ $mech->get_ok($i);
+ };
+};
+
done_testing();
diff --git a/t/app/controller/token.t b/t/app/controller/token.t
index 858838865..f7dec1a31 100644
--- a/t/app/controller/token.t
+++ b/t/app/controller/token.t
@@ -1,5 +1,4 @@
use FixMyStreet::TestMech;
-use FixMyStreet::App;
my $mech = FixMyStreet::TestMech->new;
my $user = $mech->create_user_ok('bob@example.com', name => 'Bob');
@@ -8,7 +7,6 @@ subtest 'Zurich special case for C::Tokens->problem_confirm' => sub {
FixMyStreet::override_config {
ALLOWED_COBRANDS => ['zurich'],
}, sub {
- my $c = FixMyStreet::App->new;
my $zurich = $mech->create_body_ok( 1, 'Zurich' );
my ($report) = $mech->create_problems_for_body(
1, $zurich->id,
@@ -19,7 +17,7 @@ subtest 'Zurich special case for C::Tokens->problem_confirm' => sub {
});
is $report->get_extra_metadata('email_confirmed'), undef, 'email_confirmed not yet set (sanity)';
- my $token = $c->model('DB::Token')->create({ scope => 'problem', data => $report->id });
+ my $token = FixMyStreet::DB->resultset('Token')->create({ scope => 'problem', data => $report->id });
$mech->get_ok('/P/' . $token->token);
$report->discard_changes;
diff --git a/t/app/model/user.t b/t/app/model/user.t
index 5a9c898a2..cbc0fe6cf 100644
--- a/t/app/model/user.t
+++ b/t/app/model/user.t
@@ -1,5 +1,8 @@
use FixMyStreet::TestMech;
use FixMyStreet::DB;
+use Catalyst::Test 'FixMyStreet::App';
+use HTTP::Request::Common;
+use Test::Exception;
my $mech = FixMyStreet::TestMech->new();
$mech->log_in_ok('test@example.com');
@@ -63,9 +66,16 @@ FixMyStreet::override_config {
$mech->content_like(qr/may_show_name[^>c]*>/);
};
-END {
- done_testing();
-}
+subtest 'Check non-existent methods on user object die' => sub {
+ my $c = ctx_request(POST '/auth', { username => 'test@example.com', password_sign_in => 'secret' });
+ throws_ok(
+ sub { $c->user->is_super_user },
+ qr/Can't locate object method 'is_super_user'/,
+ 'attempt to call non-existent method'
+ );
+};
+
+done_testing();
sub create_update {
my ($problem, %params) = @_;
diff --git a/web/cobrands/fixmystreet-uk-councils/roadworks.js b/web/cobrands/fixmystreet-uk-councils/roadworks.js
index f7451f801..ea95d1d40 100644
--- a/web/cobrands/fixmystreet-uk-councils/roadworks.js
+++ b/web/cobrands/fixmystreet-uk-councils/roadworks.js
@@ -183,7 +183,7 @@ fixmystreet.roadworks.display_message = function(feature) {
tag_top = config.tag_top || 'p',
colon = config.colon ? ':' : '';
- var $msg = $('<div class="js-roadworks-message-' + feature.layer.id + ' box-warning"><' + tag_top + '>Roadworks are scheduled near this location, so you may not need to report your issue.</' + tag_top + '></div>');
+ var $msg = $('<div class="js-roadworks-message js-roadworks-message-' + feature.layer.id + ' box-warning"><' + tag_top + '>Roadworks are scheduled near this location, so you may not need to report your issue.</' + tag_top + '></div>');
var $dl = $("<dl></dl>").appendTo($msg);
$dl.append("<dt>Dates" + colon + "</dt>");
$dl.append($("<dd></dd>").text(start + " until " + end));
diff --git a/web/cobrands/lincolnshire/assets.js b/web/cobrands/lincolnshire/assets.js
index 40187e188..f4b1861a9 100644
--- a/web/cobrands/lincolnshire/assets.js
+++ b/web/cobrands/lincolnshire/assets.js
@@ -140,7 +140,10 @@ fixmystreet.assets.add($.extend(true, {}, defaults, {
TYPENAME: "ST_All_Structures"
}
},
- asset_category: "Bridge",
+ asset_category: [
+ "Bridge",
+ "Bridge or Structure"
+ ],
asset_item: 'bridge',
filter_key: 'Type',
filter_value: [
diff --git a/web/cobrands/lincolnshire/roadworks.js b/web/cobrands/lincolnshire/roadworks.js
index 7b42a12f2..760e8f00e 100644
--- a/web/cobrands/lincolnshire/roadworks.js
+++ b/web/cobrands/lincolnshire/roadworks.js
@@ -11,4 +11,19 @@ fixmystreet.assets.add($.extend(true, {}, fixmystreet.roadworks.layer_future, {
// NB Lincs don't want forward planning works displayed, so
// fixmystreet.roadworks.layer_planned is deliberately missing here.
+
+// Lincs want to also display the responsible party in roadworks messages
+var original_display_message = fixmystreet.roadworks.display_message;
+fixmystreet.roadworks.display_message = function(feature) {
+ var retval = original_display_message.apply(this, arguments);
+
+ if (feature.attributes.promoter) {
+ var $dl = $(".js-roadworks-message-" + feature.layer.id + " dl");
+ $dl.append("<dt>Responsibility</dt>");
+ $dl.append($("<dd></dd>").text(feature.attributes.promoter));
+ }
+
+ return retval;
+};
+
})();