Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#51437 closed enhancement (fixed)

Streamline migrating from HTTP to HTTPS

Reported by: flixos90's profile flixos90 Owned by: flixos90's profile flixos90
Milestone: 5.7 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch has-unit-tests commit has-dev-note
Focuses: Cc:

Description

Per existing WordPress core behavior, HTTPS being enabled can be detected by looking at whether the Site Address and WordPress Address both use "https://" URLs. This detection is currently only present within Site Health, but #47577 aims to make it more generally available via a function.

A major pain point in WordPress has been the migration of a WordPress site from HTTP to HTTPS: While changing the Site Address and WordPress Address to use HTTPS is trivial, updating references to the old URLs in existing content is not. It cannot be accomplished within core UI and requires use of more advanced tools, such as WP-CLI or plugins like "Better Search Replace", which is a no-go for most users.

The migration of existing content can also happen more dynamically though, by replacing the old URLs starting with "http://" to "https://" on the fly. While replacing the content in the database is generally recommended, this presents a roadblock for most users, and this alternative is certainly feasible.

It could also be considered to build a full HTTP to HTTPS database replacement tool into WordPress core, but dynamic replacement is a pattern already established in core for years (e.g. injecting srcset, sizes, loading tags to content images) and would be allow for a simpler way of solving this problem. Two important things to consider:

  • This replacement logic should only run if the site was migrated from HTTP to HTTPS. It shouldn't run if the site was HTTPS from the start.
  • It should be possible to skip the logic entirely, e.g. for advanced users that actually replaced all URLs in the database and thus no longer need this.

I'm proposing the following:

  • Introduce a function wp_should_update_insecure_urls() which returns whether WordPress core should automatically replace "http://" versions of the home URL with "https://":
    • Rely on wp_is_using_https() from #47577.
      • If false (and the site is not a fresh_site, i.e. already has content), store a new option https_migrated and set it to 0. This will be able to indicate in the future that the site used to be HTTP. Then return false.
      • Otherwise (if true), and there is an option https_migrated, first change its value to 1 if it's still 0. Then return true.
    • Make the return value filterable, so that plugins can disable/enable the feature as needed.
  • Add hook callbacks to various content filters (e.g. the_content, widget_text_content etc, and if wp_should_update_insecure_urls() returns true, replace the non-HTTPS variant of the current home_url() with its HTTPS-variant.

Change History (28)

#1 @westonruter
4 years ago

Should this also apply to setting HTTPS from the very beginning at the point of WP install?

#2 @flixos90
4 years ago

  • Milestone changed from Awaiting Review to 5.7

Reminder: Let's incorporate the ideas from https://core.trac.wordpress.org/ticket/47577#comment:25 in here.

  • Add constant and function so that hosts can provide a custom URL to update the site to HTTPS.
  • Modify default action so that it will update the URLs immediately, perform a loopback request to check if it works, and then redirect back with a notice.

Both of these simplify what a user needs to do. A simple action button is much easier than changing two URLs from HTTP to HTTPS.

We'll also need to cater for the case though where the two URLs are defined with a constant (WP_HOME and WP_SITEURL). In this scenario, we'll need to show a message that automatically changing the URL is not possible. Let's think about good wording for this and what links could be potentially helpful.

cc @Clorith

This ticket was mentioned in PR #870 on WordPress/wordpress-develop by felixarntz.


4 years ago
#3

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

URL replacements from HTTP to HTTPS:

  • Introduces wp_replace_insecure_home_url() filter function that replaces the HTTP variant of home_url() with its HTTPS counterpart.
  • Introduces wp_should_replace_insecure_home_url() (including wp_should_replace_insecure_home_url filter for customization) which determines whether the above function should run its replacement logic or not.
  • Add wp_update_https_migrated() which hooks into update_option_home to set a https_migrated flag when home_url() is updated from HTTP to its HTTPS counterpart. If it is a "fresh site", there is no content to migrate, so the flag is set to 1. Otherwise, it is set to 0 (i.e. URL replacement logic should kick in).
  • Overall, URLs are only replaced if both home_url() and site_url() are using HTTPS and on the same domain. Special edge-cases (where e.g. the domain would differ between them) are not worth covering here since they would require additional logic, would make it more error-prone, and furthermore it is likely such sites are administered by fairly advanced WordPress users.

Updating home and siteurl URLs to HTTPS:

  • Introduces wp_update_urls_to_https() which updates both options to their HTTPS counterparts. If as a result WordPress does not recognize the site as using HTTPS (e.g. due to the option being overridden by a constant), the update is reverted.
  • Introduces update_https meta capability, which is given to users that have both manage_options and update_core.

SIte Health integration:

  • Introduces wp_get_update_https_url() which returns the URL to learn more about updating HTTPS. It is customizable via either WP_UPDATE_HTTPS_URL environment variable or wp_update_https_url filter.
  • Introduces wp_get_direct_update_https_url() which returns the "CTA" URL to actually update HTTPS, if set by the host. It can be set via either WP_DIRECT_UPDATE_HTTPS_URL environment variable or wp_direct_update_https_url filter.
  • Integrates the above host URLs into the "HTTPS Status" Site Health section if they are specified.
  • Adds a primary button if HTTPS is supported and can be enabled. The button goes either to the wp_get_direct_update_https_url() (if set) or triggers the URL update via the above wp_update_urls_to_https() function. Success/error feedback is provided accordingly upon redirect back to the Site Health page.

Trac ticket: https://core.trac.wordpress.org/ticket/51437

felixarntz commented on PR #870:


4 years ago
#4

Some screenshots of the different states of the "HTTPS Status" Site Health section with this PR:
HTTPS not supported, no host support URL set:
https://i0.wp.com/user-images.githubusercontent.com/3531426/104535204-98307780-55ca-11eb-8e71-5227a486f311.png

HTTPS not supported, host support URL set:
https://i0.wp.com/user-images.githubusercontent.com/3531426/104535305-c6ae5280-55ca-11eb-87b9-8546524500dc.png

HTTPS supported, no host-specific direct update URL set: (i.e. WordPress will update on its own)
https://i0.wp.com/user-images.githubusercontent.com/3531426/104535309-ca41d980-55ca-11eb-9cb8-46ecd6ff73d1.png

HTTPS supported, host-specific direct update URL set:
https://i0.wp.com/user-images.githubusercontent.com/3531426/104535317-cdd56080-55ca-11eb-8bea-d891ceff6aee.png

#5 @flixos90
4 years ago

@Clorith I've opened a PR for smoother migration to HTTPS and have taken into account your suggestions from https://core.trac.wordpress.org/ticket/47577#comment:25. Could you please review the copy / try it out? Curious to hear your feedback.

#6 @flixos90
4 years ago

  • Keywords needs-dev-note added

adamsilverstein commented on PR #870:


4 years ago
#7

@felixarntz This works well in my local testing, however I did find a couple of parts confusing from a user perspective.

  • I tried setting my WordPress Address (wp-admin) to https, and my front end to http - a potentially common scenario, since https was at one point considered "expensive" and only worth running on the front end when context warranted it (eg. cart checkout):

https://i0.wp.com/user-images.githubusercontent.com/2676022/105071964-7f004e80-5a42-11eb-96e7-6c04a44ae119.png

When I visit site health, the warning is confusing:

https://i0.wp.com/user-images.githubusercontent.com/2676022/105072045-9b03f000-5a42-11eb-86bd-937c35e30468.png

In this case my "WordPress Address" is using https, but my "Site Address" is not. Maybe the text here can reflect that and help explain this to users. Something like:

_Your WordPress address is set to use HTTPS, but your Site Address is not set up to use HTTPS by default._

  • My local site uses a self signed certificate, and I can see that is recorded successfully in the 'https_detection_errors' option, however -when I enable HTTPS for both WordPress and Site Address, the error detection is no longer used. So on Site Health, HTTPS shows under the passed tests. I feel like we should still want users when we detect that their certificate is invalid - what do you think?

One other question regarding the checks in wp_update_https_detection_errors - I'm not sure about caching the results in an option. Although the check takes some time to run, that seems fine for the Site Health page, and I'm worried we won't pick up changes when a user fixes the issue (for example by installing a valid certificate on their site). Do we really need to cache the results of these checks? The doc block says "This internal function is called by a regular Cron hook to ensure HTTPS support is detected and maintained.", but I didn't see that check?

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

felixarntz commented on PR #870:


4 years ago
#9

@adamsilverstein Thanks for the feedback.

In this case my "WordPress Address" is using https, but my "Site Address" is not. Maybe the text here can reflect that and help explain this to users.

Good catch, I will update the messaging there to reflect that case.

My local site uses a self signed certificate, and I can see that is recorded successfully in the 'https_detection_errors' option, however -when I enable HTTPS for both WordPress and Site Address, the error detection is no longer used. So on Site Health, HTTPS shows under the passed tests.

Great point, we already continue to have the check being made, so it's worth flagging such errors to the user in Site Health. I will update it so that, even if the site is already using HTTPS, this problem would be flagged. E.g. Your site uses an invalid SSL certificate.

One other question regarding the checks in wp_update_https_detection_errors - I'm not sure about caching the results in an option. Although the check takes some time to run, that seems fine for the Site Health page

I agree it would probably be okay to not rely on a cached value for Site Health specifically, but this value is also used by a "regular" WordPress function that is available to call in any context, so I think relying on a cron hook makes more sense. The cron hook is added in default-filters.php as expected via wp_schedule_https_detection on init.

felixarntz commented on PR #870:


4 years ago
#10

@adamsilverstein Actually, on second though, all the feedback you left here applies more to https://core.trac.wordpress.org/ticket/47577 than to https://core.trac.wordpress.org/ticket/51437, the latter of which this is for. So I'll probably open a follow-up PR for https://core.trac.wordpress.org/ticket/47577 to make the changes based on your suggestions.

TimothyBJacobs commented on PR #870:


4 years ago
#11

If it isn't too much trouble, I think it'd probably be a good idea to do a fresh call in Site Health. That way a user can be sure the information they are seeing is up-to-date. It can be done as an async test, so it won't affect page load performance.

felixarntz commented on PR #870:


4 years ago
#12

@TimothyBJacobs Thanks! Fair point, we could call the update function there manually. That's something we can do as a follow-up commit for https://core.trac.wordpress.org/ticket/47577, as mentioned. Decoupled from that, it would be great to also get some feedback on this PR's approach, related to replacing URLs and streamlining the HTTPS _migration_.

#13 @joostdevalk
4 years ago

When someone is writing the dev note for this I'm happy to review.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#15 @johnbillion
4 years ago

What's the behaviour of this functionality when the site is behind a reverse proxy which provides the HTTPS layer? For example Cloudflare Universal SSL. In this situation, the site is accessed over HTTPS at its public domain name, but its origin domain name will be HTTP.

#16 @flixos90
4 years ago

@johnbillion I think in this case the URLs in WordPress should still be using HTTPS, or not? If the site can be accessed using HTTPS, its links should also rely on HTTPS - why would we want to keep them HTTP in that scenario?

I see how technically, without the reverse proxy of course it wouldn't work anymore, but I think disabling that would have similar implications like e.g. the server's SSL certificate becoming invalid.

I may not fully understand your point, in that case could you elaborate? From what I'm thinking, this ticket shouldn't be affected by how exactly HTTPS access is implemented for a domain. It's primarily focused on making sure in such a case all site-owned URLs use HTTPS to avoid mixed content automatically where possible, without requiring the user to do a DB search-replace.

felixarntz commented on PR #870:


4 years ago
#17

@TimothyBJacobs Thanks for the feedback, I've addressed it. The previous feedback that was more related to https://core.trac.wordpress.org/ticket/47577 I've also addressed in a follow-up PR for that ticket, https://github.com/WordPress/wordpress-develop/pull/948.

TimothyBJacobs commented on PR #870:


4 years ago
#18

Code looks great to me, I should be able to give this a proper test on a site tomorrow.

Do URLs in CSS stylesheets cause mixed content warnings? Wondering if it might makes sense to also filter the customizer additional CSS rules.

felixarntz commented on PR #870:


4 years ago
#19

@TimothyBJacobs Good point, that's worth looking into. Will check tomorrow.

felixarntz commented on PR #870:


4 years ago
#20

@TimothyBJacobs I've refreshed this against latest trunk and also added the existing filter to wp_get_custom_css, so that HTTP versions of the home_url() are replaced with their HTTPS counterparts in custom CSS as well. cc @westonruter

#21 @flixos90
4 years ago

  • Keywords commit added

This ticket was mentioned in Slack in #core-committers by hellofromtonya. View the logs.


4 years ago

TimothyBJacobs commented on PR #870:


4 years ago
#23

This looks good to me from the code. I unfortunately don't have an environment right now where I can test the actual migration to https.

felixarntz commented on PR #870:


4 years ago
#24

I've recorded a quick demo that shows the user flow of switching from HTTP to HTTPS, comparing how this is done on WordPress 5.6 vs WordPress 5.7: https://www.youtube.com/watch?v=wRVAY_Lwoco

#25 @flixos90
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 50131:

Security, Site Health: Make migrating a site to HTTPS a one-click interaction.

Switching a WordPress site from HTTP to HTTPS has historically been a tedious task. While on the surface the Site Address and WordPress Address have to be updated, existing content still remains using HTTP URLs where hard-coded in the database. Furthermore, updating _two_ URLs to migrate to HTTPS is still a fairly unintuitive step which is not clearly explained.

This changeset simplifies migration from HTTP to HTTPS and, where possible, makes it a one-click interaction.

  • Automatically replace insecure versions of the Site Address (home_url()) with its HTTPS counterpart on the fly if the site has been migrated from HTTP to HTTPS. This is accomplished by introducing a https_migration_required option and enabling it when the home_url() is accordingly changed.
    • A new wp_replace_insecure_home_url() function is hooked into various pieces of content to replace URLs accordingly.
    • The migration only kicks in when the Site Address (home_url()) and WordPress Address (site_url()) match, which is the widely common case. Configurations where these differ are often maintained by more advanced users, where this migration routine would be less essential - something to potentially iterate on in the future though.
    • The migration does not actually update content in the database. More savvy users that prefer to do that can prevent the migration logic from running by either deleting the https_migration_required option or using the new wp_should_replace_insecure_home_url filter.
    • For fresh sites that do not have any content yet at the point of changing the URLs to HTTPS, the migration will also be skipped since it would not be relevant.
  • Expose a primary action in the Site Health recommendation, if HTTPS is already supported by the environment, built on top of the HTTPS detection mechanism from [49904]. When clicked, the default behavior is to update home_url() and site_url() in one go to their HTTPS counterpart.
    • A new wp_update_urls_to_https() function takes care of the update routine.
    • A new update_https meta capability is introduced to control access.
    • If the site's URLs are controlled by constants, this update is not automatically possible, so in these scenarios the user is informed about that in the HTTPS status check in Site Health.
  • Allow hosting providers to modify the URLs linked to in the HTTPS status check in Site Health, similar to how that is possible for the URLs around updating the PHP version.
    • A WP_UPDATE_HTTPS_URL environment variable or wp_update_https_url filter can be used to provide a custom URL with guidance about updating the site to use HTTPS.
    • A WP_DIRECT_UPDATE_HTTPS_URL environment variable or wp_direct_update_https_url filter can be used to provide a custom URL for the primary CTA to update the site to use HTTPS.

Props flixos90, timothyblynjacobs.
Fixes #51437.

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


4 years ago

Note: See TracTickets for help on using tickets.