Replace GET by POST
These functions should be sent as a POST to pfSense software, which should then do a redirect back to the status page. Going back through your browser history should not bring down your tunnels! Thanks.
#2 Updated by Renato Botelho over 2 years ago
- Subject changed from IPSec disconnect and reconnect should be POST not GET to Replace GET by POST
- Target version changed from 2.2 to 2.2.1
This is a change that needs to be done globally, replace this ticket to a more general description to keep track of it and postpone since it's too late for 2.2
#6 Updated by Fernando Munoz almost 2 years ago
Is there any ETA for 2.3? it seems the target for this has been moving from version since almost one year ago, that doesn't look good. There are two CSRF issues that I care about right now:
First one allows deleting VLANs and the second one allows stopping services.
#11 Updated by Jim Pingle over 1 year ago
Steve Beaver wrote:
status_services.php also converted
The main body of status_services.php is converted but it looks like the shortcut bar service controls, openvpn status, and the widget are still using it via GET. I don't see any other pages/places that are using GET to control services, so if a solution can be found for those that would wrap up that page completely.
Shortcut bar service control: source:src/usr/local/www/head.inc#L480
OpenVPN status: source:src/usr/local/www/status_openvpn.php#L180
All three use get_service_control_GET_links() from source:src/etc/inc/service-utils.inc#L488
Perhaps hitting it via GET could land at a confirmation page, similar to what happens for a package install.
#14 Updated by Jim Pingle 6 months ago
- Target version changed from Future to 2.4.0
The recent work here is excellent, for delete, enable/disable, and other actions that result in a config change or firewall state change, everything I've tried works wonderfully.
Using POST to display edit forms (e.g. act=new, act=edit, act=dup and similar) seems to have a couple negative effects though.
- It breaks breadcrumb links to the current page (leads to an empty item)
- It breaks refreshing the edit page to revert to the previous saved values
- It removes the ability to bookmark a specific item from the list
Looks like if we set 'id' using $_REQUEST in most cases we could use GET (or both) to display the edit forms and POST for actions that result in a config or state change.
#16 Updated by Phillip Davis 6 months ago
Isn't the principle here that anything that changes stuff (makes a config change, stops/starts a service, applies changes...) should not be able to be done via direct URL parameters (i.e. REQUEST/GET). That avoids users accidentally re-doing actions.
Things that are just queries (load up a page with current settings of something, load up an empty page for a new entry, display status...) can be done directly with URL parameters. That makes it easy to get the data (e.g. if testing, refreshing pages...) and for the code to know what was requested based on the URL parameters.
#18 Updated by Phillip Davis 6 months ago
Note issue https://redmine.pfsense.org/issues/7254
The dropdown list of interface names in Firewall Rules was still doing the old "ordinary" code to send "?if=opt1" type of request, but the code that decides what to do is no longer looking at that, it expects a $_POSTed value.
PR https://github.com/pfsense/pfsense/pull/3533 adds code to make that do $_POST based on the selected interface from the list.
But as Jim has commented, this approach in general means that users cannot bookmark a link to a particular interface, and the breadcrumb links would have to somehow be made smarter...
#20 Updated by Steve Beaver 6 months ago
- Status changed from Assigned to Feedback
- No packages have been converted
- The Traffic shaper has not been converted due to the complexity and risk of that task.