Bug 37765 - Fix forms that POST without an op in systempreferences
Summary: Fix forms that POST without an op in systempreferences
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P3 normal
Assignee: Phil Ringnalda
QA Contact: Julian Maurice
URL:
Keywords:
Depends on: 36192
Blocks: 37728
  Show dependency treegraph
 
Reported: 2024-08-29 02:56 UTC by Phil Ringnalda
Modified: 2024-09-13 10:07 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37765: Fix forms that POST without an op in systemprefernces (4.23 KB, patch)
2024-08-29 03:10 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 37765: Fix forms that POST without an op in systemprefernces (4.28 KB, patch)
2024-08-29 04:06 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 37765: Fix forms that POST without an op in systemprefernces (4.35 KB, patch)
2024-08-29 09:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37765: Fix forms that POST without an op in systemprefernces (4.41 KB, patch)
2024-09-06 11:51 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Phil Ringnalda 2024-08-29 02:56:18 UTC
There are two forms that have action="post" but do not send an op (so it can't be checked for whether its value starts with 'cud-') in systempreferences. Because of bug 37728, they weren't caught earlier.

One is simple, the "No, do not delete" button while starting to delete a local preference. It doesn't need to post, and changing it to a get has no behavior change.

The other is slightly more complicated: the "Yes, delete" button is supposed to then show a page saying "Data deleted" with a button to take you "Back to system preferences" but changing the op to cud-delete_confirmed left the part of the template unused, because it still thinks that delete_confirmed will be true when it needs to show that. Once the script sets delete_confirmed, the "Back to system preferences" button can just be a get.
Comment 1 Phil Ringnalda 2024-08-29 03:10:27 UTC
Created attachment 170853 [details] [review]
Bug 37765: Fix forms that POST without an op in systemprefernces

We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

The two in systempreferences are the button to cancel deleting a local
preference, which can be fixed with no visible change, and the button to
return to the preferences list after being told that your requested deletion
has been done, which makes a visible change because right now, the whole page
that tells you the preference was deleted doesn't show at all.

Test plan:
 1. Without the patch, Administration - System preferences - Local use (in the
    left sidebar)
 2. New system preference - Explanation and Variable are required, so make
    them both Trash and Save
 3. In the row for your new preference, click the Delete button
 4. In the confirmation page, click the No, do not delete button
 5. You'll be taken back to the list of Local use preferences. That's the
    behavior that you want to see unchanged after the patch
 6. Click the Delete button for your preference again, but this time click
    Yes, delete
 7. You'll be taken to a blank page with no category of preferences selected
    or listed. That's the behavior that you want to see change with the patch
 8. Apply patch, restart_all
 9. Administration - System preferences - Local use - New system preference -
    'Trash' for both Explanation and Variable - Save
10. In the row for the new preference, click the Delete button
11. In the confirmation page, click No, do not delete
12. Verify that it returns you to the list of Local use preferences just like
    before
13. Click Delete again, but this time click Yes, delete
14. Now you should get a page saying "Data deleted" with a Back to system
    preferences button. Click that button, you should return to the list
    of Local use preferences, with your Trash preference gone
Comment 2 Phil Ringnalda 2024-08-29 04:06:38 UTC
Created attachment 170854 [details] [review]
Bug 37765: Fix forms that POST without an op in systemprefernces

We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

The two in systempreferences are the button to cancel deleting a local
preference, which can be fixed with no visible change, and the button to
return to the preferences list after being told that your requested deletion
has been done, which makes a visible change because right now, the whole page
that tells you the preference was deleted doesn't show at all.

Test plan:
 1. Without the patch, Administration - System preferences - Local use (in the
    left sidebar)
 2. New system preference - Explanation and Variable are required, so make
    them both Trash and Save
 3. In the row for your new preference, click the Delete button
 4. In the confirmation page, click the No, do not delete button
 5. You'll be taken back to the list of Local use preferences. That's the
    behavior that you want to see unchanged after the patch
 6. Click the Delete button for your preference again, but this time click
    Yes, delete
 7. You'll be taken to a blank page with no category of preferences selected
    or listed. That's the behavior that you want to see change with the patch
 8. Apply patch, restart_all
 9. Administration - System preferences - Local use - New system preference -
    'Trash' for both Explanation and Variable - Save
10. In the row for the new preference, click the Delete button
11. In the confirmation page, click No, do not delete
12. Verify that it returns you to the list of Local use preferences just like
    before
13. Click Delete again, but this time click Yes, delete
14. Now you should get a page saying "Data deleted" with a Back to system
    preferences button. Click that button, you should return to the list
    of Local use preferences, with your Trash preference gone

Sponsored-by: Chetco Community Public Library
Comment 3 Jonathan Druart 2024-08-29 09:36:49 UTC
Created attachment 170856 [details] [review]
Bug 37765: Fix forms that POST without an op in systemprefernces

We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

The two in systempreferences are the button to cancel deleting a local
preference, which can be fixed with no visible change, and the button to
return to the preferences list after being told that your requested deletion
has been done, which makes a visible change because right now, the whole page
that tells you the preference was deleted doesn't show at all.

Test plan:
 1. Without the patch, Administration - System preferences - Local use (in the
    left sidebar)
 2. New system preference - Explanation and Variable are required, so make
    them both Trash and Save
 3. In the row for your new preference, click the Delete button
 4. In the confirmation page, click the No, do not delete button
 5. You'll be taken back to the list of Local use preferences. That's the
    behavior that you want to see unchanged after the patch
 6. Click the Delete button for your preference again, but this time click
    Yes, delete
 7. You'll be taken to a blank page with no category of preferences selected
    or listed. That's the behavior that you want to see change with the patch
 8. Apply patch, restart_all
 9. Administration - System preferences - Local use - New system preference -
    'Trash' for both Explanation and Variable - Save
10. In the row for the new preference, click the Delete button
11. In the confirmation page, click No, do not delete
12. Verify that it returns you to the list of Local use preferences just like
    before
13. Click Delete again, but this time click Yes, delete
14. Now you should get a page saying "Data deleted" with a Back to system
    preferences button. Click that button, you should return to the list
    of Local use preferences, with your Trash preference gone

Sponsored-by: Chetco Community Public Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Julian Maurice 2024-09-06 11:51:06 UTC
Created attachment 171133 [details] [review]
Bug 37765: Fix forms that POST without an op in systemprefernces

We intend not to have forms with method="post" without an op variable (so we
can check that the op starts with "cud-" as part of the CSRF protection), but
because of bug 37728 some were missed.

The two in systempreferences are the button to cancel deleting a local
preference, which can be fixed with no visible change, and the button to
return to the preferences list after being told that your requested deletion
has been done, which makes a visible change because right now, the whole page
that tells you the preference was deleted doesn't show at all.

Test plan:
 1. Without the patch, Administration - System preferences - Local use (in the
    left sidebar)
 2. New system preference - Explanation and Variable are required, so make
    them both Trash and Save
 3. In the row for your new preference, click the Delete button
 4. In the confirmation page, click the No, do not delete button
 5. You'll be taken back to the list of Local use preferences. That's the
    behavior that you want to see unchanged after the patch
 6. Click the Delete button for your preference again, but this time click
    Yes, delete
 7. You'll be taken to a blank page with no category of preferences selected
    or listed. That's the behavior that you want to see change with the patch
 8. Apply patch, restart_all
 9. Administration - System preferences - Local use - New system preference -
    'Trash' for both Explanation and Variable - Save
10. In the row for the new preference, click the Delete button
11. In the confirmation page, click No, do not delete
12. Verify that it returns you to the list of Local use preferences just like
    before
13. Click Delete again, but this time click Yes, delete
14. Now you should get a page saying "Data deleted" with a Back to system
    preferences button. Click that button, you should return to the list
    of Local use preferences, with your Trash preference gone

Sponsored-by: Chetco Community Public Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 5 Katrin Fischer 2024-09-13 10:07:42 UTC
Pushed for 24.11!

Well done everyone, thank you!