Add config so we can enable Rails Mailer previews in deployed envs#5521
Add config so we can enable Rails Mailer previews in deployed envs#5521zachmargolis merged 5 commits intomainfrom
Conversation
zachmargolis
commented
Oct 19, 2021
zachmargolis
commented
- Follow-up to Add Rails Mailer previews #5502
zachmargolis
requested review from
aduth,
jmhooper,
mitchellhenke and
stevegsa
config/environments/production.rb
Outdated
|
|
||
| if IdentityConfig.store.rails_mailer_previews_enabled | ||
| config.action_mailer.show_previews = true | ||
| config.action_mailer.preview_path ||= Rails.root.join('spec/mailers/previews') |
There was a problem hiding this comment.
Any reason this shouldn't be = instead? Just wondering based on the default being a folder which doesn't exist in our project (test/mailers/previews). Unsure if that default is applied directly, or evaluated based on preview_path being nil.
Also wonder if we could just set preview_path regardless of environment / configuration, and rely on show_previews setting to actually enact it.
There was a problem hiding this comment.
Also, why do we only need to set this in production, and not in other environments?
There was a problem hiding this comment.
Re: = ... yes that was a copy-paste error from a blog on enabling in prod 🤦
Re: why only in prod, I think because the configs default to these values in lower environments
|
|
||
| if IdentityConfig.store.rails_mailer_previews_enabled | ||
| # CSP 2.0 only; overriden by x_frame_options in some browsers | ||
| default_csp_config[:frame_ancestors] = %w('self') |
There was a problem hiding this comment.
Trying to sort out how to expect this to behave where we set x-frame-options as deny but frame-ancestors to self, since I see the former as implying a frame-ancestors of none (per docs). I wonder instead if we could just set x-frame-options to SAMEORIGIN?
There was a problem hiding this comment.
If I read correctly, SAMEORIGIN is even more poorly supported?
There was a problem hiding this comment.
Also I went with this because I trusted the old comment here that frame-ancestors takes precedence over x-frame-options
There was a problem hiding this comment.
coupled with the fact that this worked locally (the reason I had to add the frame options in the last PR was that the previews didn't work without that change)
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
2 participants
