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

[DO NOT MERGE] Adds SRI to js and css files #1008

Closed
wants to merge 3 commits into from
Closed

Conversation

elenatanasoiu
Copy link
Contributor

@elenatanasoiu elenatanasoiu commented Apr 24, 2017

Trello card

This will add security checks to verify that the content being served on GOV.UK hasn't been modified by third parties:

  • upgrades sprockets-rails to version 3.2.0 so that we can use the SRI check feature
  • adds integrity: true to javascript_include_tag and stylesheet_link_tag

Explicitly setting config.assets.digest to false, as the default value is set to true since sprockets-rails 3.0: rails/sprockets-rails@3cb84ea

Depends on:

In order to use SRI, the browser expects CORS to be supported. This is why we need to add crossorigin: true alongside integrity: true. See more here

@elenatanasoiu elenatanasoiu force-pushed the add-sri branch 7 times, most recently from 6201376 to 1d38ad4 Compare April 26, 2017 14:58
@elenatanasoiu elenatanasoiu changed the title Adds SRI to js and css files [DO NOT MERGE] Adds SRI to js and css files Apr 26, 2017
@elenatanasoiu elenatanasoiu force-pushed the add-sri branch 20 times, most recently from d3d1280 to 43633c2 Compare May 2, 2017 09:26
@elenatanasoiu elenatanasoiu force-pushed the add-sri branch 2 times, most recently from 6579878 to 6c20399 Compare May 15, 2017 13:28
@elenatanasoiu elenatanasoiu changed the title [DO NOT MERGE] Adds SRI to js and css files Adds SRI to js and css files May 15, 2017
elenatanasoiu pushed a commit that referenced this pull request May 16, 2017
@elenatanasoiu elenatanasoiu changed the title Adds SRI to js and css files [DO NOT MERGE] Adds SRI to js and css files May 16, 2017
@h-lame h-lame mentioned this pull request May 17, 2017
This will add security checks to verify that the content being served on GOV.UK hasn't been modified by third parties:

- upgrades `sprockets-rails` to version 3.2.0 so that we can use the SRI check feature
- adds `integrity: true` to `javascript_include_tag` and `stylesheet_link_tag`

Explicitly setting `config.assets.digest` to `false`, since the default value is set to `true` in sprockets-rails 3.0:
rails/sprockets-rails@3cb84ea

How to test
-----------

Either set `config.assets.debug = false` in config/environments/development.rb or run the application in production mode.
This is because sprockets-rails checks for this flag when deciding if it should calculate the integrity attribute: https://github.com/rails/sprockets-rails/blob/10bc1bd096b39a3dd632571dd517788314657056/lib/sprockets/rails/helper.rb#L168
Elena Tanasoiu added 2 commits May 17, 2017 16:52
Once govuk_template has been upgraded to v0.21.0 it will use a new batch of icons. The old ones no longer exist, and have been removed from the routes + tests.

See alphagov/govuk_template@82d9459 for the changes done to icons in govuk_template.
@elenatanasoiu
Copy link
Contributor Author

elenatanasoiu commented May 17, 2017

@h-lame I've moved the changes from PR 1041 here

@h-lame
Copy link
Contributor

h-lame commented May 19, 2017

The CI failures here are because we need the fix in alphagov/govuk_template#305 to be released (see: alphagov/govuk_template#307)

@h-lame
Copy link
Contributor

h-lame commented May 19, 2017

Note that this is on hold while we investigate a firefox issue with SRI (see alphagov/government-frontend#368)

@h-lame
Copy link
Contributor

h-lame commented May 22, 2017

See alphagov/govuk_template#308 and alphagov/govuk_template#301 for discussion on impact. We're holding off on SRI until there are fewer users using the version of Firefox that has an issue with calculating SRI.

@fofr
Copy link
Contributor

fofr commented Jun 5, 2017

Should we close this for now?

@fofr
Copy link
Contributor

fofr commented Jun 19, 2017

Closing while SRI is on hold.

@fofr fofr closed this Jun 19, 2017
@barrucadu barrucadu deleted the add-sri branch April 27, 2018 12:35
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.

3 participants