From b3d0db6e7bc9782cc495a2d98346d6f5c7bcc038 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 24 Oct 2025 13:06:23 +0200 Subject: [PATCH] Bug 41094: Fix search_anonymize_candidates, locked_account We should not allow anonymizing patrons with failed login attempts when the pref FailedLoginAttempts has not been set. Test plan: Run the test again from the previous patch. This should pass now. Choose a patron with a positive value (say 55) in borrowers.login_attempts or modify one. (NOTE: This patron will be anonymized in the test plan.) Empty pref value for FailedLoginAttempts. Set PatronAnonymizeDelay to 1. Fill dateexpiry for this patron to yesterday. Run cleanupdatabase.pl -confirm -v and check if the patron was NOT anonymized. * Anonymized 0 patrons Now set FailedLoginAttempts to a value (say 55) so that the patron is considered as locked. (This unrealistic value is for testing only.) MAKE SURE that you will not be anonymizing more patrons than you like in the next step! (Set login_attempts to value < 55 for all other patrons.) Run cleanupdatabase.pl -confirm -v again and check if the patron was anonymized. * Anonymized 1 patrons Restore the changed pref values. Signed-off-by: Marcel de Rooy Signed-off-by: David Nind Signed-off-by: Martin Renvoize Signed-off-by: Lucas Gass (cherry picked from commit 291c2f103d59fc624842a5917d36326daab35410) Signed-off-by: Paul Derscheid (cherry picked from commit 2fb7dab4a411f53847d4df1cc1f34aaff86ac8c2) Signed-off-by: Fridolin Somers --- Koha/Patron.pm | 8 +++++--- Koha/Patrons.pm | 10 +++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 8cefe0d23b..705caa96ad 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1803,9 +1803,11 @@ disabled. sub account_locked { my ($self) = @_; my $FailedLoginAttempts = C4::Context->preference('FailedLoginAttempts'); - return 1 if $FailedLoginAttempts - and $self->login_attempts - and $self->login_attempts >= $FailedLoginAttempts; + return 1 + if $FailedLoginAttempts + && $FailedLoginAttempts > 0 + && $self->login_attempts + && $self->login_attempts >= $FailedLoginAttempts; return 1 if ($self->login_attempts || 0) < 0; # administrative lockout return 0; } diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 04d3bf05f6..f77550d308 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -268,9 +268,13 @@ sub search_anonymize_candidates { my $str = $parser->format_datetime($dt); $cond->{dateexpiry} = { '<=' => $str }; $cond->{anonymized} = 0; # not yet done - if( $params->{locked} ) { - my $fails = C4::Context->preference('FailedLoginAttempts') || 0; - $cond->{login_attempts} = [ -and => { '!=' => undef }, { -not_in => [0, 1..$fails-1 ] } ]; # -not_in does not like undef + my $fails = C4::Context->preference('FailedLoginAttempts'); + + if ( $params->{locked} && $fails && $fails > 0 ) { # $fails should actually not be negative btw + $cond->{login_attempts} = + [ -and => { '!=' => undef }, { -not_in => [ 0, 1 .. $fails - 1 ] } ]; # -not_in does not like undef + } elsif ( $params->{locked} ) { + $cond->{login_attempts} = -1; } return $class->search( $cond ); }