Bug 36928 - Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled
Summary: Patron authentication / password validation will fail with 500 error for loca...
Status: RESOLVED DUPLICATE of bug 36575
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords: RM_priority
Depends on: 34893
Blocks:
  Show dependency treegraph
 
Reported: 2024-05-22 14:39 UTC by Kyle M Hall
Modified: 2024-05-30 14:37 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 36928: Patron authentication / password validation will fail with 500 error for local users if LDAP is enabled (1.89 KB, patch)
2024-05-22 14:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36928: Followup for CAS and Shibboleth (2.09 KB, patch)
2024-05-22 14:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36928 - Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled (3.31 KB, patch)
2024-05-22 15:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36928: Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled (3.11 KB, patch)
2024-05-23 10:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36928: Patron authentication / password validation will fail with 500 error for local users if external auth (LDAP/CAS/Shibboleth) is enabled (3.31 KB, patch)
2024-05-28 01:30 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 ***