Bug 20956: BorrowersLog is not logging permission changes

Changes to permissions are not logged in the Koha action_logs, making it impossible to trace changes to staff permissions. This patch ensures that all changes to permissions are logged.

Test plan:

    1. Go to a staff interface
    2. Search for a patron and change their permissions (note their borrowernumber)
    3. Go to tools/Log viewer
    4. Filter by Module = Patrons, Object = <borrowernumber>.
    5. Click on submit
       ==> No permission changes are logged.
    6. Apply the patch
    7. Repeat step 1, 2, 3, 4, 5
       ==> Permission changes are now logged
    8. Check that the diff column contains the change:
       select diff from action_logs where object= <borrowernumber> order by action_id desc limit 1;

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This commit is contained in:
Hammat Wele 2025-09-12 19:14:59 +00:00 committed by Martin Renvoize
parent 9837925379
commit 553a18da09
No known key found for this signature in database
GPG Key ID: 2D86E0EAEC83711B
3 changed files with 64 additions and 4 deletions

View File

@ -148,6 +148,8 @@ sub logaction {
$diff //= encode_json( diff( $original, $updated, noU => 1 ) )
if $original && ref $updated eq 'HASH';
$infos = encode_json($infos) if ( ref $infos eq 'HASH' );
Koha::ActionLog->new(
{
timestamp => \'NOW()',

View File

@ -10,6 +10,7 @@ use CGI qw ( -utf8 );
use C4::Output qw( output_and_exit_if_error output_and_exit output_html_with_http_headers );
use C4::Auth qw( get_template_and_user get_all_subpermissions get_user_subpermissions );
use C4::Context;
use C4::Log qw( logaction );
use Koha::Patron::Categories;
use Koha::Patrons;
@ -69,16 +70,19 @@ if ( $op eq 'cud-newflags' ) {
# construct flags
my $module_flags = 0;
my $sth = $dbh->prepare("SELECT bit,flag FROM userflags ORDER BY bit");
my $sth = $dbh->prepare("SELECT bit,flag,flagdesc FROM userflags ORDER BY bit");
$sth->execute();
while ( my ( $bit, $flag ) = $sth->fetchrow_array ) {
my %userflags;
while ( my ( $bit, $flag, $flagdesc ) = $sth->fetchrow_array ) {
if ( exists $all_module_perms{$flag} ) {
$module_flags += 2**$bit;
}
$userflags{$bit} = [ $flag, $flagdesc ];
}
$sth = $dbh->prepare("UPDATE borrowers SET flags=? WHERE borrowernumber=?");
my $old_flags = $patron->flags // 0;
my $old_flags = $patron->flags // 0;
my %permissions_before = get_permissions( \%userflags, $old_flags );
if ( ( $old_flags == 1 || $module_flags == 1 )
&& $old_flags != $module_flags )
{
@ -105,6 +109,8 @@ if ( $op eq 'cud-newflags' ) {
}
}
my %permissions_after = get_permissions( \%userflags, $module_flags );
logaction( 'MEMBERS', 'MODIFY', $member, \%permissions_after, undef, \%permissions_before );
print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member");
} else {
@ -200,3 +206,36 @@ if ( $op eq 'cud-newflags' ) {
output_html_with_http_headers $input, $cookie, $template->output;
}
sub get_permissions {
my ( $userflags, $old_flags ) = @_;
my %active_flag = decode_flags( $userflags, $old_flags );
my $user_perms = get_user_subpermissions( $bor->{'userid'} );
my %permissions;
my $dbh = C4::Context->dbh();
my $sth = $dbh->prepare("SELECT code, description FROM permissions");
$sth->execute();
while ( my ( $code, $desc ) = $sth->fetchrow_array ) {
$permissions{$code} = $desc;
}
for my $module ( keys %$user_perms ) {
for my $code ( keys %{ $user_perms->{$module} } ) {
$active_flag{$code} = $permissions{$code};
}
}
return %active_flag;
}
sub decode_flags {
my ( $userflags, $flags ) = @_;
my %active_flags;
foreach my $bit ( keys %$userflags ) {
if ( $flags & ( 2**$bit ) ) {
my ( $flag, $desc ) = @{ $userflags->{$bit} };
$active_flags{$flag} = $desc;
}
}
return %active_flags;
}

View File

@ -16,8 +16,9 @@
use Modern::Perl;
use Data::Dumper qw( Dumper );
use Test::NoWarnings;
use Test::More tests => 8;
use Test::More tests => 9;
use C4::Context;
use C4::Log qw( logaction cronlogaction );
@ -221,6 +222,24 @@ subtest 'Test storing diff of objects' => sub {
is( $diff->{D}->{barcode}->{N}, '_MY_TEST_BARCODE_', 'Diff of changes logged successfully' );
};
subtest 'Test storing diff of hashrefs' => sub {
plan tests => 1;
my $original_data = {
firstname => 'John',
surname => 'Doe'
};
my $edited_data = {
firstname => 'Johnny',
surname => 'Doe'
};
logaction( 'MY_MODULE', 'TEST', 12345, $edited_data, 'staff', $original_data );
my $logs = Koha::ActionLogs->search( { module => 'MY_MODULE', action => 'TEST' } )->next;
my $diff = decode_json( $logs->diff );
is( $diff->{D}->{firstname}->{N}, 'Johnny', 'Diff of changes logged successfully' );
};
subtest 'Test storing original version of an item' => sub {
plan tests => 1;