Improve non_public photo handling.

Clear the photo cache if the non_public flag is switched on, do not
cache non_public or LOGIN_REQUIRED photos, remove non_public photos
from memcached recent lists, pass through any cookies on non_public
reports/updates, and check the non_public flag on photo lookup.

6 files changed, 100 insertions, 9 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 2f4669456..5ba3e1da4 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -535,7 +535,7 @@ sub report_edit : Path('report_edit') : Args(1) {
             $self->remove_photo($c, $problem, $remove_photo_param);
         }
 
-        if ($problem->state eq 'hidden') {
+        if ($problem->state eq 'hidden' || $problem->non_public) {
             $problem->get_photoset->delete_cached;
         }
 
diff --git a/perllib/FixMyStreet/App/Controller/Photo.pm b/perllib/FixMyStreet/App/Controller/Photo.pm
index aeb40f520..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 ] );
 }
 
@@ -69,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 )
@@ -81,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..27238fa1b 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -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;
+        }
 
         if ( !$c->forward( '/admin/report_edit_location', [ $problem ] ) ) {
             # New lat/lon isn't valid, show an error
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();