Bugzilla – Attachment 188516 Details for
Bug 41094
search_anonymize_candidates returns too many candidates when FailedLoginAttempts is empty
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41094: Fix search_anonymize_candidates, locked_account
Bug-41094-Fix-searchanonymizecandidates-lockedacco.patch (text/plain), 2.88 KB, created by
Marcel de Rooy
on 2025-10-28 14:42:14 UTC
(
hide
)
Description:
Bug 41094: Fix search_anonymize_candidates, locked_account
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2025-10-28 14:42:14 UTC
Size:
2.88 KB
patch
obsolete
>From 2cda614e196a993bb64fe2a687ab3616b6ae1fc3 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 24 Oct 2025 13:06:23 +0200 >Subject: [PATCH] Bug 41094: Fix search_anonymize_candidates, locked_account >Content-Type: text/plain; charset=utf-8 > >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 <m.de.rooy@rijksmuseum.nl> >--- > Koha/Patron.pm | 5 +++-- > Koha/Patrons.pm | 7 +++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 5d68d7c97d..018192f7fe 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -2061,8 +2061,9 @@ sub account_locked { > my $FailedLoginAttempts = C4::Context->preference('FailedLoginAttempts'); > return 1 > if $FailedLoginAttempts >- and $self->login_attempts >- and $self->login_attempts >= $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 853d44dd12..6d7aaf47c2 100644 >--- a/Koha/Patrons.pm >+++ b/Koha/Patrons.pm >@@ -305,10 +305,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; >+ 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); > } >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41094
:
188511
|
188512
|
188515
|
188516
|
188525
|
188526