[Bug 30962] New: REST API: Add endpoint /patrons/:patron_id/check_password
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Bug ID: 30962 Summary: REST API: Add endpoint /patrons/:patron_id/check_password Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au CC: tomascohen@gmail.com This endpoint would check that the POSTed password matches the Koha database password. In theory, you could have other validations here as well (e.g. check if patron is expired, restricted, etc.). But maybe those should be their own endpoints. Adding this to support a Keycloak User Storage SPI as per https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17004#c39 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=17004 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |black23@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- +1 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- How will we prevent abuse for this password-guessing service? Hehe -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #2)
How will we prevent abuse for this password-guessing service? Hehe
It's not a public/anonymous endpoint. Only an authenticated and authorized user could use it. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #3)
(In reply to Tomás Cohen Arazi from comment #2)
How will we prevent abuse for this password-guessing service? Hehe
It's not a public/anonymous endpoint. Only an authenticated and authorized user could use it.
But happy to add more security to lock accounts on too many bad password checks. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to David Cook from comment #4)
(In reply to David Cook from comment #3)
(In reply to Tomás Cohen Arazi from comment #2)
How will we prevent abuse for this password-guessing service? Hehe
It's not a public/anonymous endpoint. Only an authenticated and authorized user could use it.
But happy to add more security to lock accounts on too many bad password checks.
I think it would make sense to use the existing lock feature here, also consistent. ILS-DI locks too. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #5)
(In reply to David Cook from comment #4)
(In reply to David Cook from comment #3)
(In reply to Tomás Cohen Arazi from comment #2)
How will we prevent abuse for this password-guessing service? Hehe
It's not a public/anonymous endpoint. Only an authenticated and authorized user could use it.
But happy to add more security to lock accounts on too many bad password checks.
I think it would make sense to use the existing lock feature here, also consistent.
ILS-DI locks too.
Done! -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Created attachment 136215 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136215&action=edit Bug 30962: REST API: Add endpoint /patrons/:patron_id/check_password This patch adds an endpoint for /patrons/:patron_id/check_password This allows a third-party, using an authenticated and authorized Koha API user, to check if the username and password given by a user is correct in Koha. For example, a Keycloak extension can be created using its User Storage SPI to use Koha as the user database for Keycloak. This API allows us to authenticate the user as a particular Koha user - without creating a Koha user session for them. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RESTBasicAuth 2. Enable "RESTBasicAuth" 3. Run the following commands while substituting correct values for <koha_user> and <koha_password> 3. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/patrons/51/check_password -d '{ "password": "<koha_password>" }' -v 4. Note "200 OK" response 5. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/patrons/51/check_password -d '{ "password": "this is definitely not the password" }' -v 6. Note "400 Bad Request" response and error message {"error":"Invalid password"} -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ulrich.kleiber@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136215|0 |1 is obsolete| | --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 136300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136300&action=edit Bug 30962: REST API: Add endpoint /patrons/:patron_id/check_password This patch adds an endpoint for /patrons/:patron_id/check_password This allows a third-party, using an authenticated and authorized Koha API user, to check if the username and password given by a user is correct in Koha. For example, a Keycloak extension can be created using its User Storage SPI to use Koha as the user database for Keycloak. This API allows us to authenticate the user as a particular Koha user - without creating a Koha user session for them. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RESTBasicAuth 2. Enable "RESTBasicAuth" 3. Run the following commands while substituting correct values for <koha_user> and <koha_password> 3. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/patrons/51/check_password -d '{ "password": "<koha_password>" }' -v 4. Note "200 OK" response 5. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/patrons/51/check_password -d '{ "password": "this is definitely not the password" }' -v 6. Note "400 Bad Request" response and error message {"error":"Invalid password"} Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Regarding security, I've been thinking more about this, and in theory you could lock down all non-public API routes by IP address, if your organisation has static IP addresses and requires VPNs for working from home (WFH). We do this on other systems that have admin APIs. It's possible that you might need to provide a third-party access to an admin API (like this one in bug 30962), but then you can add their IP address to the allow list. It just adds another layer of security over top of the existing security measures. We could promote the idea by adding some configuration directives to Apache that allow all IP addresses for both public and non-public API routes and include some comments about how they can lock down the non-public API routes by doing X, Y, and Z. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- I was thinking about this again, and really it's very similar to an LDAP lookup. It requires staff-level authorization, and with the account lockout functionality, it wouldn't be any different from someone trying to brute force the OPAC login. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com Status|Signed Off |Failed QA --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 1. Missing tests (you must provide tons of tests to cover the different situations) 2. Route's name should not be a verb (/password/validation maybe?) 3. Routes that returns empty should return 204 4. It's always returning "Invalid password" even for other failures (like too many attempts) 5. It allows you to check for pwd validation for a user you don't know their userid (you can force brute only by knowing the patron's id). I don't think it's a security concern as userid could be guessed anyway (?) 6. following 5, you can lock any accounts if FailedLoginAttempts is set, no need to know the userid list. How bad is that? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #12 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #11)
1. Missing tests (you must provide tons of tests to cover the different situations)
Are all the existing API tests in "t/db_dependent/api/v1/"?
2. Route's name should not be a verb (/password/validation maybe?)
I did struggle to come up with a noun for this one so I take the point. I think "validation" or "authentication" instead of "check_password" makes sense. It also makes things more flexible for the future. I suppose the concept would be that we're creating the patron validation or authentication by supplying these details...
4. It's always returning "Invalid password" even for other failures (like too many attempts)
I suppose a generic error like "Authentication failed" or "Validation failed" would be better.
5. It allows you to check for pwd validation for a user you don't know their userid (you can force brute only by knowing the patron's id). I don't think it's a security concern as userid could be guessed anyway (?) 6. following 5, you can lock any accounts if FailedLoginAttempts is set, no need to know the userid list. How bad is that?
Remember that this isn't a public endpoint. Depending on an attacker's goal, it would be silly to try to brute force /patrons/:patron_id/check_password when you could just use /patrons/:patron_id/password to set the password to whatever you want, and then use /patrons/:patron_id to get the userid in order to login. Both password API endpoints require the same level of staff-level authorization from an authenticated API user. That said, I do take the point. I'm not opposed to changing this to "/patrons/authentication" and sending userid and password (like http://localhost:8080/cgi-bin/koha/ilsdi.pl/ilsdi.pl?service=Describe&verb=AuthenticatePatron) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- While I originally created this to support a Keycloak User Storage SPI extension, I've had libraries interested in it as an alternative to the ILS-DI API "AuthenticatePatron" verb. It's essentially the 1 thing that the ILS-DI API can do that the REST API can't do, which means that libraries use the ILS-DI API (or SIP2) to authenticate patrons against the Koha database for third-parties software. So it would be great to get this into Koha core, so that we can continue to move away from the ILS-DI API. -- We've actually had more and more libraries using Keycloak with the Koha user storage extension, because it means they get to-purpose software for SSO, while still getting to manage all their patrons in Koha. It's super useful. (I also have a goal of Koha using Keycloak Oauth2 access tokens to bring SSO to Koha's REST API, so that third-parties could call public API endpoints on behalf of Koha users without third-parties needing dedicated API users with staff-level permissions, which would be a massive improvement on security/trust relationships, but one step at a time...) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Jonathan Field <jonathan.field@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.field@ptfs-europe. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #14 from David Cook <dcook@prosentient.com.au> --- I'm keen to keep working on this but without any additional feedback I don't really have any work to do... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- I'm planning on making a plugin for this, and it's got me thinking more about what people would accept in the longer term... I'm thinking /api/v1/auth/ will be the route, and it'll take "username" and "password" JSON POST parameters. I've noticed FortiAuthenticator works this way; it's the way that the ILS-DI API works; it would be difficult to abuse. I suppose we could argue about the route name. FortiAuthenticator uses that exact same route, but I suppose it could be /api/v1/patrons/auth, so it makes it more obvious that it's a route not for authenticating to use the API but rather just for authenticating patrons. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #12)
(In reply to Jonathan Druart from comment #11)
1. Missing tests (you must provide tons of tests to cover the different situations)
Are all the existing API tests in "t/db_dependent/api/v1/"?
Not sure what you mean here, but we try to enforce 100% code coverage on the tests.
2. Route's name should not be a verb (/password/validation maybe?)
I did struggle to come up with a noun for this one so I take the point.
I'd go with 'validation'.
4. It's always returning "Invalid password" even for other failures (like too many attempts)
I suppose a generic error like "Authentication failed" or "Validation failed" would be better.
If you think generic is fine, go for it. If you think there are use cases in which the caller could take advantage on the information, please catch those exceptions and return something meaningful.
5. It allows you to check for pwd validation for a user you don't know their userid (you can force brute only by knowing the patron's id). I don't think it's a security concern as userid could be guessed anyway (?) 6. following 5, you can lock any accounts if FailedLoginAttempts is set, no need to know the userid list. How bad is that?
Maybe not the right bug to talk about how to prevent brute force attacks, as you said: this is not a public route. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #16)
(In reply to David Cook from comment #12)
Are all the existing API tests in "t/db_dependent/api/v1/"?
Not sure what you mean here, but we try to enforce 100% code coverage on the tests.
Are good examples of API tests found in that location?
2. Route's name should not be a verb (/password/validation maybe?)
I did struggle to come up with a noun for this one so I take the point.
I'd go with 'validation'.
In light of the /api/v1/auth/* routes that we have these days... /api/v1/auth/validation? Or do we want another path prefix in there like "password" for /api/v1/auth/password/validation? And we'd be posting userid and password to that endpoint. -- I'm currently using /patrons/:patron_id/check_password and when I implemented bug 31982 locally it did cause a problem because I was using the OPAC interface. I switched to the Staff Interface API and then it was all good. In my case, the consumer of this API is Keycloak, which also runs locally. If it was a third-party system, I think I'd need to create an security exception to allow it through to that endpoint. I keep thinking about public vs non-public APIs... and I figure public APIs are things that every day people and front-end systems should be able to use. They offer the same functionality to the public as the OPAC but they do so in a machine-friendly way. If an action is only allowed by staff, it's part of the non-public admin API. And if there's a third-party system that needs to access that, because we trust it, then we make an explicit exception for it, and ideally keep the scope narrowed to just what it needs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136300|0 |1 is obsolete| | --- Comment #18 from David Cook <dcook@prosentient.com.au> --- Created attachment 144810 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144810&action=edit Bug 30962: REST API: Add endpoint /patrons/:patron_id/check_password This patch adds an endpoint for /patrons/:patron_id/check_password This allows a third-party, using an authenticated and authorized Koha API user, to check if the username and password given by a user is correct in Koha. For example, a Keycloak extension can be created using its User Storage SPI to use Koha as the user database for Keycloak. This API allows us to authenticate the user as a particular Koha user - without creating a Koha user session for them. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RESTBasicAuth 2. Enable "RESTBasicAuth" 3. Run the following commands while substituting correct values for <koha_user> and <koha_password> 3. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "<koha_password>" }' -v 4. Note "204 No Content" response 5. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "this is definitely not the password" }' -v 6. Note "400 Bad Request" response and error message {"error":"Validation failed"} -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #19 from David Cook <dcook@prosentient.com.au> --- I think the updated patch includes everything except the unit tests. I've just run out of time for that today, as holidays start in 5 minutes... In the new year, I'll try to model some tests using t/db_dependent/api/v1/idp.t as a template. I think that should be what's needed... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|REST API: Add endpoint |REST API: Add endpoint |/patrons/:patron_id/check_p |/auth/password/validation |assword | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144810|0 |1 is obsolete| | --- Comment #20 from David Cook <dcook@prosentient.com.au> --- Created attachment 144964 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144964&action=edit Bug 30962: REST API: Add endpoint /auth/password/validation This patch adds an endpoint for /auth/password/validation This allows a third-party, using an authenticated and authorized Koha API user, to check if the username and password given by a user is correct in Koha. For example, a Keycloak extension can be created using its User Storage SPI to use Koha as the user database for Keycloak. This API allows us to authenticate the user as a particular Koha user - without creating a Koha user session for them. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RESTBasicAuth 2. Enable "RESTBasicAuth" 3. Run the following commands while substituting correct values for <koha_user> and <koha_password> 3. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "<koha_password>" }' -v 4. Note "204 No Content" response 5. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "this is definitely not the password" }' -v 6. Note "400 Bad Request" response and error message {"error":"Validation failed"} -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #21 from David Cook <dcook@prosentient.com.au> --- Created attachment 144965 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144965&action=edit Bug 30962: Add unit tests for /auth/password/validation endpoint Test plan: 0. Apply patch 1. prove -v t/db_dependent/api/v1/password_validation.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #22 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #11)
1. Missing tests (you must provide tons of tests to cover the different situations) 2. Route's name should not be a verb (/password/validation maybe?) 3. Routes that returns empty should return 204 4. It's always returning "Invalid password" even for other failures (like too many attempts) 5. It allows you to check for pwd validation for a user you don't know their userid (you can force brute only by knowing the patron's id). I don't think it's a security concern as userid could be guessed anyway (?) 6. following 5, you can lock any accounts if FailedLoginAttempts is set, no need to know the userid list. How bad is that?
I think that I've addressed all these points now :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144964|0 |1 is obsolete| | --- Comment #23 from David Nind <david@davidnind.com> --- Created attachment 145350 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145350&action=edit Bug 30962: REST API: Add endpoint /auth/password/validation This patch adds an endpoint for /auth/password/validation This allows a third-party, using an authenticated and authorized Koha API user, to check if the username and password given by a user is correct in Koha. For example, a Keycloak extension can be created using its User Storage SPI to use Koha as the user database for Keycloak. This API allows us to authenticate the user as a particular Koha user - without creating a Koha user session for them. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RESTBasicAuth 2. Enable "RESTBasicAuth" 3. Run the following commands while substituting correct values for <koha_user> and <koha_password> 3. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "<koha_password>" }' -v 4. Note "204 No Content" response 5. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "this is definitely not the password" }' -v 6. Note "400 Bad Request" response and error message {"error":"Validation failed"} Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144965|0 |1 is obsolete| | --- Comment #24 from David Nind <david@davidnind.com> --- Created attachment 145351 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145351&action=edit Bug 30962: Add unit tests for /auth/password/validation endpoint Test plan: 0. Apply patch 1. prove -v t/db_dependent/api/v1/password_validation.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Michaela Sieber <michaela.sieber@kit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |michaela.sieber@kit.edu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |katrin.fischer@bsz-bw.de --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Giving this one a try... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32739 Status|Signed Off |Failed QA --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 1) Unit tests pass 2) QA test tools pass 3) Test plan I followed the test plan and while I have the replies 204 and 400, I don't see the error as described: error message {"error":"Validation failed"} 4) Postman I also tried to test this with Postman. * Verified that BasicAuth worked as expected by listing patrons * Tried: localhost:8081/api/v1/auth/password/validation * Params: username, password * Verb: POST * Body: { "username": "...", "password": "..." } a) Matching username + password * cardnumber + correct password = 400 - Bad request * username + correct password = 204 - No Content (that's a success?) * username + incorrect password = 400 - Bad Request - error: Validation failed * username + incorrect password so many times to make the account lock: 400 - Bad Request - error: Validation failed Notes: * The login page in Koha allows for cardnumber + password AND userid + password at the same time. I think we should extend this route in a separate bug to also support cardnumber/both to make this easier to use and also mimick what ILS-DI and SIP do as well. I've filed: Bug 32739 - REST API: Extend endpoint /auth/password/validation for cardnumber * We do want the account to lock with too many attempts, which it does. The error stays the same, but I think that's good too and matches what we do on the OPAC, we don't want to give away too much information. *thumbs up* QA fail: * The route users username, but the patrons api uses user_id. We should make things match and use user_id here as well. Almost ready to PQA, please fix! Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32739 [Bug 32739] REST API: Extend endpoint /auth/password/validation for cardnumber -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145350|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145351|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #27 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 145765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145765&action=edit Bug 30962: REST API: Add endpoint /auth/password/validation This patch adds an endpoint for /auth/password/validation This allows a third-party, using an authenticated and authorized Koha API user, to check if the username and password given by a user is correct in Koha. For example, a Keycloak extension can be created using its User Storage SPI to use Koha as the user database for Keycloak. This API allows us to authenticate the user as a particular Koha user - without creating a Koha user session for them. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RESTBasicAuth 2. Enable "RESTBasicAuth" 3. Run the following commands while substituting correct values for <koha_user> and <koha_password> 3. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "<koha_password>" }' -v 4. Note "204 No Content" response 5. curl -XPOST -H "Content-Type: application/json" -u <koha_user>:<koha_password> http://localhost:8081/api/v1/auth/password/validation -d '{ "username": "<koha_username">, "password": "this is definitely not the password" }' -v 6. Note "400 Bad Request" response and error message {"error":"Validation failed"} Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #28 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 145766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145766&action=edit Bug 30962: Add unit tests for /auth/password/validation endpoint Test plan: 0. Apply patch 1. prove -v t/db_dependent/api/v1/password_validation.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 145767 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145767&action=edit Bug 30962: (QA follow-up) Rename attribute and simplify tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #26)
* The login page in Koha allows for cardnumber + password AND userid + password at the same time. I think we should extend this route in a separate bug to also support cardnumber/both to make this easier to use and also mimick what ILS-DI and SIP do as well. I've filed: Bug 32739 - REST API: Extend endpoint /auth/password/validation for cardnumber
Nice idea!
QA fail:
* The route users username, but the patrons api uses user_id. We should make things match and use user_id here as well.
I submitted a follow-up to help this bug get pushed soon! Hope it covers your concern. dcook: hope you're fine with my tests simplification, it is much easier to maintain and similar to what we do everywhere (with two exceptions I think). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #31 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #30)
dcook: hope you're fine with my tests simplification, it is much easier to maintain and similar to what we do everywhere (with two exceptions I think).
I've taken a quick look and if you're happy with it, I'm happy with it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145766|0 |1 is obsolete| | --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 145883 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145883&action=edit Bug 30962: Add unit tests for /auth/password/validation endpoint Test plan: 0. Apply patch 1. prove -v t/db_dependent/api/v1/password_validation.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145767|0 |1 is obsolete| | --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 145884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145884&action=edit Bug 30962: (QA follow-up) Rename attribute and simplify tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #34 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Jacob O'Mara <jacob.omara@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|23.05.00 |23.05.00,22.11.03 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #35 from Jacob O'Mara <jacob.omara@ptfs-europe.com> --- Nice work, thanks everyone! Pushed to 22.11.x for the next release. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to stable |Needs documenting --- Comment #36 from Lucas Gass <lucas@bywatersolutions.com> --- Enhancement will not be backported to 22.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16652 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|REST API: Add endpoint |Add POST endpoint for |/auth/password/validation |validating a user password CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |new feature -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #37 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Back end stuff, doesn't affect the end user so there's nothing to add/amend in the manual. I'm told the API docs is automated, so there's no need to put those bugs as Needs documenting. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21577 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35204 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35204 [Bug 35204] REST API: POST endpoint /auth/password/validation dies on patron with expired password -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36575 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36561 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36561 [Bug 36561] Inappropriate permission for "/api/v1/auth/password/validation" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 wainuiwitikapark@catalyst.net.nz changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #38 from wainuiwitikapark@catalyst.net.nz --- Hi there, I need to backport these patches to 22.05 as this bug is a dependency of Bug 36575. However, the patches don't apply cleanly, could I please get some patches for 22.05? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 --- Comment #39 from David Cook <dcook@prosentient.com.au> --- (In reply to wainuiwitikapark from comment #38)
Hi there,
I need to backport these patches to 22.05 as this bug is a dependency of Bug 36575.
However, the patches don't apply cleanly, could I please get some patches for 22.05?
Hmm... are you sure that you need this dependency? My suggestion for bug 36575 would be to just not backport the changes to t/db_dependent/api/v1/password_validation.t Nothing else in bug 36575 should touch any code here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30962 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40023 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40023 [Bug 40023] Allow embedding `patron` when validating credentials -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org