-
Notifications
You must be signed in to change notification settings - Fork 97
-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create a UI to force HTTPS if it's supported #17
Comments
Instead of “click here” I think we should be using links with descriptive text like “re-check” or “set HTTPS”. Also, buttons may be more appropriate than links in some cases.
Instead of “not supported” and “supported” I think it should be “Support detected” and “Support not detected”. There should probably be some link off to somewhere to get more information about HTTPS if support is not detected.
When support is detected, there should then be a checkbox presented to force the redirection from HTTP to HTTPS, in addition to changing the home/site URLs from HTTP to HTTPS. I think that this should be done without any page reloads because the user may have changed another setting on the page and it could be lost otherwise. So I think that clicking the “Check” button should do an Ajax request to check for support (even using the same request client-side that is being made via cron, which could have the added benefit of using the user's SSL certs). Then when that Ajax request returns affirmatively, then the table row with the checkbox could be shown. Upon clicking the checkbox, then JS can be used to update the home/site URL inputs to use HTTPS. Once the user then saves the changes, then the HTTPS redirection would be put in place at that point. Note that the user may end up getting prompted to re-login at this point if the admin also switched from HTTP to HTTPS. |
This is essentially implementing a UI for the constant described in wp#28521:
|
Revised ACs Hi @westonruter, These ACs add your suggestions, though the AC numbers don't map to the previous ones. AC1. Use the HTTPS detection logic from Issue #6 AC2. On the General Settings page, add a AC3. When support is detected, and the WordPress Address (URL) and Site Address (URL) also use HTTPS, display: AC4. When support is not detected, display: AC5. Clicking "Re-check" triggers an AJAX request, not a page reload AC6. If it then detects HTTPS support, and the WordPress Address (URL) and Site Address (URL) don't use HTTPS, display: AC7. This can also display on initially loading the General Settings page, if HTTPS is supported, but the WordPress Address (URL) and Site Address (URL) don't use HTTPS AC8. On clicking the checkbox, the WordPress Address (URL) and Site Address (URL) are changed via JavaScript in the AC9. On clicking "Save Changes," the setting to redirect is saved, and the redirection is in effect AC10. A AC11. If someone wants to turn off HTTPS support, for some reason, there should be a way to do that. So unchecking the checkbox can suspend the addition of the filter that forces the home/site URLs to be HTTPS. |
Use JavaScript to move it to the correct location, immediately following the home/site URLs.
Don't you mean when they are not using HTTPS?
Changed via JavaScript in the input fields. No request is made to change the option in the background before clicking Save Changes. Note, however, that a If someone wants to turn off HTTPS support, for some reason, there should be a way to do that. So unchecking the checkbox can suspend the addition of the filter that forces the home/site URLs to be HTTPS. |
Hi @westonruter,
Sounds good, I updated AC2 above.
I was thinking of when HTTPS is supported, and the home/site URLs also use HTTPS. In that case, there would be no need for a notice, or a checkbox to force HTTPS. Maybe there would be no need for an 'HTTPS Support'
Thanks, I updated AC8 with that.
Thanks, I added that verbatim as AC10.
Thanks, I added that verbatim as AC11. |
If the current site URL is HTTP, then the notice should be displayed for sure. If that's what you're saying then yes, agreed. If the site's URLs are currently HTTPS, then the notice maybe wouldn't need to be displayed except for being able to turn off HTTPS. So in that way, checking if |
Hi @westonruter,
Thanks, that's what I was thinking.
That sounds good. |
Hi @westonruter, in #18 (comment) I described a scenario where caution should be placed in moving to full HSTS support. Perhaps somewhere in this dialogue a prompt to describe details (why their site isn't HTTPS/HSTS ready and what they can do to fix it) could be added? I'd also recommend @jwold help design a userflow here as well to help make this a tad easier from a usability standpoint. |
Design Thanks, @postphotos! If there are changes to the requirements here based on design work, it'd be good if they were done soon. I might start developing for this soon. |
Update: waiting to develop for this, pending a design discussion on Monday. |
Would be great to also consider adding a help button/link for those who may want to learn what this does (linking out to a post or official docs). |
We had a discussion earlier this week and went over the possible UI changes. Following is a sketch showing our work in progress for displaying the update notice. Feedback is welcome! Additional things we could discuss further based on the notes above:
|
Instead of “images” it should probably just say “assets”, as the issue will be regarding media (images, videos, audio) and also scripts and stylesheets. Also, it should say “at least X issues” because we won't be able to maintain a comprehensive index of the entire site's URL space and the assets that get requested. |
Possible UI Hi @jwold and @westonruter, This is based on the discovery document and discussion from #17. The "Upgrade all mixed content" checkbox would add the upgrade-insecure-requests directive. Thanks! |
This is a great start on a UI and at a high level I think it works.
|
Thanks, Great Points Hi @jwold,
|
Two more considerations:
These could go into separate issues perhaps. |
Initial install, warning in dashboard Hi @westonruter, |
New Proposed UI Hi @jwold and @westonruter, This UI makes recommendations, where the previous one I proposed was more for developers. It uses a loopback request to the homepage (with HTTPS forced), and a match for insecure URLS (using output buffering). This would happen before the page loads, like the AMP validator does for the post editor. Here are the settings pages for when a site isn't using using HTTPS, based on what kind of mixed content it finds. The first 3 screenshots are for when the HTTPS request succeeds, and the last is for when it fails. Thanks to 10up/sketchpress for the great templates and elements that this uses. |
Great work Ryan! Got a few questions |
@jwold, thanks for your great points. I'm working on a response now. |
Hi @jwold,
<a href="http://example.com/foo">http://example.com/foo</a> If it's alright, I'll reply later about where the "More details" links go to. |
Thanks, good idea to use an expanding accordion. |
@jwold Thoughts on why Reading Settings is the best place for the HTTPS settings? I think it makes more sense adjoined to the “WordPress Address (URL)” and “Site Address (URL)” settings on the General Settings screen since it is inherently tied to the URL, and enabling HTTPS will impact the URL. |
@westonruter that was the initial thought I had, put it next to a place that affects the site URL. In chatting with Leo and Ryan we felt that Reading represented the viewing interaction that would happen with the site. For instance, we have “discourage search engines” there. Now that I say it that sounds a tad weak. @postphotos or @kienstra can you weigh in with what you were thinking as well? |
Which Settings Page? Hi @jwold, Especially if we're listing the invalid URLs (see screenshot #3 here). Or if we have a recheck button with notices. But HTTPS probably matches the context of the "General Settings" page better than any other page. |
Can some of the elements be hidden behind an expandable container? I'm thinking that once you set HTTPS that usually you won't really need to change the settings again. Having an expandable section under the URL settings to reveal the HTTPS settings could be a good solution. If HTTPS is not enabled the section could be expanded since users need to really enable it more than ever. But once enabled the section could be collapsed, and once there are HTTPS issues discovered then there could be a red indication with a count for the number of issues which are revealed upon expansion. |
My first thoughts were to have it on the "General Settings" too. As @westonruter pointed out, the site's URLs are there. You get a sense of whether you have HTTP or HTTPS by that screen. +1 for expandable container for all the extra details, such as a list of invalid URLs. We may want to have a more intuitive UI within that container that provides more contextual information, such as what the list is, why it's there, and what to do to resolve each. |
Here are the UI/User flow notes from my call today with @jwold:
Questions that remain: Let us know if there's enough fidelity here for you @kienstra. I'm also happy to work with you on the language here. |
Mockups, HSTS Hi @postphotos, What do you think of enabling HSTS automatically if the user has checked "HTTPS Upgrade" and "Upgrade Insecure URLs"? And not having a checkbox for it. It looks like HSTS has a similar effect on insecure content as the upgrade-insecure-requests header (which checking "Upgrade Insecure URLs" adds). If a script hosted on the same domain has a HTTP URL, it forces it to HTTPS. Though upgrade-insecure-requests also applies to requests outside the domain. HSTS can cause issues if the HTTPS request somehow fails. There's no fallback:
But this is something we'd probably have to handle even if the user checked an "HSTS" box, and HTTPS later failed. We might do this:
|
Insecure Content Checkbox Hi @postphotos, I think it'd be good to hide the "Upgrade Insecure URLs" checkbox if this only finds passive insecure content, if we can get a good enough way to find which content is passive or active. There wouldn't be an error in the browser anyway, only a warning in the JS console. And upgrading them could cause them to fail if the HTTPS requests fail. |
Current UI From PR #36
|
Maybe Remove "Upgrade Insecure URLs" Checkbox Based on some feedback in the sync today, maybe we could remove the "Upgrade Insecure URLs" checkbox. And automatically upgrade the insecure requests, only if there is active insecure content on the homepage. Normally, the risk in upgrading insecure requests is that when a HTTP URL is upgraded to HTTPS, it could fail. And there's no fallback. But if there are active insecure URLs, the browser already blocks them. So they're failing already, and upgrading them at least gives them a chance. The main reason to have the checkbox, instead of upgrading all requests by default, is to prevent upgrading passive insecure URLs that might fail. For example, the So this wouldn't upgrade insecure requests unless there's active insecure content. This could still display the insecure URLs if there's active insecure content, like in screenshot 3 above. |
@kienstra just synced with Leo and we propose the following updated UI. It removes sidebars and puts all list items in a bit of a box, with alternating row colors and a button to load more. Please let us know if any further questions. Thanks! |
Thanks For The UI Above Hi @jwold,
But I don't think there will be more than 50 or 100 insecure URLs very often. It probably wouldn't be common to show those URLs at all. They're only shown if all of the following are true:
We've also been discussing the UI in #36.
Thanks, Joshua! |
Just had a mini-workshop w/ @kienstra.
We also moved the number count out of the copy button and into the overflow button.
At this point, pending code review for #36 and closing out a few minor tasks left (AC8 from #18 was added here as a related item) I feel this issue will soon be ready for close. Thanks for all the hard work, folks! 🎉 |
Great ideas on the design here, @postphotos! This looks really good. |
Thanks to Leo's design above, and some style rules he added, here's the current UI as applied in #36: |
Really happy with where this landed! A few thoughts:
|
Thanks Hi @jwold, Here are what your points 1. and 5. look like. Of course, this would be easy to change pending your suggestions:
|
Copy Changes Hi @westonruter, |
Should Also Upgrade Passive Mixed Content Passive mixed content isn't currently upgraded in #36. It's still a security risk, but not as much as scripts or stylesheets. So it would be better to upgrade all mixed content via Upgrade-Insecure-Requests. And as @amedina mentioned, passive mixed content like HTTP images are blocked by service workers. The blocker to this before was that there's no fallback if the upgraded URL fails. For example, if an image at http://example.com/foo.jpeg is upgraded to HTTPS and fails. But @westonruter had a good idea of making requests for all insecure content, at the upgraded HTTPS URL, to ensure the requests don't fail. |
Please see the "Revised ACs" comment for the ACs, instead of this comment.
Hi @westonruter and @postphotos,
What do you think about these ACs? Feel free to edit this.
User Story: As a WordPress administrator, I want a UI to ensure that my site uses HTTPS if it's supported.
<tr>
for HTTPS support (use JavaScript to move it after the home/site URLs)AC6. Clicking "Recheck" triggers the loopback request from Add reliable method for detecting HTTPS support #6. Maybe there should be a notice at the top, like "HTTPS is still not supported" or "HTTP is supported."
AC7. If HTTPS is supported, and the WordPress Address (URL) and Site Address (URL) use HTTPS, display:
AC8. If HTTPS is enabled, show the user a toggle for HSTS (see issue Add support for HTTP Strict Transport Security (HSTS) #18).
AC9. Propose a UI and match the designs provided to meet WP core standards and look-and-feel, all to load on the general settings page.
The text was updated successfully, but these errors were encountered: