From 0d40ddc822fcf5936dbdbd6648e0eeb3b6173ecc Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 31 Jul 2025 11:11:43 -0300 Subject: [PATCH] Bug 30724: Allow superlibrarians to manage 2FA for other patrons This enhances the two-factor authentication management interface to allow superlibrarians to disable 2FA for other patrons when they lose access to their authenticator device. Controller changes (members/two_factor_auth.pl): * Accept borrowernumber parameter to select target patron * Implement authorization checks (self-service or superlibrarian) * Use proper HTTP status codes (403/404) for error conditions * Update all patron operations to use selected patron * Integrate with new reset_2fa() method for consistency * Pass another_user flag to template for conditional display Template changes (two_factor_auth.tt): * Use has_2fa_enabled() method instead of auth_method string comparison * Prevent superlibrarians from enabling 2FA for other users * Show explanatory message when 2FA setup is restricted * Maintain proper conditional display logic UI Integration changes: * Update members toolbar to show 2FA option for superlibrarians * Pass borrowernumber parameter in all 2FA-related URLs * Maintain context when canceling 2FA registration * Use consistent parameter naming (borrowernumber vs patron_id) To test: 1. Enable TwoFactorAuthentication system preference 2. Set up 2FA for a test patron 3. As superlibrarian, visit patron details page 4. Click 'Manage two-factor authentication' in toolbar => SUCCESS: 2FA management page loads for the selected patron 5. Disable 2FA for the patron => SUCCESS: 2FA is disabled for the target patron, not the superlibrarian 6. Verify 'Enable 2FA' button is hidden with explanatory text => SUCCESS: Shows message that users must enable 2FA themselves 7. Test authorization: try accessing as non-superlibrarian for different patron => FAIL: Returns 403 Forbidden error 8. Sign off :-D --- .../prog/en/includes/auth-two-factor.inc | 2 +- .../prog/en/includes/members-toolbar.inc | 4 +- .../en/modules/members/two_factor_auth.tt | 16 +++--- members/two_factor_auth.pl | 49 ++++++++++++------- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/auth-two-factor.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/auth-two-factor.inc index f7fe863c609..767f6c23556 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/auth-two-factor.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/auth-two-factor.inc @@ -26,7 +26,7 @@
- Cancel + Cancel
[% END %] 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 c40f34ebceb..0d8499a350f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc @@ -93,8 +93,8 @@ Set permissions [% END %] - [% IF ( Koha.Preference('TwoFactorAuthentication') == 'enforced' || Koha.Preference('TwoFactorAuthentication') == 'enabled' ) && logged_in_user.borrowernumber == patron.borrowernumber %] -
  • Manage two-factor authentication
  • + [% IF ( Koha.Preference('TwoFactorAuthentication') == 'enforced' || Koha.Preference('TwoFactorAuthentication') == 'enabled' ) && ( logged_in_user.borrowernumber == patron.borrowernumber || logged_in_user.is_superlibrarian ) %] +
  • Manage two-factor authentication
  • [% END %] [% IF CAN_user_borrowers_edit_borrowers && useDischarge %] 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 3359686effb..bbdcde441b1 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 @@ -46,7 +46,7 @@ [% PROCESS registration_form %]
    -
    +
    Status: Enabled
    @@ -56,13 +56,17 @@
    -
    +
    Status: Disabled
    - [% IF Koha.Preference('TwoFactorAuthentication') == 'enforced' %] -
    Two-factor authentication is mandatory to login. If you do not enable now it will be asked at your next login.
    - [% END %] + [% IF another_user %] +
    You cannot enable two-factor authentication for a user. They need to do it themselves.
    + [% ELSE %] + [% IF Koha.Preference('TwoFactorAuthentication') == 'enforced' %] +
    Two-factor authentication is mandatory to login. If you do not enable now it will be asked at your next login.
    + [% END %] - + + [% END %]
    [% END %] diff --git a/members/two_factor_auth.pl b/members/two_factor_auth.pl index 303440d9ae3..3c980e10f85 100755 --- a/members/two_factor_auth.pl +++ b/members/two_factor_auth.pl @@ -42,33 +42,47 @@ if ( $TwoFactorAuthentication ne 'enabled' && $TwoFactorAuthentication ne 'enfor exit; } +my $op = $cgi->param('op') // ''; +my $patron_id = $cgi->param('borrowernumber') // ''; + +my $another_user = $patron_id ne $loggedinuser; + my $logged_in_user = Koha::Patrons->find($loggedinuser); -my $op = $cgi->param('op') // ''; + +unless ( !$another_user || $logged_in_user->is_superlibrarian() ) { + print $cgi->redirect("/cgi-bin/koha/errors/403.pl"); + exit; +} + +my $patron = + $another_user + ? Koha::Patrons->find($patron_id) + : $logged_in_user; + +if ( !$patron ) { + print $cgi->redirect("/cgi-bin/koha/errors/404.pl"); + exit; +} 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 'cud-disable-2FA' ) { - 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( + + $patron->reset_2fa(); + + if ( $patron->notice_email_address ) { + $patron->queue_notice( { letter_params => { module => 'members', letter_code => '2FA_DISABLE', - branchcode => $logged_in_user->branchcode, - lang => $logged_in_user->lang, + branchcode => $patron->branchcode, + lang => $patron->lang, tables => { - branches => $logged_in_user->branchcode, - borrowers => $logged_in_user->id + branches => $patron->branchcode, + borrowers => $patron->id }, }, message_transports => ['email'], @@ -79,8 +93,9 @@ if ( !C4::Context->config('encryption_key') ) { } $template->param( - patron => $logged_in_user, - op => $op, + another_user => $another_user, + op => $op, + patron => $patron, ); output_html_with_http_headers $cgi, $cookie, $template->output; -- 2.50.1