Bugzilla – Attachment 186542 Details for
Bug 20638
Add audit logging for API key actions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20638: Update apikeys.pl and template with proper exception handling
Bug-20638-Update-apikeyspl-and-template-with-prope.patch (text/plain), 7.01 KB, created by
Tomás Cohen Arazi (tcohen)
on 2025-09-17 17:49:43 UTC
(
hide
)
Description:
Bug 20638: Update apikeys.pl and template with proper exception handling
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2025-09-17 17:49:43 UTC
Size:
7.01 KB
patch
obsolete
>From ff28e88a3df6e5cc8ac47623b3663d5ca1b4416c Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= <tomascohen@theke.io> >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 @@ > > <h1>API keys for [% INCLUDE 'patron-title.inc' %]</h1> > >+ [% FOREACH m IN messages %] >+ <div class="alert alert-[% m.type | html %]"> >+ [% SWITCH m.code %] >+ [% CASE 'already_revoked' %] >+ <span>This API key is already revoked.</span> >+ [% CASE 'already_active' %] >+ <span>This API key is already active.</span> >+ [% CASE 'key_not_found' %] >+ <span>The specified API key was not found.</span> >+ [% CASE 'unhandled_exception' %] >+ <span>An unexpected error occurred while processing your request.</span> >+ [% IF m.error_string %] >+ <p><strong>Error details:</strong> [% m.error_string | html %]</p> >+ [% END %] >+ [% CASE %] >+ <span>[% m.code | html %]</span> >+ [% END %] >+ </div> >+ [% END %] >+ > [% IF fresh_api_key %] > [%# A fresh api key was generated, display the secret, only once %] > <div class="alert alert-info">Make sure to copy your API secret now. You wonât be able to see it again!</div> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20638
:
186540
|
186541
| 186542 |
186543
|
186544