Bug 37614 - Printing patron cards from patron lists broken by CSRF protection
Summary: Printing patron cards from patron lists broken by CSRF protection
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Phil Ringnalda
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 36192
  Show dependency tree / graph
 
Reported: 2024-08-09 20:33 UTC by Phil Ringnalda
Modified: 2024-10-21 23:28 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes printing patron cards from a patron list (Tools > Patrons and circulation > Patron lists > Actions > Print patron cards > Export). When clicking on Export, the progress icon keeps spinning and doesn't finish - resulting in no PDF file to download. (This is related to the CSRF changes added in Koha 24.05 to improve form security.)
Version(s) released in:
24.11.00,24.05.04
Circulation function:

Attachments
Bug 37614: Printing patron cards from patron lists should be a GET (5.03 KB, patch)
2024-08-16 01:19 UTC, Phil Ringnalda 		Details | Diff | Splinter Review
Bug 37614: Printing patron cards from patron lists should be a GET (5.08 KB, patch)
2024-08-16 02:52 UTC, David Nind 		Details | Diff | Splinter Review
Bug 37614: Printing patron cards from patron lists should be a GET (5.18 KB, patch)
2024-08-16 06:22 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Phil Ringnalda 2024-08-09 20:33:05 UTC 
Steps to reproduce:

1. Tools - Patrons lists, if you don't already have one then New list, name and save, Search for patrons, add at least one patron, then return to Patron lists.
2. In the Actions menu for your patron list, select Print patron cards
3. In that perfectly normal looking popup, click Export. You now have a popup with an Loading throbber, which will hang forever, and in your console you have a 403 error from trying to GET patroncards/print.pl. If you go back and inspect the page where you clicked Export, you'll find that it is a button type='submit' in a form with method='post' which makes the GET seem a bit odd.

To see that it sometimes posts,

1. Tools - Patron card creator - New card batch - Add at least one patron
2. Export card batch
3. You get a popup with a lovely Windows 95 aesthetic courtesy of greybox.inc, but it is that same form method='post' with the same type='submit' Export button. Click Export and you will POST to patroncards/print.pl and things will work just fine.

Not at all clear to me how opening the popup one way turns the POST to a GET, but it does.

There's a FIXME in patroncards/print.pl saying that the op=cud-export looks sus, which it does. You could try to make the case that it's sensible to POST because in the case where you have selected some but not all of the patrons in a label batch, the number of label_id params in the URL could be so high that it exceeds Apache's max URL length, but then the result of the POST is a link to create the PDF that is the actual "print" export, and that link is a GET which contains all of them with an over-escaped &label_id= param for each one. So a pragmatic fix would be to just change from a POST to a GET, change cud-export to export, and not worry about why called one way it POSTs and called another way it GETs.
Comment 1 Phil Ringnalda 2024-08-15 00:26:34 UTC 
Oh, it's a GET because the entire popup <div> (where I never would have looked for a handler) has an onsubmit handler which cancels the submit, drags the fields out of the form, stuffs them in a query string, and GETs the .action URL with it.

https://git.koha-community.org/Koha-community/Koha/src/commit/52dccfe27beec80003a86f3f73257b87f0118eeb/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt#L204

That's... something.
Comment 2 Phil Ringnalda 2024-08-16 01:19:56 UTC 
Created attachment 170416 [details] [review]
Bug 37614: Printing patron cards from patron lists should be a GET

There's no creating, updating, or deleting about printing patron cards, and
the only time there's a lot of data (selecting individual patrons in a card
creator batch), it doesn't matter if the "Export selected" URL exceeds the
maxlength for Apache since the actual link to create the PDF also will.

Test plan:
 1. Without the patch, Tools - Patron lists - New patron list - Name it and
    Save
 2. Type three characters in the Patron search form (mar works well) to get
    at least three patrons. Click on each of three, then click Add patrons
 3. You can only print cards from the list of lists, so back to Tools -
    Patron lists
 4. In the Actions menu choose Print patron cards, in the popup click Export
 5. Watch the throbber spin around for a while (it will never stop), then
    close the popup, apply the patch, restart_all
 6. Tools - Patron lists - Actions menu - Print patron cards - Export
 7. Click the PDF link, make sure it has all three of your patrons
 8. Tools - Patron card creator - New - Card batch
 9. Without putting anything in the textarea, click Add patrons, in the
    search popup search for your three characters (mar) again, and click
    the checkboxes to the left of three of the names, then Add selected
    patrons, then Close, then Add patrons
10. Click the checkboxes for two of the three patrons, then Export selected
    card(s), then Export in the popup
11. Hover the link to the PDF, verify that it doesn't have stray 'amp;'
    after the & and before label_id= anymore
12. Click the PDF link, verify it has your two patrons
13. Back at the card batch, click Export card batch, then Export
14. Check the PDF to verify it has all three of your patrons
15. Tools - Patron lists - click your list's name to open it
16. Click the card number for a patron, then the Patron lists tab in Checkout
17. Actions menu - Print patron cards - Export, verify the PDF has all three
    patrons
Comment 3 David Nind 2024-08-16 02:52:30 UTC 
Created attachment 170417 [details] [review]
Bug 37614: Printing patron cards from patron lists should be a GET

There's no creating, updating, or deleting about printing patron cards, and
the only time there's a lot of data (selecting individual patrons in a card
creator batch), it doesn't matter if the "Export selected" URL exceeds the
maxlength for Apache since the actual link to create the PDF also will.

Test plan:
 1. Without the patch, Tools - Patron lists - New patron list - Name it and
    Save
 2. Type three characters in the Patron search form (mar works well) to get
    at least three patrons. Click on each of three, then click Add patrons
 3. You can only print cards from the list of lists, so back to Tools -
    Patron lists
 4. In the Actions menu choose Print patron cards, in the popup click Export
 5. Watch the throbber spin around for a while (it will never stop), then
    close the popup, apply the patch, restart_all
 6. Tools - Patron lists - Actions menu - Print patron cards - Export
 7. Click the PDF link, make sure it has all three of your patrons
 8. Tools - Patron card creator - New - Card batch
 9. Without putting anything in the textarea, click Add patrons, in the
    search popup search for your three characters (mar) again, and click
    the checkboxes to the left of three of the names, then Add selected
    patrons, then Close, then Add patrons
10. Click the checkboxes for two of the three patrons, then Export selected
    card(s), then Export in the popup
11. Hover the link to the PDF, verify that it doesn't have stray 'amp;'
    after the & and before label_id= anymore
12. Click the PDF link, verify it has your two patrons
13. Back at the card batch, click Export card batch, then Export
14. Check the PDF to verify it has all three of your patrons
15. Tools - Patron lists - click your list's name to open it
16. Click the card number for a patron, then the Patron lists tab in Checkout
17. Actions menu - Print patron cards - Export, verify the PDF has all three
    patrons

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-08-16 02:57:30 UTC 
I've signed off, however I think step 17 needs fixing.

With the patch applied, when you select Actions > Print patron cards, the export page and results from selecting 'Export' are not in a pop-up window.

This is how it works on main as well. However, I think this needs fixing - maybe as a separate bug if you don't want to fix it here?
Comment 5 Phil Ringnalda 2024-08-16 03:24:05 UTC 
Very much a separate bug, thanks. I'd like to let someone who thinks it's a good idea to have UI to print a whole batch of patron cards while checking out to one patron in the list decide what that UI should look like.
Comment 6 Marcel de Rooy 2024-08-16 06:22:22 UTC 
Created attachment 170422 [details] [review]
Bug 37614: Printing patron cards from patron lists should be a GET

There's no creating, updating, or deleting about printing patron cards, and
the only time there's a lot of data (selecting individual patrons in a card
creator batch), it doesn't matter if the "Export selected" URL exceeds the
maxlength for Apache since the actual link to create the PDF also will.

Test plan:
 1. Without the patch, Tools - Patron lists - New patron list - Name it and
    Save
 2. Type three characters in the Patron search form (mar works well) to get
    at least three patrons. Click on each of three, then click Add patrons
 3. You can only print cards from the list of lists, so back to Tools -
    Patron lists
 4. In the Actions menu choose Print patron cards, in the popup click Export
 5. Watch the throbber spin around for a while (it will never stop), then
    close the popup, apply the patch, restart_all
 6. Tools - Patron lists - Actions menu - Print patron cards - Export
 7. Click the PDF link, make sure it has all three of your patrons
 8. Tools - Patron card creator - New - Card batch
 9. Without putting anything in the textarea, click Add patrons, in the
    search popup search for your three characters (mar) again, and click
    the checkboxes to the left of three of the names, then Add selected
    patrons, then Close, then Add patrons
10. Click the checkboxes for two of the three patrons, then Export selected
    card(s), then Export in the popup
11. Hover the link to the PDF, verify that it doesn't have stray 'amp;'
    after the & and before label_id= anymore
12. Click the PDF link, verify it has your two patrons
13. Back at the card batch, click Export card batch, then Export
14. Check the PDF to verify it has all three of your patrons
15. Tools - Patron lists - click your list's name to open it
16. Click the card number for a patron, then the Patron lists tab in Checkout
17. Actions menu - Print patron cards - Export, verify the PDF has all three
    patrons

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Katrin Fischer 2024-08-16 15:37:45 UTC 
Pushed for 24.11!

Well done everyone, thank you!
Comment 8 Lucas Gass (lukeg) 2024-09-11 16:39:58 UTC 
Backported to 24.05.x for upcoming 24.05.04
Comment 9 Fridolin Somers 2024-10-21 12:05:53 UTC 
Not for 23.11.x