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 <paul.derscheid@lmscloud.de>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Tomás Cohen Arazi 2025-09-17 14:11:19 -03:00 committed by Lucas Gass
parent be1c64aa46
commit bc6110ae10
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 66 additions and 29 deletions

View File

@ -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>

View File

@ -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 } );
if ($key) {
$key->active(0);
$key->store;
}
print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id );
exit;
}
if ( $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->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' };
}
} 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 } );
if ($key) {
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;