Bug 20638 - Add audit logging for API key actions
Summary: Add audit logging for API key actions
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords: release-notes-needed
Depends on: 20568
Blocks: 22794
  Show dependency treegraph
 
Reported: 2018-04-23 14:05 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-11-07 16:00 UTC (History)
10 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 20638: Add ApiKeyLog system preference (4.37 KB, patch)
2025-09-17 17:49 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey (14.25 KB, patch)
2025-09-17 17:49 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Update apikeys.pl and template with proper exception handling (7.01 KB, patch)
2025-09-17 17:49 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add APIKEYS support to log viewer (10.58 KB, patch)
2025-09-17 17:49 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Include APIKEYS in 'Modification log' (1.29 KB, patch)
2025-09-17 17:49 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add ApiKeyLog system preference (4.43 KB, patch)
2025-09-18 13:26 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey (14.32 KB, patch)
2025-09-18 13:26 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 20638: Update apikeys.pl and template with proper exception handling (7.07 KB, patch)
2025-09-18 13:26 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 20638: Add APIKEYS support to log viewer (10.64 KB, patch)
2025-09-18 13:27 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 20638: Include APIKEYS in 'Modification log' (1.35 KB, patch)
2025-09-18 13:27 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 20638: Add ApiKeyLog system preference (4.37 KB, patch)
2025-09-18 19:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey (14.25 KB, patch)
2025-09-18 19:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Update apikeys.pl and template with proper exception handling (7.01 KB, patch)
2025-09-18 19:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add APIKEYS support to log viewer (10.58 KB, patch)
2025-09-18 19:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Include APIKEYS in 'Modification log' (1.29 KB, patch)
2025-09-18 19:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Improve ApiKey logging to use structured data (5.81 KB, patch)
2025-09-18 19:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add ApiKeyLog system preference (4.43 KB, patch)
2025-09-18 19:27 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey (14.31 KB, patch)
2025-09-18 19:27 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Update apikeys.pl and template with proper exception handling (7.07 KB, patch)
2025-09-18 19:27 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add APIKEYS support to log viewer (10.64 KB, patch)
2025-09-18 19:27 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Include APIKEYS in 'Modification log' (1.35 KB, patch)
2025-09-18 19:27 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Improve ApiKey logging to use structured data (5.87 KB, patch)
2025-09-18 19:28 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add ApiKeyLog system preference (3.90 KB, patch)
2025-09-26 18:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey (14.31 KB, patch)
2025-09-26 18:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Update apikeys.pl and template with proper exception handling (7.07 KB, patch)
2025-09-26 18:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add APIKEYS support to log viewer (10.64 KB, patch)
2025-09-26 18:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Include APIKEYS in 'Modification log' (1.35 KB, patch)
2025-09-26 18:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Improve ApiKey logging to use structured data (7.08 KB, patch)
2025-09-26 18:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: (follow-up) Silence viewlog.pl warning (2.15 KB, patch)
2025-09-26 18:48 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 20638: Add ApiKeyLog system preference (3.96 KB, patch)
2025-09-26 20:58 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey (14.38 KB, patch)
2025-09-26 20:59 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20638: Update apikeys.pl and template with proper exception handling (7.13 KB, patch)
2025-09-26 20:59 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20638: Add APIKEYS support to log viewer (10.70 KB, patch)
2025-09-26 20:59 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20638: Include APIKEYS in 'Modification log' (1.41 KB, patch)
2025-09-26 20:59 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20638: Improve ApiKey logging to use structured data (7.14 KB, patch)
2025-09-26 20:59 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20638: (follow-up) Silence viewlog.pl warning (2.21 KB, patch)
2025-09-26 20:59 UTC, Lisette Scheer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2018-04-23 14:05:23 UTC
Actions on API keys shoudl be logged and auditable (i.e. who creates the API keys, who revokes, etc).
Comment 1 Tomás Cohen Arazi (tcohen) 2025-09-17 17:49:36 UTC
Created attachment 186540 [details] [review]
Bug 20638: Add ApiKeyLog system preference

This patch adds the ApiKeyLog system preference to control logging
of API key actions (creation, deletion, revocation, activation).

The preference follows the same pattern as BorrowersLog and other
logging preferences.

Test plan:
1. Apply patch and run database update
2. Go to Administration > System preferences > Logs
3. Verify ApiKeyLog preference appears with Yes/No options
4. Verify default value is 'Don't log' (0)
Comment 2 Tomás Cohen Arazi (tcohen) 2025-09-17 17:49:40 UTC
Created attachment 186541 [details] [review]
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey

This patch adds two new methods to Koha::ApiKey:
- revoke(): Sets active to 0, throws exception if already revoked
- activate(): Sets active to 1, throws exception if already active

Both methods include proper exception handling and comprehensive tests.

Test plan:
1. prove t/db_dependent/Koha/ApiKey.t
2. Verify new subtests pass for revoke() and activate() methods
3. Confirm exceptions are thrown for invalid state transitions
Comment 3 Tomás Cohen Arazi (tcohen) 2025-09-17 17:49:43 UTC
Created attachment 186542 [details] [review]
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
Comment 4 Tomás Cohen Arazi (tcohen) 2025-09-17 17:49:46 UTC
Created attachment 186543 [details] [review]
Bug 20638: Add APIKEYS support to log viewer

Add comprehensive support for viewing API key logs in the log viewer:

tools/viewlog.pl:
- Add APIKEYS to patron lookup logic alongside MEMBERS, CIRCULATION, FINES
- Enable patron name display for API key log entries

koha-tmpl/.../viewlog.tt:
- Add APIKEYS to the modules list for filtering
- Add ACTIVATE and REVOKE to the actions list for filtering
- Include APIKEYS in patron display logic for object column

koha-tmpl/.../action-logs.inc:
- Add APIKEYS module translation: 'API keys'
- Include ApiKeyLog system preference check with warning icon
- Add ACTIVATE and REVOKE action translations
- Maintain alphabetical order in translation blocks

This enables librarians to view and filter API key creation, activation,
revocation, and deletion activities in the standard log viewer interface,
with proper patron name resolution and preference-based logging status
indicators.
Comment 5 Tomás Cohen Arazi (tcohen) 2025-09-17 17:49:50 UTC
Created attachment 186544 [details] [review]
Bug 20638: Include APIKEYS in 'Modification log'
Comment 6 Paul Derscheid 2025-09-18 13:26:55 UTC
Created attachment 186558 [details] [review]
Bug 20638: Add ApiKeyLog system preference

This patch adds the ApiKeyLog system preference to control logging
of API key actions (creation, deletion, revocation, activation).

The preference follows the same pattern as BorrowersLog and other
logging preferences.

Test plan:
1. Apply patch and run database update
2. Go to Administration > System preferences > Logs
3. Verify ApiKeyLog preference appears with Yes/No options
4. Verify default value is 'Don't log' (0)

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 7 Paul Derscheid 2025-09-18 13:26:57 UTC
Created attachment 186559 [details] [review]
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey

This patch adds two new methods to Koha::ApiKey:
- revoke(): Sets active to 0, throws exception if already revoked
- activate(): Sets active to 1, throws exception if already active

Both methods include proper exception handling and comprehensive tests.

Test plan:
1. prove t/db_dependent/Koha/ApiKey.t
2. Verify new subtests pass for revoke() and activate() methods
3. Confirm exceptions are thrown for invalid state transitions

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 8 Paul Derscheid 2025-09-18 13:26:59 UTC
Created attachment 186560 [details] [review]
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>
Comment 9 Paul Derscheid 2025-09-18 13:27:02 UTC
Created attachment 186561 [details] [review]
Bug 20638: Add APIKEYS support to log viewer

Add comprehensive support for viewing API key logs in the log viewer:

tools/viewlog.pl:
- Add APIKEYS to patron lookup logic alongside MEMBERS, CIRCULATION, FINES
- Enable patron name display for API key log entries

koha-tmpl/.../viewlog.tt:
- Add APIKEYS to the modules list for filtering
- Add ACTIVATE and REVOKE to the actions list for filtering
- Include APIKEYS in patron display logic for object column

koha-tmpl/.../action-logs.inc:
- Add APIKEYS module translation: 'API keys'
- Include ApiKeyLog system preference check with warning icon
- Add ACTIVATE and REVOKE action translations
- Maintain alphabetical order in translation blocks

This enables librarians to view and filter API key creation, activation,
revocation, and deletion activities in the standard log viewer interface,
with proper patron name resolution and preference-based logging status
indicators.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 10 Paul Derscheid 2025-09-18 13:27:04 UTC
Created attachment 186562 [details] [review]
Bug 20638: Include APIKEYS in 'Modification log'

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 11 Paul Derscheid 2025-09-18 13:28:02 UTC
Hi Tomás, this is awesome!

qa script complains, though:
qa
Tag v25.05.00-1 found in /kohadevbox/koha — checking out branch main in /kohadevbox/qa-test-tools
Already on 'main'
Your branch is up to date with 'origin/main'.
testing 5 commit(s) (applied to 1516f22 'f9c Bug 40665: Fix Cypress tests')

Processing files
[RUNNING] Koha/ApiKey.pm
[RUNNING] Koha/Exceptions/ApiKey.pm
[RUNNING] installer/data/mysql/atomicupdate/bug_20638.pl
[RUNNING] installer/data/mysql/mandatory/sysprefs.sql
[FAIL] installer/data/mysql/mandatory/sysprefs.sql
[RUNNING] koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc
[SKIP] Koha/Exceptions/ApiKey.pm
[FAIL] installer/data/mysql/atomicupdate/bug_20638.pl
[PASS] Koha/ApiKey.pm
[RUNNING] koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
[RUNNING] koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref
[RUNNING] koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt
[PASS] koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref
[PASS] koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc
[RUNNING] koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
[RUNNING] members/apikeys.pl
[PASS] koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
[RUNNING] t/db_dependent/Koha/ApiKey.t
[PASS] koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
[PASS] members/apikeys.pl
[PASS] koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt
[RUNNING] tools/viewlog.pl
[PASS] t/db_dependent/Koha/ApiKey.t
[PASS] tools/viewlog.pl

Result:
[PASS] Koha/ApiKey.pm
[SKIP] Koha/Exceptions/ApiKey.pm
   SKIP   pod_coverage
                No POD exists
[FAIL] installer/data/mysql/atomicupdate/bug_20638.pl
   FAIL   forbidden_patterns
                forbidden pattern: Have you considered sharing the new syspref to HEA? (see bug 23849 and C4/UsageStats.pm) (line 13)
[FAIL] installer/data/mysql/mandatory/sysprefs.sql
   FAIL   semicolon
                semicolon found instead of comma at line 896
[PASS] koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc
[PASS] koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
[PASS] koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref
[PASS] koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt
[PASS] koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
[PASS] members/apikeys.pl
[PASS] t/db_dependent/Koha/ApiKey.t
[PASS] tools/viewlog.pl

Processing additional checks PASS!


        * Was this bug sponsored by anyone?

Running tests (1)
        * Proving /kohadevbox/koha/t/db_dependent/Koha/ApiKey.t PASS
Comment 13 Tomás Cohen Arazi (tcohen) 2025-09-18 14:13:49 UTC
(In reply to Paul Derscheid from comment #11)
> Hi Tomás, this is awesome!
> 
> qa script complains, though

They all seem wrong checks.

* Exceptions file contains POD, this check always fails. I think it is because our POD describes the various exceptions, which are defined inside the `use` statement. And there are no methods defined so... we might need to fix the QA tools or just not care about this particular error.
* The semi-colon one I don't understand it. It is probably another QA script bug.
* The HEA one I don't really understand it. I mentioned on the bug that it needs to be revisited. Probably as a warning and with different wording.

(In reply to Lisette Scheer from comment #12)
> Additionally new logs need to use the diff
> https://wiki.koha-community.org/wiki/Coding_Guidelines#ACTN1:
> _New_additions_to_actions_logs_should_use_the_JSON_Diff_format

Thanks for this one. I really didn't know about it.
Comment 14 Tomás Cohen Arazi (tcohen) 2025-09-18 19:24:12 UTC Comment hidden (obsolete)
Comment 15 Tomás Cohen Arazi (tcohen) 2025-09-18 19:24:15 UTC Comment hidden (obsolete)
Comment 16 Tomás Cohen Arazi (tcohen) 2025-09-18 19:24:18 UTC Comment hidden (obsolete)
Comment 17 Tomás Cohen Arazi (tcohen) 2025-09-18 19:24:22 UTC Comment hidden (obsolete)
Comment 18 Tomás Cohen Arazi (tcohen) 2025-09-18 19:24:25 UTC Comment hidden (obsolete)
Comment 19 Tomás Cohen Arazi (tcohen) 2025-09-18 19:24:28 UTC Comment hidden (obsolete)
Comment 20 Tomás Cohen Arazi (tcohen) 2025-09-18 19:27:31 UTC
Created attachment 186583 [details] [review]
Bug 20638: Add ApiKeyLog system preference

This patch adds the ApiKeyLog system preference to control logging
of API key actions (creation, deletion, revocation, activation).

The preference follows the same pattern as BorrowersLog and other
logging preferences.

Test plan:
1. Apply patch and run database update
2. Go to Administration > System preferences > Logs
3. Verify ApiKeyLog preference appears with Yes/No options
4. Verify default value is 'Don't log' (0)

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 21 Tomás Cohen Arazi (tcohen) 2025-09-18 19:27:35 UTC
Created attachment 186584 [details] [review]
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey

This patch adds two new methods to Koha::ApiKey:
- revoke(): Sets active to 0, throws exception if already revoked
- activate(): Sets active to 1, throws exception if already active

Both methods include proper exception handling and comprehensive tests.

Test plan:
1. prove t/db_dependent/Koha/ApiKey.t
2. Verify new subtests pass for revoke() and activate() methods
3. Confirm exceptions are thrown for invalid state transitions

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 22 Tomás Cohen Arazi (tcohen) 2025-09-18 19:27:39 UTC
Created attachment 186585 [details] [review]
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>
Comment 23 Tomás Cohen Arazi (tcohen) 2025-09-18 19:27:47 UTC
Created attachment 186586 [details] [review]
Bug 20638: Add APIKEYS support to log viewer

Add comprehensive support for viewing API key logs in the log viewer:

tools/viewlog.pl:
- Add APIKEYS to patron lookup logic alongside MEMBERS, CIRCULATION, FINES
- Enable patron name display for API key log entries

koha-tmpl/.../viewlog.tt:
- Add APIKEYS to the modules list for filtering
- Add ACTIVATE and REVOKE to the actions list for filtering
- Include APIKEYS in patron display logic for object column

koha-tmpl/.../action-logs.inc:
- Add APIKEYS module translation: 'API keys'
- Include ApiKeyLog system preference check with warning icon
- Add ACTIVATE and REVOKE action translations
- Maintain alphabetical order in translation blocks

This enables librarians to view and filter API key creation, activation,
revocation, and deletion activities in the standard log viewer interface,
with proper patron name resolution and preference-based logging status
indicators.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 24 Tomás Cohen Arazi (tcohen) 2025-09-18 19:27:52 UTC
Created attachment 186587 [details] [review]
Bug 20638: Include APIKEYS in 'Modification log'

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 25 Tomás Cohen Arazi (tcohen) 2025-09-18 19:28:01 UTC
Created attachment 186588 [details] [review]
Bug 20638: Improve ApiKey logging to use structured data

This patch enhances the ApiKey logging mechanism to pass structured
object data instead of formatted strings to logaction(), following
the ACTN1 coding guideline for action logs.

Changes:
- Pass the ApiKey object directly to logaction() for CREATE/REVOKE/ACTIVATE
- Pass undef for info and the object in diff field for DELETE operations
- Refactor store/revoke/activate methods to improve logging consistency
- Update tests to handle the new structured logging format

The new approach provides richer, more structured log data that can be
better utilized by log analysis tools and maintains consistency with
other Koha object logging patterns.

Test plan:
1. Enable ApiKeyLog system preference
2. Create, revoke, activate, and delete API keys
3. Verify action_logs table contains structured object data
4. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/ApiKey.t
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 26 Lisette Scheer 2025-09-26 17:01:57 UTC
The info doesn't seem to be accurate, it's showing what it was before instead of what it is now (not sure if that's expected) and the just shows {}

Create "$VAR1 = { 'active' => 1,"
Revoke "$VAR1 = { 'active' => 1,"
Activate" $VAR1 = { 'active' => 0,"

Cataloging log shoes before, but it shares that it's before:
 	
biblio BEFORE=>

Patron log also shows before in the info
{ "middle_name" : { "after" : "middle name", "before" : null } } 

There's also some discussion on Bug 20956 about if the example in the ACTN1 is the way we should be doing this.
Comment 27 Tomás Cohen Arazi (tcohen) 2025-09-26 18:43:06 UTC
Created attachment 187004 [details] [review]
Bug 20638: Add ApiKeyLog system preference

This patch adds the ApiKeyLog system preference to control logging
of API key actions (creation, deletion, revocation, activation).

The preference follows the same pattern as BorrowersLog and other
logging preferences.

Test plan:
1. Apply patch and run database update
2. Go to Administration > System preferences > Logs
3. Verify ApiKeyLog preference appears with Yes/No options
4. Verify default value is 'Don't log' (0)

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 28 Tomás Cohen Arazi (tcohen) 2025-09-26 18:43:09 UTC
Created attachment 187005 [details] [review]
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey

This patch adds two new methods to Koha::ApiKey:
- revoke(): Sets active to 0, throws exception if already revoked
- activate(): Sets active to 1, throws exception if already active

Both methods include proper exception handling and comprehensive tests.

Test plan:
1. prove t/db_dependent/Koha/ApiKey.t
2. Verify new subtests pass for revoke() and activate() methods
3. Confirm exceptions are thrown for invalid state transitions

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 29 Tomás Cohen Arazi (tcohen) 2025-09-26 18:43:13 UTC
Created attachment 187006 [details] [review]
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>
Comment 30 Tomás Cohen Arazi (tcohen) 2025-09-26 18:43:17 UTC
Created attachment 187007 [details] [review]
Bug 20638: Add APIKEYS support to log viewer

Add comprehensive support for viewing API key logs in the log viewer:

tools/viewlog.pl:
- Add APIKEYS to patron lookup logic alongside MEMBERS, CIRCULATION, FINES
- Enable patron name display for API key log entries

koha-tmpl/.../viewlog.tt:
- Add APIKEYS to the modules list for filtering
- Add ACTIVATE and REVOKE to the actions list for filtering
- Include APIKEYS in patron display logic for object column

koha-tmpl/.../action-logs.inc:
- Add APIKEYS module translation: 'API keys'
- Include ApiKeyLog system preference check with warning icon
- Add ACTIVATE and REVOKE action translations
- Maintain alphabetical order in translation blocks

This enables librarians to view and filter API key creation, activation,
revocation, and deletion activities in the standard log viewer interface,
with proper patron name resolution and preference-based logging status
indicators.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 31 Tomás Cohen Arazi (tcohen) 2025-09-26 18:43:20 UTC
Created attachment 187008 [details] [review]
Bug 20638: Include APIKEYS in 'Modification log'

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 32 Tomás Cohen Arazi (tcohen) 2025-09-26 18:43:24 UTC
Created attachment 187009 [details] [review]
Bug 20638: Improve ApiKey logging to use structured data

This patch enhances the ApiKey logging mechanism to pass structured
object data instead of formatted strings to logaction(), following
the ACTN1 coding guideline for action logs.

Changes:
- Pass the ApiKey object directly to logaction() for CREATE/REVOKE/ACTIVATE
- Pass undef for info and the object in diff field for DELETE operations
- Refactor store/revoke/activate methods to improve logging consistency
- Update tests to handle the new structured logging format

The new approach provides richer, more structured log data that can be
better utilized by log analysis tools and maintains consistency with
other Koha object logging patterns.

Test plan:
1. Enable ApiKeyLog system preference
2. Create, revoke, activate, and delete API keys
3. Verify action_logs table contains structured object data
4. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/ApiKey.t
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 33 Tomás Cohen Arazi (tcohen) 2025-09-26 18:48:53 UTC
Created attachment 187011 [details] [review]
Bug 20638: (follow-up) Silence viewlog.pl warning

This patch fixes useless warnings being printed by `viewlog.pl` I
noticed while testing this new feature.

To test:
1. Test the feature introduced by this patchset
2. Make sure you are running:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
=> FAIL: You see things like:

```
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:43:44] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:43:44] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
```
3. Refresh the viewlog page
=> FAIL: They are consistent
4. Apply this patch
5. Repeat 3
=> SUCCESS: No more warnings!
6. Sign off :-D

Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Comment 34 Tomás Cohen Arazi (tcohen) 2025-09-26 18:54:49 UTC
Thanks Lisette for testing!

I found out the `C4::Log::actionlog` method is actually performing an implicit `$infos->get_from_storage()` which breaks the logic I implemented, as my original implementation was:

1. modify the object ($self)
2. create an unmodified copy of the object ($self->get_from_storage) [1]
3. pass both to `logaction()`
4. call `store()`

So it was always comparing the untouched object with itself, even though I was passing a modified one!

I changed it to match what `actionlog` is designed to, even though I dislike this a lot :-D. About [1] I opted to use `clone` and save a DB query.
Comment 35 Lisette Scheer 2025-09-26 20:58:55 UTC
Created attachment 187020 [details] [review]
Bug 20638: Add ApiKeyLog system preference

This patch adds the ApiKeyLog system preference to control logging
of API key actions (creation, deletion, revocation, activation).

The preference follows the same pattern as BorrowersLog and other
logging preferences.

Test plan:
1. Apply patch and run database update
2. Go to Administration > System preferences > Logs
3. Verify ApiKeyLog preference appears with Yes/No options
4. Verify default value is 'Don't log' (0)

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 36 Lisette Scheer 2025-09-26 20:59:07 UTC
Created attachment 187021 [details] [review]
Bug 20638: Add revoke() and activate() methods to Koha::ApiKey

This patch adds two new methods to Koha::ApiKey:
- revoke(): Sets active to 0, throws exception if already revoked
- activate(): Sets active to 1, throws exception if already active

Both methods include proper exception handling and comprehensive tests.

Test plan:
1. prove t/db_dependent/Koha/ApiKey.t
2. Verify new subtests pass for revoke() and activate() methods
3. Confirm exceptions are thrown for invalid state transitions

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 37 Lisette Scheer 2025-09-26 20:59:17 UTC
Created attachment 187022 [details] [review]
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>
Comment 38 Lisette Scheer 2025-09-26 20:59:25 UTC
Created attachment 187023 [details] [review]
Bug 20638: Add APIKEYS support to log viewer

Add comprehensive support for viewing API key logs in the log viewer:

tools/viewlog.pl:
- Add APIKEYS to patron lookup logic alongside MEMBERS, CIRCULATION, FINES
- Enable patron name display for API key log entries

koha-tmpl/.../viewlog.tt:
- Add APIKEYS to the modules list for filtering
- Add ACTIVATE and REVOKE to the actions list for filtering
- Include APIKEYS in patron display logic for object column

koha-tmpl/.../action-logs.inc:
- Add APIKEYS module translation: 'API keys'
- Include ApiKeyLog system preference check with warning icon
- Add ACTIVATE and REVOKE action translations
- Maintain alphabetical order in translation blocks

This enables librarians to view and filter API key creation, activation,
revocation, and deletion activities in the standard log viewer interface,
with proper patron name resolution and preference-based logging status
indicators.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 39 Lisette Scheer 2025-09-26 20:59:33 UTC
Created attachment 187024 [details] [review]
Bug 20638: Include APIKEYS in 'Modification log'

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 40 Lisette Scheer 2025-09-26 20:59:39 UTC
Created attachment 187025 [details] [review]
Bug 20638: Improve ApiKey logging to use structured data

This patch enhances the ApiKey logging mechanism to pass structured
object data instead of formatted strings to logaction(), following
the ACTN1 coding guideline for action logs.

Changes:
- Pass the ApiKey object directly to logaction() for CREATE/REVOKE/ACTIVATE
- Pass undef for info and the object in diff field for DELETE operations
- Refactor store/revoke/activate methods to improve logging consistency
- Update tests to handle the new structured logging format

The new approach provides richer, more structured log data that can be
better utilized by log analysis tools and maintains consistency with
other Koha object logging patterns.

Test plan:
1. Enable ApiKeyLog system preference
2. Create, revoke, activate, and delete API keys
3. Verify action_logs table contains structured object data
4. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/ApiKey.t
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 41 Lisette Scheer 2025-09-26 20:59:46 UTC
Created attachment 187026 [details] [review]
Bug 20638: (follow-up) Silence viewlog.pl warning

This patch fixes useless warnings being printed by `viewlog.pl` I
noticed while testing this new feature.

To test:
1. Test the feature introduced by this patchset
2. Make sure you are running:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
=> FAIL: You see things like:

```
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:37:39] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:43:44] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
[2025/09/26 18:43:44] [WARN] Use of uninitialized value in substr at /kohadevbox/koha/tools/viewlog.pl line 156.
```
3. Refresh the viewlog page
=> FAIL: They are consistent
4. Apply this patch
5. Repeat 3
=> SUCCESS: No more warnings!
6. Sign off :-D

Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 42 Lucas Gass (lukeg) 2025-10-10 18:28:34 UTC
Sorry, I think I have to ask more an additional external QA stamp here. Or if Paul is willing to trade SO/QA with Lisette that would work.
Comment 43 Lucas Gass (lukeg) 2025-10-10 18:29:50 UTC
(In reply to Lucas Gass (lukeg) from comment #42)
> Sorry, I think I have to ask more an additional external QA stamp here. Or
> if Paul is willing to trade SO/QA with Lisette that would work.

When I say external, I mean I would like a non-ByWater QA here.