diff options
32 files changed, 469 insertions, 346 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 0deef93a5..084a819ff 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-superuser staff to use 2FA, and optional enforcement of 2FA. - 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 428a3668c..8b6771037 100644 --- a/perllib/Catalyst/Authentication/Credential/2FA.pm +++ b/perllib/Catalyst/Authentication/Credential/2FA.pm @@ -21,13 +21,55 @@ 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; - return $user_obj unless $user_obj->get_extra_metadata('2fa_secret'); + + # We don't care unless user has a 2FA secret, or the cobrand mandates it + return $user_obj unless $user_obj->has_2fa || $c->cobrand->call_hook('must_have_2fa', $user_obj); $c->stash->{token} = $c->get_param('token'); - if ($self->check_2fa($c, $user_obj)) { + if (!$user_obj->has_2fa) { + $c->stash->{template} = 'auth/2fa/intro.html'; + my $action = $c->get_param('2fa_action') || ''; + + my $secret; + if ($action eq 'confirm') { + $secret = $c->get_param('secret32'); + if ($c->check_2fa($secret)) { + $user_obj->set_extra_metadata('2fa_secret' => $secret); + $user_obj->update; + if ($c->stash->{token}) { + my $token = $c->forward('/tokens/load_auth_token', [ $c->stash->{token}, '2fa' ]); + # Will contain a detach_to and report/update data + $c->stash($token->data); + } else { + $c->stash->{stage} = 'success'; + $c->stash->{detach_to} = '/auth/two_factor_setup_success'; + } + return $user_obj; + } else { + $action = 'activate'; # Incorrect code, reshow + } + } + + if ($action eq 'activate') { + my $auth = Auth::GoogleAuth->new; + $c->stash->{qr_code} = $auth->qr_code($secret, $user_obj->email, 'FixMyStreet'); + $c->stash->{secret32} = $auth->secret32; + $c->stash->{stage} = 'activate'; + } + + if ($c->stash->{tfa_data}) { + my $token = $c->model("DB::Token")->create( { + scope => '2fa', + data => $c->stash->{tfa_data}, + }); + $c->stash->{token} = $token->token; + } + + $c->detach; + } + + if ($c->check_2fa($user_obj->has_2fa)) { if ($c->stash->{token}) { my $token = $c->forward('/tokens/load_auth_token', [ $c->stash->{token}, '2fa' ]); # Will contain a detach_to and report/update data @@ -44,23 +86,11 @@ sub authenticate { $c->stash->{token} = $token->token; } - $c->stash->{template} = 'auth/2faform.html'; + $c->stash->{template} = 'auth/2fa/form.html'; $c->detach; } } -sub check_2fa { - my ($self, $c, $user) = @_; - - if (my $code = $c->get_param('2fa_code')) { - my $auth = Auth::GoogleAuth->new; - my $secret32 = $user->get_extra_metadata('2fa_secret'); - return 1 if $auth->verify($code, 2, $secret32); - $c->stash->{incorrect_code} = 1; - } - return 0; -} - __PACKAGE__; __END__ @@ -91,8 +121,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.pm b/perllib/FixMyStreet/App.pm index 27a5a4580..1173523bc 100644 --- a/perllib/FixMyStreet/App.pm +++ b/perllib/FixMyStreet/App.pm @@ -13,6 +13,7 @@ use FixMyStreet::Email::Sender; use FixMyStreet::PhotoStorage; use Utils; +use Auth::GoogleAuth; use Path::Tiny 'path'; use Try::Tiny; use Text::CSV; @@ -516,6 +517,23 @@ sub set_param { $c->req->params->{$param} = $value; } +=head2 check_2fa + +Given a user's secret, verifies a submitted code. + +=cut + +sub check_2fa { + my ($c, $secret32) = @_; + + if (my $code = $c->get_param('2fa_code')) { + my $auth = Auth::GoogleAuth->new; + return 1 if $auth->verify($code, 2, $secret32); + $c->stash->{incorrect_code} = 1; + } + return 0; +} + =head1 SEE ALSO L<FixMyStreet::App::Controller::Root>, L<Catalyst> diff --git a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm index 165fdc783..098c29ad4 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm @@ -269,14 +269,6 @@ sub update_contact : Private { } else { $contact->unset_extra_metadata( 'photo_required' ); } - if ( $c->get_param('inspection_required') ) { - $contact->set_extra_metadata( inspection_required => 1 ); - } else { - $contact->unset_extra_metadata( 'inspection_required' ); - } - if ( $c->get_param('reputation_threshold') ) { - $contact->set_extra_metadata( reputation_threshold => int($c->get_param('reputation_threshold')) ); - } if ( my @group = $c->get_param_list('group') ) { @group = grep { $_ } @group; if (scalar @group == 0) { diff --git a/perllib/FixMyStreet/App/Controller/Admin/Users.pm b/perllib/FixMyStreet/App/Controller/Admin/Users.pm index fd18caf21..0d7c23fff 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/Users.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/Users.pm @@ -371,35 +371,6 @@ sub edit : Path : Args(1) { $user->area_ids( @area_ids ? \@area_ids : undef ); } - # Handle 'trusted' flag(s) - my @trusted_bodies = $c->get_param_list('trusted_bodies'); - if ( $c->user->is_superuser ) { - $user->user_body_permissions->search({ - body_id => { -not_in => \@trusted_bodies }, - permission_type => 'trusted', - })->delete; - foreach my $body_id (@trusted_bodies) { - $user->user_body_permissions->find_or_create({ - body_id => $body_id, - permission_type => 'trusted', - }); - } - } elsif ( $c->user->from_body ) { - my %trusted = map { $_ => 1 } @trusted_bodies; - my $body_id = $c->user->from_body->id; - if ( $trusted{$body_id} ) { - $user->user_body_permissions->find_or_create({ - body_id => $body_id, - permission_type => 'trusted', - }); - } else { - $user->user_body_permissions->search({ - body_id => $body_id, - permission_type => 'trusted', - })->delete; - } - } - # Update the categories this user operates in if ( $user->from_body ) { $c->stash->{body} = $user->from_body; 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..041a8b76e 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -241,11 +241,11 @@ sub token : Path('/M') : Args(1) { && (!$c->user_exists || $c->user->id ne $data->{old_user_id}); my $type = $data->{login_type} || 'email'; - $c->detach( '/auth/process_login', [ $data, $type ] ); + $c->detach( '/auth/process_login', [ $data, $type, $url_token ] ); } sub process_login : Private { - my ( $self, $c, $data, $type ) = @_; + my ( $self, $c, $data, $type, $url_token ) = @_; # sign out in case we are another user $c->logout(); @@ -257,8 +257,9 @@ 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 - $c->detach( '/page_error_403_access_denied', [] ) if $user->has_2fa; + # People using 2FA need to supply a code + $c->forward( 'token_2fa', [ $user, $url_token ] ) if $user->has_2fa; + $c->forward( 'signup_2fa', [ $user ] ) if $c->cobrand->call_hook('must_have_2fa', $user); if ($data->{old_user_id}) { # Were logged in as old_user_id, want to switch to $user @@ -303,6 +304,53 @@ sub process_login : Private { $c->detach( 'redirect_on_signin', [ $data->{r}, $data->{p} ] ); } +=head2 token_2fa + +Used after clicking an email token link to request a 2FA code + +=cut + +sub token_2fa : Private { + my ($self, $c, $user, $url_token) = @_; + + return if $c->check_2fa($user->has_2fa); + + $c->stash->{form_action} = $c->req->path; + $c->stash->{token} = $url_token; + $c->stash->{template} = 'auth/2fa/form.html'; + $c->detach; +} + +sub signup_2fa : Private { + my ($self, $c, $user) = @_; + + $c->stash->{form_action} = $c->req->path; + $c->stash->{template} = 'auth/2fa/intro.html'; + my $action = $c->get_param('2fa_action') || ''; + + my $secret; + if ($action eq 'confirm') { + $secret = $c->get_param('secret32'); + if ($c->check_2fa($secret)) { + $user->set_extra_metadata('2fa_secret' => $secret); + $user->update; + $c->stash->{stage} = 'success'; + return; + } else { + $action = 'activate'; # Incorrect code, reshow + } + } + + if ($action eq 'activate') { + my $auth = Auth::GoogleAuth->new; + $c->stash->{qr_code} = $auth->qr_code($secret, $user->email, 'FixMyStreet'); + $c->stash->{secret32} = $auth->secret32; + $c->stash->{stage} = 'activate'; + } + + $c->detach; +} + =head2 redirect_on_signin Used after signing in to take the person back to where they were. @@ -540,6 +588,11 @@ sub check_auth : Local { return; } +sub two_factor_setup_success : Private { + my ($self, $c) = @_; + # Only here to be detached to after setup success +} + __PACKAGE__->meta->make_immutable; 1; diff --git a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm index 87aff2261..91ffac205 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm @@ -188,23 +188,38 @@ sub generate_token : Path('/auth/generate_token') { if ($c->get_param('generate_token')) { my $token = mySociety::AuthToken::random_token(); $c->user->set_extra_metadata('access_token', $token); + $c->user->update; $c->stash->{token_generated} = 1; } - if ($c->get_param('toggle_2fa') && $c->user->is_superuser) { - if ($has_2fa) { - $c->user->unset_extra_metadata('2fa_secret'); - $c->stash->{toggle_2fa_off} = 1; + my $action = $c->get_param('2fa_action') || ''; + $action = 'deactivate' if $c->get_param('2fa_deactivate'); + $action = 'activate' if $c->get_param('2fa_activate'); + $action = 'activate' if $action eq 'deactivate' && $has_2fa && $c->cobrand->call_hook('must_have_2fa', $c->user); + + my $secret; + if ($action eq 'deactivate') { + $c->user->unset_extra_metadata('2fa_secret'); + $c->user->update; + $c->stash->{toggle_2fa_off} = 1; + } elsif ($action eq 'confirm') { + $secret = $c->get_param('secret32'); + if ($c->check_2fa($secret)) { + $c->user->set_extra_metadata('2fa_secret', $secret); + $c->user->update; + $c->stash->{stage} = 'success'; + $has_2fa = 1; } else { - my $auth = Auth::GoogleAuth->new; - $c->stash->{qr_code} = $auth->qr_code(undef, $c->user->email, 'FixMyStreet'); - $c->stash->{secret32} = $auth->secret32; - $c->user->set_extra_metadata('2fa_secret', $auth->secret32); - $c->stash->{toggle_2fa_on} = 1; + $action = 'activate'; # Incorrect code, reshow } } - $c->user->update(); + if ($action eq 'activate') { + my $auth = Auth::GoogleAuth->new; + $c->stash->{qr_code} = $auth->qr_code($secret, $c->user->email, 'FixMyStreet'); + $c->stash->{secret32} = $auth->secret32; + $c->stash->{stage} = 'activate'; + } } $c->stash->{has_2fa} = $has_2fa ? 1 : 0; diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 692379de6..89350b1cb 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -356,8 +356,6 @@ sub delete :Chained('id') :Args(0) { $p->lastupdate( \'current_timestamp' ); $p->update; - $p->user->update_reputation(-1); - $c->model('DB::AdminLog')->create( { user => $c->user->obj, admin_user => $c->user->from_body->name, @@ -408,7 +406,6 @@ sub inspect : Private { my $valid = 1; my $update_text = ''; - my $reputation_change = 0; my %update_params = (); if ($permissions->{report_inspect}) { @@ -463,8 +460,6 @@ sub inspect : Private { $update_params{problem_state} = $problem->state; my $state = $problem->state; - $reputation_change = 1 if $c->cobrand->reputation_increment_states->{$state}; - $reputation_change = -1 if $c->cobrand->reputation_decrement_states->{$state}; # If an inspector has changed the state, subscribe them to # updates @@ -518,9 +513,6 @@ sub inspect : Private { $c->cobrand->call_hook(report_inspect_update_extra => $problem); if ($valid) { - if ( $reputation_change != 0 ) { - $problem->user->update_reputation($reputation_change); - } $problem->lastupdate( \'current_timestamp' ); $problem->update; if ($update_text || %update_params) { diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 907834b3a..612c76c0c 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/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index d2929dcc0..620183078 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -748,12 +748,6 @@ sub available_permissions { contribute_as_body => _("Create reports/updates as the council"), default_to_body => _("Default to creating reports/updates as the council"), view_body_contribute_details => _("See user detail for reports created as the council"), - - # NB this permission is special in that it can be assigned to users - # without their from_body being set. It's included here for - # reference, but left commented out because it's not assigned in the - # same way as other permissions. - # trusted => _("Trusted to make reports that don't need to be inspected"), }, _("Users") => { user_edit => _("Edit users' details/search for their reports"), @@ -1227,16 +1221,6 @@ sub category_extra_hidden { return 0; } -=item reputation_increment_states/reputation_decrement_states - -Get a hashref of states that cause the reporting user's reputation to be -incremented/decremented, if a report is changed to this state upon inspection. - -=cut - -sub reputation_increment_states { {} }; -sub reputation_decrement_states { {} }; - sub traffic_management_options { return [ _("Yes"), diff --git a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm index e830b10b2..c69156c64 100644 --- a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm +++ b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm @@ -310,4 +310,10 @@ sub suppress_reporter_alerts { return 0; } +sub must_have_2fa { + my ($self, $user) = @_; + return 1 if $user->is_superuser; + return 0; +} + 1; diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm index f99cdf84d..6f6284c7a 100644 --- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm +++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm @@ -172,12 +172,6 @@ sub admin_pages { return $pages; } -sub reputation_increment_states { - return { - 'action scheduled' => 1, - }; -} - sub user_extra_fields { return [ 'initials' ]; } diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index 85fdc790b..805ea4776 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -438,9 +438,6 @@ sub has_permission_to { my $cobrand = $self->result_source->schema->cobrand; my $cobrand_perms = $cobrand->available_permissions; my %available = map { %$_ } values %$cobrand_perms; - # The 'trusted' permission is never set in the cobrand's - # available_permissions (see note there in Default.pm) so include it here. - $available{trusted} = 1; return 0 unless $available{$permission_type}; return 1 if $self->is_superuser; @@ -498,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 { @@ -600,14 +597,6 @@ sub is_planned_report { return scalar grep { $_->report_id == $id } @{$self->active_user_planned_reports}; } -sub update_reputation { - my ( $self, $change ) = @_; - - my $reputation = $self->get_extra_metadata('reputation') || 0; - $self->set_extra_metadata( reputation => $reputation + $change); - $self->update; -} - has categories => ( is => 'ro', lazy => 1, diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm index 59de86db1..9725f1781 100644 --- a/perllib/FixMyStreet/Script/Reports.pm +++ b/perllib/FixMyStreet/Script/Reports.pm @@ -148,25 +148,6 @@ sub send(;$) { } $reporters{ $sender } ||= $sender->new(); - my $inspection_required = $sender_info->{contact} - ? $sender_info->{contact}->get_extra_metadata('inspection_required') - : undef; - if ( $inspection_required ) { - my $reputation_threshold = $sender_info->{contact}->get_extra_metadata('reputation_threshold') || 0; - my $reputation_threshold_met = 0; - if ( $reputation_threshold > 0 ) { - my $user_reputation = $row->user->get_extra_metadata('reputation') || 0; - $reputation_threshold_met = $user_reputation >= $reputation_threshold; - } - unless ( - $row->user->has_permission_to( trusted => $row->bodies_str_ids ) || - $reputation_threshold_met - ) { - $skip = 1; - debug_print("skipped because not yet inspected", $row->id) if $debug_mode; - } - } - if ( $reporters{ $sender }->should_skip( $row, $debug_mode ) ) { $skip = 1; debug_print("skipped by sender " . $sender_info->{method} . " (might be due to previous failed attempts?)", $row->id) if $debug_mode; diff --git a/t/app/controller/admin/reportextrafields.t b/t/app/controller/admin/reportextrafields.t index 1714e8521..1aa4c6a42 100644 --- a/t/app/controller/admin/reportextrafields.t +++ b/t/app/controller/admin/reportextrafields.t @@ -9,6 +9,8 @@ sub allow_report_extra_fields { 1 } sub area_types { [ 'UTA' ] } +sub must_have_2fa { 0 } + package FixMyStreet::Cobrand::SecondTester; diff --git a/t/app/controller/admin/users.t b/t/app/controller/admin/users.t index 7361ab619..17256a214 100644 --- a/t/app/controller/admin/users.t +++ b/t/app/controller/admin/users.t @@ -249,7 +249,6 @@ my %default_perms = ( "permissions[template_edit]" => undef, "permissions[responsepriority_edit]" => undef, "permissions[category_edit]" => undef, - trusted_bodies => undef, ); # Start this section with user having no name diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t index ffabc75f3..cd72ab550 100644 --- a/t/app/controller/auth.t +++ b/t/app/controller/auth.t @@ -1,3 +1,10 @@ +package FixMyStreet::Cobrand::Dummy; +use parent 'FixMyStreet::Cobrand::Default'; + +sub must_have_2fa { 1 } + +package main; + use Test::MockModule; use FixMyStreet::TestMech; @@ -7,10 +14,6 @@ my $test_email = 'test@example.com'; my $test_email3 = 'newuser@example.org'; my $test_password = 'foobar123'; -END { - done_testing(); -} - $mech->get_ok('/auth'); # check that we can't reach a page that is only available to authenticated users @@ -290,7 +293,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; @@ -305,3 +307,97 @@ subtest "Test two-factor authentication login" => sub { $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" ); $mech->logged_in_ok; }; + +subtest "Test enforced two-factor authentication" => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => 'dummy', + }, sub { + my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); + $user->unset_extra_metadata('2fa_secret'); + $user->update; + + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { with_fields => { username => $test_email, password_sign_in => 'password' } }, + "sign in using form" ); + + $mech->content_contains('requires two-factor'); + $mech->submit_form_ok({ with_fields => { '2fa_action' => 'activate' } }, "submit 2FA activation"); + my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/; + + use Auth::GoogleAuth; + my $auth = Auth::GoogleAuth->new({ secret32 => $token }); + my $code = $auth->code; + my $wrong_code = $auth->code(undef, time() - 120); + + $mech->submit_form_ok({ with_fields => { '2fa_code' => $wrong_code } }, "provide wrong 2FA code" ); + $mech->content_contains('Try again'); + $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" ); + $mech->content_contains('successfully enabled two-factor authentication', "2FA activated"); + + $user->discard_changes(); + my $user_token = $user->get_extra_metadata('2fa_secret'); + is $token, $user_token, '2FA secret set'; + + $mech->logged_in_ok; + }; +}; + +subtest "Test enforced two-factor authentication, no password yet set" => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => 'dummy', + }, sub { + my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); + $user->unset_extra_metadata('2fa_secret'); + $user->update; + + $mech->clear_emails_ok; + $mech->get_ok('/auth'); + $mech->submit_form_ok({ + fields => { username => $test_email, password_register => $test_password }, + button => 'sign_in_by_code', + }, "log in by email"); + + my $link = $mech->get_link_from_email; + $mech->get_ok($link); + + $mech->content_contains('requires two-factor'); + $mech->submit_form_ok({ with_fields => { '2fa_action' => 'activate' } }, "submit 2fa activation"); + my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/; + + my $auth = Auth::GoogleAuth->new({ secret32 => $token }); + my $code = $auth->code; + $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2fa code" ); + + $user->discard_changes(); + my $user_token = $user->get_extra_metadata('2fa_secret'); + is $token, $user_token, '2FA secret set'; + + $mech->logged_in_ok; + }; +}; + +subtest "Check two-factor log in by email works" => sub { + use Auth::GoogleAuth; + my $auth = Auth::GoogleAuth->new; + my $code = $auth->code; + + my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); + $user->set_extra_metadata('2fa_secret', $auth->secret32); + $user->update; + + $mech->clear_emails_ok; + $mech->get_ok('/auth'); + $mech->submit_form_ok({ + fields => { username => $test_email, password_register => $test_password }, + button => 'sign_in_by_code', + }, "log in by email"); + + my $link = $mech->get_link_from_email; + $mech->get_ok($link); + $mech->content_contains('Please generate a two-factor code'); + $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" ); + $mech->logged_in_ok; +}; + +done_testing(); diff --git a/t/app/controller/auth_profile.t b/t/app/controller/auth_profile.t index 2f0ef6a11..4b7db19b5 100644 --- a/t/app/controller/auth_profile.t +++ b/t/app/controller/auth_profile.t @@ -1,3 +1,10 @@ +package FixMyStreet::Cobrand::Dummy; +use parent 'FixMyStreet::Cobrand::Default'; + +sub must_have_2fa { 1 } + +package main; + use Test::MockModule; use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; @@ -359,6 +366,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 +383,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"; @@ -425,29 +432,80 @@ subtest "Test generate token page" => sub { $mech->log_out_ok; $mech->add_header('Authorization', "Bearer $token"); $mech->logged_in_ok; + $mech->delete_header('Authorization'); }; 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'); - $mech->submit_form_ok({ button => 'toggle_2fa' }, "submit 2FA activation"); + $mech->submit_form_ok({ button => '2fa_activate' }, "submit 2FA activation"); + my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/; + + use Auth::GoogleAuth; + my $auth = Auth::GoogleAuth->new({ secret32 => $token }); + my $code = $auth->code; + my $wrong_code = $auth->code(undef, time() - 120); + + $mech->submit_form_ok({ with_fields => { '2fa_code' => $wrong_code } }, "provide wrong 2FA code" ); + $mech->content_contains('Try again'); + $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" ); + $mech->content_contains('has been activated', "2FA activated"); $user->discard_changes(); - my $token = $user->get_extra_metadata('2fa_secret'); - ok $token, '2FA secret set'; - - $mech->content_contains($token, 'secret displayed'); + my $user_token = $user->get_extra_metadata('2fa_secret'); + is $token, $user_token, '2FA secret set'; $mech->get_ok('/auth/generate_token'); $mech->content_lacks($token, 'secret no longer displayed'); - $mech->submit_form_ok({ button => 'toggle_2fa' }, "submit 2FA deactivation"); + $mech->submit_form_ok({ button => '2fa_deactivate' }, "submit 2FA deactivation"); $mech->content_contains('has been deactivated', "2FA deactivated"); + } +}; + +subtest "Test enforced two-factor authentication" => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => 'dummy', + }, sub { + use Auth::GoogleAuth; + my $auth = Auth::GoogleAuth->new; + my $code = $auth->code; + + # Sign in with 2FA + my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); + $user->password('password'); + $user->set_extra_metadata('2fa_secret', $auth->secret32); + $user->update; + + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { with_fields => { username => $test_email, password_sign_in => 'password' } }, + "sign in using form" ); + $mech->content_contains('Please generate a two-factor code'); + $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" ); + + $mech->get_ok('/auth/generate_token'); + $mech->content_contains('Change two-factor'); + $mech->content_lacks('Deactivate two-factor'); + + my ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + $mech->post_ok('/auth/generate_token', { + '2fa_deactivate' => 1, + 'token' => $csrf, + }); + $mech->content_lacks('has been deactivated', "2FA not deactivated"); + $mech->content_contains('Please scan this image', 'Change 2FA page shown instead'); + }; }; done_testing(); diff --git a/t/app/controller/dashboard.t b/t/app/controller/dashboard.t index 061809aaf..15c718c74 100644 --- a/t/app/controller/dashboard.t +++ b/t/app/controller/dashboard.t @@ -1,5 +1,9 @@ use Test::MockTime ':all'; +package FixMyStreet::Cobrand::No2FA; +use parent 'FixMyStreet::Cobrand::FixMyStreet'; +sub must_have_2fa { 0 } + package FixMyStreet::Cobrand::Tester; use parent 'FixMyStreet::Cobrand::Default'; # Allow access if CSV export for a body, otherwise deny @@ -38,13 +42,13 @@ my $area_id = '60705'; my $alt_area_id = '62883'; my $last_month = DateTime->now->subtract(months => 2); -$mech->create_problems_for_body(2, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'fixmystreet' }); -$mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'fixmystreet', dt => $last_month }); -$mech->create_problems_for_body(1, $body->id, 'Title', { areas => ",$alt_area_id,2651,", category => 'Litter', cobrand => 'fixmystreet' }); +$mech->create_problems_for_body(2, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'no2fat' }); +$mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'no2fa', dt => $last_month }); +$mech->create_problems_for_body(1, $body->id, 'Title', { areas => ",$alt_area_id,2651,", category => 'Litter', cobrand => 'no2fa' }); -my @scheduled_problems = $mech->create_problems_for_body(7, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'fixmystreet' }); -my @fixed_problems = $mech->create_problems_for_body(4, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'fixmystreet' }); -my @closed_problems = $mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'fixmystreet' }); +my @scheduled_problems = $mech->create_problems_for_body(7, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'no2fa' }); +my @fixed_problems = $mech->create_problems_for_body(4, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'no2fa' }); +my @closed_problems = $mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'no2fa' }); my $first_problem_id; my $first_update_id; @@ -73,7 +77,7 @@ my $categories = scraper { }; FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + ALLOWED_COBRANDS => 'no2fa', MAPIT_URL => 'http://mapit.uk/', }, sub { diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index b6498e840..61fa821b2 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -132,7 +132,6 @@ FixMyStreet::override_config { $user->user_body_permissions->create({ body => $oxon, permission_type => 'planned_reports' }); $report->state('confirmed'); $report->update; - my $reputation = $report->user->get_extra_metadata("reputation"); $mech->get_ok("/report/$report_id"); $mech->submit_form_ok({ button => 'save', with_fields => { public_update => "This is a public update.", include_update => "1", @@ -145,7 +144,6 @@ FixMyStreet::override_config { $report->discard_changes; my $comment = ($report->comments( undef, { order_by => { -desc => 'id' } } )->all)[1]->text; is $comment, "This is a public update.", 'Update was created'; - is $report->user->get_extra_metadata('reputation'), $reputation, "User reputation wasn't changed"; $mech->get_ok("/report/$report_id"); my $meta = $mech->extract_update_metas; like $meta->[0], qr/State changed to: Action scheduled/, 'First update mentions action scheduled'; @@ -583,38 +581,6 @@ FixMyStreet::override_config { return $perms; }); - subtest "test negative reputation" => sub { - my $reputation = $report->user->get_extra_metadata("reputation") || 0; - - $mech->get_ok("/report/$report_id"); - $mech->submit_form( button => 'remove_from_site' ); - - $report->discard_changes; - is $report->user->get_extra_metadata('reputation'), $reputation-1, "User reputation was decreased"; - $report->update({ state => 'confirmed' }); - }; - - subtest "test positive reputation" => sub { - $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_instruct' }); - $report->update; - $report->inspection_log_entry->delete if $report->inspection_log_entry; - my $reputation = $report->user->get_extra_metadata("reputation") || 0; - $mech->get_ok("/report/$report_id"); - $mech->submit_form_ok({ button => 'save', with_fields => { - state => 'in progress', include_update => undef, - } }); - $report->discard_changes; - - $mech->submit_form_ok({ button => 'save', with_fields => { - state => 'action scheduled', include_update => undef, - } }); - $report->discard_changes; - is $report->user->get_extra_metadata('reputation'), $reputation+1, "User reputation was increased"; - - $mech->submit_form_ok({ button => 'save', with_fields => { - state => 'action scheduled', include_update => undef, - } }); - }; subtest "Oxfordshire-specific traffic management options are shown" => sub { $report->update({ state => 'confirmed' }); diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index 20eecb50e..48169c148 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -914,8 +914,9 @@ subtest "test password errors for a user who is signing in as they report" => su }; foreach my $test ( - { two_factor => 0, desc => '', }, - { two_factor => 1, desc => ' with two-factor', }, + { two_factor => '', desc => '', }, + { two_factor => 'yes', desc => ' with two-factor', }, + { two_factor => 'new', desc => ' with mandated two-factor, not yet set up', }, ) { subtest "test report creation for a user who is signing in as they report$test->{desc}" => sub { $mech->log_out_ok; @@ -932,21 +933,25 @@ foreach my $test ( name => 'Joe Bloggs', phone => '01234 567 890', password => 'secret2', + $test->{two_factor} ? (is_superuser => 1) : (), } ), "set user details"; my $auth; - if ($test->{two_factor}) { + my $mock; + if ($test->{two_factor} eq 'yes') { use Auth::GoogleAuth; $auth = Auth::GoogleAuth->new; - $user->is_superuser(1); $user->set_extra_metadata('2fa_secret', $auth->generate_secret32); $user->update; + } elsif ($test->{two_factor} eq 'new') { + $mock = Test::MockModule->new('FixMyStreet::Cobrand::FixMyStreet'); + $mock->mock(must_have_2fa => sub { 1 }); } # submit initial pc form $mech->get_ok('/around'); FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + ALLOWED_COBRANDS => 'fixmystreet', MAPIT_URL => 'http://mapit.uk/', }, sub { $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, @@ -971,13 +976,23 @@ foreach my $test ( "submit good details" ); - if ($test->{two_factor}) { + if ($test->{two_factor} eq 'yes') { my $code = $auth->code; my $wrong_code = $auth->code(undef, time() - 120); $mech->content_contains('Please generate a two-factor code'); $mech->submit_form_ok({ with_fields => { '2fa_code' => $wrong_code } }, "provide wrong 2FA code" ); $mech->content_contains('Try again'); $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" ); + } elsif ($test->{two_factor} eq 'new') { + $mech->content_contains('requires two-factor'); + $mech->submit_form_ok({ with_fields => { '2fa_action' => 'activate' } }, "submit 2FA activation"); + my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/; + + use Auth::GoogleAuth; + my $auth = Auth::GoogleAuth->new({ secret32 => $token }); + my $code = $auth->code; + print $mech->encoded_content; + $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" ); } # check that we got the message expected @@ -998,7 +1013,7 @@ foreach my $test ( my $report = $user->problems->first; ok $report, "Found the report"; - if (!$test->{two_factor}) { + if ($test->{two_factor} eq '') { # The superuser account will be immediately redirected $mech->content_contains('Thank you for reporting this issue'); } diff --git a/t/app/sendreport/inspection_required.t b/t/app/sendreport/inspection_required.t deleted file mode 100644 index 5eff516f5..000000000 --- a/t/app/sendreport/inspection_required.t +++ /dev/null @@ -1,85 +0,0 @@ -use FixMyStreet; -use FixMyStreet::DB; -use FixMyStreet::TestMech; -use FixMyStreet::Script::Reports; - -ok( my $mech = FixMyStreet::TestMech->new, 'Created mech object' ); - -my $user = $mech->create_user_ok( 'user@example.com' ); - -my $body = $mech->create_body_ok( 2237, 'Oxfordshire County Council'); -# $body->update({ send_method => 'Email' }); - -my $contact = $mech->create_contact_ok( - body_id => $body->id, - category => 'Pothole', - email => 'test@example.org', -); -$contact->set_extra_metadata(inspection_required => 1); -$contact->update; - -my @reports = $mech->create_problems_for_body( 1, $body->id, 'Test', { - cobrand => 'oxfordshire', - category => $contact->category, - user => $user, -}); -my $report = $reports[0]; - -subtest "Report isn't sent if uninspected" => sub { - $mech->clear_emails_ok; - - FixMyStreet::Script::Reports::send(); - - $mech->email_count_is( 0 ); - is $report->whensent, undef, "Report hasn't been sent"; -}; - -subtest 'Uninspected report is sent when made by trusted user' => sub { - $mech->clear_emails_ok; - $report->whensent( undef ); - $report->update; - - $user->user_body_permissions->find_or_create({ - body => $body, - permission_type => 'trusted', - }); - ok $user->has_permission_to('trusted', $report->bodies_str_ids), 'User can make trusted reports'; - - FixMyStreet::Script::Reports::send(); - - $report->discard_changes; - $mech->email_count_is( 1 ); - ok $report->whensent, 'Report marked as sent'; -}; - -subtest "Uninspected report isn't sent when user rep is too low" => sub { - $mech->clear_emails_ok; - $report->whensent( undef ); - $report->update; - - $user->user_body_permissions->delete; - $user->set_extra_metadata(reputation => 15); - $user->update; - - $contact->set_extra_metadata(reputation_threshold => 20); - $contact->update; - - FixMyStreet::Script::Reports::send(); - - $report->discard_changes; - $mech->email_count_is( 0 ); - is $report->whensent, undef, "Report hasn't been sent"; -}; - -subtest 'Uninspected report is sent when user rep is high enough' => sub { - $user->set_extra_metadata(reputation => 21); - $user->update; - - FixMyStreet::Script::Reports::send(); - - $report->discard_changes; - $mech->email_count_is( 1 ); - ok $report->whensent, 'Report marked as sent'; -}; - -done_testing(); diff --git a/t/cobrand/fixmystreet.t b/t/cobrand/fixmystreet.t index a4d00d032..a54e782f2 100644 --- a/t/cobrand/fixmystreet.t +++ b/t/cobrand/fixmystreet.t @@ -111,6 +111,22 @@ FixMyStreet::override_config { }; }; -END { - done_testing(); -} +FixMyStreet::override_config { + ALLOWED_COBRANDS => 'fixmystreet', +}, sub { + subtest 'test enforced 2FA for superusers' => sub { + my $test_email = 'test@example.com'; + my $user = FixMyStreet::DB->resultset('User')->find_or_create({ email => $test_email }); + $user->password('password'); + $user->is_superuser(1); + $user->update; + + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { with_fields => { username => $test_email, password_sign_in => 'password' } }, + "sign in using form" ); + $mech->content_contains('requires two-factor'); + }; +}; + +done_testing(); diff --git a/templates/web/base/admin/bodies/contact-form.html b/templates/web/base/admin/bodies/contact-form.html index ba47327cd..921cb1380 100644 --- a/templates/web/base/admin/bodies/contact-form.html +++ b/templates/web/base/admin/bodies/contact-form.html @@ -58,21 +58,6 @@ <textarea id="disabled-message" name="disable_message" class="form-control">[% contact.disable_form_field.description %]</textarea> </p> - <p class="form-check"> - <input type="checkbox" name="inspection_required" value="1" id="inspection_required" data-toggle-visibility="#js-inspection-reputation-box" [% 'checked' IF contact.get_extra_metadata('inspection_required') %]> - <label for="inspection_required">[% loc('Reports in this category must be inspected before being sent') %]</label> - </p> - - <p class="form-group form-group--indented [% 'hidden-js' IF NOT contact.get_extra_metadata('inspection_required') %]" id="js-inspection-reputation-box"> - <label for="reputation_threshold">[% loc('Reputation threshold') %]</label> - <span class="form-hint" id="reputation_threshold_hint"> - [% loc("Reports will automatically be sent without needing to be inspected if the user's <strong>reputation</strong> is at or above this value. Set to <strong>0</strong> if all reports must be inspected regardless.") %] - </span> - <input type="text" class="form-control" name="reputation_threshold" id="reputation_threshold" - value="[% contact.get_extra_metadata('reputation_threshold') | html %]" size="30" - aria-describedby="reputation_threshold_hint"> - </p> - [% IF body.can_be_devolved %] <div class="admin-hint"> <p> diff --git a/templates/web/base/admin/users/form.html b/templates/web/base/admin/users/form.html index c9cc6463b..f141dc02c 100644 --- a/templates/web/base/admin/users/form.html +++ b/templates/web/base/admin/users/form.html @@ -116,29 +116,6 @@ </label> </li> - [% UNLESS user.is_superuser %] - <li> - <div class="admin-hint"> - <p> - [% loc("Reports made by trusted users will be sent to the responsible body without being inspected first.") %] - </p> - </div> - [% IF c.user.is_superuser %] - <label for="trusted_bodies">[% loc('Trusted by bodies:') %]</label> - <select class="form-control js-multiple" id='trusted_bodies' name='trusted_bodies' multiple> - [% FOR body IN bodies %] - <option value="[% body.id %]"[% ' selected' IF user.has_permission_to('trusted', body.id) %]>[% body.name %]</option> - [% END %] - </select> - [% ELSE %] - <label> - [% loc('Trusted:') %] - <input type="checkbox" id="trusted_bodies" name="trusted_bodies" value="[% c.user.from_body.id %]" [% 'checked' IF user.has_permission_to('trusted', c.user.from_body.id) %]> - </label> - [% END %] - </li> - [% END %] - [% IF c.user.is_superuser %] <li> <div class="admin-hint"> diff --git a/templates/web/base/auth/2fa/form-add.html b/templates/web/base/auth/2fa/form-add.html new file mode 100644 index 000000000..706f1a31d --- /dev/null +++ b/templates/web/base/auth/2fa/form-add.html @@ -0,0 +1,17 @@ +<p>[% loc('Please scan this image with your app, or enter the text code into your app, then generate a new one-time code and enter it below:') %]</p> + +<p align="center"><img src="[% qr_code %]"></p> +<p align="center">[% secret32.replace('(....)', '$1 ') %]</p> + +[% IF incorrect_code %] + <div class="form-error">[% loc('Sorry, that wasn’t the correct code') %]. + [% loc('Try again') %]:</div> +[% END %] + +<label for="2fa_code">[% loc('Code') %]</label> +<div class="form-txt-submit-box"> + <input autofocus class="form-control" type="number" id="2fa_code" name="2fa_code" value="" required> + <input type="submit" value="[% loc('Submit') %]" class="btn-primary"> +</div> +<input type="hidden" name="secret32" value="[% secret32 %]"> +<input type="hidden" name="2fa_action" value="confirm"> diff --git a/templates/web/base/auth/2faform.html b/templates/web/base/auth/2fa/form.html index bd6a7bd18..e093f6554 100644 --- a/templates/web/base/auth/2faform.html +++ b/templates/web/base/auth/2fa/form.html @@ -8,7 +8,7 @@ <h1>[% loc("Nearly done! Now check your phone…") %]</h1> <p>[% loc("Please generate a two-factor code and enter it below:") %]</p> [% END %] - <form action="/auth" method="post"> + <form action="/[% form_action OR 'auth' %]" method="post"> <input type="hidden" name="username" value="[% c.get_param('username') | html %]"> <input type="hidden" name="password_sign_in" value="[% c.get_param('password_sign_in') | html %]"> <input type="hidden" name="r" value="[% c.get_param('r') | html %]"> diff --git a/templates/web/base/auth/2fa/intro.html b/templates/web/base/auth/2fa/intro.html new file mode 100644 index 000000000..7813507a3 --- /dev/null +++ b/templates/web/base/auth/2fa/intro.html @@ -0,0 +1,32 @@ +[% +INCLUDE 'header.html', title = loc('Two-factor authentication'), bodyclass = 'fullwidthpage' +%] + +<div class="confirmation-header confirmation-header--phone"> + <h1>[% loc('Two-factor authentication') %]</h1> + + <form action="/[% form_action OR 'auth' %]" method="post"> + + [% IF stage == 'success' %] + <p>[% loc('Thanks, you have successfully enabled two-factor authentication on your account.') %]</p> + <p><a href="/my">[% loc('Your account') %]</a></p> + + [% ELSIF stage == 'activate' %] + [% PROCESS 'auth/2fa/form-add.html' %] + + [% ELSE # stage is intro %] + <p align="center">[% loc('Your account requires two-factor authentication to be set up.') %]</p> + <p align="center"> + <input class="btn-primary" type="submit" value="[% loc('Activate two-factor authentication') %]"> + </p> + <input type="hidden" name="2fa_action" value="activate"> + [% END %] + + <input type="hidden" name="username" value="[% c.get_param('username') | html %]"> + <input type="hidden" name="password_sign_in" value="[% c.get_param('password_sign_in') | html %]"> + <input type="hidden" name="r" value="[% c.get_param('r') | html %]"> + <input type="hidden" name="token" value="[% token | html %]"> + </form> +</div> + +[% INCLUDE 'footer.html' %] diff --git a/templates/web/base/auth/generate_token.html b/templates/web/base/auth/generate_token.html index f7061be45..9152d0cb3 100644 --- a/templates/web/base/auth/generate_token.html +++ b/templates/web/base/auth/generate_token.html @@ -8,37 +8,63 @@ INCLUDE 'header.html', title = loc('Security'), bodyclass = 'fullwidthpage' <h1>[% loc('Your token has been generated') %]</h1> <p> - <strong>[% loc('Token:') %]</strong> + <strong>[% loc('Token') %]:</strong> <span>[% existing_token | html %]</span> </p> <p><a href="/my">[% loc('Your account') %]</a></p> </div> -[% ELSIF toggle_2fa_on %] +[% ELSIF toggle_2fa_off %] <div class="confirmation-header"> - <h1>[% loc('Two-factor authentication has been activated') %]</h1> - - <p align="center"><img src="[% qr_code %]"></p> - <p align="center">[% secret32.replace('(....)', '$1 ') %]</p> + <h1>[% loc('Two-factor authentication has been deactivated') %]</h1> <p><a href="/my">[% loc('Your account') %]</a></p> </div> -[% ELSIF toggle_2fa_off %] +[% ELSIF stage == 'success' %] <div class="confirmation-header"> - <h1>[% loc('Two-factor authentication has been deactivated') %]</h1> - + <h1>[% loc('Two-factor authentication has been activated') %]</h1> + <p>[% loc('Thanks, you have successfully enabled two-factor authentication on your account.') %]</p> <p><a href="/my">[% loc('Your account') %]</a></p> </div> +[% ELSIF stage == 'activate' %] + <div class="confirmation-header confirmation-header--phone"> + <h1>[% loc('Two-factor authentication') %]</h1> + + <form action="[% c.uri_for_action('/auth/profile/generate_token') %]" method="post" name="generate_token"> + <input type="hidden" name="token" value="[% csrf_token %]"> + [% PROCESS 'auth/2fa/form-add.html' %] + </form> + [% ELSE %] <h1>[% loc('Security') %]</h1> <form action="[% c.uri_for_action('/auth/profile/generate_token') %]" method="post" name="generate_token"> + +<h2>[% loc('Two-factor authentication') %]</h2> + + <input type="hidden" name="token" value="[% csrf_token %]"> + + <p> + [% IF c.user.is_superuser || c.user.from_body %] + [% IF has_2fa %] + <input name="2fa_activate" type="submit" class="btn" value="[% loc('Change two-factor authentication') %]"> + [% IF !c.cobrand.call_hook('must_have_2fa', c.user) %] + <input name="2fa_deactivate" type="submit" class="btn" value="[% loc('Deactivate two-factor authentication') %]"> + [% END %] + [% ELSE %] + <input name="2fa_activate" type="submit" class="btn" value="[% loc('Activate two-factor authentication') %]"> + [% END %] + [% END %] + </p> + +<h2>[% loc('Token') %]</h2> + <input type="hidden" name="token" value="[% csrf_token %]"> [% IF existing_token %] @@ -50,17 +76,15 @@ 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 %] - <input name="toggle_2fa" type="submit" class="btn" value="[% has_2fa ? loc('Deactivate two-factor authentication') : loc('Activate two-factor authentication') %]"> - [% END %] </p> -</form> [% IF existing_token %] <p> [% loc('If you generate a new token the existing token will no longer work.') %] </p> [% END %] + +</form> [% END %] [% INCLUDE 'footer.html' %] diff --git a/templates/web/oxfordshire/admin/user-form-extra-fields.html b/templates/web/oxfordshire/admin/user-form-extra-fields.html index 4109a3075..c7697fa35 100644 --- a/templates/web/oxfordshire/admin/user-form-extra-fields.html +++ b/templates/web/oxfordshire/admin/user-form-extra-fields.html @@ -1,15 +1,6 @@ <li> <div class="admin-hint"> <p> - [% loc("Reports from users with high enough reputation will be sent immediately without requiring inspection. Each category's threshold can be managed on its edit page. Users earn reputation when a report they have made is marked as inspected by inspectors.") %] - </p> - </div> - [% loc('Reputation:') %] [% user.get_extra_metadata('reputation') %] -</li> - -<li> - <div class="admin-hint"> - <p> [% loc( "The user's initials are used when sending inspections to Exor. Only inspectors need to have this field filled in.") |