From 891b8fcc1aa9e3da33f39beba52eda402625b77c Mon Sep 17 00:00:00 2001 From: Saiful Amin Date: Thu, 29 Jan 2026 14:14:54 +0000 Subject: [PATCH] Bug 41310: (follow-up) Add ActionLogsEnableIPLogging system preference Update to previous patch: - Add ActionLogsEnableIPLogging system preference with GDPR compliance notice - Add ActionLogsEnableIPLogging toggle in System preferences > Logs > Debugging - Log IP address only if ActionLogsEnableIPLogging is set to 1 - Update kohastructure.sql: add `ip_address` column to action_logs table Test Plan: 1. Apply the patch 2. Run the tests: `prove t/db_dependent/TestIpLogging.t` - Verify that all tests pass 3. Verification with IP Logging Disabled: - Go to Administration > System preferences > Logs > Debugging - Ensure that 'ActionLogsEnableIPLogging' is set to 'No' - Go to Administration > System preferences > Logs > Logging - Toggle any preference, e.g., AcquisitionLog - Connect to the Koha database (koha-mysql kohadev) and run: `SELECT * FROM action_logs ORDER BY action_id DESC LIMIT 1;` - The 'ip_address' column should be NULL 4. Verification with IP Logging Enabled: - Go to Administration > System preferences > Logs > Debugging - Ensure that 'ActionLogsEnableIPLogging' is set to 'Yes' - Go to Administration > System preferences > Logs > Logging - Toggle any preference, e.g., AcquisitionLog - Connect to the Koha database (koha-mysql kohadev) and run: `SELECT * FROM action_logs ORDER BY action_id DESC LIMIT 1;` - The 'ip_address' column should contain a valid IP address 5. Sign off Future improvement: Add the IP Address column to Tools > Log viewer --- C4/Log.pm | 27 +++++---- Koha/Schema/Result/ActionLog.pm | 10 ++++ installer/data/mysql/db_revs/bug_41310.pl | 45 ++++++++++++++ installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/mandatory/sysprefs.sql | 1 + .../en/modules/admin/preferences/logs.pref | 7 +++ t/db_dependent/TestIpLogging.t | 58 +++++++++++++++++++ 7 files changed, 139 insertions(+), 10 deletions(-) create mode 100644 installer/data/mysql/db_revs/bug_41310.pl create mode 100644 t/db_dependent/TestIpLogging.t diff --git a/C4/Log.pm b/C4/Log.pm index 603bc8695a..dfcf27ada8 100644 --- a/C4/Log.pm +++ b/C4/Log.pm @@ -140,18 +140,25 @@ sub logaction { $diff //= encode_json( diff( $original, $updated, noU => 1 ) ) if $original && ref $updated eq 'HASH'; + # Get the IP address from the environment if IP logging is enabled + my $ip_address = undef; + if ( C4::Context->preference('ActionLogsEnableIPLogging') ) { + $ip_address = $ENV{REMOTE_ADDR} || undef; + } + Koha::ActionLog->new( { - timestamp => \'NOW()', - user => $usernumber, - module => $modulename, - action => $actionname, - object => $objectnumber, - info => $infos, - interface => $interface, - script => $script, - trace => $trace, - diff => $diff, + timestamp => \'NOW()', + user => $usernumber, + module => $modulename, + action => $actionname, + object => $objectnumber, + info => $infos, + interface => $interface, + script => $script, + trace => $trace, + diff => $diff, + ip_address => $ip_address, } )->store(); diff --git a/Koha/Schema/Result/ActionLog.pm b/Koha/Schema/Result/ActionLog.pm index c97ffdcf21..82ff990004 100644 --- a/Koha/Schema/Result/ActionLog.pm +++ b/Koha/Schema/Result/ActionLog.pm @@ -106,6 +106,14 @@ An optional stack trace enabled by ActionLogsTraceDepth Stores a diff of the changed object +=head2 ip_address + + data_type: 'varchar' + is_nullable: 1 + size: 45 + +IP address of the client that performed the action + =cut __PACKAGE__->add_columns( @@ -136,6 +144,8 @@ __PACKAGE__->add_columns( { data_type => "text", is_nullable => 1 }, "diff", { data_type => "longtext", is_nullable => 1 }, + "ip_address", + { data_type => "varchar", is_nullable => 1, size => 45 }, ); =head1 PRIMARY KEY diff --git a/installer/data/mysql/db_revs/bug_41310.pl b/installer/data/mysql/db_revs/bug_41310.pl new file mode 100644 index 0000000000..62f8cd53f8 --- /dev/null +++ b/installer/data/mysql/db_revs/bug_41310.pl @@ -0,0 +1,45 @@ +#!/usr/bin/perl + +use Modern::Perl; + +use C4::Context; +use Koha::Database; + +use base 'C4::Installer::DBUpgrade'; + +our $db_rev = '41310'; + +sub db_rev { + return $db_rev; +} + +sub upgrade { + my ($args) = @_; + my $upgrade_context = $args->{upgrade_context}; + my $schema = $upgrade_context->schema; + my $dbh = $upgrade_context->dbh; + + unless ( column_exists( 'action_logs', 'ip_address' ) ) { + $dbh->do( + q{ + ALTER TABLE action_logs + ADD COLUMN ip_address VARCHAR(45) NULL DEFAULT NULL + COMMENT 'IP address of the client that performed the action' + } + ); + + say $upgrade_context->output("Added column 'action_logs.ip_address'"); + } + + $dbh->do( + q{ + INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES + ('ActionLogsEnableIPLogging', '0', NULL, 'If enabled, store the IP address of the client in action logs for security auditing.', 'YesNo') + } + ); + say $upgrade_context->output("Added new system preference 'ActionLogsEnableIPLogging'"); + + return 1; +} + +1; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 1222ef6387..bf278db8da 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -180,6 +180,7 @@ CREATE TABLE `action_logs` ( `script` varchar(255) DEFAULT NULL COMMENT 'the name of the cron script that caused this change', `trace` text DEFAULT NULL COMMENT 'An optional stack trace enabled by ActionLogsTraceDepth', `diff` longtext DEFAULT NULL COMMENT 'Stores a diff of the changed object', + `ip_address` varchar(45) DEFAULT NULL COMMENT 'IP address of the client that performed the action', PRIMARY KEY (`action_id`), KEY `timestamp_idx` (`timestamp`), KEY `user_idx` (`user`), diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 585d7a5d8d..4e7c608a61 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -12,6 +12,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AcquisitionsDefaultReplyTo', '', NULL, 'Default email address used as reply-to for notices sent by the acquisitions module.', 'Free'), ('AcqViewBaskets','user','user|branch|all','Define which baskets a user is allowed to view: their own only, any within their branch, or all','Choice'), ('AcqWarnOnDuplicateInvoice','0',NULL,'Warn librarians when they try to create a duplicate invoice','YesNo'), +('ActionLogsEnableIPLogging', '0', NULL, 'If enabled, store the IP address of the client in action logs for security auditing.', 'YesNo'), ('ActionLogsTraceDepth', '0', NULL, 'Sets the maximum depth of the action logs stack trace', 'Integer'), ('AdditionalContentLog','0',NULL,'If ON, log OPAC news changes','YesNo'), ('AdditionalContentsEditor','tinymce','tinymce|codemirror','Choose tool for editing News.', 'Choice'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref index 1861a0de4e..b83b88361c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref @@ -140,6 +140,13 @@ Logging: - item transfers. Debugging: + - + - "When logging actions, store the IP address of the client" + - pref: "ActionLogsEnableIPLogging" + choices: + 1: "Yes" + 0: "No (recommended for GDPR compliance)" + - "Note: IP addresses may be considered personal data under GDPR." - - "When logging actions, store a stack trace that goes at most" - pref: "ActionLogsTraceDepth" diff --git a/t/db_dependent/TestIpLogging.t b/t/db_dependent/TestIpLogging.t new file mode 100644 index 0000000000..6a034eac56 --- /dev/null +++ b/t/db_dependent/TestIpLogging.t @@ -0,0 +1,58 @@ +#!/usr/bin/perl + +# Test script to verify IP address logging in action_logs +# This script tests that the IP address is properly stored in action_logs +# based on the ActionLogsEnableIPLogging system preference + +use Modern::Perl; +use Test::More tests => 10; + +use C4::Context; +use C4::Log qw(logaction); +use Koha::Database; +use Koha::ActionLogs; +use t::lib::TestBuilder; +use t::lib::Mocks; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +my $builder = t::lib::TestBuilder->new; + +# Mock REMOTE_ADDR +$ENV{REMOTE_ADDR} = '192.168.1.100'; + +# Clear any existing action logs for this test +Koha::ActionLogs->search->delete; + +# Perform a test action log +my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + +# Test 1: IP logging DISABLED (default) +t::lib::Mocks::mock_preference( 'ActionLogsEnableIPLogging', 0 ); + +logaction( "TESTING", "ADD_DISABLED", $patron->borrowernumber, "Test IP logging disabled", "intranet" ); + +my $action_log_disabled = Koha::ActionLogs->search( { module => 'TESTING', action => 'ADD_DISABLED' } )->next; +ok( $action_log_disabled, "Action log was created with IP logging disabled" ); +is( $action_log_disabled->module, "TESTING", "Module is correct when disabled" ); +is( $action_log_disabled->action, "ADD_DISABLED", "Action is correct when disabled" ); +is( $action_log_disabled->info, "Test IP logging disabled", "Info is correct when disabled" ); +is( $action_log_disabled->ip_address, undef, "IP address is NOT logged when disabled" ); + +# Test 2: IP logging ENABLED +t::lib::Mocks::mock_preference( 'ActionLogsEnableIPLogging', 1 ); + +logaction( "TESTING", "ADD_ENABLED", $patron->borrowernumber, "Test IP logging enabled", "intranet" ); + +my $action_log_enabled = Koha::ActionLogs->search( { module => 'TESTING', action => 'ADD_ENABLED' } )->next; +ok( $action_log_enabled, "Action log was created with IP logging enabled" ); +is( $action_log_enabled->module, "TESTING", "Module is correct when enabled" ); +is( $action_log_enabled->action, "ADD_ENABLED", "Action is correct when enabled" ); +is( $action_log_enabled->info, "Test IP logging enabled", "Info is correct when enabled" ); +is( $action_log_enabled->ip_address, "192.168.1.100", "IP address is logged when enabled" ); + +# Clean up +$schema->storage->txn_rollback; + +done_testing(); -- 2.39.5