aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md1
-rw-r--r--perllib/Catalyst/Authentication/Credential/2FA.pm7
-rw-r--r--perllib/FixMyStreet/App/Controller/Alert.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth/Profile.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Report/New.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Report/Update.pm2
-rw-r--r--perllib/FixMyStreet/DB/Result/User.pm2
-rw-r--r--t/app/controller/auth.t1
-rw-r--r--t/app/controller/auth_profile.t12
-rw-r--r--templates/web/base/auth/generate_token.html2
11 files changed, 20 insertions, 15 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0deef93a5..49188e212 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,7 @@
- Allow categories/Open311 questions to disable the reporting form. #2599
- Improve category edit form. #2469
- Allow editing of category name. #1398
+ - Allow non-superusers to store 2FA secrets.
- New features:
- Categories can be listed under more than one group #2475
- OpenID Connect login support. #2523
diff --git a/perllib/Catalyst/Authentication/Credential/2FA.pm b/perllib/Catalyst/Authentication/Credential/2FA.pm
index 154959ce3..22f4b4cff 100644
--- a/perllib/Catalyst/Authentication/Credential/2FA.pm
+++ b/perllib/Catalyst/Authentication/Credential/2FA.pm
@@ -21,8 +21,7 @@ sub authenticate {
my $user_obj = $realm->find_user($userfindauthinfo, $c);
if (ref($user_obj)) {
- # We don't care unless user is a superuser and has a 2FA secret
- return $user_obj unless $user_obj->is_superuser;
+ # We don't care unless user has a 2FA secret
return $user_obj unless $user_obj->get_extra_metadata('2fa_secret');
$c->stash->{token} = $c->get_param('token');
@@ -91,8 +90,8 @@ with a two-factor authentication code.
This authentication credential checker takes authentication information
(most often a username), and only passes if a valid 2FA code is then
-entered. It only works for Users that have an is_superuser flag set,
-plus store the 2FA secret in a FixMyStreet::Role::Extra metadata key.
+entered. It only works for Users that have a 2FA secret stored in a
+FixMyStreet::Role::Extra metadata key.
=head1 CONFIGURATION
diff --git a/perllib/FixMyStreet/App/Controller/Alert.pm b/perllib/FixMyStreet/App/Controller/Alert.pm
index 1060c080b..755602562 100644
--- a/perllib/FixMyStreet/App/Controller/Alert.pm
+++ b/perllib/FixMyStreet/App/Controller/Alert.pm
@@ -283,7 +283,7 @@ sub send_confirmation_email : Private {
my $user = $c->stash->{alert}->user;
- # Superusers using 2FA can not log in by code
+ # People using 2FA can not log in by code
$c->detach( '/page_error_403_access_denied', [] ) if $user->has_2fa;
my $token = $c->model("DB::Token")->create(
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index e23690368..9680a5624 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -257,7 +257,7 @@ sub process_login : Private {
$c->detach( '/page_error_403_access_denied', [] )
if FixMyStreet->config('SIGNUPS_DISABLED') && !$user->in_storage && !$data->{old_user_id};
- # Superusers using 2FA can not log in by code
+ # People using 2FA can not log in by code
$c->detach( '/page_error_403_access_denied', [] ) if $user->has_2fa;
if ($data->{old_user_id}) {
diff --git a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm
index 87aff2261..107720aee 100644
--- a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm
@@ -191,7 +191,7 @@ sub generate_token : Path('/auth/generate_token') {
$c->stash->{token_generated} = 1;
}
- if ($c->get_param('toggle_2fa') && $c->user->is_superuser) {
+ if ($c->get_param('toggle_2fa')) {
if ($has_2fa) {
$c->user->unset_extra_metadata('2fa_secret');
$c->stash->{toggle_2fa_off} = 1;
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 03623259c..cac50e34d 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -1613,7 +1613,7 @@ sub redirect_or_confirm_creation : Private {
return 1;
}
- # Superusers using 2FA can not log in by code
+ # People using 2FA can not log in by code
$c->detach( '/page_error_403_access_denied', [] ) if $report->user->has_2fa;
# otherwise email or text a confirm token to them.
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 28a58d4f8..1dc337c48 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -501,7 +501,7 @@ sub redirect_or_confirm_creation : Private {
return 1;
}
- # Superusers using 2FA can not log in by code
+ # People using 2FA can not log in by code
$c->detach( '/page_error_403_access_denied', [] ) if $update->user->has_2fa;
my $data = $c->stash->{token_data};
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index c5824af36..805ea4776 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -495,7 +495,7 @@ sub admin_user_body_permissions {
sub has_2fa {
my $self = shift;
- return $self->is_superuser && $self->get_extra_metadata('2fa_secret');
+ return $self->get_extra_metadata('2fa_secret');
}
sub contributing_as {
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index ffabc75f3..fc1966b17 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -290,7 +290,6 @@ subtest "Test two-factor authentication login" => sub {
my $wrong_code = $auth->code(undef, time() - 120);
my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
- $user->is_superuser(1);
$user->password('password');
$user->set_extra_metadata('2fa_secret', $auth->secret32);
$user->update;
diff --git a/t/app/controller/auth_profile.t b/t/app/controller/auth_profile.t
index 815098caa..6cab1fb6c 100644
--- a/t/app/controller/auth_profile.t
+++ b/t/app/controller/auth_profile.t
@@ -359,6 +359,8 @@ subtest "Test superuser can access generate token page" => sub {
$mech->get_ok('/auth/generate_token');
};
+my $body = $mech->create_body_ok(2237, 'Oxfordshire');
+
subtest "Test staff user can access generate token page" => sub {
my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
ok $user->update({ is_superuser => 0 }), 'user not superuser';
@@ -374,8 +376,6 @@ subtest "Test staff user can access generate token page" => sub {
$mech->content_lacks('Security');
- my $body = $mech->create_body_ok(2237, 'Oxfordshire');
-
$mech->get('/auth/generate_token');
is $mech->res->code, 403, "access denied";
@@ -428,8 +428,13 @@ subtest "Test generate token page" => sub {
};
subtest "Test two-factor authentication admin" => sub {
+ for (0, 1) {
my $user = $mech->log_in_ok($test_email);
- ok $user->update({ is_superuser => 1 }), 'user set to superuser';
+ if ($_) {
+ ok $user->update({ is_superuser => 1, from_body => undef }), 'user set to superuser';
+ } else {
+ ok $user->update({ is_superuser => 0, from_body => $body }), 'user set to staff user';
+ }
$mech->get_ok('/auth/generate_token');
ok !$user->get_extra_metadata('2fa_secret');
@@ -448,4 +453,5 @@ subtest "Test two-factor authentication admin" => sub {
$mech->submit_form_ok({ button => 'toggle_2fa' }, "submit 2FA deactivation");
$mech->content_contains('has been deactivated', "2FA deactivated");
+ }
};
diff --git a/templates/web/base/auth/generate_token.html b/templates/web/base/auth/generate_token.html
index f7061be45..3c9dbf6aa 100644
--- a/templates/web/base/auth/generate_token.html
+++ b/templates/web/base/auth/generate_token.html
@@ -50,7 +50,7 @@ INCLUDE 'header.html', title = loc('Security'), bodyclass = 'fullwidthpage'
<p>
<input name="generate_token" type="submit" class="btn" value="[% existing_token ? loc('Replace token') : loc('Generate token') %]">
- [% IF c.user.is_superuser %]
+ [% IF c.user.is_superuser || c.user.from_body %]
<input name="toggle_2fa" type="submit" class="btn" value="[% has_2fa ? loc('Deactivate two-factor authentication') : loc('Activate two-factor authentication') %]">
[% END %]
</p>
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-14 00:22:50 +0000