From ff28e88a3df6e5cc8ac47623b3663d5ca1b4416c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Wed, 17 Sep 2025 14:11:19 -0300 Subject: [PATCH] Bug 20638: Update apikeys.pl and template with proper exception handling This patch comprehensively updates the API keys management interface to use the new revoke() and activate() methods with proper exception handling. Controller changes (members/apikeys.pl): - Add Try::Tiny and Scalar::Util imports for exception handling - Replace direct field manipulation (active(0)/active(1)) with proper method calls (revoke()/activate()) to ensure logging - Wrap method calls in try/catch blocks to handle specific exceptions: * Koha::Exceptions::ApiKey::AlreadyRevoked * Koha::Exceptions::ApiKey::AlreadyActive - Implement standard Koha messages pattern with @messages array - Improve code structure with elsif chain instead of multiple if statements - Add proper error handling for key_not_found cases - Include error_string for unhandled exceptions to aid debugging Template changes (koha-tmpl/.../apikeys.tt): - Add comprehensive error message handling for all exception types: * already_revoked: Clear message for revoked keys * already_active: Clear message for active keys * key_not_found: Message for missing keys * unhandled_exception: Generic message with detailed error string Key benefits: - All API key state changes are now properly logged when ApiKeyLog is enabled - Better user experience with specific, actionable error messages - Robust exception handling prevents crashes and provides debugging info - Improved code maintainability and extensibility --- .../prog/en/modules/members/apikeys.tt | 20 ++++++ members/apikeys.pl | 71 ++++++++++++------- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt index 7f0fcd739e..edfbf2ab2e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt @@ -36,6 +36,26 @@

API keys for [% INCLUDE 'patron-title.inc' %]

+ [% FOREACH m IN messages %] +
+ [% SWITCH m.code %] + [% CASE 'already_revoked' %] + This API key is already revoked. + [% CASE 'already_active' %] + This API key is already active. + [% CASE 'key_not_found' %] + The specified API key was not found. + [% CASE 'unhandled_exception' %] + An unexpected error occurred while processing your request. + [% IF m.error_string %] +

Error details: [% m.error_string | html %]

+ [% END %] + [% CASE %] + [% m.code | html %] + [% END %] +
+ [% END %] + [% IF fresh_api_key %] [%# A fresh api key was generated, display the secret, only once %]
Make sure to copy your API secret now. You won’t be able to see it again!
diff --git a/members/apikeys.pl b/members/apikeys.pl index d1513e09b4..e2f67c27ca 100755 --- a/members/apikeys.pl +++ b/members/apikeys.pl @@ -20,6 +20,8 @@ use Modern::Perl; use CGI; +use Try::Tiny qw( catch try ); +use Scalar::Util qw( blessed ); use C4::Auth qw( get_template_and_user ); use C4::Output qw( output_and_exit output_html_with_http_headers ); @@ -28,6 +30,7 @@ use Koha::ApiKeys; use Koha::Patrons; my $cgi = CGI->new; +my @messages; my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -73,48 +76,62 @@ if ($op) { ); $api_key->store; - $template->param( - fresh_api_key => $api_key, - api_keys => Koha::ApiKeys->search( { patron_id => $patron_id } ), - ); - } - - if ( $op eq 'cud-delete' ) { + $template->param( fresh_api_key => $api_key, ); + } elsif ( $op eq 'cud-delete' ) { my $api_key_id = $cgi->param('key'); - my $key = Koha::ApiKeys->find( { patron_id => $patron_id, client_id => $api_key_id } ); + + my $key = Koha::ApiKeys->find( { patron_id => $patron_id, client_id => $api_key_id } ); + if ($key) { $key->delete; + } else { + push @messages, { type => 'alert', code => 'key_not_found' }; } - print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id ); - exit; - } - - if ( $op eq 'cud-revoke' ) { + $template->param( + key_deleted => 1, + ); + } elsif ( $op eq 'cud-revoke' ) { my $api_key_id = $cgi->param('key'); - my $key = Koha::ApiKeys->find( { patron_id => $patron_id, client_id => $api_key_id } ); + + my $key = Koha::ApiKeys->find( { patron_id => $patron_id, client_id => $api_key_id } ); + if ($key) { - $key->active(0); - $key->store; + try { + $key->revoke(); + } catch { + if ( blessed $_ && $_->isa('Koha::Exceptions::ApiKey::AlreadyRevoked') ) { + push @messages, { type => 'alert', code => 'already_revoked' }; + } else { + push @messages, { type => 'alert', code => 'unhandled_exception', error_string => $_ }; + } + }; + } else { + push @messages, { type => 'alert', code => 'key_not_found' }; } - print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id ); - exit; - } - - if ( $op eq 'cud-activate' ) { + } elsif ( $op eq 'cud-activate' ) { my $api_key_id = $cgi->param('key'); - my $key = Koha::ApiKeys->find( { patron_id => $patron_id, client_id => $api_key_id } ); + + my $key = Koha::ApiKeys->find( { patron_id => $patron_id, client_id => $api_key_id } ); if ($key) { - $key->active(1); - $key->store; + try { + $key->activate(); + } catch { + if ( blessed $_ && $_->isa('Koha::Exceptions::ApiKey::AlreadyActive') ) { + push @messages, { type => 'alert', code => 'already_active' }; + } else { + push @messages, { type => 'alert', code => 'unhandled_exception', error_string => $_ }; + } + }; + } else { + push @messages, { type => 'alert', code => 'key_not_found' }; } - print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id ); - exit; } } $template->param( api_keys => Koha::ApiKeys->search( { patron_id => $patron_id } ), - patron => $patron + patron => $patron, + messages => \@messages ); output_html_with_http_headers $cgi, $cookie, $template->output; -- 2.51.0