From 0af63e9f95bcb107830d822156919af9d154607d Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 22 Apr 2022 10:46:06 +0100 Subject: [PATCH] Bug 28998: (QA follow-up) Improve output on error This patch updates the output_and_exit call to instead pass an error parameter in the template and use it to display the warning instead of the rest of the page content in the 'Manage two-factor authentication' page. This allows for translation and makes the page adhere to normal practices. Signed-off-by: Martin Renvoize --- .../en/modules/members/two_factor_auth.tt | 105 ++++++----- members/two_factor_auth.pl | 166 ++++++++++-------- 2 files changed, 150 insertions(+), 121 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/two_factor_auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/two_factor_auth.tt index feb86b9586..1c2c16cfae 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/two_factor_auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/two_factor_auth.tt @@ -33,60 +33,69 @@ [% INCLUDE 'members-toolbar.inc' %] - [% IF op == 'register' %] -

Register two-factor authenticator

-
-

We recommend cloud-based mobile authenticator apps such as Authy, Duo Mobile, and LastPass. They can restore access if you lose your hardware device.

-

Can't scan the code?

-

To add the entry manually, provide the following details to the application on your phone.

-

Account: [% issuer | html %]

-

Key: [% key_id | html %]

-

Time based: Yes

-
+

Manage two-factor authentication

- [% IF invalid_pin %] -
Invalid pin code
- [% END %] -
-
- - - -
    -
  1. - - -
  2. -
  3. - - -
  4. -
-
-
- - Cancel -
-
+ [% IF ( missing_key ) %] +
+

+ Operation cannot proceed, please define an encryption key in your configuration. +

+
[% ELSE %] -

Two-factor authentication

- [% IF patron.auth_method == "two-factor" %] -
Status: Enabled
+ [% IF op == 'register' %] +
+

We recommend cloud-based mobile authenticator apps such as Authy, Duo Mobile, and LastPass. They can restore access if you lose your hardware device.

+

Can't scan the code?

+

To add the entry manually, provide the following details to the application on your phone.

+

Account: [% issuer | html %]

+

Key: [% key_id | html %]

+

Time based: Yes

+
+ + [% IF invalid_pin %] +
Invalid pin code
+ [% END %]
- - - +
+ + + +
    +
  1. + + +
  2. +
  3. + + +
  4. +
+
+
+ + Cancel +
[% ELSE %] -
Status: Disabled
- -
- - - -
- + [% IF patron.auth_method == "two-factor" %] +
Status: Enabled
+ +
+ + + +
+ [% ELSE %] +
Status: Disabled
+ +
+ + + +
+ + [% END %] [% END %] [% END %] diff --git a/members/two_factor_auth.pl b/members/two_factor_auth.pl index 14040dc1ec..71ca71da57 100755 --- a/members/two_factor_auth.pl +++ b/members/two_factor_auth.pl @@ -42,86 +42,106 @@ unless ( C4::Context->preference('TwoFactorAuthentication') ) { exit; } -output_and_exit( $cgi, $cookie, $template, 'Entry encryption_key is missing in koha-conf.xml' ) - if !C4::Context->config('encryption_key'); - my $logged_in_user = Koha::Patrons->find($loggedinuser); +my $op = $cgi->param('op') // ''; -my $op = $cgi->param('op') // ''; -my $csrf_pars = { - session_id => scalar $cgi->cookie('CGISESSID'), - token => scalar $cgi->param('csrf_token'), -}; - -if ( $op eq 'register-2FA' ) { - output_and_exit( $cgi, $cookie, $template, 'wrong_csrf_token' ) - unless Koha::Token->new->check_csrf($csrf_pars); - - my $pin_code = $cgi->param('pin_code'); - my $secret32 = $cgi->param('secret32'); - my $auth = Koha::Auth::TwoFactorAuth->new( - { patron => $logged_in_user, secret32 => $secret32 } ); - - my $verified = $auth->verify( - $pin_code, - 1, # range - $secret32, - undef, # timestamp (defaults to now) - 30, # interval (default 30) - ); - - if ($verified) { - # FIXME Generate a (new?) secret - $logged_in_user->encode_secret($secret32); - $logged_in_user->auth_method('two-factor')->store; - $op = 'registered'; - if( $logged_in_user->notice_email_address ) { - $logged_in_user->queue_notice({ - letter_params => { - module => 'members', letter_code => '2FA_ENABLE', - branchcode => $logged_in_user->branchcode, lang => $logged_in_user->lang, - tables => { branches => $logged_in_user->branchcode, borrowers => $logged_in_user->id }, - }, - message_transports => [ 'email' ], - }); +if ( !C4::Context->config('encryption_key') ) { + $template->param( missing_key => 1 ); +} +else { + + my $csrf_pars = { + session_id => scalar $cgi->cookie('CGISESSID'), + token => scalar $cgi->param('csrf_token'), + }; + + if ( $op eq 'register-2FA' ) { + output_and_exit( $cgi, $cookie, $template, 'wrong_csrf_token' ) + unless Koha::Token->new->check_csrf($csrf_pars); + + my $pin_code = $cgi->param('pin_code'); + my $secret32 = $cgi->param('secret32'); + my $auth = Koha::Auth::TwoFactorAuth->new( + { patron => $logged_in_user, secret32 => $secret32 } ); + + my $verified = $auth->verify( + $pin_code, + 1, # range + $secret32, + undef, # timestamp (defaults to now) + 30, # interval (default 30) + ); + + if ($verified) { + + # FIXME Generate a (new?) secret + $logged_in_user->encode_secret($secret32); + $logged_in_user->auth_method('two-factor')->store; + $op = 'registered'; + if ( $logged_in_user->notice_email_address ) { + $logged_in_user->queue_notice( + { + letter_params => { + module => 'members', + letter_code => '2FA_ENABLE', + branchcode => $logged_in_user->branchcode, + lang => $logged_in_user->lang, + tables => { + branches => $logged_in_user->branchcode, + borrowers => $logged_in_user->id + }, + }, + message_transports => ['email'], + } + ); + } + } + else { + $template->param( invalid_pin => 1, ); + $op = 'enable-2FA'; } } - else { - $template->param( invalid_pin => 1, ); - $op = 'enable-2FA'; - } -} -if ( $op eq 'enable-2FA' ) { - my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); - my $auth = Koha::Auth::TwoFactorAuth->new( - { patron => $logged_in_user, secret => $secret } ); + if ( $op eq 'enable-2FA' ) { + my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); + my $auth = Koha::Auth::TwoFactorAuth->new( + { patron => $logged_in_user, secret => $secret } ); + + $template->param( + issuer => $auth->issuer, + key_id => $auth->key_id, + qr_code => $auth->qr_code, + secret32 => $auth->secret32, - $template->param( - issuer => $auth->issuer, - key_id => $auth->key_id, - qr_code => $auth->qr_code, - secret32 => $auth->secret32, # IMPORTANT: get secret32 after qr_code call ! - ); - $auth->clear; - $op = 'register'; -} -elsif ( $op eq 'disable-2FA' ) { - output_and_exit( $cgi, $cookie, $template, 'wrong_csrf_token' ) - unless Koha::Token->new->check_csrf($csrf_pars); - my $auth = Koha::Auth::TwoFactorAuth->new({ patron => $logged_in_user }); - $logged_in_user->secret(undef); - $logged_in_user->auth_method('password')->store; - if( $logged_in_user->notice_email_address ) { - $logged_in_user->queue_notice({ - letter_params => { - module => 'members', letter_code => '2FA_DISABLE', - branchcode => $logged_in_user->branchcode, lang => $logged_in_user->lang, - tables => { branches => $logged_in_user->branchcode, borrowers => $logged_in_user->id }, - }, - message_transports => [ 'email' ], - }); + ); + $auth->clear; + $op = 'register'; + } + elsif ( $op eq 'disable-2FA' ) { + output_and_exit( $cgi, $cookie, $template, 'wrong_csrf_token' ) + unless Koha::Token->new->check_csrf($csrf_pars); + my $auth = + Koha::Auth::TwoFactorAuth->new( { patron => $logged_in_user } ); + $logged_in_user->secret(undef); + $logged_in_user->auth_method('password')->store; + if ( $logged_in_user->notice_email_address ) { + $logged_in_user->queue_notice( + { + letter_params => { + module => 'members', + letter_code => '2FA_DISABLE', + branchcode => $logged_in_user->branchcode, + lang => $logged_in_user->lang, + tables => { + branches => $logged_in_user->branchcode, + borrowers => $logged_in_user->id + }, + }, + message_transports => ['email'], + } + ); + } } } -- 2.20.1