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

Add nonced versions of Rails link/include tags #372

Merged
merged 1 commit into from
Nov 20, 2017

Conversation

steveh
Copy link
Contributor

@steveh steveh commented Nov 14, 2017

Adds support for nonced versions of Rails' CSS, JavaScript and Webpacker link/include tags.

  • nonced_stylesheet_link_tag
  • nonced_javascript_include_tag
  • nonced_javascript_pack_tag

All PRs:

  • Has tests
  • Documentation updated

Adds support for nonced versions of Rails' CSS, JavaScript and Webpacker link/include tags.

* nonced_stylesheet_link_tag
* nonced_javascript_include_tag
* nonced_javascript_pack_tag
Copy link
Contributor

@oreoshake oreoshake left a comment

Choose a reason for hiding this comment

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

I'm happy with the overall goal and docs but we'll need some tests and some clarification on the route taken.

# Public: create a script tag using the content security policy nonce.
# Instructs secure_headers to append a nonce to style/script-src directives.
# Instructs secure_headers to append a nonce to script-src directive.
#
# Returns an html-safe script tag with the nonce attribute.
def nonced_javascript_tag(content_or_options = {}, &block)
nonced_tag(:script, content_or_options, block)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should just be a passthrough call to nonced_javascript_include_tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Rails API is a little different:

javascript_tag(content_or_options_with_block = nil, html_options = {}, &block)

javascript_include_tag(*sources)

It might work as a passthrough though, if that's preferred?

@@ -7,21 +7,45 @@ module ViewHelpers
class UnexpectedHashedScriptException < StandardError; end

# Public: create a style tag using the content security policy nonce.
# Instructs secure_headers to append a nonce to style/script-src directives.
# Instructs secure_headers to append a nonce to style-src directive.
#
# Returns an html-safe style tag with the nonce attribute.
def nonced_style_tag(content_or_options = {}, &block)
nonced_tag(:style, content_or_options, block)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a passthrough to nonced_stylesheet_link_tag instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, nonced_style_tag generates <style> whereas nonced_stylesheet_link_tag generates a <link />

content_tag(:script, nil, options.merge(src: source))
end

alias_method :javascript_pack_tag, :javascript_include_tag
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for the alias? Can we accomplish the same thing without the metaprogramming?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, but for the purpose of this test the method is identical. Would you prefer that?

@oreoshake oreoshake merged commit 565b819 into github:master Nov 20, 2017
@oreoshake
Copy link
Contributor

Released in 5.0.3

Sorry for the confusion, I was completely misreading things 😄

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.

None yet

2 participants