With the `identifier` parameter passed for validating a password, as introduced by bug 32739, the identifier will be matched against `userid` and `cardnumber`. It would be useful to return both the userid and cardnumber of the patron if validation is correct.
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...)