diff options
21 files changed, 77 insertions, 46 deletions
diff --git a/conf/general.yml-example b/conf/general.yml-example index 3b2c597b9..6c694024d 100644 --- a/conf/general.yml-example +++ b/conf/general.yml-example @@ -41,14 +41,18 @@ DO_NOT_REPLY_EMAIL: 'do-not-reply@example.org' # Whether this is a development site or not. It will mean e.g. templates/ # CSS modified times aren't cached STAGING_SITE: 1 -# Normally, a staging site will route all reports to the reporter on a -# development site (when STAGING_SITE is 1), to guard against sending fake -# reports to live places. Set this to 1 if you want a dev site to route -# reports as normal. -SEND_REPORTS_ON_STAGING: 0 -# Manual testing of multiple cobrands can be made easier by skipping some -# checks they have in them, if this variable is set -SKIP_CHECKS_ON_STAGING: 0 +# Developers may want a staging site to act differently from a live site. +# Possible flags include: +# - send_reports: Normally, a staging site will route all reports to the +# reporter, to guard against sending fake reports to live places. Set +# this to 1 if you want a staging site to route reports as normal. +# - skip_checks: Manual testing of multiple cobrands can be made easier by +# skipping some checks they have in them, if this variable is set. +# - enable_appcache: Whether the appcache should be active. +STAGING_FLAGS: + send_reports: 0 + skip_checks: 0 + enable_appcache: 0 # What to use as front page/alert example places placeholder # Defaults to High Street, Main Street diff --git a/perllib/FixMyStreet.pm b/perllib/FixMyStreet.pm index 14f3f3607..1f4579293 100644 --- a/perllib/FixMyStreet.pm +++ b/perllib/FixMyStreet.pm @@ -212,4 +212,18 @@ sub set_time_zone { $dt->set_time_zone($tz_f) if $tz_f; } +# Development functions + +sub staging_flag { + my ($cls, $flag, $value) = @_; + $value = 1 unless defined $value; + return unless $cls->config('STAGING_SITE'); + my $flags = $cls->config('STAGING_FLAGS'); + unless ($flags && ref $flags eq 'HASH') { + # Assume all flags 0 if missing + return !$value; + } + return $flags->{$flag} == $value; +} + 1; diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 592d37d4e..d8c5cdf6d 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -1006,10 +1006,9 @@ sub load_template_body : Private { my ($self, $c, $body_id) = @_; my $zurich_user = $c->user->from_body && $c->cobrand->moniker eq 'zurich'; - my $has_permission = $c->user->has_body_permission_to('template_edit') && - $c->user->from_body->id eq $body_id; + my $has_permission = $c->user->has_body_permission_to('template_edit', $body_id); - unless ( $c->user->is_superuser || $zurich_user || $has_permission ) { + unless ( $zurich_user || $has_permission ) { $c->detach( '/page_error_404_not_found', [] ); } @@ -1235,7 +1234,7 @@ sub user_edit : Path('user_edit') : Args(1) { my $user = $c->cobrand->users->find( { id => $id } ); $c->detach( '/page_error_404_not_found', [] ) unless $user; - unless ( $c->user->is_superuser || $c->user->has_body_permission_to('user_edit') || $c->cobrand->moniker eq 'zurich' ) { + unless ( $c->user->has_body_permission_to('user_edit') || $c->cobrand->moniker eq 'zurich' ) { $c->detach('/page_error_403_access_denied', []); } diff --git a/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm b/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm index 032e593c6..a6c13c117 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm @@ -92,10 +92,9 @@ sub edit : Path : Args(2) { sub load_user_body : Private { my ($self, $c, $body_id) = @_; - my $has_permission = $c->user->has_body_permission_to('responsepriority_edit') && - $c->user->from_body->id eq $body_id; + my $has_permission = $c->user->has_body_permission_to('responsepriority_edit', $body_id); - unless ( $c->user->is_superuser || $has_permission ) { + unless ( $has_permission ) { $c->detach( '/page_error_404_not_found' ); } diff --git a/perllib/FixMyStreet/App/Controller/Offline.pm b/perllib/FixMyStreet/App/Controller/Offline.pm index 9acb33f7e..5f3b69f2e 100644 --- a/perllib/FixMyStreet/App/Controller/Offline.pm +++ b/perllib/FixMyStreet/App/Controller/Offline.pm @@ -11,6 +11,7 @@ FixMyStreet::App::Controller::Offline - Catalyst Controller =head1 DESCRIPTION Offline pages Catalyst Controller. +On staging site, appcache only for people who want it. =head1 METHODS @@ -18,6 +19,10 @@ Offline pages Catalyst Controller. sub manifest : Path("/offline/appcache.manifest") { my ($self, $c) = @_; + if (FixMyStreet->staging_flag('enable_appcache', 0)) { + $c->response->status(404); + $c->response->body('NOT FOUND'); + } $c->res->content_type('text/cache-manifest; charset=utf-8'); $c->res->header(Cache_Control => 'no-cache, no-store'); } @@ -25,6 +30,10 @@ sub manifest : Path("/offline/appcache.manifest") { sub appcache : Path("/offline/appcache") { my ($self, $c) = @_; $c->detach('/page_error_404_not_found', []) if keys %{$c->req->params}; + if (FixMyStreet->staging_flag('enable_appcache', 0)) { + $c->response->status(404); + $c->response->body('NOT FOUND'); + } } __PACKAGE__->meta->make_immutable; diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index 27111deb2..61982c47a 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -646,27 +646,27 @@ sub admin_pages { $pages->{config} = [ _('Configuration'), 9]; }; # And some that need special permissions - if ( $user->is_superuser || $user->has_body_permission_to('category_edit') ) { + if ( $user->has_body_permission_to('category_edit') ) { my $page_title = $user->is_superuser ? _('Bodies') : _('Categories'); $pages->{bodies} = [ $page_title, 1 ]; $pages->{body} = [ undef, undef ]; } - if ( $user->is_superuser || $user->has_body_permission_to('report_edit') ) { + if ( $user->has_body_permission_to('report_edit') ) { $pages->{reports} = [ _('Reports'), 2 ]; $pages->{report_edit} = [ undef, undef ]; $pages->{update_edit} = [ undef, undef ]; $pages->{abuse_edit} = [ undef, undef ]; } - if ( $user->is_superuser || $user->has_body_permission_to('template_edit') ) { + if ( $user->has_body_permission_to('template_edit') ) { $pages->{templates} = [ _('Templates'), 3 ]; $pages->{template_edit} = [ undef, undef ]; }; - if ( $user->is_superuser || $user->has_body_permission_to('responsepriority_edit') ) { + if ( $user->has_body_permission_to('responsepriority_edit') ) { $pages->{responsepriorities} = [ _('Priorities'), 4 ]; $pages->{responsepriority_edit} = [ undef, undef ]; }; - if ( $user->is_superuser || $user->has_body_permission_to('user_edit') ) { + if ( $user->has_body_permission_to('user_edit') ) { $pages->{users} = [ _('Users'), 6 ]; $pages->{user_edit} = [ undef, undef ]; } diff --git a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm index 242735073..cf0d72f8e 100644 --- a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm +++ b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm @@ -30,7 +30,7 @@ sub disambiguate_location { sub area_types { my $self = shift; - return $self->next::method() if FixMyStreet->config('STAGING_SITE') && FixMyStreet->config('SKIP_CHECKS_ON_STAGING'); + return $self->next::method() if FixMyStreet->staging_flag('skip_checks'); [ 'NKO', 'NFY', 'NRA' ]; } diff --git a/perllib/FixMyStreet/Cobrand/FixaMinGata.pm b/perllib/FixMyStreet/Cobrand/FixaMinGata.pm index 5b78b3fa1..324811008 100644 --- a/perllib/FixMyStreet/Cobrand/FixaMinGata.pm +++ b/perllib/FixMyStreet/Cobrand/FixaMinGata.pm @@ -31,7 +31,7 @@ sub disambiguate_location { sub area_types { my $self = shift; - return $self->next::method() if FixMyStreet->config('STAGING_SITE') && FixMyStreet->config('SKIP_CHECKS_ON_STAGING'); + return $self->next::method() if FixMyStreet->staging_flag('skip_checks'); [ 'KOM' ]; } diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm index c22224307..64ca7fc62 100644 --- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm +++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm @@ -42,13 +42,13 @@ sub restriction { sub problems_restriction { my ($self, $rs) = @_; - return $rs if FixMyStreet->config('STAGING_SITE') && FixMyStreet->config('SKIP_CHECKS_ON_STAGING'); + return $rs if FixMyStreet->staging_flag('skip_checks'); return $rs->to_body($self->council_id); } sub updates_restriction { my ($self, $rs) = @_; - return $rs if FixMyStreet->config('STAGING_SITE') && FixMyStreet->config('SKIP_CHECKS_ON_STAGING'); + return $rs if FixMyStreet->staging_flag('skip_checks'); return $rs->to_body($self->council_id); } @@ -105,7 +105,7 @@ sub enter_postcode_text { sub area_check { my ( $self, $params, $context ) = @_; - return 1 if FixMyStreet->config('STAGING_SITE') && FixMyStreet->config('SKIP_CHECKS_ON_STAGING'); + return 1 if FixMyStreet->staging_flag('skip_checks'); my $councils = $params->{all_areas}; my $council_match = defined $councils->{$self->council_id}; diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index b34be674a..cf6de9a76 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -287,23 +287,26 @@ sub has_permission_to { =head2 has_body_permission_to -Checks if the User has a from_body set, and the specified permission on that body. +Checks if the User has a from_body set, the specified permission on that body, +and optionally that their from_body is one particular body. Instead of saying: - ($user->from_body && $user->has_permission_to('user_edit', $user->from_body->id)) + ($user->from_body && $user->from_body->id == $body_id && $user->has_permission_to('user_edit', $body_id)) You can just say: - $user->has_body_permission_to('user_edit') - -NB unlike has_permission_to, this doesn't blindly return 1 if the user is a superuser. + $user->has_body_permission_to('user_edit', $body_id) =cut sub has_body_permission_to { - my ($self, $permission_type) = @_; + my ($self, $permission_type, $body_id) = @_; + + return 1 if $self->is_superuser; + return unless $self->from_body; + return if $body_id && $self->from_body->id != $body_id; return $self->has_permission_to($permission_type, $self->from_body->id); } diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm index 7d614bc30..193c5fa41 100644 --- a/perllib/FixMyStreet/Script/Reports.pm +++ b/perllib/FixMyStreet/Script/Reports.pm @@ -211,7 +211,7 @@ sub send(;$) { . " ]\n\n"; } - if (FixMyStreet->config('STAGING_SITE') && !FixMyStreet->config('SEND_REPORTS_ON_STAGING')) { + if (FixMyStreet->staging_flag('send_reports', 0)) { # on a staging server send emails to ourselves rather than the bodies %reporters = map { $_ => $reporters{$_} } grep { /FixMyStreet::SendReport::Email/ } keys %reporters; unless (%reporters) { diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm index 2eab1c754..4cee58d42 100644 --- a/perllib/FixMyStreet/SendReport/Email.pm +++ b/perllib/FixMyStreet/SendReport/Email.pm @@ -67,7 +67,7 @@ sub send { my $recips = $self->build_recipient_list( $row, $h ); # on a staging server send emails to ourselves rather than the bodies - if (FixMyStreet->config('STAGING_SITE') && !FixMyStreet->config('SEND_REPORTS_ON_STAGING') && !FixMyStreet->test_mode) { + if (FixMyStreet->staging_flag('send_reports', 0) && !FixMyStreet->test_mode) { $recips = 1; @{$self->to} = [ $row->user->email, $self->to->[0][1] || $row->name ]; } diff --git a/t/cobrand/bromley.t b/t/cobrand/bromley.t index 43d936684..a7cc563dc 100644 --- a/t/cobrand/bromley.t +++ b/t/cobrand/bromley.t @@ -55,7 +55,7 @@ subtest 'testing special Open311 behaviour', sub { $body->update( { send_method => 'Open311', endpoint => 'http://bromley.endpoint.example.com', jurisdiction => 'FMS', api_key => 'test' } ); my $test_data; FixMyStreet::override_config { - SEND_REPORTS_ON_STAGING => 1, + STAGING_FLAGS => { send_reports => 1 }, ALLOWED_COBRANDS => [ 'fixmystreet', 'bromley' ], }, sub { $test_data = FixMyStreet::DB->resultset('Problem')->send_reports(); diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t index ddaae1f90..85cada27a 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -28,12 +28,12 @@ ok $sample_file->exists, "sample file $sample_file exists"; my $sample_photo = $sample_file->slurp_raw; # This is a helper method that will send the reports but with the config -# correctly set - notably SEND_REPORTS_ON_STAGING needs to be true, and +# correctly set - notably STAGING_FLAGS send_reports needs to be true, and # zurich must be allowed cobrand if we want to be able to call cobrand # methods on it. sub send_reports_for_zurich { FixMyStreet::override_config { - SEND_REPORTS_ON_STAGING => 1, + STAGING_FLAGS => { send_reports => 1 }, ALLOWED_COBRANDS => ['zurich'] }, sub { # Actually send the report diff --git a/t/sendreport/open311.t b/t/sendreport/open311.t index 636faba31..c40b64d12 100644 --- a/t/sendreport/open311.t +++ b/t/sendreport/open311.t @@ -26,7 +26,7 @@ subtest 'testing Open311 behaviour', sub { $body->update( { send_method => 'Open311', endpoint => 'http://endpoint.example.com', jurisdiction => 'FMS', api_key => 'test' } ); my $test_data; FixMyStreet::override_config { - SEND_REPORTS_ON_STAGING => 1, + STAGING_FLAGS => { send_reports => 1 }, ALLOWED_COBRANDS => [ 'fixmystreet' ], }, sub { $test_data = FixMyStreet::DB->resultset('Problem')->send_reports(); diff --git a/templates/web/base/admin/bodies.html b/templates/web/base/admin/bodies.html index e98e2d350..9bd85940b 100644 --- a/templates/web/base/admin/bodies.html +++ b/templates/web/base/admin/bodies.html @@ -14,9 +14,9 @@ </p> [% ELSE %] - [% IF c.config.STAGING_SITE and !c.config.SEND_REPORTS_ON_STAGING %] + [% IF c.config.STAGING_SITE and !c.config.STAGING_FLAGS.send_reports %] <p class="fms-admin-warning"> - [% tprintf(loc("As this is a staging site and %s is false, reports made on this site will be sent to the problem reporter, not the contact given for the report’s category."), "<a class='admin-offsite-link' href='http://fixmystreet.org/customising/config/#send_reports_on_staging'><code>SEND_REPORTS_ON_STAGING</code></a>") %] + [% tprintf(loc("As this is a staging site and %s is false, reports made on this site will be sent to the problem reporter, not the contact given for the report’s category."), "<a class='admin-offsite-link' href='http://fixmystreet.org/customising/config/#send_reports_on_staging'><code>STAGING_FLAGS send_reports</code></a>") %] </p> [% END %] diff --git a/templates/web/base/admin/body.html b/templates/web/base/admin/body.html index 5c9f4f9b9..5e8c6a164 100644 --- a/templates/web/base/admin/body.html +++ b/templates/web/base/admin/body.html @@ -59,9 +59,9 @@ <br> [% loc("Add a contact using the form below.") %] </p> -[% ELSIF c.config.STAGING_SITE and !c.config.SEND_REPORTS_ON_STAGING %] +[% ELSIF c.config.STAGING_SITE and !c.config.STAGING_FLAGS.send_reports %] <p class="fms-admin-warning"> - [% tprintf(loc("As this is a staging site and %s is false, reports made on this site will be sent to the problem reporter, not the contact given for the report’s category."), "<a class='admin-offsite-link' href='http://fixmystreet.org/customising/config/#send_reports_on_staging'><code>SEND_REPORTS_ON_STAGING</code></a>") %] + [% tprintf(loc("As this is a staging site and %s is false, reports made on this site will be sent to the problem reporter, not the contact given for the report’s category."), "<a class='admin-offsite-link' href='http://fixmystreet.org/customising/config/#send_reports_on_staging'><code>STAGING_FLAGS send_reports</code></a>") %] </p> [% END %] diff --git a/templates/web/base/admin/config_page.html b/templates/web/base/admin/config_page.html index 67661c597..f35cd6adb 100644 --- a/templates/web/base/admin/config_page.html +++ b/templates/web/base/admin/config_page.html @@ -118,7 +118,11 @@ running version <strong>[% git_version || 'unknown' %]</strong>. [% INCLUDE subsection heading="Development" %] [% INCLUDE just_value value="STAGING_SITE" %] -[% INCLUDE just_value value="SEND_REPORTS_ON_STAGING" %] +[% staging_conf = FOR k IN c.config.STAGING_FLAGS %] + [% k.key %]:[% k.value %] + [%- ',' IF NOT loop.last %] +[% END %] +[% INCLUDE just_value value="STAGING_FLAGS" conf = staging_conf %] [% INCLUDE just_value value="UPLOAD_DIR" %] [% INCLUDE just_value value="GEO_CACHE" %] [% INCLUDE just_value value="TESTING_COUNCILS" %] diff --git a/templates/web/base/admin/user-form.html b/templates/web/base/admin/user-form.html index 17230e940..0f5452b0a 100644 --- a/templates/web/base/admin/user-form.html +++ b/templates/web/base/admin/user-form.html @@ -47,7 +47,7 @@ [% loc("Staff users have permission to log in to the admin.") %] </p> </div> - [% loc('Staff:') %] <input type="checkbox" id="body" name="body" value="[% c.user.from_body.id %]" [% user.from_body.id == c.user.from_body.id ? ' checked' : '' %] [% 'disabled' UNLESS c.user.is_superuser OR c.user.has_body_permission_to('user_assign_body') %]> + [% loc('Staff:') %] <input type="checkbox" id="body" name="body" value="[% c.user.from_body.id %]" [% user.from_body.id == c.user.from_body.id ? ' checked' : '' %] [% 'disabled' UNLESS c.user.has_body_permission_to('user_assign_body') %]> </li> [% END %] @@ -162,7 +162,7 @@ [% FOREACH permission IN group.value %] <li> <label class="inline"> - <input type="checkbox" id="perms_[% permission.key %]" name="permissions[[% permission.key %]]" [% "checked" IF user.has_body_permission_to(permission.key) %]> + <input type="checkbox" id="perms_[% permission.key %]" name="permissions[[% permission.key %]]" [% "checked" IF NOT user.is_superuser AND user.has_body_permission_to(permission.key) %]> [% permission.value %] </label> </li> diff --git a/templates/web/base/admin/users.html b/templates/web/base/admin/users.html index 757046bcf..47901aed8 100644 --- a/templates/web/base/admin/users.html +++ b/templates/web/base/admin/users.html @@ -26,7 +26,7 @@ <td>[% PROCESS value_or_nbsp value=user.name %]</td> <td><a href="[% c.uri_for( 'reports', search => user.email ) %]">[% PROCESS value_or_nbsp value=user.email %]</a></td> <td>[% PROCESS value_or_nbsp value=user.from_body.name %] - [% IF user.has_body_permission_to('moderate') %] * [% END %] + [% IF user.is_superuser %] * [% END %] </td> [% IF c.cobrand.moniker != 'zurich' %] <td>[% user.flagged == 2 ? loc('(Email in abuse table)') : user.flagged ? loc('Yes') : ' ' %]</td> diff --git a/templates/web/base/offline/manifest.html b/templates/web/base/offline/manifest.html index f5a9fddcc..93d26cb94 100644 --- a/templates/web/base/offline/manifest.html +++ b/templates/web/base/offline/manifest.html @@ -5,7 +5,6 @@ CACHE MANIFEST CACHE: [% version('/cobrands/' _ c.cobrand.asset_moniker _ '/base.css') %] [% version('/cobrands/' _ c.cobrand.asset_moniker _ '/layout.css') %] - [% FOR script IN scripts ~%] [%- script %] [% END %] |