Bug 37111

Summary: OPAC renewal - CSRF "op must be set"
Product: Koha Reporter: David Nind <david>
Component: OPACAssignee: Lucas Gass (lukeg) <lucas>
Status: RESOLVED FIXED QA Contact: Kyle M Hall (khall) <kyle>
Severity: major    
Priority: P5 - low CC: alexander.wagner, fridolin.somers, kyle, lucas, martin.renvoize, nick, phil
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36246
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
This fixes an error that occurs when patron's attempt to renew items from their OPAC account (Your account > Summary). The error was related to the CSRF changes to improve the security for forms added Koha 24.05.
Version(s) released in:
24.11.00,24.05.04
Circulation function:
Bug Depends on: 34478, 36246    
Bug Blocks: 36192    
Attachments: Bug 37111: Turn Renew link into a POST form
Bug 37111: Add submit-form asset to OPAC assets
Bug 37111: Fix renew link on opac-user.tt
Bug 37111: Add submit-form asset to OPAC assets
Bug 37111: Fix renew link on opac-user.tt
Bug 37111: Add submit-form asset to OPAC assets
Bug 37111: Fix renew link on opac-user.tt
Bug 37111: (follow-up) data attribute names should be lowercase
Bug 37111: Add submit-form asset to OPAC assets
Bug 37111: Fix renew link on opac-user.tt

Description David Nind 2024-06-18 08:14:29 UTC
In the OPAC, when a patron clicks the "renew" link for an item they get an error:

op must be set at /kohadevbox/koha/opac/opac-renew.pl line 36

    in (eval) at /kohadevbox/koha/opac/opac-renew.pl line 36

       33: my $query = CGI->new;
       34: my $op = $query->param('op') || q{};
       35: 
       36: die "op must be set" unless $op eq 'cud-renew';
...

To replicate (using KTD):
1. Check some items out to a patron
2. Set the username and apssword for the patron so that you can log in as that patron.
3. Log in to the OAPC as that patron.
4. Go to Your account > Summary (the default landing page after you log in).
5. Click "Renew" for one of the items.
6. You get the error as above.
Comment 1 Alexander Wagner 2024-06-18 14:07:52 UTC
It seems the error results from l.456 in [`opac.tt`](https://github.com/Koha-Community/Koha/blob/8ab3989654b900cef2954c469b12ddce4375c9b8/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt#L456) where the URL generated does not add the missing parameter.
Comment 2 Lucas Gass (lukeg) 2024-06-18 17:15:41 UTC
Created attachment 167860 [details] [review]
Bug 37111: Turn Renew link into a POST form

To test:
1. Check some items out to a patron
2. Set the username and apssword for the patron so that you can log in as that patron.
3. Log in to the OAPC as that patron.
4. Go to Your account > Summary (the default landing page after you log in).
5. Click "Renew" for one of the items.
6. You get the error as above.
7. APPLY PATCH
8. Try steps 1 -5 again, you should not get an error.
Comment 3 Lucas Gass (lukeg) 2024-06-18 17:23:32 UTC
I think the link to renew needs to be a POST form.
Comment 4 David Nind 2024-06-18 18:20:15 UTC
Thanks Lucas for looking at this so quickly!

If I tick the box and select "Renew selected" above the table header or at the bottom of the table, it works as expected. (No change from before the patch.)

If I select "Renew all" above the table header or at the bottom of the table, I get a pop-up message, "Nothing has been selected. Check the box for each item you want to renew". (This was working before the patch.)

If I click the "Renew" button for the individual item, I'm still getting the error.

Also, is it possible to leave "Renew" in the table for an individual item as a text link? It looks quite big and out of place at the moment.
Comment 5 Owen Leonard 2024-06-18 18:50:45 UTC
The whole table is wrapped in a <form> so you can't put individual forms inside the table. I think our most common solution for that is to but the "batch" form after the table and use JS to populate the selections.
Comment 6 Nick Clemens (kidclamp) 2024-06-18 19:01:11 UTC
(In reply to Owen Leonard from comment #5)
> The whole table is wrapped in a <form> so you can't put individual forms
> inside the table. I think our most common solution for that is to but the
> "batch" form after the table and use JS to populate the selections.

We now have a method to submit a link as a form on bug 36246
Comment 7 Lucas Gass (lukeg) 2024-06-18 19:38:23 UTC
Created attachment 167866 [details] [review]
Bug 37111: Add submit-form asset to OPAC assets
Comment 8 Lucas Gass (lukeg) 2024-06-18 19:38:24 UTC
Created attachment 167867 [details] [review]
Bug 37111: Fix renew link on opac-user.tt

To test:
1. Check some items out to a patron
2. Set the username and apssword for the patron so that you can log in as that patron.
3. Log in to the OAPC as that patron.
4. Go to Your account > Summary (the default landing page after you log in).
5. Click "Renew" for one of the items.
6. You get the error as above.
7. APPLY PATCH
8. Try steps 1 -5 again, you should not get an error.
Comment 9 Lucas Gass (lukeg) 2024-06-18 19:38:58 UTC
Thanks everyone, new patchset submitted.
Comment 10 David Nind 2024-06-18 20:00:41 UTC
Created attachment 167869 [details] [review]
Bug 37111: Add submit-form asset to OPAC assets

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2024-06-18 20:00:43 UTC
Created attachment 167870 [details] [review]
Bug 37111: Fix renew link on opac-user.tt

To test:
1. Check some items out to a patron
2. Set the username and apssword for the patron so that you can log in as that patron.
3. Log in to the OAPC as that patron.
4. Go to Your account > Summary (the default landing page after you log in).
5. Click "Renew" for one of the items.
6. You get the error as above.
7. APPLY PATCH
8. Try steps 1 -5 again, you should not get an error.

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Alexander Wagner 2024-06-20 07:46:59 UTC
Created attachment 167921 [details] [review]
Bug 37111: Add submit-form asset to OPAC assets

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexander Wagner <alexander.wagner@desy.de>
Comment 13 Alexander Wagner 2024-06-20 07:47:02 UTC
Created attachment 167922 [details] [review]
Bug 37111: Fix renew link on opac-user.tt

To test:
1. Check some items out to a patron
2. Set the username and apssword for the patron so that you can log in as that patron.
3. Log in to the OAPC as that patron.
4. Go to Your account > Summary (the default landing page after you log in).
5. Click "Renew" for one of the items.
6. You get the error as above.
7. APPLY PATCH
8. Try steps 1 -5 again, you should not get an error.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexander Wagner <alexander.wagner@desy.de>
Comment 14 Alexander Wagner 2024-06-20 07:47:30 UTC
Thanks for the quick fix! :)
Comment 15 Lucas Gass (lukeg) 2024-06-25 21:42:10 UTC
Created attachment 168103 [details] [review]
Bug 37111: (follow-up) data attribute names should be lowercase

See Bug 37190

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 16 Katrin Fischer 2024-06-27 14:58:01 UTC
Adding RM_Priority - please QA as this affects end users.
Comment 17 Lucas Gass (lukeg) 2024-06-27 15:05:04 UTC
This won't work as is because Bug 37190 was RESOLVED INVALID. I will make the necessary changes.
Comment 18 Phil Ringnalda 2024-07-08 02:48:04 UTC
Isn't the necessary change just to obsolete your followup patch? I applied the first two patches, added a data-confirmation-msg to the link, and it worked perfectly, submit-form.js saw it as confirmationMsg and asked the question and acted appropriately.
Comment 19 Kyle M Hall (khall) 2024-07-18 17:16:02 UTC
(In reply to Phil Ringnalda (back in August) from comment #18)
> Isn't the necessary change just to obsolete your followup patch? I applied
> the first two patches, added a data-confirmation-msg to the link, and it
> worked perfectly, submit-form.js saw it as confirmationMsg and asked the
> question and acted appropriately.

I've having no issues with or without the followup. It seems better to leave it off in that case.
Comment 20 Kyle M Hall (khall) 2024-07-18 17:19:15 UTC
Created attachment 169157 [details] [review]
Bug 37111: Add submit-form asset to OPAC assets

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexander Wagner <alexander.wagner@desy.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Kyle M Hall (khall) 2024-07-18 17:19:22 UTC
Created attachment 169158 [details] [review]
Bug 37111: Fix renew link on opac-user.tt

To test:
1. Check some items out to a patron
2. Set the username and apssword for the patron so that you can log in as that patron.
3. Log in to the OAPC as that patron.
4. Go to Your account > Summary (the default landing page after you log in).
5. Click "Renew" for one of the items.
6. You get the error as above.
7. APPLY PATCH
8. Try steps 1 -5 again, you should not get an error.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexander Wagner <alexander.wagner@desy.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Martin Renvoize (ashimema) 2024-07-22 06:37:57 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 23 Lucas Gass (lukeg) 2024-08-28 19:37:26 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 24 Fridolin Somers 2024-09-10 11:42:39 UTC
Depends on Bug 34478 not in 23.11.x
Comment 25 David Nind 2024-11-10 18:25:20 UTC
Bug fix, no change to UI, no changes required to the documentation.