To recreate: 1. Add a list 2. Go to that list ( virtualshelves/shelves.pl?op=view&shelfnumber=X ) 3. From the toolbar click the 'Edit' dropdown. 4. From the dropdown try either 'Edit list' or 'Delete list'. 5. CSRF error
Created attachment 154926 [details] [review] Bug 34650: Add CSRF token to edit/delete toolbar links To test: 1. Add a list 2. Go to that list ( virtualshelves/shelves.pl?op=view&shelfnumber=X ) 3. From the toolbar click the 'Edit' dropdown. 4. From the dropdown try either 'Edit list' or 'Delete list'. 5. CSRF error 6. APPLY PATCH 7. Try 2 - 4 again, no more error.
Technically, we should be looking at changing these GETs to POSTs, although I don't know if we have at-hand styling to do that. I will take a quick look to see if I can do a little alternative. If not, I'll sign off...
op=edit_form shouldn't need a CSRF token either since it's not state changing...
(In reply to David Cook from comment #3) > op=edit_form shouldn't need a CSRF token either since it's not state > changing... I mean the previous change wasn't correct.
Created attachment 154933 [details] [review] Bug 34650: Remove unnecessary CSRF check on edit_form The op "edit_form" doesn't change state. It just renders the edit form. Therefore, it doesn't need a CSRF token/check.
Created attachment 154934 [details] [review] Bug 34650: Convert list toolbar delete into form POST This patch adds a HTML form with a CSRF token to POST the list delete, which is triggered by a click handler on the A element. The A element is still needed for existing style reasons. Test plan: 0. Apply patch 1. koha-plack --reload kohadev 2. In the staff interface, add a list 3. Go into that list (e.g. virtualshelves/shelves.pl?op=view&shelfnumber=X) 4. From the toolbar click the "Edit" dropdown 5. From the dropdown try either "Edit list" or "Delete list" 6. Note no CSRF error and operation completes as expected
Sorry for taking this one over, Lucas, but I think this is a better way to go.
(In reply to David Cook from comment #7) > Sorry for taking this one over, Lucas, but I think this is a better way to > go. No problem, thanks!
Created attachment 154955 [details] [review] Bug 34650: Remove unnecessary CSRF check on edit_form The op "edit_form" doesn't change state. It just renders the edit form. Therefore, it doesn't need a CSRF token/check. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 154956 [details] [review] Bug 34650: Convert list toolbar delete into form POST This patch adds a HTML form with a CSRF token to POST the list delete, which is triggered by a click handler on the A element. The A element is still needed for existing style reasons. Test plan: 0. Apply patch 1. koha-plack --reload kohadev 2. In the staff interface, add a list 3. Go into that list (e.g. virtualshelves/shelves.pl?op=view&shelfnumber=X) 4. From the toolbar click the "Edit" dropdown 5. From the dropdown try either "Edit list" or "Delete list" 6. Note no CSRF error and operation completes as expected Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Looking here
Created attachment 155089 [details] [review] Bug 34650: Remove unnecessary CSRF check on edit_form The op "edit_form" doesn't change state. It just renders the edit form. Therefore, it doesn't need a CSRF token/check. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 155090 [details] [review] Bug 34650: Convert list toolbar delete into form POST This patch adds a HTML form with a CSRF token to POST the list delete, which is triggered by a click handler on the A element. The A element is still needed for existing style reasons. Test plan: 0. Apply patch 1. koha-plack --reload kohadev 2. In the staff interface, add a list 3. Go into that list (e.g. virtualshelves/shelves.pl?op=view&shelfnumber=X) 4. From the toolbar click the "Edit" dropdown 5. From the dropdown try either "Edit list" or "Delete list" 6. Note no CSRF error and operation completes as expected Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.11. Nice work everyone, thanks!
Would it be possible to backport this to oldstable and oldoldstable? Thanks, all!
(In reply to Elise Konya from comment #15) > Would it be possible to backport this to oldstable and oldoldstable? Thanks, > all! +1
Pushed to 23.05.x for 23.05.04
Nice work everyone! Pushed to oldstable for 22.11.x