Skip to content

Attempts to resolve CSP violations#762

Merged
pkarman merged 1 commit intomasterfrom
ab-csp-errors
Nov 28, 2016
Merged

Attempts to resolve CSP violations#762
pkarman merged 1 commit intomasterfrom
ab-csp-errors

Conversation

@el-mapache
Copy link
Contributor

@el-mapache el-mapache commented Nov 22, 2016

Why:
Our Content Security Policy dictates that external resources that
execute inline, such as those loaded via <script> or <style> tags,
won't be loaded by the browser unless nonces are added to the tags in question.

How:

  • Updates CSP config to include additional domains from which content is
    loaded
  • Uses nonced_javascript_tag where necessary
  • Alters GA script to set on nonce on the dynamic script tag

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the nonced_javascript_tag method need a block? we might be able to simplify this to:

<%= nonced_javascript_tag src: dap_source, async: true, id: '_fed_an_ua_tag' %>

Copy link
Contributor Author

@el-mapache el-mapache Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weirdly enough, it does. Although the method signature contains a parameter called 'content_or_options', it will always try and put the first argument into the tag as content, unless you also supply a block. See the code here.

I verified this by just passing hash to nonced_javascript_tag, and the method shown above tried to call html_safe on it. Passing a string 'works', although it's put inside the tag as content.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My ruby / rails is a little rusty so maybe there is a more appropriate way to use the library...but this seems to resolve the issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it works, it works ¯\_(ツ)_/¯

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be '<%= Figaro.env.google_analytics_key %>' since you converted it from Slim to ERB.

@monfresh
Copy link
Contributor

Other than the ERB variable fix, I think this looks good. I tested it locally by setting participate_in_dap: 'true' and adding a google_analytics_key entry in the development section of application.yml.

**Why**:
Our Content Security Policy dictates that external resources that
execute inline, such as javascript or css won't be loaded by the browser
unless we add nonces are added to the tags in question

**How**:
- Updates CSP config to include additional domain from which content is
loaded
- Uses nonced_javascript_tag where necessary
- Alters GA script to set on nonce on the dynamic script tag

Use erb syntax in google analytics partial

**Why**:
I updated the file type from slim to erb, but neglected to change the
the variable interpolation around the analytics key
@pkarman pkarman merged commit 306b86b into master Nov 28, 2016
@jessieay jessieay deleted the ab-csp-errors branch November 28, 2016 16:59
@jessieay
Copy link
Contributor

Hello! I know I am late to the game on this but curious: why was changing this to erb part of the solution?

Was there something happening in the slim templates that we couldn't fix without converting? (tried to see if this was explained in the PR convo anywhere but couldn't find it)

@el-mapache
Copy link
Contributor Author

el-mapache commented Nov 28, 2016

Yes! The same issue popped up here (PR), basically, I couldnt get the nonced_javascript_tag view helper to output correctly in in slim. I tried a few different permutations (nonced_javascript_tag: and = nonced_javascript_tag do come to mind) but none of them worked.

amoose pushed a commit that referenced this pull request Dec 1, 2016
**Why**:
Our Content Security Policy dictates that external resources that
execute inline, such as javascript or css won't be loaded by the browser
unless we add nonces are added to the tags in question

**How**:
- Updates CSP config to include additional domain from which content is
loaded
- Uses nonced_javascript_tag where necessary
- Alters GA script to set on nonce on the dynamic script tag

Use erb syntax in google analytics partial

**Why**:
I updated the file type from slim to erb, but neglected to change the
the variable interpolation around the analytics key
amoose pushed a commit that referenced this pull request Dec 1, 2016
**Why**:
Our Content Security Policy dictates that external resources that
execute inline, such as javascript or css won't be loaded by the browser
unless we add nonces are added to the tags in question

**How**:
- Updates CSP config to include additional domain from which content is
loaded
- Uses nonced_javascript_tag where necessary
- Alters GA script to set on nonce on the dynamic script tag

Use erb syntax in google analytics partial

**Why**:
I updated the file type from slim to erb, but neglected to change the
the variable interpolation around the analytics key
@el-mapache el-mapache self-assigned this Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants