Bug 34313

Summary: Make password validation endpoint return patron IDs
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: RESOLVED FIXED QA Contact:
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
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 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 (kidclamp) 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 (kidclamp) 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...)