diff options
26 files changed, 677 insertions, 122 deletions
diff --git a/.gitignore b/.gitignore index fc1d063c6..2c2171260 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,7 @@ gh_fixmycommunity *[Ff]ix[Mm]indelo* *[Dd]ans[Mm]on[Qq]wat* *[Cc]uido[Mm]i[Cc]iudad* + +# Commercial +/fixmystreet-commercial +*[Gg]round[Cc]ontrol* diff --git a/.travis.yml b/.travis.yml index de9603218..13bbee380 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,7 +37,7 @@ addons: install: - .travis/install - - 'if [ "$TRAVIS_PERL_VERSION" = "5.24" ]; then cpanm --quiet --notest Devel::Cover::Report::Codecov; fi' + - 'if [ "$TRAVIS_PERL_VERSION" = "5.24" ]; then cpanm --quiet --notest https://github.com/mysociety/codecov-perl/tarball/cover-uncoverables; fi' before_script: - commonlib/bin/gettext-makemo FixMyStreet - 'if [ "$TRAVIS_PERL_VERSION" = "5.24" ]; then export HARNESS_PERL_SWITCHES="-MDevel::Cover=+ignore,local/lib/perl5,commonlib,perllib/Catalyst,perllib/DBIx,perllib/Email,perllib/Template,^t"; fi' diff --git a/CHANGELOG.md b/CHANGELOG.md index d8547af48..991ad63ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,14 +7,18 @@ - Body users can now create reports as an anonymous user. #1796 - Extra fields can be added to report form site-wide. #1743 - Body users can filter reports by all states. #1790 + - `LOGIN_REQUIRED` config key to limit site access to logged-in users. + - `SIGNUPS_DISABLED` config key to prevent new user registrations. - Front end improvements: - Always show pagination figures even if only one page. #1787 - Report pages list every update to a report. #1806 + - Cobrands can implement `hide_areas_on_reports` to hide outline on map. - Admin improvements: - Highlight current shortlisted user in list tooltip. #1788 - Extra fields on contacts can be edited. #1743 - Clearer highlight for selected duplicate on inspect form. #1798 - Include MapIt API key on admin config page. #1778 + - Redirect to same map view after inspection. #1820 - Bugfixes: - Set up action scheduled field when report loaded. #1789 - Fix display of thumbnail images on page reload. #1815 @@ -26,6 +30,7 @@ - Duplicate list not loading when phone number present. #1803 - Don't list multiple fixed states all as Fixed in dropdown. #1824 - Development improvements: + - Debug toolbar added. #1823 - `switch-site` script to automate switching config.yml files. #1741 - `make_css --watch` can run custom script after each compilation. @@ -2,7 +2,14 @@ use strict; use warnings; use FixMyStreet::App; +use Plack::Builder; +use Catalyst::Utils; my $app = FixMyStreet::App->apply_default_middlewares(FixMyStreet::App->psgi_app); -$app; +builder { + enable 'Debug', panels => [ qw(Parameters Response DBIC::QueryLog CatalystLog Timer Memory) ] + if Catalyst::Utils::env_value( 'FixMyStreet::App', 'DEBUG' ); + + $app; +}; diff --git a/bin/make_css b/bin/make_css index 7c7a5290d..b99459cda 100755 --- a/bin/make_css +++ b/bin/make_css @@ -14,12 +14,12 @@ BEGIN { use CSS::Sass; use File::ChangeNotify; use File::Find::Rule; -use File::Slurp; use Getopt::Long; use MIME::Base64; use MIME::Types; use Path::Tiny; use Pod::Usage; +use Try::Tiny; # Store ARGV in case we need to restart later. my @ARGVorig = @ARGV; @@ -125,9 +125,15 @@ sub compile { # Write a file, only if it has changed. sub write_if_different { my ($fn, $data) = @_; - my $current = File::Slurp::read_file($fn, { binmode => ':utf8', err_mode => 'quiet' }); + $fn = path($fn); + my $current = try { + $fn->slurp_utf8; + } catch { + return if $_->{err} eq 'No such file or directory'; + die $_; + }; if (!$current || $current ne $data) { - File::Slurp::write_file($fn, { binmode => ':utf8' }, $data); + $fn->spew_utf8($data); return 1; } return 0; diff --git a/conf/general.yml-example b/conf/general.yml-example index 0e437fac3..345a6426d 100644 --- a/conf/general.yml-example +++ b/conf/general.yml-example @@ -204,3 +204,10 @@ TESTING_COUNCILS: '' # if you're using Message Manager, include the URL here (see https://github.com/mysociety/message-manager/) MESSAGE_MANAGER_URL: '' + +# If you want to hide all pages from non-logged-in users, set this to 1. +LOGIN_REQUIRED: 0 + +# If you want to stop new users from registering, set this to 1. +# NB: This also disables all Facebook/Twitter logins. +SIGNUPS_DISABLED: 0 @@ -115,6 +115,12 @@ requires 'File::ChangeNotify'; requires 'Path::Tiny', '0.104'; requires 'File::Find::Rule'; +# Modules used for development +requires 'Plack::Middleware::Debug'; +requires 'Plack::Middleware::Debug::DBIC::QueryLog'; +recommends 'Linux::Inotify2' if $^O eq 'linux'; +recommends 'Mac::FSEvents' if $^O eq 'darwin'; + # Modules used by the test suite requires 'Test::PostgreSQL'; requires 'CGI::Simple'; diff --git a/cpanfile.snapshot b/cpanfile.snapshot index a5f8a9bbb..7e516ecff 100644 --- a/cpanfile.snapshot +++ b/cpanfile.snapshot @@ -1221,6 +1221,20 @@ DISTRIBUTIONS DBIx::Class 0 ExtUtils::MakeMaker 6.72 Test::More 0 + DBIx-Class-QueryLog-1.005001 + pathname: F/FR/FREW/DBIx-Class-QueryLog-1.005001.tar.gz + provides: + DBIx::Class::QueryLog 1.005001 + DBIx::Class::QueryLog::Analyzer 1.005001 + DBIx::Class::QueryLog::NotifyOnMax 1.005001 + DBIx::Class::QueryLog::Query 1.005001 + DBIx::Class::QueryLog::Transaction 1.005001 + requirements: + DBIx::Class 0.07000 + ExtUtils::MakeMaker 0 + Moo 2 + Time::HiRes 0 + Types::Standard 0 DBIx-Class-Schema-Loader-0.07035 pathname: G/GE/GENEHACK/DBIx-Class-Schema-Loader-0.07035.tar.gz provides: @@ -4775,6 +4789,70 @@ DISTRIBUTIONS URI 1.59 parent 0 perl 5.008001 + Plack-Middleware-DBIC-QueryLog-0.05 + pathname: J/JJ/JJNAPIORK/Plack-Middleware-DBIC-QueryLog-0.05.tar.gz + provides: + Plack::Middleware::DBIC::QueryLog 0.05 + requirements: + DBIx::Class::QueryLog v1.3.0 + Data::Dump 0 + ExtUtils::MakeMaker 6.42 + HTTP::Request::Common 0 + Moo 0.009004 + Plack 0.9957 + Plack::Middleware::Debug 0 + Scalar::Util 0 + Test::Fatal 0 + Test::More 0 + perl 5.008008 + Plack-Middleware-Debug-0.16 + pathname: M/MI/MIYAGAWA/Plack-Middleware-Debug-0.16.tar.gz + provides: + Plack::Middleware::Debug 0.16 + Plack::Middleware::Debug::Base 0.16 + Plack::Middleware::Debug::CatalystLog 0.16 + Plack::Middleware::Debug::DBITrace 0.16 + Plack::Middleware::Debug::Environment 0.16 + Plack::Middleware::Debug::Memory 0.16 + Plack::Middleware::Debug::ModuleVersions 0.16 + Plack::Middleware::Debug::Panel undef + Plack::Middleware::Debug::Parameters undef + Plack::Middleware::Debug::PerlConfig 0.16 + Plack::Middleware::Debug::Response 0.16 + Plack::Middleware::Debug::Session undef + Plack::Middleware::Debug::Timer 0.16 + Plack::Middleware::Debug::TrackObjects undef + requirements: + Class::Method::Modifiers 1.05 + Data::Dump 0 + Encode 2.23 + File::ShareDir 1.00 + Module::Build::Tiny 0.026 + Plack 0 + Text::MicroTemplate 0.15 + parent 0 + perl 5.008001 + Plack-Middleware-Debug-DBIC-QueryLog-0.09 + pathname: J/JJ/JJNAPIORK/Plack-Middleware-Debug-DBIC-QueryLog-0.09.tar.gz + provides: + Plack::Middleware::Debug::DBIC::QueryLog 0.09 + requirements: + DBIx::Class::QueryLog 0 + DBIx::Class::QueryLog::Analyzer 0 + Data::Dump 0 + ExtUtils::MakeMaker 7.1002 + HTTP::Request::Common 0 + Moo 0.009005 + Plack 0 + Plack::Builder 0 + Plack::Middleware::DBIC::QueryLog 0.05 + Plack::Middleware::Debug 0 + Plack::Test 0 + SQL::Abstract 1.70 + Scalar::Util 0 + Test::Fatal 0 + Test::More 0.96 + perl 5.008008 Plack-Middleware-ReverseProxy-0.15 pathname: M/MI/MIYAGAWA/Plack-Middleware-ReverseProxy-0.15.tar.gz provides: @@ -5846,6 +5924,17 @@ DISTRIBUTIONS requirements: Module::Build 0.36 Test::More 0 + Text-MicroTemplate-0.24 + pathname: K/KA/KAZUHO/Text-MicroTemplate-0.24.tar.gz + provides: + Text::MicroTemplate 0.24 + Text::MicroTemplate::EncodedString 0.24 + Text::MicroTemplate::File undef + requirements: + ExtUtils::MakeMaker 6.59 + File::Temp 0 + Test::More 0 + perl 5.00800 Text-SimpleTable-2.03 pathname: M/MR/MRAMBERG/Text-SimpleTable-2.03.tar.gz provides: @@ -5994,6 +6083,48 @@ DISTRIBUTIONS constant 0 strict 0 warnings 0 + Type-Tiny-1.002001 + pathname: T/TO/TOBYINK/Type-Tiny-1.002001.tar.gz + provides: + Devel::TypeTiny::Perl56Compat 1.002001 + Devel::TypeTiny::Perl58Compat 1.002001 + Error::TypeTiny 1.002001 + Error::TypeTiny::Assertion 1.002001 + Error::TypeTiny::Compilation 1.002001 + Error::TypeTiny::WrongNumberOfParameters 1.002001 + Eval::TypeTiny 1.002001 + Reply::Plugin::TypeTiny 1.002001 + Test::TypeTiny 1.002001 + Type::Coercion 1.002001 + Type::Coercion::FromMoose 1.002001 + Type::Coercion::Union 1.002001 + Type::Library 1.002001 + Type::Params 1.002001 + Type::Parser 1.002001 + Type::Registry 1.002001 + Type::Tiny 1.002001 + Type::Tiny::Class 1.002001 + Type::Tiny::Duck 1.002001 + Type::Tiny::Enum 1.002001 + Type::Tiny::Intersection 1.002001 + Type::Tiny::Role 1.002001 + Type::Tiny::Union 1.002001 + Type::Utils 1.002001 + Types::Common::Numeric 1.002001 + Types::Common::String 1.002001 + Types::Standard 1.002001 + Types::Standard::ArrayRef 1.002001 + Types::Standard::CycleTuple 1.002001 + Types::Standard::Dict 1.002001 + Types::Standard::HashRef 1.002001 + Types::Standard::Map 1.002001 + Types::Standard::ScalarRef 1.002001 + Types::Standard::Tuple 1.002001 + Types::TypeTiny 1.002001 + requirements: + Exporter::Tiny 0.026 + ExtUtils::MakeMaker 6.17 + perl 5.006001 UNIVERSAL-can-1.20140328 pathname: C/CH/CHROMATIC/UNIVERSAL-can-1.20140328.tar.gz provides: diff --git a/perllib/Catalyst/Plugin/Compress/Gzip.pm b/perllib/Catalyst/Plugin/Compress/Gzip.pm deleted file mode 100644 index 06532c84c..000000000 --- a/perllib/Catalyst/Plugin/Compress/Gzip.pm +++ /dev/null @@ -1,82 +0,0 @@ -package Catalyst::Plugin::Compress::Gzip; -use strict; -use warnings; -use MRO::Compat; - -use Compress::Zlib (); - -sub finalize_headers { - my $c = shift; - - if ( $c->response->content_encoding ) { - return $c->next::method(@_); - } - - unless ( $c->response->body ) { - return $c->next::method(@_); - } - - unless ( $c->response->status == 200 ) { - return $c->next::method(@_); - } - - unless ( $c->response->content_type =~ /^text|xml$|javascript$/ ) { - return $c->next::method(@_); - } - - my $accept = $c->request->header('Accept-Encoding') || ''; - - unless ( index( $accept, "gzip" ) >= 0 ) { - return $c->next::method(@_); - } - - - my $body = $c->response->body; - eval { local $/; $body = <$body> } if ref $body; - die "Response body is an unsupported kind of reference" if ref $body; - - $c->response->body( Compress::Zlib::memGzip( $body ) ); - $c->response->content_length( length( $c->response->body ) ); - $c->response->content_encoding('gzip'); - $c->response->headers->push_header( 'Vary', 'Accept-Encoding' ); - - $c->next::method(@_); -} - -1; - -__END__ - -=head1 NAME - -Catalyst::Plugin::Compress::Gzip - Gzip response - -=head1 SYNOPSIS - - use Catalyst qw[Compress::Gzip]; - - -=head1 DESCRIPTION - -Gzip compress response if client supports it. Changed from CPAN version to -overload finalize_headers, rather than finalize. - -=head1 METHODS - -=head2 finalize_headers - -=head1 SEE ALSO - -L<Catalyst>. - -=head1 AUTHOR - -Christian Hansen, C<ch@ngmedia.com> -Matthew Somerville. - -=head1 LICENSE - -This library is free software . You can redistribute it and/or modify it under -the same terms as perl itself. - -=cut diff --git a/perllib/Catalyst/TraitFor/Model/DBIC/Schema/QueryLog/AdoptPlack.pm b/perllib/Catalyst/TraitFor/Model/DBIC/Schema/QueryLog/AdoptPlack.pm new file mode 100644 index 000000000..22509568e --- /dev/null +++ b/perllib/Catalyst/TraitFor/Model/DBIC/Schema/QueryLog/AdoptPlack.pm @@ -0,0 +1,128 @@ +# Local version to clone schema in enable_dbic_querylogging + +package Catalyst::TraitFor::Model::DBIC::Schema::QueryLog::AdoptPlack; +our $VERSION = "0.07"; + +use 5.008004; +use Moose::Role; +use Plack::Middleware::DBIC::QueryLog; +use Scalar::Util 'blessed'; + +with 'Catalyst::Component::InstancePerContext'; + +requires 'storage'; + +has show_missing_ql_warning => (is=>'rw', default=>1); + +sub get_querylog_from_env { + my ($self, $env) = @_; + return Plack::Middleware::DBIC::QueryLog->get_querylog_from_env($env); +} + +sub infer_env_from { + my ($self, $ctx) = @_; + if($ctx->engine->can('env')) { + return $ctx->engine->env; + } elsif($ctx->request->can('env')) { + return $ctx->request->env; + } else { return } +} + +sub enable_dbic_querylogging { + my ($self, $querylog) = @_; + my $clone = $self->clone; + $clone->storage->debugobj($querylog); + $clone->storage->debug(1); +} + +sub die_missing_querylog { + shift->show_missing_ql_warning(0); + die <<DEAD; +You asked me to querylog DBIC, but there is no querylog object in the Plack +\$env. You probably forgot to enable Plack::Middleware::Debug::DBIC::QueryLog +in your debugging panel. +DEAD +} + +sub die_not_plack { + die "Not a Plack Engine or compatible interface!" +} + +sub build_per_context_instance { + my ( $self, $ctx ) = @_; + return $self unless blessed($ctx); + + if(my $env = $self->infer_env_from($ctx)) { + if(my $querylog = $self->get_querylog_from_env($env)) { + $self->enable_dbic_querylogging($querylog); + } else { + $self->die_missing_querylog() if + $self->show_missing_ql_warning; + } + } else { + die_not_plack(); + } + + return $self; +} + +1; + +=head1 NAME + +Catalyst::TraitFor::Model::DBIC::Schema::QueryLog::AdoptPlack - Use a Plack Middleware QueryLog + +=head1 SYNOPSIS + + package MyApp::Web::Model::Schema; + use parent 'Catalyst::Model::DBIC::Schema'; + + __PACKAGE__->config({ + schema_class => 'MyApp::Schema', + traits => ['QueryLog::AdoptPlack'], + ## .. rest of configuration + }); + +=head1 DESCRIPTION + +This is a trait for L<Catalyst::Model::DBIC::Schema> which adopts a L<Plack> +created L<DBIx::Class::QueryLog> and logs SQL for a given request cycle. It is +intended to be compatible with L<Catalyst::TraitFor::Model::DBIC::Schema::QueryLog> +which you may already be using. + +It picks up the querylog from C<< $env->{'plack.middleware.dbic.querylog'} >> +or from C<< $env->{'plack.middleware.debug.dbic.querylog'} >> which is generally +provided by the L<Plack> middleware L<Plack::Middleware::Debug::DBIC::QueryLog> +In fact you will probably use these two modules together. Please see the documentation +in L<Plack::Middleware::Debug::DBIC::QueryLog> for an example. + +PLEASE NOTE: Starting with the 0.04 version of L<Plack::Middleware::Debug::DBIC::QueryLog> +we will canonicalize on C<< $env->{'plack.middleware.dbic.querylog'} >>. For now +both listed keys will work, but within a release or two the older key will warn and +prompt you to upgrade your version of L<Plack::Middleware::Debug::DBIC::QueryLog>. +Sorry for the trouble. + +=head1 SEE ALSO + +L<Plack::Middleware::Debug::DBIC::QueryLog>, +L<Catalyst::TraitFor::Model::DBIC::Schema::QueryLog>, +L<Catalyst::Model::DBIC::Schema>, +L<Plack::Middleware::Debug> + +=head1 ACKNOWLEGEMENTS + +This code inspired from L<Catalyst::TraitFor::Model::DBIC::Schema::QueryLog> +and the author owes a debt of gratitude for the original authors. + +=head1 AUTHOR + +John Napiorkowski, C<< <jjnapiork@cpan.org> >> + +=head1 COPYRIGHT & LICENSE + +Copyright 2012, John Napiorkowski + +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 d782890bc..a09f86f85 100644 --- a/perllib/FixMyStreet/App.pm +++ b/perllib/FixMyStreet/App.pm @@ -25,7 +25,6 @@ use Catalyst ( 'Session::State::Cookie', # FIXME - we're using our own override atm 'Authentication', 'SmartURI', - 'Compress::Gzip', ); extends 'Catalyst'; diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index 83fb0554c..825066026 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -128,6 +128,18 @@ sub email_sign_in : Private { return; } + # If user registration is disabled then bail out at this point + # if there's not already a user with this email address. + # NB this uses the same template as a successful sign in to stop + # enumeration of valid email addresses. + if ( FixMyStreet->config('SIGNUPS_DISABLED') + && !$c->model('DB::User')->search({ email => $good_email })->count + && !$c->stash->{current_user} # don't break the change email flow + ) { + $c->stash->{template} = 'auth/token.html'; + return; + } + my $user_params = {}; $user_params->{password} = $c->get_param('password_register') if $c->get_param('password_register'); @@ -199,6 +211,10 @@ sub token : Path('/M') : Args(1) { my $user = $c->model('DB::User')->find_or_new({ email => $data->{email} }); + # Bail out if this is a new user and SIGNUPS_DISABLED is set + $c->detach( '/page_error_403_access_denied', [] ) + if FixMyStreet->config('SIGNUPS_DISABLED') && !$user->in_storage && !$data->{old_email}; + if ($data->{old_email}) { # Were logged in as old_email, want to switch to email ($user) if ($user->in_storage) { @@ -244,6 +260,8 @@ sub fb : Private { sub facebook_sign_in : Private { my ( $self, $c ) = @_; + $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED'); + my $fb = $c->forward('/auth/fb'); my $url = $fb->get_authorization_url(scope => ['email']); @@ -302,6 +320,8 @@ sub tw : Private { sub twitter_sign_in : Private { my ( $self, $c ) = @_; + $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED'); + my $twitter = $c->forward('/auth/tw'); my $url = $twitter->get_authentication_url(callback => $c->uri_for('/auth/Twitter')); diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index c617f5733..60d373a16 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -316,6 +316,10 @@ sub inspect : Private { $c->stash->{templates_by_category} = $templates_by_category; } + if ($c->user->has_body_permission_to('planned_reports')) { + $c->stash->{post_inspect_url} = $c->req->referer; + } + if ( $c->get_param('save') ) { $c->forward('/auth/check_csrf_token'); @@ -438,33 +442,36 @@ sub inspect : Private { anonymous => 0, %update_params, } ); - # This problem might no longer be visible on the current cobrand, - # if its body has changed (e.g. by virtue of the category changing) - # so redirect to a cobrand where it can be seen if necessary - $problem->discard_changes; + my $redirect_uri; - if ( $c->cobrand->is_council && !$c->cobrand->owns_problem($problem) ) { + $problem->discard_changes; + + # If inspector, redirect back to the map view they came from + # with the right filters. If that wasn't set, go to /around at this + # report's location. + # We go here rather than the shortlist because it makes it much + # simpler to inspect many reports in the same location. The + # shortlist is always a single click away, being on the main nav. + if ($c->user->has_body_permission_to('planned_reports')) { + unless ($redirect_uri = $c->get_param("post_inspect_url")) { + my $categories = join(',', @{ $c->user->categories }); + my $params = { + lat => $problem->latitude, + lon => $problem->longitude, + }; + $params->{filter_category} = $categories if $categories; + $params->{js} = 1 if $c->get_param('js'); + $redirect_uri = $c->uri_for( "/around", $params ); + } + } elsif ( $c->cobrand->is_council && !$c->cobrand->owns_problem($problem) ) { + # This problem might no longer be visible on the current cobrand, + # if its body has changed (e.g. by virtue of the category changing) + # so redirect to a cobrand where it can be seen if necessary $redirect_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; } else { $redirect_uri = $c->uri_for( $problem->url ); } - # Or if inspector, redirect back to /around at this report's - # location with the right filters. We go here rather than the - # shortlist because it makes it much simpler to inspect many reports - # in the same location. The shortlist is always a single click away, - # being on the main nav. - if ($c->user->has_body_permission_to('planned_reports')) { - my $categories = join(',', @{ $c->user->categories }); - my $params = { - lat => $problem->latitude, - lon => $problem->longitude, - }; - $params->{filter_category} = $categories if $categories; - $params->{js} = 1 if $c->get_param('js'); - $redirect_uri = $c->uri_for( "/around", $params ); - } - $c->log->debug( "Redirecting to: " . $redirect_uri ); $c->res->redirect( $redirect_uri ); } diff --git a/perllib/FixMyStreet/App/Controller/Root.pm b/perllib/FixMyStreet/App/Controller/Root.pm index 64d7fa6ae..7f70623ae 100644 --- a/perllib/FixMyStreet/App/Controller/Root.pm +++ b/perllib/FixMyStreet/App/Controller/Root.pm @@ -16,6 +16,18 @@ FixMyStreet::App::Controller::Root - Root Controller for FixMyStreet::App =head1 METHODS +=head2 begin + +Any pre-flight checking for all requests + +=cut +sub begin : Private { + my ( $self, $c ) = @_; + + $c->forward( 'check_login_required' ); +} + + =head2 auto Set up general things for this instance @@ -130,6 +142,27 @@ sub page_error : Private { $c->response->status($code); } +sub check_login_required : Private { + my ($self, $c) = @_; + + return if $c->user_exists || !FixMyStreet->config('LOGIN_REQUIRED'); + + # Whitelisted URL patterns are allowed without login + my $whitelist = qr{ + ^auth(/|$) + | ^js/translation_strings\.(.*?)\.js + | ^[PACQM]/ # various tokens that log the user in + }x; + return if $c->request->path =~ $whitelist; + + # Blacklisted URLs immediately 404 + # This is primarily to work around a Safari bug where the appcache + # URL is requested in an infinite loop if it returns a 302 redirect. + $c->detach('/page_error_404_not_found', []) if $c->request->path =~ /^offline/; + + $c->detach( '/auth/redirect' ); +} + =head2 end Attempt to render a view, if needed. diff --git a/perllib/FixMyStreet/App/Model/DB.pm b/perllib/FixMyStreet/App/Model/DB.pm index ffd867485..db8e72c27 100644 --- a/perllib/FixMyStreet/App/Model/DB.pm +++ b/perllib/FixMyStreet/App/Model/DB.pm @@ -5,6 +5,7 @@ use strict; use warnings; use FixMyStreet; +use Catalyst::Utils; use Moose; with 'Catalyst::Component::InstancePerContext'; @@ -13,6 +14,10 @@ __PACKAGE__->config( schema_class => 'FixMyStreet::DB::Schema', connect_info => sub { FixMyStreet::DB->schema->storage->dbh }, ); +__PACKAGE__->config( + traits => ['QueryLog::AdoptPlack'], +) + if Catalyst::Utils::env_value( 'FixMyStreet::App', 'DEBUG' ); sub build_per_context_instance { my ( $self, $c ) = @_; diff --git a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm index 1052bac0e..c50721334 100644 --- a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm +++ b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm @@ -21,6 +21,8 @@ sub path_to_email_templates { sub add_response_headers { my $self = shift; + # uncoverable branch true + return if $self->{c}->debug; my $csp_nonce = $self->{c}->stash->{csp_nonce} = unpack('h*', mySociety::Random::random_bytes(16, 1)); $self->{c}->res->header('Content-Security-Policy', "script-src 'self' www.google-analytics.com www.googleadservices.com 'unsafe-inline' 'nonce-$csp_nonce'") } diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index fcffc1e97..77190679b 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -890,15 +890,21 @@ sub photos { my $id = $self->id; 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 + # 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 = (FixMyStreet->config('LOGIN_REQUIRED')) ? "&cookie_passthrough=1" : ""; my ($hash, $format) = split /\./, $_; { id => $hash, - url_temp => "/photo/temp.$hash.$format", - url_temp_full => "/photo/fulltemp.$hash.$format", - url => "/photo/$id.$i.$format?$cachebust", - url_full => "/photo/$id.$i.full.$format?$cachebust", - url_tn => "/photo/$id.$i.tn.$format?$cachebust", - url_fp => "/photo/$id.$i.fp.$format?$cachebust", + url_temp => "/photo/temp.$hash.$format$extra", + url_temp_full => "/photo/fulltemp.$hash.$format$extra", + url => "/photo/$id.$i.$format?$cachebust$extra", + url_full => "/photo/$id.$i.full.$format?$cachebust$extra", + url_tn => "/photo/$id.$i.tn.$format?$cachebust$extra", + url_fp => "/photo/$id.$i.fp.$format?$cachebust$extra", idx => $i++, } } $photoset->all_ids; diff --git a/perllib/FixMyStreet/Roles/Extra.pm b/perllib/FixMyStreet/Roles/Extra.pm index dc2e5c241..445f6d91c 100644 --- a/perllib/FixMyStreet/Roles/Extra.pm +++ b/perllib/FixMyStreet/Roles/Extra.pm @@ -175,4 +175,20 @@ sub get_extra { return $extra; } +=head2 get_extra_field_value + +Return the value of a field stored in `_fields` in extra, or undefined if +it's not present. + +=cut + +sub get_extra_field_value { + my ($self, $name) = @_; + + my @fields = @{ $self->get_extra_fields() }; + + my ($field) = grep { $_->{name} eq $name } @fields; + return $field->{value}; +} + 1; diff --git a/script/server b/script/server index 2ed265a28..b1d72269e 100755 --- a/script/server +++ b/script/server @@ -3,4 +3,5 @@ set -e cd "$(dirname "$0")/.." -script/fixmystreet_app_server.pl --fork --restart --debug $@ +export FIXMYSTREET_APP_DEBUG=${FIXMYSTREET_APP_DEBUG=1} +bin/cron-wrapper local/bin/plackup -s Starman --listen :3000 --Reload perllib,conf/general.yml diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t index 388216a1f..cb7d16969 100644 --- a/t/app/controller/auth.t +++ b/t/app/controller/auth.t @@ -5,6 +5,7 @@ my $mech = FixMyStreet::TestMech->new; my $test_email = 'test@example.com'; my $test_email2 = 'test@example.net'; +my $test_email3 = 'newuser@example.org'; my $test_password = 'foobar'; END { @@ -279,6 +280,94 @@ subtest "sign in but have email form autofilled" => sub { is $mech->uri->path, '/my', "redirected to correct page"; }; +$mech->log_out_ok; -# more test: -# TODO: test that email are always lowercased +subtest "sign in with uppercase email" => sub { + $mech->get_ok('/auth'); + my $uc_test_email = uc $test_email; + $mech->submit_form_ok( + { + form_name => 'general_auth', + fields => { + email => $uc_test_email, + password_sign_in => $test_password, + }, + button => 'sign_in', + }, + "sign in with '$uc_test_email' and auto-completed name" + ); + is $mech->uri->path, '/my', "redirected to correct page"; + + $mech->content_contains($test_email); + $mech->content_lacks($uc_test_email); + + my $count = FixMyStreet::App->model('DB::User')->search( { email => $uc_test_email } )->count; + is $count, 0, "uppercase user wasn't created"; +}; + + +FixMyStreet::override_config { + SIGNUPS_DISABLED => 1, +}, sub { + subtest 'signing in with an unknown email address disallowed' => sub { + $mech->log_out_ok; + # create a new account + $mech->clear_emails_ok; + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { + form_name => 'general_auth', + fields => { email => $test_email3, }, + button => 'email_sign_in', + }, + "create a new account" + ); + + ok $mech->email_count_is(0); + + my $count = FixMyStreet::App->model('DB::User')->search( { email => $test_email3 } )->count; + is $count, 0, "no user exists"; + }; + + subtest 'signing in as known email address with new password is allowed' => sub { + my $new_password = "myshinynewpassword"; + + $mech->clear_emails_ok; + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { + form_name => 'general_auth', + fields => { + email => "$test_email", + password_register => $new_password, + r => 'faq', # Just as a test + }, + button => 'email_sign_in', + }, + "email_sign_in with '$test_email'" + ); + + $mech->not_logged_in_ok; + + ok $mech->email_count_is(1); + my $link = $mech->get_link_from_email; + $mech->get_ok($link); + is $mech->uri->path, '/faq', "redirected to the Help page"; + + $mech->log_out_ok; + + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { + form_name => 'general_auth', + fields => { + email => $test_email, + password_sign_in => $new_password, + }, + button => 'sign_in', + }, + "sign in with '$test_email' and new password" + ); + is $mech->uri->path, '/my', "redirected to correct page"; + }; +}; diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index 4000a9da8..68f9063cf 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -190,6 +190,42 @@ FixMyStreet::override_config { $report->update({ state => $old_state }); }; + subtest "post-inspect redirect is to the right place if URL set" => sub { + $user->user_body_permissions->create({ body => $oxon, permission_type => 'planned_reports' }); + $mech->get_ok("/report/$report_id"); + my $update_text = "This text was entered as an update by the user."; + $mech->submit_form_ok({ button => 'save', with_fields => { + public_update => $update_text, + include_update => "1", + post_inspect_url => "/" + }}); + is $mech->res->code, 200, "got 200"; + is $mech->res->previous->code, 302, "got 302 for redirect"; + is $mech->uri->path, '/', 'redirected to front page'; + $user->user_body_permissions->search({ body_id => $oxon->id, permission_type => 'planned_reports' })->delete; + }; + + subtest "post-inspect redirect is to the right place if URL not set" => sub { + $user->user_body_permissions->create({ body => $oxon, permission_type => 'planned_reports' }); + $user->set_extra_metadata(categories => [ $contact->id ]); + $user->update; + $mech->get_ok("/report/$report_id"); + my $update_text = "This text was entered as an update by the user."; + $mech->submit_form_ok({ button => 'save', with_fields => { + public_update => $update_text, + include_update => "1", + post_inspect_url => "" + }}); + is $mech->res->code, 200, "got 200"; + is $mech->res->previous->code, 302, "got 302 for redirect"; + is $mech->uri->path, '/around', 'redirected to /around'; + my %params = $mech->uri->query_form; + is $params{lat}, $report->latitude, "latitude param is correct"; + is $params{lon}, $report->longitude, "longitude param is correct"; + is $params{filter_category}, $contact->category, "categories param is correct"; + $user->user_body_permissions->search({ body_id => $oxon->id, permission_type => 'planned_reports' })->delete; + }; + foreach my $test ( { type => 'report_edit_priority', priority => 1 }, { type => 'report_edit_category', category => 1 }, diff --git a/t/app/controller/root.t b/t/app/controller/root.t new file mode 100644 index 000000000..413341d89 --- /dev/null +++ b/t/app/controller/root.t @@ -0,0 +1,76 @@ +use FixMyStreet::TestMech; + +ok( my $mech = FixMyStreet::TestMech->new, 'Created mech object' ); + +my @urls = ( + "/", + "/reports", + "/about/faq", + "/around?longitude=-1.351488&latitude=51.847235" +); + + +FixMyStreet::override_config { + LOGIN_REQUIRED => 0, + MAPIT_URL => 'http://mapit.uk/' +}, sub { + subtest 'LOGIN_REQUIRED = 0 behaves correctly' => sub { + foreach my $url (@urls) { + $mech->get_ok($url); + is $mech->res->code, 200, "got 200 for page"; + is $mech->res->previous, undef, 'No redirect'; + } + }; +}; + + +FixMyStreet::override_config { + LOGIN_REQUIRED => 1, + MAPIT_URL => 'http://mapit.uk/' +}, sub { + subtest 'LOGIN_REQUIRED = 1 redirects to /auth if not logged in' => sub { + foreach my $url (@urls) { + $mech->get_ok($url); + is $mech->res->code, 200, "got 200 for final destination"; + is $mech->res->previous->code, 302, "got 302 for redirect"; + is $mech->uri->path, '/auth'; + } + }; + + subtest 'LOGIN_REQUIRED = 1 does not redirect if logged in' => sub { + $mech->log_in_ok('user@example.org'); + foreach my $url (@urls) { + $mech->get_ok($url); + is $mech->res->code, 200, "got 200 for final destination"; + is $mech->res->previous, undef, 'No redirect'; + } + $mech->log_out_ok; + }; + + subtest 'LOGIN_REQUIRED = 1 allows whitelisted URLs' => sub { + my @whitelist = ( + '/auth', + '/js/translation_strings.en-gb.js' + ); + + foreach my $url (@whitelist) { + $mech->get_ok($url); + is $mech->res->code, 200, "got 200 for final destination"; + is $mech->res->previous, undef, 'No redirect'; + } + }; + + subtest 'LOGIN_REQUIRED = 1 404s blacklisted URLs' => sub { + my @blacklist = ( + '/offline/appcache', + ); + + foreach my $url (@blacklist) { + $mech->get($url); + ok !$mech->res->is_success(), "want a bad response"; + is $mech->res->code, 404, "got 404"; + } + }; +}; + +done_testing(); diff --git a/t/app/model/extra.t b/t/app/model/extra.t index 17f34c6c1..a5e3e3574 100644 --- a/t/app/model/extra.t +++ b/t/app/model/extra.t @@ -98,4 +98,46 @@ subtest 'Default hash layout' => sub { }; }; +subtest 'Get named field values' => sub { + my $user = $db->resultset('User')->create({ + email => 'test-moderation@example.com', + name => 'Test User' + }); + my $report = $db->resultset('Problem')->create( + { + postcode => 'BR1 3SB', + bodies_str => "", + areas => "", + category => 'Other', + title => 'Good bad good', + detail => 'Good bad bad bad good bad', + used_map => 't', + name => 'Test User 2', + anonymous => 'f', + state => 'confirmed', + lang => 'en-gb', + service => '', + cobrand => 'default', + latitude => '51.4129', + longitude => '0.007831', + user_id => $user->id, + }); + + $report->push_extra_fields( + { + name => "field1", + description => "This is a test field", + value => "value 1", + }, + { + name => "field 2", + description => "Another test", + value => "this is a test value", + } + ); + + is $report->get_extra_field_value("field1"), "value 1", "field1 has correct value"; + is $report->get_extra_field_value("field 2"), "this is a test value", "field 2 has correct value"; +}; + done_testing(); diff --git a/templates/web/base/auth/token.html b/templates/web/base/auth/token.html index a4dedcec3..9a79a5e67 100644 --- a/templates/web/base/auth/token.html +++ b/templates/web/base/auth/token.html @@ -14,8 +14,14 @@ <div class="confirmation-header confirmation-header--inbox"> - <h1>[% loc("Nearly done! Now check your email…") %]</h1> - <p>[% loc("Click the link in our confirmation email to sign in.") %]</p> + [% IF c.config.SIGNUPS_DISABLED %] + <h1>[% loc("Nearly done!") %]</h1> + <p>[% loc("If there's a user associated with the address you entered, we've sent a confirmation email.") %]</p> + <p>[% loc("Click the link in that email to sign in.") %]</p> + [% ELSE %] + <h1>[% loc("Nearly done! Now check your email…") %]</h1> + <p>[% loc("Click the link in our confirmation email to sign in.") %]</p> + [% END %] <p> [% loc("Can’t find our email? Check your spam folder – that’s the solution 99% of the time.") %] diff --git a/templates/web/base/maps/openlayers.html b/templates/web/base/maps/openlayers.html index e8d6c2e06..a9758f738 100644 --- a/templates/web/base/maps/openlayers.html +++ b/templates/web/base/maps/openlayers.html @@ -6,7 +6,9 @@ <input type="hidden" name="zoom" value="[% map.zoom %]"> <div id="js-map-data" +[%- UNLESS c.cobrand.call_hook('hide_areas_on_reports') %] data-area="[% map.area.join(',') %]" +[%- END %] data-all_pins='[% all_pins %]' data-latitude=[% map.latitude %] data-longitude=[% map.longitude %] @@ -23,7 +25,7 @@ data-map_type="[% map.map_type %]" [% IF include_key -%] data-key='[% c.config.BING_MAPS_API_KEY %]' -[%- END %] +[%- END -%] > </div> <div id="map_box" aria-hidden="true"> diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index cd34b147b..fb58a0cfa 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -174,7 +174,10 @@ [% END %] <p> - <input type="hidden" name="token" value="[% csrf_token %]"> + <input type="hidden" name="token" value="[% csrf_token %]" /> + [% IF permissions.planned_reports %] + <input type="hidden" name="post_inspect_url" value="[% post_inspect_url | html %]" /> + [% END %] <input class="btn btn-primary" type="submit" value="[% loc('Save changes') %]" data-value-original="[% loc('Save changes') %]" data-value-duplicate="[% loc('Save + close as duplicate') %]" name="save" /> </p> </div> |