nikolalsvk / render_async

render_async lets you include pages asynchronously with AJAX
https://rubygems.org/gems/render_async/
MIT License
1.08k stars 75 forks source link

Feature idea: Always pass nonce if config is set #99

Closed colinxfleming closed 4 years ago

colinxfleming commented 4 years ago

hi @nikolalsvk ! It looks like render_async is increasingly starting to take initializer configs, like config.turbolinks = true or config.jquery = true. I kinda like that pattern a lot, as the turbolinks option is removing a lot of redundant stuff from my codebase, and I appreciate that!

A similar thing I am doing (to literally every render_async call) is html_options: { nonce: true } because I need to get the javascript past my CSP. So I was wondering: Do you have any interest in adding a 'turn on nonces by default' as an option in RenderAsync.configuration, or do you think it would be overkill? Would probably be something like (spitballing):

RenderAsync.configure do |config|
  config.nonces = true
end

# _render_async.html.erb
...
javascript_tag html_options.merge({nonce: RenderAsync.configuration.nonces}) do
  ...
end

and that would basically do the same thing as what you can currently accomplish with:

render_async users_path, html_options: { nonce: true }

(The other thing I kinda thought about was a default_html_options config, which would merge into html_options but I think I like the specific option better.)

More than happy to take a stab at this work if it sounds good or interesting? I kinda think it would be a nice 'turn on extra security feature by default' kinda deal. Let me know what you think, or if I can clarify anything.

walterdavis commented 4 years ago

Would merge be the best choice here? Would that allow override on an individual element where (for some reason) you didn't want to include the nonce? What about ||= in there somewhere?

colinxfleming commented 4 years ago

@walterdavis oh yeah, it's almost definitely not the best choice for an actual implementation - was just some late night pseudocode to get the idea across.

nikolalsvk commented 4 years ago

This sounds pretty good and useful to people who use nonce! I'd say you can go ahead and take a stab at implementing this. I'd be more than glad to help out with coding / review 👍

nikolalsvk commented 4 years ago

Hey, @colinxfleming, I released a config option for nonce in 2.1.8 version. Check it out and let me know if it works for you.

You can now do this:

RenderAsync.configure do |config|
  config.nonces = true
end

And not care about passing nonce in each render_async call.

Cheers 🍻

colinxfleming commented 3 years ago

@nikolalsvk sorry for posting the idea and then promptly not having the time and space to work on it! But this works fantastic; thank you very much! Stoked to put it to work in my corner of the world.

nikolalsvk commented 3 years ago

No problem, @colinxfleming. Let me know if it works for you and if we can add anything else.