Bug 37111 - OPAC renewal - CSRF "op must be set"
Summary: OPAC renewal - CSRF "op must be set"
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Lucas Gass (lukeg)
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on: 34478 36246
Blocks: 36192
  Show dependency treegraph
 
Reported: 2024-06-18 08:14 UTC by David Nind
Modified: 2024-11-14 14:37 UTC (History)
7 users (show)

See Also:
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:


Attachments
Bug 37111: Turn Renew link into a POST form (2.94 KB, patch)
2024-06-18 17:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37111: Add submit-form asset to OPAC assets (1.75 KB, patch)
2024-06-18 19:38 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37111: Fix renew link on opac-user.tt (2.55 KB, patch)
2024-06-18 19:38 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37111: Add submit-form asset to OPAC assets (1.80 KB, patch)
2024-06-18 20:00 UTC, David Nind
Details | Diff | Splinter Review
Bug 37111: Fix renew link on opac-user.tt (2.60 KB, patch)
2024-06-18 20:00 UTC, David Nind
Details | Diff | Splinter Review
Bug 37111: Add submit-form asset to OPAC assets (1.86 KB, patch)
2024-06-20 07:46 UTC, Alexander Wagner
Details | Diff | Splinter Review
Bug 37111: Fix renew link on opac-user.tt (2.65 KB, patch)
2024-06-20 07:47 UTC, Alexander Wagner
Details | Diff | Splinter Review
Bug 37111: (follow-up) data attribute names should be lowercase (1.19 KB, patch)
2024-06-25 21:42 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 37111: Add submit-form asset to OPAC assets (1.92 KB, patch)
2024-07-18 17:19 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37111: Fix renew link on opac-user.tt (2.71 KB, patch)
2024-07-18 17:19 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 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.