Bug 30199

Summary: self checkout login by cardnumber is broken if you input a non-existent cardnumber
Product: Koha Reporter: Lucas Gass <lucas>
Component: Self checkoutAssignee: Jonathan Druart <jonathan.druart+koha>
Status: Pushed to oldoldstable --- QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: andrewfh, jonathan.druart+koha, kyle, martin.renvoize, sbcornell, victor, wainuiwitikapark
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.05,21.05.15,20.11.19
Bug Depends on: 29543    
Bug Blocks:    
Attachments: Bug 30199: self checkout login by cardnumber is broken if you input a non-existent cardnumber
Bug 30199: self checkout login by cardnumber is broken if you input a non-existent cardnumber
Bug 30199: [ALTERNATIVE-PATH] Do not generate jtw if patron does not exist
Bug 30199: Do not generate jtw if patron does not exist
Bug 30199: Do not generate jwt if patron does not exist
Bug 30199: Do not generate jwt if patron does not exist

Description Lucas Gass 2022-02-28 23:13:53 UTC
To recreate:

1. Set SelfCheckoutByLogin to 'cardnumber'.
2. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
3. Enter an invalid carnumber like 'thisisabadcardnumber'
4. See the appropriate message "Sorry The userid thisisabadcardnumber was not found in the database. Please try again."
5. Try again with a different cardnumber and notice the same exact error message including the 'thisisabadcardnumber' cardnumber.
6. The only thing that solves it for me is to direct my browser to this URL:

http://localhost:8080/cgi-bin/koha/sco/sco-main.pl?op=logout

That properly logs me out and allows me to input a new cardnumber.
Comment 1 Kyle M Hall 2022-03-03 19:26:16 UTC
Created attachment 131348 [details] [review]
Bug 30199: self checkout login by cardnumber is broken if you input a non-existent cardnumber

Test Plan:
1. Set SelfCheckoutByLogin to 'cardnumber'.
2. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
3. Enter an invalid carnumber like 'thisisabadcardnumber'
4. See the appropriate message "Sorry The userid thisisabadcardnumber was not found in the database. Please try again."
5. Try again with a different cardnumber and notice the same exact error message including the 'thisisabadcardnumber' cardnumber.
6. Apply this patch
7. Restart all the things!
8. Repeat steps 2-4
9. Try again with a different cardnumber, you should now see the correct
   cardnumber!
Comment 2 Lucas Gass 2022-03-03 20:17:19 UTC
Created attachment 131364 [details] [review]
Bug 30199: self checkout login by cardnumber is broken if you input a non-existent cardnumber

Test Plan:
1. Set SelfCheckoutByLogin to 'cardnumber'.
2. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
3. Enter an invalid carnumber like 'thisisabadcardnumber'
4. See the appropriate message "Sorry The userid thisisabadcardnumber was not found in the database. Please try again."
5. Try again with a different cardnumber and notice the same exact error message including the 'thisisabadcardnumber' cardnumber.
6. Apply this patch
7. Restart all the things!
8. Repeat steps 2-4
9. Try again with a different cardnumber, you should now see the correct
   cardnumber!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 3 Katrin Fischer 2022-03-06 14:40:19 UTC
As this is the area we just had the security fixes for, we might want to verify if this is related (I suspect it is) to make sure it gets backported to all the right versions.

I am updating the severity as for a standalone self check station this could be quite an issue with no staff around to restart things.
Comment 4 Jonathan Druart 2022-03-18 09:28:11 UTC
Created attachment 131860 [details] [review]
Bug 30199: [ALTERNATIVE-PATH] Do not generate jtw if patron does not exist
Comment 5 Jonathan Druart 2022-03-18 09:28:58 UTC
Good catch, Kyle!

Can you have a look at this alternative patch please?
I think the correct fix is to not generate the JWT if the patron's info are not valid.
Comment 6 Jonathan Druart 2022-03-18 09:30:12 UTC
Certainly caused by bug 29543.
Comment 7 Jonathan Druart 2022-03-21 14:07:16 UTC
Lucas, Kyle, could you test this alternative patch?
Comment 8 Lucas Gass 2022-03-21 14:34:09 UTC
Created attachment 131973 [details] [review]
Bug 30199: Do not generate jtw if patron does not exist

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 9 Lucas Gass 2022-03-21 14:39:01 UTC
Jonathan,

Your patch works nicely so I signed off and obsoleted Kyle's.
Comment 10 Jonathan Druart 2022-03-21 15:01:22 UTC
Created attachment 131978 [details] [review]
Bug 30199: Do not generate jwt if patron does not exist

This is a follow-up patch for bug 29543. If an invalid cardnumber has
been used we should not generate a token.

Test Plan:
1. Set SelfCheckoutByLogin to 'cardnumber'.
2. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
3. Enter an invalid carnumber like 'thisisabadcardnumber'
4. See the appropriate message "Sorry The userid thisisabadcardnumber was not found in the database. Please try again."
5. Try again with a different cardnumber and notice the same exact error message including the 'thisisabadcardnumber' cardnumber.
6. Apply this patch
7. Restart all the things!
8. Repeat steps 2-4
9. Try again with a different cardnumber, you should now see the correct
   cardnumber!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 11 Jonathan Druart 2022-03-21 15:01:47 UTC
(In reply to Lucas Gass from comment #9)
> Jonathan,
> 
> Your patch works nicely so I signed off and obsoleted Kyle's.

Thanks!
Comment 12 Martin Renvoize 2022-03-24 16:45:23 UTC
Created attachment 132183 [details] [review]
Bug 30199: Do not generate jwt if patron does not exist

This is a follow-up patch for bug 29543. If an invalid cardnumber has
been used we should not generate a token.

Test Plan:
1. Set SelfCheckoutByLogin to 'cardnumber'.
2. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
3. Enter an invalid carnumber like 'thisisabadcardnumber'
4. See the appropriate message "Sorry The userid thisisabadcardnumber was not found in the database. Please try again."
5. Try again with a different cardnumber and notice the same exact error message including the 'thisisabadcardnumber' cardnumber.
6. Apply this patch
7. Restart all the things!
8. Repeat steps 2-4
9. Try again with a different cardnumber, you should now see the correct
   cardnumber!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2022-03-24 16:46:08 UTC
Nice simple fix, works as expected

PQA
Comment 14 Fridolin Somers 2022-03-25 00:25:41 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 15 Lucas Gass 2022-03-30 21:05:41 UTC
If possible can we get this backported to 21.11 and 21.05?
Comment 16 Jonathan Druart 2022-03-31 08:50:39 UTC
(In reply to Lucas Gass from comment #15)
> If possible can we get this backported to 21.11 and 21.05?

Yes, looks like it should be backported into all stables.
Comment 17 Sarah Cornell 2022-04-26 13:42:57 UTC
We'd love to have this backported to 21.05 as soon as possible.  It's starting to show up on machines that were previously well-behaved.  Thanks!
Comment 18 Kyle M Hall 2022-04-26 14:59:49 UTC
Pushed to 21.11.x for 21.11.05
Comment 19 Andrew Fuerste-Henry 2022-05-03 20:15:09 UTC
Pushed to 21.05.x for 21.05.15
Comment 20 Victor Grousset/tuxayo 2022-05-10 01:48:02 UTC
Backported: Pushed to 20.11.x branch for 20.11.19
Comment 21 wainuiwitikapark 2022-05-18 22:52:16 UTC
Does this need to be backported to 19.11.x?