Bug 40061

Summary: Cannot delete image from patron card creator
Product: Koha Reporter: Caroline Cyr La Rose <caroline.cyr-la-rose>
Component: Label/patron card printingAssignee: Owen Leonard <oleonard>
Status: Pushed to stable --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: m.de.rooy
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.01
Circulation function:
Attachments: Bug 40061: Add CSRF protection to patron image deletion
Bug 40061: (follow-up) Delete last image using checkbox
Bug 40061: Add CSRF protection to patron image deletion
Bug 40061: (follow-up) Delete last image using checkbox
Bug 40061: Add CSRF protection to patron image deletion
Bug 40061: (follow-up) Delete last image using checkbox

Description Caroline Cyr La Rose 2025-06-03 16:41:25 UTC
I noticed this in 24.05, but it also happens in KTD on main.

When you upload an image in the patron card creator, if you try to delete it, it returns an error.

1. Go to Tools > Patron card creator
2. Click New > Image
3. Upload an image (I used a png with transparent background, hence why I had to delete it after, but I also tried with a jpg with the same result)
4. Enter a name for the image (I tried "Logo Bibliothèque" as well as a name all in lowercase letters without spaces or diacritics "tomhanks" with the same result)
5. Click Upload
6. Click Delete on the right-hand side
--> Error 
"WARNING: An error has occurred. Please have your system administrator check the error log for details."

plack-intranet-error.log says
[2025/06/03 16:40:36] [WARN] No image ids passed in to delete. at /kohadevbox/koha/patroncards/image-manage.pl line 131.
Comment 1 Owen Leonard 2025-06-04 11:48:24 UTC
Created attachment 182946 [details] [review]
Bug 40061: Add CSRF protection to patron image deletion

The patron image deletion operation was not fully updated in Bug 34478.
The individual "Delete" buttons in the table of images need to be
updated to use form-submit.js

Unrelated: I removed an invalid "align" attribute from a <td>.

To test, apply the patch and go to Tools -> Patron card creator -> Image
manager.

- Upload multiple images
- Test the individual "Delete" buttons in the table of images.
  - Test that both the "OK" and "Cancel" operations work correctly.
- Test the process of checking one or more checkboxes and clicking the
  "Delete selected" button.
   - Test that both the "OK" and "Cancel" operations work correctly.

Sponsored-by: Athens County Public Libraries
Comment 2 Caroline Cyr La Rose 2025-06-04 13:16:40 UTC
Thanks Owen! The Delete button works now.

I know it wasn't part of the original bug report, but I also tested the checkbox + Delete selected button, and for some reason, when I try to delete the last image this way, it's not deleted...?

If there is one image, and you check it and click Delete selected, it's not deleted. If there is more than one and you check one or more, they are deleted.

Is it the same problem?
Comment 3 Owen Leonard 2025-06-04 18:21:28 UTC
Created attachment 182957 [details] [review]
Bug 40061: (follow-up) Delete last image using checkbox

This followup corrects a problem in image deletion which isn't really
the same issue (it's not CSRF related) but is another problem with
deletion. The JS for grabbing all the checkboxes by name wasn't working
when there is only one checkbox.

To test, apply the patch and delete all but one image from the patron
card creator image manager.

- Check the checkbox for the last image and click the "Delete selected"
  button.
- The image should be deleted successfully.

Sponsored-by: Athens County Public Libraries
Comment 4 Owen Leonard 2025-06-04 18:22:05 UTC
(In reply to Caroline Cyr La Rose from comment #2)
> Is it the same problem?

Technically not the same problem but it makes sense to me to include the fix here.
Comment 5 Caroline Cyr La Rose 2025-06-04 18:32:01 UTC
Created attachment 182959 [details] [review]
Bug 40061: Add CSRF protection to patron image deletion

The patron image deletion operation was not fully updated in Bug 34478.
The individual "Delete" buttons in the table of images need to be
updated to use form-submit.js

Unrelated: I removed an invalid "align" attribute from a <td>.

To test, apply the patch and go to Tools -> Patron card creator -> Image
manager.

- Upload multiple images
- Test the individual "Delete" buttons in the table of images.
  - Test that both the "OK" and "Cancel" operations work correctly.
- Test the process of checking one or more checkboxes and clicking the
  "Delete selected" button.
   - Test that both the "OK" and "Cancel" operations work correctly.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 6 Caroline Cyr La Rose 2025-06-04 18:32:03 UTC
Created attachment 182960 [details] [review]
Bug 40061: (follow-up) Delete last image using checkbox

This followup corrects a problem in image deletion which isn't really
the same issue (it's not CSRF related) but is another problem with
deletion. The JS for grabbing all the checkboxes by name wasn't working
when there is only one checkbox.

To test, apply the patch and delete all but one image from the patron
card creator image manager.

- Check the checkbox for the last image and click the "Delete selected"
  button.
- The image should be deleted successfully.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 7 Caroline Cyr La Rose 2025-06-04 18:32:26 UTC
Perfect! Thank you!
Comment 8 Marcel de Rooy 2025-06-06 06:46:22 UTC
Created attachment 183039 [details] [review]
Bug 40061: Add CSRF protection to patron image deletion

The patron image deletion operation was not fully updated in Bug 34478.
The individual "Delete" buttons in the table of images need to be
updated to use form-submit.js

Unrelated: I removed an invalid "align" attribute from a <td>.

To test, apply the patch and go to Tools -> Patron card creator -> Image
manager.

- Upload multiple images
- Test the individual "Delete" buttons in the table of images.
  - Test that both the "OK" and "Cancel" operations work correctly.
- Test the process of checking one or more checkboxes and clicking the
  "Delete selected" button.
   - Test that both the "OK" and "Cancel" operations work correctly.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2025-06-06 06:46:28 UTC
Created attachment 183040 [details] [review]
Bug 40061: (follow-up) Delete last image using checkbox

This followup corrects a problem in image deletion which isn't really
the same issue (it's not CSRF related) but is another problem with
deletion. The JS for grabbing all the checkboxes by name wasn't working
when there is only one checkbox.

To test, apply the patch and delete all but one image from the patron
card creator image manager.

- Check the checkbox for the last image and click the "Delete selected"
  button.
- The image should be deleted successfully.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Lucas Gass (lukeg) 2025-06-06 15:46:33 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 11 Paul Derscheid 2025-06-22 21:47:28 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03