Bug 36928

Summary: Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled
Product: Koha Reporter: Kyle M Hall <kyle>
Component: REST APIAssignee: Kyle M Hall <kyle>
Status: RESOLVED DUPLICATE QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: critical    
Priority: P5 - low CC: dcook, nick, tomascohen, victor
Version: unspecifiedKeywords: RM_priority
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 34893    
Bug Blocks:    
Attachments: Bug 36928: Patron authentication / password validation will fail with 500 error for local users if LDAP is enabled
Bug 36928: Followup for CAS and Shibboleth
Bug 36928 - Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled
Bug 36928: Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled
Bug 36928: Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled

Description Kyle M Hall 2024-05-22 14:39:51 UTC
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.
Comment 1 Kyle M Hall 2024-05-22 14:46:25 UTC
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!
Comment 2 Kyle M Hall 2024-05-22 14:58:58 UTC
Created attachment 167048 [details] [review]
Bug 36928: Followup for CAS and Shibboleth
Comment 3 Kyle M Hall 2024-05-22 15:00:31 UTC
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!
Comment 4 Kyle M Hall 2024-05-22 15:01:19 UTC
Upgrading severity since it affects all external authentications and will affect third party integration with Koha.
Comment 5 Katrin Fischer 2024-05-22 15:02:19 UTC
Can we tell what caused this and which branches/versions are affected?
Comment 6 Kyle M Hall 2024-05-22 15:03:08 UTC
This also affects ILSDI and may affect other code utilizing checkpw
Comment 7 Kyle M Hall 2024-05-22 15:08:17 UTC
(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
Comment 8 Katrin Fischer 2024-05-22 15:09:14 UTC
(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.
Comment 9 Tomás Cohen Arazi 2024-05-22 15:10:56 UTC
I think we need some tests here.

For C4::Auth. I might be able to help.
Comment 10 Kyle M Hall 2024-05-22 15:52:57 UTC
(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!
Comment 11 Kyle M Hall 2024-05-23 10:59:31 UTC
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!
Comment 12 Victor Grousset/tuxayo 2024-05-28 01:30:22 UTC
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>
Comment 13 Victor Grousset/tuxayo 2024-05-28 01:34:33 UTC
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`
Comment 14 Victor Grousset/tuxayo 2024-05-28 01:47:29 UTC
(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.
Comment 15 Victor Grousset/tuxayo 2024-05-28 01:59:34 UTC
(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.
Comment 16 Tomás Cohen Arazi 2024-05-30 14:37:30 UTC
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 ***