Opened 5 years ago
Last modified 5 years ago
#47954 new enhancement
Create a sanity check for siteurl and home changes
Reported by: | Otto42 | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-patch |
Focuses: | ui, administration | Cc: |
Description (last modified by )
One of the most common problems is when people change the site URL fields on the settings general screen, without full understanding of what it means. It's a common enough problem that we've had documentation on ways to fix it for ages:
https://wordpress.org/support/article/changing-the-site-url/
The idea is to enhance this field so as to perform some kind of sanity check before actually changing the fields. Health check exists, we have checks for plugin and theme changes to some degree. This could be checked as well.
Concept: Change the field, make an internal request, see if it fails, if so change them back, then warn the user with a "hey, this change seems like it's gonna break things real bad, are you really sure?" Or equivalent.
Second thought: when these change, email the admin user about them, with a revert link that will work no matter what.
I'm sure that other ideas exist as well.
Attachments (2)
Change History (14)
#3
@
5 years ago
This is interesting. IMHO we can do both of the approach @Otto42 mentioned above.
Change the field, make an internal request, see if it fails, if so change them back, then warn the user with a "hey, this change seems like it's gonna break things real bad, are you really sure?" Or equivalent.
At the top of my head, maybe we can try to use the constants WP_HOME
and WP_SITEURL
when doing the internal request. The request can be simple, maybe just see if both front-end and wp-admin both sends back 200 HTTP?
I'll try to create an initial patch when time permits.
#4
@
5 years ago
Here's an initial code that should perform the second approach @Otto42 suggested.
Second thought: when these change, email the admin user about them, with a revert link that will work no matter what.
Feel free to try this code by adding in your theme's functions.php
. It still needs unit testing and putting the code to a much suited place. I'll try to put it as a patch later.
<?php /** * Setup restore plan to give the admin a way to revert the 'siteurl' and/or 'home' changes. * * This function performs the following tasks. * 1. Creates a 'siteurl_restore_key' transient. * 2. Save the old 'siteurl' and 'home' options as transients. * 3. Email the admin the regarding the change and give a restore link. * * Known Issue / Can be enhanced. * 1. The email action part is firing twice if both 'siteurl' and 'home' is updated in * 'wp-admin/options-general.php' at the same time. * 2. Provide a filter for the email subject and message. * * @since 5.3.0 * * @param string $option * @param string $old_value * @param string $value */ function setup_siteurl_restore( $option, $old_value, $value ) { if ( 'siteurl' === $option || 'home' === $option ) { $restore_key = wp_generate_password( 12, false ); set_transient( 'siteurl_restore_key', $restore_key, 1800 ); set_transient( "old_{$option}", $old_value, 1800 ); $restore_url = add_query_arg( [ 'srk' => $restore_key, ], $old_value ); wp_mail( get_option('admin_email'), 'Your WordPress site url was changed.', "You can undo this change by clicking this link {$restore_url}" ); } } add_action( 'updated_option', 'setup_siteurl_restore', 10, 3 ); /** * Perform the siteurl restore operation. * * Note: I only hook this function in 'registered_taxonomy' because I think the sooner this * function is fired, the better. * * @since 5.3.0 */ function perform_siteurl_restore() { if ( isset( $_GET['srk'] ) && ! empty( $_GET['srk'] ) ) { $restore_key = get_transient( 'siteurl_restore_key' ); if ( $_GET['srk'] === $restore_key ) { $old_siteurl = get_transient( 'old_siteurl' ); $old_home = get_transient( 'old_home' ); $is_restore_success = false; if ( $old_siteurl ) { $update_siteurl = update_option( 'siteurl', $old_siteurl ); if ( $update_siteurl ) { delete_transient( 'old_siteurl' ); $is_restore_success = true; } } if ( $old_home ) { $update_home = update_option( 'home', $old_home ); if ( $update_home ) { delete_transient( 'old_home' ); $is_restore_success = true; } } if ( $is_restore_success ) { delete_transient( 'siteurl_restore_key' ); // Track success. set_transient( 'siteurl_restore_success', '1', 300 ); $restored_site_admin_url = trailingslashit( get_option( 'home' ) ) . 'wp-admin'; // Success redirect url. $success_redirect_url = add_query_arg( 'srsuccess', '1', $restored_site_admin_url ); wp_redirect( $success_redirect_url ); exit; } } else { wp_die( __( 'Restore key is invalid.' ) ); exit; } } } add_action( 'registered_taxonomy', 'perform_siteurl_restore' ); /** * Display success notice if the restore siteurl operation is a success. * * @since 5.3.0 */ function restore_siteurl_success_notice() { if ( isset( $_GET['srsuccess'] ) && '1' === $_GET['srsuccess'] ) { $restore_success = get_transient( 'siteurl_restore_success' ); if ( '1' === $restore_success ) { add_action( 'admin_notices', 'restore_siteurl_success_notice__success'); delete_transient( 'siteurl_restore_success' ); } } } add_action( 'admin_init', 'restore_siteurl_success_notice' ); /** * Restore siteurl success notice. * * @since 5.3.0 */ function restore_siteurl_success_notice__success() { $class = 'notice notice-success'; $message = __( 'Your WordPress site url was successfully restored.' ); printf( '<div class="%1$s"><p>%2$s</p></div>', esc_attr( $class ), esc_html( $message ) ); }
#5
@
5 years ago
Upon looking further, I just found out that since 5.2.0, WP_Recovery_Mode
class was introduced. Correct me if i'm wrong, but from what I see, it's use as a way for site admins to enter the website if ever a plugin / theme fails. Do you think this Change Siteurl recovery
should also be part of WP_Recovery_Mode
?
I am asking this because the header in WP_Recovery_Mode
class file states
<?php /** * Error Protection API: WP_Recovery_Mode class * * @package WordPress * @since 5.2.0 */
And this new Change Siteurl recover
feature can be counted as an Error Protection
function.
Any thoughts?
#6
@
5 years ago
- Keywords has-patch added
The attached patch, 47954.diff, should does the second approach @Otto42 mentioned.
Second thought: when these change, email the admin user about them, with a revert link that will work no matter what.
Please feel free to test it so we can further enhance it or fix any bugs on it.
As for
Concept: Change the field, make an internal request, see if it fails, if so change them back, then warn the user with a "hey, this change seems like it's gonna break things real bad, are you really sure?" Or equivalent.
I'll be working on this when time permits.
I also created a fork branch for this in Github. Again, feel free to help out.
https://github.com/donmhico/wordpress-develop/tree/trac-47954
@Otto42 @SergeyBiryukov - I'm not sure if this should be early
for the next release but it would be awesome if we can add this to 5.3
milestone even just this email to admin a restore link
that it currently has as IMHO, it would really help site admins, especially non-techie ones. Thanks.
This ticket was mentioned in Slack in #core by donmhico. View the logs.
5 years ago
#8
@
5 years ago
Had a quick look, and the concept isn't bad.
I would prefer if it used core constants for the expiry-time, instead of plain numbers, it makes it easier to discern what the time actually is, and would probably also do well with a filter to modify the expiry time. The default time of 30 minutes sounds reasonable though.
Using 30 * MINUTE_IN_SECONDS
would make this default much clearer.
I noticed the restore feature is hooked to registered_taxonomy
, this needs a much earlier hook, a quick test with init
shows that even that is too late. WordPress does an automatic redirect to the URL configured, and that redirect is a 301, which means browsers will cache the redirect, making it near impossible for the average user to utilize the restore URL.
--
For the part that's not done yet, which I personally think is the more important aspect of this, the loopback to check if a new URL will still work before the user is locked out, this should be one function for all of core (I've made mention of this elsewhere as well today), and I would prefer to see a proper implementation of such a function for use here first.
#9
@
5 years ago
Thanks for the feedback @Clorith. The latest patch, 47954.2.diff, applies the changes you recommended. The expiry time is now using 30 * MINUTE_IN_SECONDS
and also filterable. See https://github.com/donmhico/wordpress-develop/commit/19617ea6d50573919d4b775072fcb8e8a170e3e8
Regarding where it is hooked. Correct me if i'm wrong, but registered_taxonomy
is the earliest possible action I can hook it to where the pluggable.php
is already loaded. The restore function uses wp_redirect()
which is declared in pluggable.php
, hence if I want to hook it in muplugins_loaded
, I need to either load pluggable.php
when needed to or not use wp_redirect()
. Unless there's any reason why it should be hooked in muplugins_loaded
, then it's good to hook it when pluggable.php
is already loaded.
#10
@
5 years ago
Concept: Change the field, make an internal request, see if it fails, if so change them back, then warn the user with a "hey, this change seems like it's gonna break things real bad, are you really sure?" Or equivalent.
What would be a sane way to check? Looking at https://wordpress.org/support/article/moving-wordpress/, it mentions on some steps that 404 is expected since the instructions prioritize the Change of urls
before moving the files / database. In this case, it's hard to think of a sanity check.
Of course, it would be different if we are just trying is to prevent 'accidental' URL change in which case, we can just check if home
or siteurl
is about to be updated and just ask for a "Are you sure?" confirmation to the admin.
#12
@
5 years ago
I agree that it is a point that would really improve the user experience. Also, maybe we could consider just taking this fields out.
We could take out the option to users edit it on wp-admin/options-general.php
, so they just would be able to do it on the hidden wp-admin/options.php
inside the admin panel. Maybe we could still show it there, but do not allow the change.
Related: #10970, #18769, #40353.