diff options
-rw-r--r-- | perllib/Catalyst/Authentication/Store/FixMyStreetUser.pm | 54 | ||||
-rw-r--r-- | perllib/FixMyStreet/App.pm | 21 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin/Users.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Moderate.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Photo.pm | 33 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report.pm | 5 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Reports.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Model/PhotoSet.pm | 7 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/ResultSet/Problem.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/Roles/PhotoSet.pm | 7 | ||||
-rw-r--r-- | t/app/controller/photo.t | 71 | ||||
-rw-r--r-- | t/app/controller/token.t | 4 | ||||
-rw-r--r-- | t/app/model/user.t | 16 | ||||
-rw-r--r-- | web/cobrands/fixmystreet-uk-councils/roadworks.js | 2 | ||||
-rw-r--r-- | web/cobrands/lincolnshire/assets.js | 5 | ||||
-rw-r--r-- | web/cobrands/lincolnshire/roadworks.js | 15 |
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; +}; + })(); |