https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29783 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29783 --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Created attachment 128984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128984&action=edit Bug 29783: Add account lockout patron userid lookup condition This patch checks that $q_userid is not blank before trying to do a patron lookup. This prevents a spurrious account locked message from appearing to the user when there is a user in Koha that has an empty userid and too many login_attempts. Test plan: 0) Do not apply the patch yet 1) Set system preference FailedLoginAttempts to 3 2) Modify a borrower to have an empty userid and 10 login_attempts e.g. update borrowers set userid = '', login_attempts = 10 where cardnumber = 23529001223636; 3) Visit the staff interface (e.g. http://localhost:8081/) 4) Note the message "Error: This account has been locked!" even though you have not tried to log in 5) Apply the patch 6) Visit the staff interface (e.g. http://localhost:8081/) 7) Note that there is no error message about your account being locked 8) Profit. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29783 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #2)

Hi David,

2 things here:

1) How did you manage to add a user with an empty userid? Using the GUI and using the patron import tool this should always be automatically generated if not given. We can use SQL for the test of course.

I'm not sure how it happened. My guess would be some migration script. For testing, I used SQL.

2) You write in your test plan: 1) Set system preference FailedLoginAttempts to 3 2) Modify a borrower to have an empty userid and 10 login_attempts

So in this case the user account SHOULD be locked, shouldn't it?

Yes, the user account should be locked. The purpose of that test is to see whether or not that user account is being used automatically by Koha without any user input. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29783 --- Comment #5 from Fridolin Somers <fridolin.somers@biblibre.com> --- I think we should not fetch Koha::Patron at this place. For logged-in patron, its datas are fetch here : https://git.koha-community.org/Koha-community/Koha/src/commit/2f39f426b5e748... We should add 'account_locked' there no ? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29783 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + my $patron = Koha::Patrons->find({ userid => $q_userid }) if $q_userid; "my $var if condition" does not meet the coding guidelines, at least I hope so :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29783 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Looks like the problematic line is gone now... although I think it may have broken some template variables... In this scope, I don't think $patorn is even initialized... too_many_login_attempts => ( $patron and $patron->account_locked ), password_has_expired => ( $patron and $patron->password_expired ), We seem to fetch the patron object quite a few times in checkauth()... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29783 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- This is still an issue -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.