aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--.gitignore4
-rw-r--r--CHANGELOG.md3
-rw-r--r--conf/general.yml-example7
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth.pm20
-rw-r--r--perllib/FixMyStreet/App/Controller/Root.pm33
-rw-r--r--perllib/FixMyStreet/DB/Result/Problem.pm18
-rw-r--r--perllib/FixMyStreet/Roles/Extra.pm16
-rw-r--r--t/app/controller/auth.t93
-rw-r--r--t/app/controller/root.t76
-rw-r--r--t/app/model/extra.t42
-rw-r--r--templates/web/base/auth/token.html10
-rw-r--r--templates/web/base/maps/openlayers.html4
12 files changed, 315 insertions, 11 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/CHANGELOG.md b/CHANGELOG.md
index ba7225e4b..d9326b2b4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,9 +6,12 @@
- 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
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
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/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/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index a74a04828..51805fa51 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -920,15 +920,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/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/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&hellip;") %]</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&hellip;") %]</h1>
+ <p>[% loc("Click the link in our confirmation email to sign in.") %]</p>
+ [% END %]
<p>
[% loc("Can&rsquo;t find our email? Check your spam folder&nbsp;&ndash; that&rsquo;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">
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-15 20:13:47 +0000