Bug 34313 - Make password validation endpoint return patron IDs
Summary: Make password validation endpoint return patron IDs
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 32739
Blocks:
  Show dependency treegraph
 
Reported: 2023-07-19 19:05 UTC by Tomás Cohen Arazi
Modified: 2024-04-08 00:32 UTC (History)
4 users (show)

See Also:
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


Attachments
Bug 34313: Unit tests (2.01 KB, patch)
2023-07-19 19:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34313: Add patron information in pass validation response (2.57 KB, patch)
2023-07-19 19:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34313: Unit tests (2.12 KB, patch)
2023-07-19 20:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34313: Add patron information in pass validation response (2.89 KB, patch)
2023-07-19 20:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34313: Unit tests (2.18 KB, patch)
2023-07-21 13:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34313: Add patron information in pass validation response (2.95 KB, patch)
2023-07-21 13:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34313: Unit tests (2.24 KB, patch)
2023-08-16 11:27 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 34313: Add patron information in pass validation response (3.00 KB, patch)
2023-08-16 11:27 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2023-07-19 19:05:22 UTC
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.
Comment 1 Tomás Cohen Arazi 2023-07-19 19:55:36 UTC
Created attachment 153685 [details] [review]
Bug 34313: Unit tests
Comment 2 Tomás Cohen Arazi 2023-07-19 19:55:39 UTC
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
Comment 3 Tomás Cohen Arazi 2023-07-19 20:35:46 UTC
Created attachment 153690 [details] [review]
Bug 34313: Unit tests
Comment 4 Tomás Cohen Arazi 2023-07-19 20:35:49 UTC
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
Comment 5 David Cook 2023-07-19 23:31:14 UTC
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.
Comment 6 Katrin Fischer 2023-07-21 13:14:38 UTC
Created attachment 153787 [details] [review]
Bug 34313: Unit tests

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 7 Katrin Fischer 2023-07-21 13:14:41 UTC
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>
Comment 8 Tomás Cohen Arazi 2023-07-21 13:25:44 UTC
(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
Comment 9 David Cook 2023-07-24 00:25:40 UTC
(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.
Comment 10 Nick Clemens 2023-08-16 11:27:50 UTC
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>
Comment 11 Nick Clemens 2023-08-16 11:27:56 UTC
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>
Comment 12 Tomás Cohen Arazi 2023-08-29 17:58:11 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 13 Fridolin Somers 2023-08-30 07:10:46 UTC
Pushed to 23.05.x for 23.05.04
Comment 14 Matt Blenkinsop 2023-08-31 10:22:36 UTC
Enhancement - not backporting to 22.11.x

Nice work everyone!
Comment 15 Tomás Cohen Arazi 2023-08-31 10:28:50 UTC
(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.
Comment 16 Matt Blenkinsop 2023-08-31 10:51:29 UTC
VAR: Decision overturned

Tomas' point makes sense, will reset the bug and backport to 22.11.x
Comment 17 Matt Blenkinsop 2023-08-31 10:52:35 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 18 David Cook 2024-04-08 00:32:50 UTC
Note: If the user has a userid but no cardnumber, they will fail to authenticate. (One day, we'll return borrowernumber when we authenticate...)