If LDAP is enabled, and use of /api/v1/auth/password/validation will fail with a 500 internal server error like: Can't call method "cardnumber" on an undefined value at /usr/share/koha/lib/Koha/REST/V1/Auth/Password.pm line 83, <DATA> line 1490. This is because the valid patron object is being overwritten with an undefined value if there is no LDAP patron returned. Since a failed LDAP query falls back to local user auth, we try to use the now undefined patron object thus generating an error.
Created attachment 167047 [details] [review] Bug 36928: Patron authentication / password validation will fail with 500 error for local users if LDAP is enabled If LDAP is enabled, and use of /api/v1/auth/password/validation will fail with a 500 internal server error like: Can't call method "cardnumber" on an undefined value at /usr/share/koha/lib/Koha/REST/V1/Auth/Password.pm line 83, <DATA> line 1490. This is because the valid patron object is being overwritten with an undefined value if there is no LDAP patron returned. Since a failed LDAP query falls back to local user auth, we try to use the now undefined patron object thus generating an error. Test Plan: 1) Set up and enable LDAP 2) POST JSON to /api/v1/auth/password/validation like: { "userid": "koha", "password": "koha" } 3) Note the 500 erorr 4) Apply this patch 5) Restart all the things! 6) POST the data again 7) It works!
Created attachment 167048 [details] [review] Bug 36928: Followup for CAS and Shibboleth
Created attachment 167049 [details] [review] Bug 36928 - Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled If external auth ( such as LDAP ) is enabled, and use of /api/v1/auth/password/validation will fail with a 500 internal server error like: Can't call method "cardnumber" on an undefined value at /usr/share/koha/lib/Koha/REST/V1/Auth/Password.pm line 83, <DATA> line 1490. This is because the valid patron object is being overwritten with an undefined value if there is no LDAP patron returned. Since a failed LDAP query falls back to local user auth, we try to use the now undefined patron object thus generating an error. Test Plan: 1) Set up and enable LDAP 2) POST JSON to /api/v1/auth/password/validation like: { "userid": "koha", "password": "koha" } 3) Note the 500 erorr 4) Apply this patch 5) Restart all the things! 6) POST the data again 7) It works!
Upgrading severity since it affects all external authentications and will affect third party integration with Koha.
Can we tell what caused this and which branches/versions are affected?
This also affects ILSDI and may affect other code utilizing checkpw
(In reply to Katrin Fischer from comment #5) > Can we tell what caused this and which branches/versions are affected? This appears to be a missed edge case for the bug 34893
(In reply to Kyle M Hall from comment #7) > (In reply to Katrin Fischer from comment #5) > > Can we tell what caused this and which branches/versions are affected? > > This appears to be a missed edge case for the bug 34893 That's pretty bad then.
I think we need some tests here. For C4::Auth. I might be able to help.
(In reply to Tomás Cohen Arazi from comment #9) > I think we need some tests here. > > For C4::Auth. I might be able to help. Agreed. Thanks Tomas!
Created attachment 167097 [details] [review] Bug 36928: Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled If external auth ( such as LDAP ) is enabled, and use of /api/v1/auth/password/validation will fail with a 500 internal server error like: Can't call method "cardnumber" on an undefined value at /usr/share/koha/lib/Koha/REST/V1/Auth/Password.pm line 83, <DATA> line 1490. This is because the valid patron object is being overwritten with an undefined value if there is no LDAP patron returned. Since a failed LDAP query falls back to local user auth, we try to use the now undefined patron object thus generating an error. Test Plan: 1) Set up and enable LDAP 2) POST JSON to /api/v1/auth/password/validation like: { "userid": "koha", "password": "koha" } 3) Note the 500 erorr 4) Apply this patch 5) Restart all the things! 6) POST the data again 7) It works!
Created attachment 167202 [details] [review] Bug 36928: Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled If external auth ( such as LDAP ) is enabled, and use of /api/v1/auth/password/validation will fail with a 500 internal server error like: Can't call method "cardnumber" on an undefined value at /usr/share/koha/lib/Koha/REST/V1/Auth/Password.pm line 83, <DATA> line 1490. This is because the valid patron object is being overwritten with an undefined value if there is no LDAP patron returned. Since a failed LDAP query falls back to local user auth, we try to use the now undefined patron object thus generating an error. Test Plan: 1) Set up and enable LDAP https://wiki.koha-community.org/wiki/Ldap_testing (restart plack) 2) POST JSON to /api/v1/auth/password/validation like: { "userid": "koha", "password": "koha" } Or run: prove t/db_dependent/api/v1/password_validation.t 3) Note the 500 error(s) 4) Apply this patch 5) Restart all the things! 6) POST the data again or run the tests 7) It works! Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works! :) --- > will fail with a 500 internal server error like: Can't call method "cardnumber" on an undefined value at /usr/share/koha/lib/Koha/REST/V1/Auth/Password.pm line 83 Indeed, observed in /var/log/koha/kohadev/api-error.log --- Test plan amended to suggest as an alternative to run `prove t/db_dependent/api/v1/password_validation.t`
(In reply to Kyle M Hall from comment #7) > This appears to be a missed edge case for the bug 34893 Indeed, with LDAP enabled password_validation.t passes before and fails after.
(In reply to Tomás Cohen Arazi from comment #9) > I think we need some tests here. > > For C4::Auth. I might be able to help. Idea for higher levels tests: a test from t/db_dependent/api/v1/password_validation.t could be copied in t/db_dependent/Auth_with_ldap.t I'm not sure where it wouldn't be too much out of place.
I was tracking the root cause of this, which took me to ONLY the LDAP use case. And the solution was to make sure the most basic successful case returned the Koha::Patron object. Which is what one of the follow-ups on bug 36575 does. I'm closing this one as duplicate. Feel free to re-open if you have another POV. *** This bug has been marked as a duplicate of bug 36575 ***