Many SIP services send an empty password field (AD). Even if allow_empty_passwords is enabled for the given SIP account, this empty password is run though Koha's password checker which increments the number of login attempts for a patron. Thus repeated patron information requests can lock a patron out! Empty password fields in SIP should not call for a password check if allow_empty_passwords is enabled.
Created attachment 83183 [details] [review] Bug 21997 - SIP patron information requests can lock patron out of account Many SIP services send an empty password field (AD). Even if allow_empty_passwords is enabled for the given SIP account, this empty password is run though Koha's password checker which increments the number of login attempts for a patron. Thus repeated patron information requests can lock a patron out! Empty password fields in SIP should not call for a password check if allow_empty_passwords is enabled. Test Plan: 1) Enable a patron password attempt with a limit of 3 2) Send 4 patron information requests with an empty AD field 3) Note the patron's account is now locked 4) Apply this patch 5) Repeat step 2 with a different patron 6) Note the patron's account does not get locked!
Created attachment 84230 [details] [review] Bug 21997 - SIP patron information requests can lock patron out of account Many SIP services send an empty password field (AD). Even if allow_empty_passwords is enabled for the given SIP account, this empty password is run though Koha's password checker which increments the number of login attempts for a patron. Thus repeated patron information requests can lock a patron out! Empty password fields in SIP should not call for a password check if allow_empty_passwords is enabled. Test Plan: 1) Enable a patron password attempt with a limit of 3 2) Send 4 patron information requests with an empty AD field 3) Note the patron's account is now locked 4) Apply this patch 5) Repeat step 2 with a different patron 6) Note the patron's account does not get locked! Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Can we have an accompanying test pretty please.. I think there's enough SIP test scaffolding in place to make this viable. Failing QA
Created attachment 84989 [details] [review] Bug 21997: Unit tests
Created attachment 84990 [details] [review] Bug 21997 - SIP patron information requests can lock patron out of account Many SIP services send an empty password field (AD). Even if allow_empty_passwords is enabled for the given SIP account, this empty password is run though Koha's password checker which increments the number of login attempts for a patron. Thus repeated patron information requests can lock a patron out! Empty password fields in SIP should not call for a password check if allow_empty_passwords is enabled. Test Plan: 1) Enable a patron password attempt with a limit of 3 2) Send 4 patron information requests with an empty AD field 3) Note the patron's account is now locked 4) Apply this patch 5) Repeat step 2 with a different patron 6) Note the patron's account does not get locked! Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Created attachment 84991 [details] [review] Bug 21997: Unit tests
Created attachment 84992 [details] [review] Bug 21997 - SIP patron information requests can lock patron out of account Many SIP services send an empty password field (AD). Even if allow_empty_passwords is enabled for the given SIP account, this empty password is run though Koha's password checker which increments the number of login attempts for a patron. Thus repeated patron information requests can lock a patron out! Empty password fields in SIP should not call for a password check if allow_empty_passwords is enabled. Test Plan: 1) Enable a patron password attempt with a limit of 3 2) Send 4 patron information requests with an empty AD field 3) Note the patron's account is now locked 4) Apply this patch 5) Repeat step 2 with a different patron 6) Note the patron's account does not get locked! Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Awesome, thanks for adding the tests Kyle.. sorry it's taken me a while to get back to it.
Created attachment 85360 [details] [review] Bug 21997: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 85361 [details] [review] Bug 21997: SIP patron information requests can lock patron out of account Many SIP services send an empty password field (AD). Even if allow_empty_passwords is enabled for the given SIP account, this empty password is run though Koha's password checker which increments the number of login attempts for a patron. Thus repeated patron information requests can lock a patron out! Empty password fields in SIP should not call for a password check if allow_empty_passwords is enabled. Test Plan: 1) Enable a patron password attempt with a limit of 3 2) Send 4 patron information requests with an empty AD field 3) Note the patron's account is now locked 4) Apply this patch 5) Repeat step 2 with a different patron 6) Note the patron's account does not get locked! Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works for me, no regressions found, passes qa test scripts.. Passing QA
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.04
backported to 18.05.x for 18.05.11