Skip to content
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

Fix HTTPS mixed content warnings/blocking for codeforamerica.org #912

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

konklone
Copy link
Contributor

@konklone konklone commented Feb 5, 2015

This does a huge once-over on the codeforamerica.org repo.

Broadly:

  • http://codeforamerica.org -> https://www.codeforamerica.org
  • moved all protocol-relative URLs to https://
  • made sure <form> tags pointed to secure endpoints
  • replaced all vimeo, youtube, slideshare, iframe, and other embeds with secure versions
  • updated image links wherever possible with https versions
  • fix JS/CSS imports to Mapbox to use secure endpoints
  • a smattering of random https fixes

There were something like 20 images that had no secure alternative, visible by running grep -r "src=\"http:" * in the project root. Many of them are actually 404s now anyway. For the ones that remain, the right solution is to download them and inline them into the repo, to be served from https://www.codeforamerica.org.

This doesn't cover /blog/ or /library/, which apparently are handled somewhere else, not in GitHub? If you open source it, I'll finish the job. :)

In any case, once merged, the rest of codeforamerica.org should be basically ready for an all-HTTPS future.

@konklone
Copy link
Contributor Author

konklone commented Feb 5, 2015

It might be easier to look at individual commits here than the whole Files Changed tab, as there are a lot of changes.

@konklone konklone changed the title Https Fix HTTPS mixed content warnings/blocking for codeforamerica.org Feb 5, 2015
@migurski
Copy link
Contributor

migurski commented Feb 5, 2015

This is huge, thank you for your incredible effort. I don’t actually want to migrate to SSL, I just want to make sure the issue finder works with SSL so it’s compatible with you guys. In particular, I'd prefer for things to remain protocol-relative where possible, and to wait on updating links until we can move to a new certificate that’s compatible with both the apex and subdomain URLs. Is it possible to break this down a bit?

@konklone
Copy link
Contributor Author

konklone commented Feb 5, 2015

It's definitely possible to break this down, and re-submit with some cherry-picked commits.

But I really suggest merging this as-is, because nothing here breaks the site when loaded under http://. By having as many of the resources point to https:// as possible, the site is more secure even for visitors to the insecure version (for example, an insecure asset is how the Syrian Electronic Army defaced Gigya).

The Google CDN and cdnjs have both changed their recommended URLs from protocol-relative to https://, and Paul Irish, who popularized the protocol-relative URL, now recommends against its use.

I didn't go through and update all <a href> links to point to the https:// version of the site, and those won't cause any mixed content issues anyway. So the site will continue working just fine, and the http:// version will remain canonical. This PR makes it trivial later for you to flip the switch, with some additional protection in the meantime.

@migurski
Copy link
Contributor

migurski commented Feb 5, 2015

Okay. It’s a bit much for me to evaluate, maybe I can conscript @davidrleonard to give this a more in-depth look.

@prestonrhea
Copy link
Contributor

Interested in bumping this. I created an account on forum.codeforamerica.org and noticed the connection wasn't HTTPS. I get sweaty when I put in a password over an unsecured website.

@konklone
Copy link
Contributor Author

This PR doesn't fix forum.codeforamerica.org, but you're right to feel sweaty - you should never be asked to put in a password over an insecure connection.

@konklone
Copy link
Contributor Author

I'm happy to fix the merge conflicts that have developed in this branch, once I have a 👍 that it's slated for revision/merging.

@konklone konklone mentioned this pull request Apr 11, 2015
@junosuarez
Copy link
Member

Related: Mozilla this morning joined the Chrome team in their intent to deprecate HTTP. The post includes a good recap of the rationale: https://groups.google.com/forum/#!topic/mozilla.dev.platform/xaGffxAM-hs

@junosuarez
Copy link
Member

Any status update available for this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants