Summary: | Make password validation endpoint return patron IDs | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | REST API | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | dcook, fridolin.somers, matt.blenkinsop, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.04,22.11.10
|
|
Circulation function: | |||
Bug Depends on: | 32739 | ||
Bug Blocks: | |||
Attachments: |
Bug 34313: Unit tests
Bug 34313: Add patron information in pass validation response Bug 34313: Unit tests Bug 34313: Add patron information in pass validation response Bug 34313: Unit tests Bug 34313: Add patron information in pass validation response Bug 34313: Unit tests Bug 34313: Add patron information in pass validation response |
Description
Tomás Cohen Arazi (tcohen)
2023-07-19 19:05:22 UTC
Created attachment 153685 [details] [review] Bug 34313: Unit tests Created attachment 153686 [details] [review] Bug 34313: Add patron information in pass validation response This patch makes the password validation response return the following patron attributes to the API consumer: * cardnumber * userid This will give hints on what was used to validate in the fallback bahvior the endpoint has. To test: 1. Apply the unit tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/password_validation.t => FAIL: The endpoint doesn't return this valuable data 3. Apply this patch 4. Repeat 2 => SUCESS: Tests pass! We got the cardnumber and the userid! 5. Sign off :-D Created attachment 153690 [details] [review] Bug 34313: Unit tests Created attachment 153691 [details] [review] Bug 34313: Add patron information in pass validation response This patch makes the password validation response return the following patron attributes to the API consumer: * cardnumber * userid This will give hints on what was used to validate in the fallback bahvior the endpoint has. To test: 1. Apply the unit tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/password_validation.t => FAIL: The endpoint doesn't return this valuable data 3. Apply this patch 4. Repeat 2 => SUCESS: Tests pass! We got the cardnumber and the userid! 5. Sign off :-D This will have some potential consequences for users of the existing API but they should be manageable. I think this sounds like a reasonable change. At a minimum, getting the patron_id back is useful. Created attachment 153787 [details] [review] Bug 34313: Unit tests Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 153788 [details] [review] Bug 34313: Add patron information in pass validation response This patch makes the password validation response return the following patron attributes to the API consumer: * cardnumber * userid This will give hints on what was used to validate in the fallback bahvior the endpoint has. To test: 1. Apply the unit tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/password_validation.t => FAIL: The endpoint doesn't return this valuable data 3. Apply this patch 4. Repeat 2 => SUCESS: Tests pass! We got the cardnumber and the userid! 5. Sign off :-D Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> (In reply to David Cook from comment #5) > This will have some potential consequences for users of the existing API but > they should be manageable. You mean the change of the returned status code? It will still mean success for all libraries I know about, and people are not using the response body so there will be no surprise, I hope :-D (In reply to Tomás Cohen Arazi from comment #8) > (In reply to David Cook from comment #5) > > This will have some potential consequences for users of the existing API but > > they should be manageable. > > You mean the change of the returned status code? It will still mean success > for all libraries I know about, and people are not using the response body > so there will be no surprise, I hope :-D I have some code that checks for 204 specifically. I should've just had it check for a success. Not a big deal to fix that one though. Created attachment 154462 [details] [review] Bug 34313: Unit tests Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 154463 [details] [review] Bug 34313: Add patron information in pass validation response This patch makes the password validation response return the following patron attributes to the API consumer: * cardnumber * userid This will give hints on what was used to validate in the fallback bahvior the endpoint has. To test: 1. Apply the unit tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/password_validation.t => FAIL: The endpoint doesn't return this valuable data 3. Apply this patch 4. Repeat 2 => SUCESS: Tests pass! We got the cardnumber and the userid! 5. Sign off :-D Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.04 Enhancement - not backporting to 22.11.x Nice work everyone! (In reply to Matt Blenkinsop from comment #14) > Enhancement - not backporting to 22.11.x > > Nice work everyone! Van you reconsider for the LTS release? It could be used by external systems to avoid using ILS-DI for validating users. VAR: Decision overturned Tomas' point makes sense, will reset the bug and backport to 22.11.x Nice work everyone! Pushed to oldstable for 22.11.x Note: If the user has a userid but no cardnumber, they will fail to authenticate. (One day, we'll return borrowernumber when we authenticate...) |