Bug 23771 - CAS/Shib Authentication can fail when multiple users with no userid/cardnumber defined and one of them is locked
Summary: CAS/Shib Authentication can fail when multiple users with no userid/cardnumbe...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-07 20:08 UTC by Nick Clemens (kidclamp)
Modified: 2021-06-14 21:28 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.05
Circulation function:


Attachments
Bug 23771: Don't look for patrons if we don't have a userid (1.36 KB, patch)
2019-10-07 20:13 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 23771: Don't look for patrons if we don't have a userid (1.38 KB, patch)
2019-10-07 23:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 23771: Don't look for patrons if we don't have a userid (1.46 KB, patch)
2019-10-10 13:54 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 23771: Don't look for patrons if we don't have a userid (1.52 KB, patch)
2019-10-11 13:32 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2019-10-07 20:08:56 UTC
When logging in via cas no userid is passed to checkpw, however, we still seek out users with a null userid/cardnumber. If one of those users has a locked account and it happens to be the one chosen, we won't continue to CAS/Shib login (or LDAP, but LDAP shouldn't get a blank userid)
Comment 1 Nick Clemens (kidclamp) 2019-10-07 20:13:02 UTC
Created attachment 93847 [details] [review]
Bug 23771: Don't look for patrons if we don't have a userid

While the code requires external auth to test, the change is simple enough I
think a good read would suffice

To test:
1 - You either need Shib or CAS enabled and setup
2 - Add two users with blank.null cardnumbers
3 - Lock those user accounts
    Set failedloginattempts to 1
    Try a bad login with the userid
    Or set via the DB
4 - Try a good login via the external auth
5 - Note it fails
6 - Apply patch
7 - Login now succeeds
Comment 2 Nick Clemens (kidclamp) 2019-10-07 23:22:27 UTC
Created attachment 93860 [details] [review]
Bug 23771: Don't look for patrons if we don't have a userid

While the code requires external auth to test, the change is simple enough I
think a good read would suffice

To test:
1 - You either need Shib or CAS enabled and setup
2 - Add two users with blank.null cardnumbers
3 - Lock those user accounts
    Set failedloginattempts to 1
    Try a bad login with the userid
    Or set via the DB
4 - Try a good login via the external auth
5 - Note it fails
6 - Apply patch
7 - Login now succeeds
Comment 3 Liz Rea 2019-10-10 13:54:33 UTC
Created attachment 93951 [details] [review]
Bug 23771: Don't look for patrons if we don't have a userid

While the code requires external auth to test, the change is simple enough I
think a good read would suffice

To test:
1 - You either need Shib or CAS enabled and setup
2 - Add two users with blank.null cardnumbers
3 - Lock those user accounts
    Set failedloginattempts to 1
    Try a bad login with the userid
    Or set via the DB
4 - Try a good login via the external auth
5 - Note it fails
6 - Apply patch
7 - Login now succeeds

Signed-off-by: Mike Somers - Bridgewater State University <msomers@bridgew.edu>
Comment 4 Martin Renvoize (ashimema) 2019-10-11 13:32:53 UTC
Created attachment 94018 [details] [review]
Bug 23771: Don't look for patrons if we don't have a userid

While the code requires external auth to test, the change is simple enough I
think a good read would suffice

To test:
1 - You either need Shib or CAS enabled and setup
2 - Add two users with blank.null cardnumbers
3 - Lock those user accounts
    Set failedloginattempts to 1
    Try a bad login with the userid
    Or set via the DB
4 - Try a good login via the external auth
5 - Note it fails
6 - Apply patch
7 - Login now succeeds

Signed-off-by: Mike Somers - Bridgewater State University <msomers@bridgew.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize (ashimema) 2019-10-11 13:33:18 UTC
Works well, Passing QA
Comment 6 Martin Renvoize (ashimema) 2019-10-11 13:37:02 UTC
Nice work!

Pushed to master for 19.11.00
Comment 7 Fridolin Somers 2019-11-08 14:41:47 UTC
Pushed to 19.05.x for 19.05.05