Bug 37612

Summary:	Batch modifying patrons from patron lists broken by CSRF protection
Product:	Koha	Reporter:	Phil Ringnalda <phil>
Component:	Tools	Assignee:	Phil Ringnalda <phil>
Status:	RESOLVED FIXED	QA Contact:	Martin Renvoize (ashimema) <martin.renvoize>
Severity:	major
Priority:	P5 - low	CC:	david, dcook, fridolin.somers, lucas, martin.renvoize
Version:	Main
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37614
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:	This fixes batch editing patrons from a patron list (Tools > Patrons and circulation > Patron lists > Actions > Batch edit patrons). When attempting to batch edit patrons, it didn't load the page to batch edit the patrons, and displayed the message "No patron card numbers or borrowernumbers given." (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:
Bug Depends on:
Bug Blocks:	36192
Attachments:	Bug 37612: Batch patron modification should accept both cud-show and show ops Bug 37612: Batch patron modification should accept both cud-show and show ops Bug 37612: Batch patron modification should accept both cud-show and show ops Bug 37612: Batch patron modification should accept both cud-show and show ops

Description Phil Ringnalda 2024-08-09 18:30:28 UTC

Patron lists have an Actions menuitem to Batch edit patrons, which sends a GET to tools/modborrowers.pl?patron_list_id=1&op=show. But because the other two ways to modborrowers are to upload a file of borrowernumbers or to stick a possibly-huge number of borrowernumbers into a textarea, both of which require a POST, the op is now cud-show, not because it does anything CUD, but because it has to accept POSTs. It just needs the same solution as https://git.koha-community.org/Koha-community/Koha/src/commit/99c2064126978f377f8fb23f61a2db3e201a33c1/tools/batchMod.pl#L209 to accept both cud-show and show.

Comment 1 Phil Ringnalda 2024-08-09 22:59:07 UTC Comment hidden (obsolete)

Created attachment 170223 [details] [review]
Bug 37612: Batch patron modification should accept both cud-show and show ops

Just like with batch item modification, batch patron modification can accept
either a POST of a lot of data, which might be more than Apache's default
URL length limit, or a GET of a little data. Or at least it could, if both
the op 'cud-show' and the op 'show' were accepted. Show isn't doing any
creation or updating or deleting, it just has to be cud-show because it needs
to be able to accept large POSTs. So when it is only getting a little data, it
should be willing to take a GET with op=show just like batch item
modification does.

Test plan:
1. Tools - Patron lists - New patron list - give it a name and Save
2. Type enough characters in the Patron search input to find a patron (I
   like ace for poor often-used Henry Acevedo)
3. Add patrons
4. Return to Patron lists, and in the Actions menu for your list, choose
   Batch edit patrons. Note that the page that loads doesn't show any patrons
   or UI to edit them, only a message about "No patron card numbers or
   borrowernumbers given."
5. Apply patch, restart_all
6. Repeat step 4, but this time get a page with your patron listed, and a
   form to change things about the patron record.

Comment 2 Phil Ringnalda 2024-08-16 16:53:12 UTC

Created attachment 170459 [details] [review]
Bug 37612: Batch patron modification should accept both cud-show and show ops

Just like with batch item modification, batch patron modification can accept
either a POST of a lot of data, which might be more than Apache's default
URL length limit, or a GET of a little data. Or at least it could, if both
the op 'cud-show' and the op 'show' were accepted. Show isn't doing any
creation or updating or deleting, it just has to be cud-show because it needs
to be able to accept large POSTs. So when it is only getting a little data, it
should be willing to take a GET with op=show just like batch item
modification does.

Test plan:
1. Without the patch, Tools - Patron lists - New patron list - give it a
   name and Save
2. Type enough characters in the Patron search input to find a patron (I
   like ace for poor often-used Henry Acevedo) and click on a patron in
   the list of results
3. Click Add patrons
4. Click Patron lists, and in the Actions menu for your list, choose
   Batch edit patrons. Note that the page that loads doesn't show any patrons
   or UI to edit them, only a message about "No patron card numbers or
   borrowernumbers given."
5. Apply patch, restart_all
6. Repeat step 4, but this time get a page with your patron listed, and a
   form to change things about the patron record.

Sponsored-by: Chetco Community Public Library

Comment 3 David Nind 2024-08-17 03:55:53 UTC

Created attachment 170462 [details] [review]
Bug 37612: Batch patron modification should accept both cud-show and show ops

Just like with batch item modification, batch patron modification can accept
either a POST of a lot of data, which might be more than Apache's default
URL length limit, or a GET of a little data. Or at least it could, if both
the op 'cud-show' and the op 'show' were accepted. Show isn't doing any
creation or updating or deleting, it just has to be cud-show because it needs
to be able to accept large POSTs. So when it is only getting a little data, it
should be willing to take a GET with op=show just like batch item
modification does.

Test plan:
1. Without the patch, Tools - Patron lists - New patron list - give it a
   name and Save
2. Type enough characters in the Patron search input to find a patron (I
   like ace for poor often-used Henry Acevedo) and click on a patron in
   the list of results
3. Click Add patrons
4. Click Patron lists, and in the Actions menu for your list, choose
   Batch edit patrons. Note that the page that loads doesn't show any patrons
   or UI to edit them, only a message about "No patron card numbers or
   borrowernumbers given."
5. Apply patch, restart_all
6. Repeat step 4, but this time get a page with your patron listed, and a
   form to change things about the patron record.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>

Comment 4 David Cook 2024-08-19 05:26:02 UTC

This sounds all right.

Comment 5 Martin Renvoize (ashimema) 2024-08-21 07:43:52 UTC

Created attachment 170528 [details] [review]
Bug 37612: Batch patron modification should accept both cud-show and show ops

Just like with batch item modification, batch patron modification can accept
either a POST of a lot of data, which might be more than Apache's default
URL length limit, or a GET of a little data. Or at least it could, if both
the op 'cud-show' and the op 'show' were accepted. Show isn't doing any
creation or updating or deleting, it just has to be cud-show because it needs
to be able to accept large POSTs. So when it is only getting a little data, it
should be willing to take a GET with op=show just like batch item
modification does.

Test plan:
1. Without the patch, Tools - Patron lists - New patron list - give it a
   name and Save
2. Type enough characters in the Patron search input to find a patron (I
   like ace for poor often-used Henry Acevedo) and click on a patron in
   the list of results
3. Click Add patrons
4. Click Patron lists, and in the Actions menu for your list, choose
   Batch edit patrons. Note that the page that loads doesn't show any patrons
   or UI to edit them, only a message about "No patron card numbers or
   borrowernumbers given."
5. Apply patch, restart_all
6. Repeat step 4, but this time get a page with your patron listed, and a
   form to change things about the patron record.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Comment 6 Katrin Fischer 2024-08-22 14:47:40 UTC

Pushed for 24.11!

Well done everyone, thank you!

Comment 7 Lucas Gass (lukeg) 2024-09-12 22:13:58 UTC

Backported to 24.05.x for upcoming 24.05.04

Comment 8 Fridolin Somers 2024-10-21 12:32:22 UTC

Not for 23.11.x

Comment 9 David Nind 2024-10-21 18:01:10 UTC

This fixes a bug - no interface changes, so no updates to the manual required.