From 8511750de92e98b53e5bd9c0eaec045deba49016 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 25 Jul 2022 15:58:56 +0200 Subject: [PATCH] Bug 30588: Adjust existing occurrences of TwoFactorAuthentication We need to replace 0 with 'disabled', and 1 with 'enabled' Sponsored-by: Rijksmuseum, Netherlands Signed-off-by: Nick Clemens Signed-off-by: Tomas Cohen Arazi --- C4/Auth.pm | 9 +++++++-- .../intranet-tmpl/prog/en/includes/members-toolbar.inc | 2 +- .../en/modules/admin/preferences/staff_interface.pref | 6 +++--- members/two_factor_auth.pl | 3 ++- t/db_dependent/Auth.t | 6 +++--- t/db_dependent/Koha/Auth/TwoFactorAuth.t | 4 ++-- t/db_dependent/selenium/authentication_2fa.t | 4 ++-- 7 files changed, 20 insertions(+), 14 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index a006baa8d8..a292c36a92 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -816,7 +816,10 @@ sub checkauth { my $session; my $invalid_otp_token; - my $require_2FA = ( C4::Context->preference('TwoFactorAuthentication') && $type ne "opac" ) ? 1 : 0; + my $require_2FA = + ( $type ne "opac" # Only available for the staff interface + && C4::Context->preference('TwoFactorAuthentication') ne "disabled" ) # If "enabled" or "enforced" + ? 1 : 0; # Basic authentication is incompatible with the use of Shibboleth, # as Shibboleth may return REMOTE_USER as a Shibboleth attribute, @@ -1267,7 +1270,9 @@ sub checkauth { # Auth is completed unless an additional auth is needed if ( $require_2FA ) { my $patron = Koha::Patrons->find({userid => $userid}); - if ( $patron->auth_method eq 'two-factor' ) { + if ( C4::Context->preference('TwoFactorAuthentication') eq "enforced" + || $patron->auth_method eq 'two-factor' ) + { # Ask for the OTP token $auth_state = 'additional-auth-needed'; $session->param('waiting-for-2FA', 1); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc index 056d855bd9..b59cdde3ce 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc @@ -56,7 +56,7 @@
  • Set permissions
    • [% END %] - [% IF Koha.Preference('TwoFactorAuthentication') && logged_in_user.borrowernumber == patron.borrowernumber %] + [% IF ( Koha.Preference('TwoFactorAuthentication') == 'enforced' || Koha.Preference('TwoFactorAuthentication') == 'enabled' ) && logged_in_user.borrowernumber == patron.borrowernumber %]
  • Manage two-factor authentication
    • [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_interface.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_interface.pref index 29c51d8d16..0e5bb27f4c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_interface.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_interface.pref @@ -209,7 +209,7 @@ Staff interface: - - pref: TwoFactorAuthentication choices: - "enforce": Enforce - "enable": Enable - "disable": "Don't enable" + "enforced": Enforce + "enabled": Enable + "disabled": "Don't enable" - two-factor authentication (2FA) for staff members. diff --git a/members/two_factor_auth.pl b/members/two_factor_auth.pl index 71ca71da57..3a93732d52 100755 --- a/members/two_factor_auth.pl +++ b/members/two_factor_auth.pl @@ -37,7 +37,8 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); -unless ( C4::Context->preference('TwoFactorAuthentication') ) { +my $TwoFactorAuthentication = C4::Context->preference('TwoFactorAuthentication'); +if ( $TwoFactorAuthentication ne 'enabled' && $TwoFactorAuthentication ne 'enforced' ) { print $cgi->redirect("/cgi-bin/koha/errors/404.pl"); exit; } diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index 50d0f4c79b..e918ef546a 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -190,7 +190,7 @@ subtest 'checkauth() tests' => sub { $logout = 0; } - t::lib::Mocks::mock_preference( 'TwoFactorAuthentication', 0 ); + t::lib::Mocks::mock_preference( 'TwoFactorAuthentication', 'disabled' ); $patron->auth_method('password')->store; ( $userid, $cookie, $sessionID, $flags ) = C4::Auth::checkauth( $cgi, 'authrequired', undef, 'intranet' ); is( $userid, $patron->userid, 'Succesful login' ); @@ -203,7 +203,7 @@ subtest 'checkauth() tests' => sub { is( C4::Auth::get_session($sessionID)->param('waiting-for-2FA'), undef, 'Second auth not required' ); logout($cgi); - t::lib::Mocks::mock_preference( 'TwoFactorAuthentication', 1 ); + t::lib::Mocks::mock_preference( 'TwoFactorAuthentication', 'enabled' ); t::lib::Mocks::mock_config('encryption_key', '1234tH1s=t&st'); $patron->auth_method('password')->store; ( $userid, $cookie, $sessionID, $flags ) = C4::Auth::checkauth( $cgi, 'authrequired', undef, 'intranet' ); @@ -238,7 +238,7 @@ subtest 'checkauth() tests' => sub { is( $userid, $patron->userid, 'Succesful login at the OPAC' ); is( C4::Auth::get_session($sessionID)->param('waiting-for-2FA'), undef, 'No second auth required at the OPAC' ); - t::lib::Mocks::mock_preference( 'TwoFactorAuthentication', 0 ); + t::lib::Mocks::mock_preference( 'TwoFactorAuthentication', 'disabled' ); }; C4::Context->_new_userenv; # For next tests diff --git a/t/db_dependent/Koha/Auth/TwoFactorAuth.t b/t/db_dependent/Koha/Auth/TwoFactorAuth.t index 7dd1a9d2d0..41fed4f41d 100755 --- a/t/db_dependent/Koha/Auth/TwoFactorAuth.t +++ b/t/db_dependent/Koha/Auth/TwoFactorAuth.t @@ -21,7 +21,7 @@ subtest 'new' => sub { plan tests => 10; $schema->storage->txn_begin; - t::lib::Mocks::mock_preference('TwoFactorAuthentication', 1); + t::lib::Mocks::mock_preference('TwoFactorAuthentication', 'enabled'); t::lib::Mocks::mock_config('encryption_key', 'bad_example'); # Trivial test: no patron, no object @@ -63,7 +63,7 @@ subtest 'qr_code' => sub { $schema->storage->txn_begin; - t::lib::Mocks::mock_preference('TwoFactorAuthentication', 1); + t::lib::Mocks::mock_preference('TwoFactorAuthentication', 'enabled'); t::lib::Mocks::mock_config('encryption_key', 'bad_example'); my $patron = $builder->build_object({ class => 'Koha::Patrons' }); $patron->encode_secret('you2wont2guess2it'); # this is base32 btw diff --git a/t/db_dependent/selenium/authentication_2fa.t b/t/db_dependent/selenium/authentication_2fa.t index 7c995d496a..c48b8b1ffa 100755 --- a/t/db_dependent/selenium/authentication_2fa.t +++ b/t/db_dependent/selenium/authentication_2fa.t @@ -55,11 +55,11 @@ SKIP: { fill_login_form($s); like( $driver->get_title, qr(Koha staff interface), 'Patron with flags superlibrarian should be able to login' ); - C4::Context->set_preference('TwoFactorAuthentication', 0); + C4::Context->set_preference('TwoFactorAuthentication', 'disabled'); $driver->get($s->base_url . q|members/two_factor_auth.pl|); like( $driver->get_title, qr(Error 404), 'Must be redirected to 404 is the pref is off' ); - C4::Context->set_preference('TwoFactorAuthentication', 1); + C4::Context->set_preference('TwoFactorAuthentication', 'enabled'); $driver->get($s->base_url . q|members/two_factor_auth.pl|); like( $driver->get_title, qr(Two-factor authentication), 'Must be on the page with the pref on' ); -- 2.39.5