Bug 37407 - Fast add / fast cataloging from patron checkout does not checkout item
Summary: Fast add / fast cataloging from patron checkout does not checkout item
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Brendan Lawlor
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on:
Blocks: 36192
  Show dependency treegraph
 
Reported: 2024-07-18 16:59 UTC by Donna
Modified: 2024-09-10 14:27 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.04
Circulation function:


Attachments
Bug 37407: Fill barcode field when fast cataloging from patron checkout (3.28 KB, patch)
2024-07-22 16:05 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37407: Fix automatic checkout for fast cataloging (2.15 KB, patch)
2024-07-23 12:42 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37407: Fix automatic checkout for fast cataloging (2.14 KB, patch)
2024-07-23 12:48 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37407: Fix automatic checkout for fast cataloging (2.19 KB, patch)
2024-07-24 15:53 UTC, Eric Garcia
Details | Diff | Splinter Review
Bug 37407: Fix automatic checkout for fast cataloging (2.25 KB, patch)
2024-07-25 17:01 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Donna 2024-07-18 16:59:05 UTC
To replicate:
1.  Go to any patron checkout screen
2.  Enter a barcode that is not in the system
3.  Click on the "Add using Fast Cataloging" in Barcode not found warning
4.  Enter bib/item info and click on Save
5.  You are returned to the patron checkout screen but it is not checked out

The bib record and item record are created, but staff will need to scan the item again in order to check it out.  Since this is the workflow a lot of libraries use for ephemeral items, magazines, etc, this will add a lot of clicks and cause confusion.

Worked as expected as late as 23.11.05
Comment 1 Brendan Lawlor 2024-07-19 15:53:08 UTC
This is related to CSRF protection because doing the checkout now requires op=cud-checkout.

When you add the item, additem.pl does a redirect to circulation.pl, which is like a GET request so we can't just add the op to the redirect.

# additem.pl line 491
if ( $frameworkcode eq 'FA' && $fa_circborrowernumber ) {
        print $input->redirect( '/cgi-bin/koha/circ/circulation.pl?'

Probably not the best solution, but one thought I had was to populate the barcode field in circulation.tt and trigger the checkout button with javascript
Comment 2 David Cook 2024-07-22 05:20:45 UTC
Yeah that's an interesting one. 

I suppose one option would be to do the redirect in a way that it re-populates the barcode input box, and then requires the user to actually click the checkout button again. 

On one hand, I don't love the GET request param being used to fill in the input box, but it's probably the shortest path forward. Plus, if we add data verification (like we should), we'd check that the URL parameter barcode actually exists in the database before popping it in the box. 

Not something I'm planning on working on ATM, but that's how I'd do it I think.
Comment 3 Brendan Lawlor 2024-07-22 16:05:07 UTC
Created attachment 169316 [details] [review]
Bug 37407: Fill barcode field when fast cataloging from patron checkout

This patch fills the barcode field on the patron checkout screen when
an item is fast cataloged at checkout.

Test plan:
1. Apply patch
2. Enter a barcode not in the system, eg 99999
3. Click '+ Add record using fast cataloging'
4. Fill required bib fields 000, 008 and 245a and click 'Save'
5. Add required item field y - Koha item type and click 'Add item'
6. Notice the barcode you entered previously is filled in the barcode field
7. Hit 'Enter' or click 'Checkout' and verify the item is checked out
Comment 4 Brendan Lawlor 2024-07-22 16:19:39 UTC
This could be added to the javascript on circulation tt to automatically submit the form, but I think that would go against the CSRF protection so I did not include that part in the patch. 

[% IF fa_barcode %]
  $('#mainform').submit();
[% END %]
Comment 5 Donna 2024-07-22 16:27:30 UTC
I think the checkout should be automatic, since that is the behavior now.
Comment 6 David Cook 2024-07-22 23:16:13 UTC
(In reply to Donna from comment #5)
> I think the checkout should be automatic, since that is the behavior now.

If the checkout were to be automatic, it would have to be done on the additem.pl page, which is going to come with a number of issues and take a lot more work.

With the redirect, as Brendan says, automatic checkout would be a CSRF vulnerability. 

An alternative to Brendan's patch would be for additem.pl to not redirect to circulation.pl, but rather to show a new confirmation asking if they wanted to checkout. They'd then click confirm, and that would POST back to circulation.pl.

But either way the user is going to have an extra click without a significant. I think this is a pretty good compromise. At least for now.
Comment 7 Brendan Lawlor 2024-07-23 12:42:39 UTC
Created attachment 169415 [details] [review]
Bug 37407: Fix automatic checkout for fast cataloging

This patch adds a check for the referrer to the circulation page.
If the referrer is from the same origin's additem.pl then get the
barcode from the url parameters, fill the form and submit.

Test plan:
1. Apply patch
2. Enter a barcode not in the system, eg 99999
3. Click '+ Add record using fast cataloging'
4. Fill required bib fields 000, 008 and 245a and click 'Save'
5. Add required item field y - Koha item type and click 'Add item'
6. Notice the barcode is filled and the form is submitted automatically
7. Confirm the item is checked out and the dutedate specified works
8. Add an html customization somewhere else in koha with a link like
   http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=38&barcode=99999&duedatespec=&stickyduedate=
9. Click on the link to simulate a csrf attack
10. Confirm the checkout page is loaded for that patron but no checkout is made
Comment 8 Brendan Lawlor 2024-07-23 12:48:21 UTC
Created attachment 169416 [details] [review]
Bug 37407: Fix automatic checkout for fast cataloging

This patch adds a check for the referrer to the circulation page.
If the referrer is from the same origin's additem.pl then get the
barcode from the url parameters, fill the form and submit.

Test plan:
1. Apply patch
2. Enter a barcode not in the system, eg 99999
3. Click '+ Add record using fast cataloging'
4. Fill required bib fields 000, 008 and 245a and click 'Save'
5. Add required item field y - Koha item type and click 'Add item'
6. Notice the barcode is filled and the form is submitted automatically
7. Confirm the item is checked out and the dutedate specified works
8. Add an html customization somewhere else in koha with a link like
   http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=38&barcode=99999&duedatespec=&stickyduedate=
9. Click on the link to simulate a csrf attack
10. Confirm the checkout page is loaded for that patron but no checkout is made
Comment 9 Brendan Lawlor 2024-07-23 12:54:57 UTC
The latest patch is pure javascript so until this bug is resolved this can be added to IntranetUserJS as a workaround:

$(document).ready(function() {
  // Handle checkout for fast cataloging
  // Check the referrer to prevent csrf, fill and submit form
  if(document.referrer.split('?')[0] === window.location.origin +'/cgi-bin/koha/cataloguing/additem.pl') {
    let urlParams = new URLSearchParams(window.location.search);
    let barcode = urlParams.get('barcode');
    $('#barcode').val(barcode);
    $('#mainform').submit();
  }  
});

This code checks that the referrer is the same origin and from the additem page, but it might not be as secure as the token based csrf protection.
Comment 10 Eric Garcia 2024-07-24 15:53:08 UTC
Created attachment 169502 [details] [review]
Bug 37407: Fix automatic checkout for fast cataloging

This patch adds a check for the referrer to the circulation page.
If the referrer is from the same origin's additem.pl then get the
barcode from the url parameters, fill the form and submit.

Test plan:
1. Apply patch
2. Enter a barcode not in the system, eg 99999
3. Click '+ Add record using fast cataloging'
4. Fill required bib fields 000, 008 and 245a and click 'Save'
5. Add required item field y - Koha item type and click 'Add item'
6. Notice the barcode is filled and the form is submitted automatically
7. Confirm the item is checked out and the dutedate specified works
8. Add an html customization somewhere else in koha with a link like
   http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=38&barcode=99999&duedatespec=&stickyduedate=
9. Click on the link to simulate a csrf attack
10. Confirm the checkout page is loaded for that patron but no checkout is made

Signed-off-by: Eric Garcia <cubingguy714@gmail.com>
Comment 11 David Cook 2024-07-25 01:01:44 UTC
At a glance, this should work, but... conventional wisdom is that HTTP Referer/document.referrer shouldn't be used for security. 

This feels like a workaround. 

That said, at the moment, I can't see a particular technical flaw in it. In the context that document.referrer is being used... I can't see a way it could be manipulated to trigger this form submit (other than XSS but that's an issue for token based anti-CSRF too).

I thought Javascript could manipulate the Referer for AJAX requests, but the Internet tells me that's not the case. 

So yeah... I think this is OK, but I don't like it. 

So I won't Fail QA, but I'm not going to Pass QA either. A different QAer may feel differently.
Comment 12 Kyle M Hall (khall) 2024-07-25 17:01:09 UTC
Created attachment 169602 [details] [review]
Bug 37407: Fix automatic checkout for fast cataloging

This patch adds a check for the referrer to the circulation page.
If the referrer is from the same origin's additem.pl then get the
barcode from the url parameters, fill the form and submit.

Test plan:
1. Apply patch
2. Enter a barcode not in the system, eg 99999
3. Click '+ Add record using fast cataloging'
4. Fill required bib fields 000, 008 and 245a and click 'Save'
5. Add required item field y - Koha item type and click 'Add item'
6. Notice the barcode is filled and the form is submitted automatically
7. Confirm the item is checked out and the dutedate specified works
8. Add an html customization somewhere else in koha with a link like
   http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=38&barcode=99999&duedatespec=&stickyduedate=
9. Click on the link to simulate a csrf attack
10. Confirm the checkout page is loaded for that patron but no checkout is made

Signed-off-by: Eric Garcia <cubingguy714@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Kyle M Hall (khall) 2024-07-25 17:02:18 UTC
I've been unable to find a better solution either.
Comment 14 Martin Renvoize (ashimema) 2024-07-26 12:56:02 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 15 Lucas Gass (lukeg) 2024-08-29 14:32:18 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 16 Fridolin Somers 2024-09-10 14:27:28 UTC
"cud-" not in 23.11.x