From 241ec3cd9c598db500df44150e2e55d2bc12d85a 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 Signed-off-by: Paul Derscheid --- .../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 7f0fcd739ed..edfbf2ab2ee 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 d1513e09b49..e2f67c27ca9 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.39.5