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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Martin Renvoize 2022-04-22 10:46:06 +01:00 committed by Fridolin Somers
parent f300a7d363
commit e225ee3b15
2 changed files with 144 additions and 115 deletions

View File

@ -33,60 +33,69 @@
[% INCLUDE 'members-toolbar.inc' %]
[% IF op == 'register' %]
<h1>Register two-factor authenticator</h1>
<div class="dialog message">
<p>We recommend cloud-based mobile authenticator apps such as Authy, Duo Mobile, and LastPass. They can restore access if you lose your hardware device.</p>
<p>Can't scan the code?</p>
<p>To add the entry manually, provide the following details to the application on your phone.</p>
<p>Account: [% issuer | html %]</p>
<p>Key: [% key_id | html %]</p>
<p>Time based: Yes</p>
<h1>Manage two-factor authentication</h1>
[% IF ( missing_key ) %]
<div id="error_message" class="dialog message">
<p>
Operation cannot proceed, please define an encryption key in your configuration.
</p>
</div>
[% IF invalid_pin %]
<div class="dialog error">Invalid pin code</div>
[% END %]
<form id="two-factor-auth" action="/cgi-bin/koha/members/two_factor_auth.pl" method="post">
<fieldset class="rows">
<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
<input type="hidden" name="op" value="register-2FA" />
<input type="hidden" name="secret32" value="[% secret32 | html %]" />
<ol>
<li>
<label for="qr_code">QR code: </label>
<img id="qr_code" src="[% qr_code | $raw %]" />
</li>
<li>
<label for="pin_code">Pin code: </label>
<input type="text" id="pin_code" name="pin_code" value="" />
</li>
</ol>
</fieldset>
<fieldset class="action">
<input type="submit" value="Register with two-factor app" />
<a class="cancel" href="/cgi-bin/koha/members/two_factor_auth.pl">Cancel</a>
</fieldset>
</form>
[% ELSE %]
<h1>Two-factor authentication</h1>
[% IF patron.auth_method == "two-factor" %]
<div class="two-factor-status">Status: Enabled</div>
[% IF op == 'register' %]
<div class="dialog message">
<p>We recommend cloud-based mobile authenticator apps such as Authy, Duo Mobile, and LastPass. They can restore access if you lose your hardware device.</p>
<p>Can't scan the code?</p>
<p>To add the entry manually, provide the following details to the application on your phone.</p>
<p>Account: [% issuer | html %]</p>
<p>Key: [% key_id | html %]</p>
<p>Time based: Yes</p>
</div>
[% IF invalid_pin %]
<div class="dialog error">Invalid pin code</div>
[% END %]
<form id="two-factor-auth" action="/cgi-bin/koha/members/two_factor_auth.pl" method="post">
<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
<input type="hidden" name="op" value="disable-2FA" />
<input type="submit" value="Disable two-factor authentication" />
<fieldset class="rows">
<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
<input type="hidden" name="op" value="register-2FA" />
<input type="hidden" name="secret32" value="[% secret32 | html %]" />
<ol>
<li>
<label for="qr_code">QR code: </label>
<img id="qr_code" src="[% qr_code | $raw %]" />
</li>
<li>
<label for="pin_code">Pin code: </label>
<input type="text" id="pin_code" name="pin_code" value="" />
</li>
</ol>
</fieldset>
<fieldset class="action">
<input type="submit" value="Register with two-factor app" />
<a class="cancel" href="/cgi-bin/koha/members/two_factor_auth.pl">Cancel</a>
</fieldset>
</form>
[% ELSE %]
<div class="two-factor-status">Status: Disabled</div>
[% IF patron.auth_method == "two-factor" %]
<div class="two-factor-status">Status: Enabled</div>
<form id="two-factor-auth" action="/cgi-bin/koha/members/two_factor_auth.pl" method="post">
<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
<input type="hidden" name="op" value="enable-2FA" />
<input type="submit" value="Enable two-factor authentication" />
</form>
<form id="two-factor-auth" action="/cgi-bin/koha/members/two_factor_auth.pl" method="post">
<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
<input type="hidden" name="op" value="disable-2FA" />
<input type="submit" value="Disable two-factor authentication" />
</form>
[% ELSE %]
<div class="two-factor-status">Status: Disabled</div>
<form id="two-factor-auth" action="/cgi-bin/koha/members/two_factor_auth.pl" method="post">
<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
<input type="hidden" name="op" value="enable-2FA" />
<input type="submit" value="Enable two-factor authentication" />
</form>
[% END %]
[% END %]
[% END %]
</main>

View File

@ -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 ( !C4::Context->config('encryption_key') ) {
$template->param( missing_key => 1 );
}
else {
if ( $op eq 'register-2FA' ) {
output_and_exit( $cgi, $cookie, $template, 'wrong_csrf_token' )
unless Koha::Token->new->check_csrf($csrf_pars);
my $csrf_pars = {
session_id => scalar $cgi->cookie('CGISESSID'),
token => scalar $cgi->param('csrf_token'),
};
my $pin_code = $cgi->param('pin_code');
my $secret32 = $cgi->param('secret32');
my $auth = Koha::Auth::TwoFactorAuth->new(
{ patron => $logged_in_user, secret32 => $secret32 } );
if ( $op eq 'register-2FA' ) {
output_and_exit( $cgi, $cookie, $template, 'wrong_csrf_token' )
unless Koha::Token->new->check_csrf($csrf_pars);
my $verified = $auth->verify(
$pin_code,
1, # range
$secret32,
undef, # timestamp (defaults to now)
30, # interval (default 30)
);
my $pin_code = $cgi->param('pin_code');
my $secret32 = $cgi->param('secret32');
my $auth = Koha::Auth::TwoFactorAuth->new(
{ patron => $logged_in_user, secret32 => $secret32 } );
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' ],
});
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'],
}
);
}
}
}