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

Per instance hook #146

Closed
rupurt opened this issue Jun 17, 2015 · 5 comments
Closed

Per instance hook #146

rupurt opened this issue Jun 17, 2015 · 5 comments

Comments

@rupurt
Copy link

rupurt commented Jun 17, 2015

Howdy,

Great gem, thanks! I was wondering if there's any way to modify the headers on a per instance basis? For example using the CSP preferences for the current user.

I had a quick peek into the source code and couldn't find a hook point that would achieve this. However with a small modification I think this could be achieved. I was thinking that instead of calling self.class.options_for it could call an instance method self.options_for which would do the same thing. That would then allow you to use super and you can modify the returned options hash. Something along these lines.

# lib/secure_headers.rb
module SecureHeaders
   module InstanceMethods
      # ... stuff

      def set_csp_header(req = nil, config=nil)
         # ... more stuff

         config = self.class.secure_headers_options[:csp] if config.nil?
         config = options_for :csp, config

         return if config == false

         # ... even more stuff
      end

      # This can now be overridden
      def options_for(type, options)
         options.nil? ? ::SecureHeaders::Configuration.send(type) : options
      end
   end
end
@rupurt rupurt changed the title Modify CSP headers per action Per action hook Jun 17, 2015
@oreoshake
Copy link
Contributor

That's a great idea! It's been asked for before.

BTW all csp config values accept procs which can work for simple cases. I do like this approach, but options_for isn't descriptive enough to plop in someone else's namespace.

This could also be extended to the rest of the config so maybe something like def secure_headers_options which returns the entire hash, likely a modified version of super's.

wdyt?

@rupurt rupurt changed the title Per action hook Per instance hook Jun 17, 2015
@rupurt
Copy link
Author

rupurt commented Jun 17, 2015

Cool.

I just realized that what I'm doing doesn't need something on a per action basis. It needs it on a per instance basis. I've updated the title of the issue to reflect this. Sorry.

I agree that options_for is probably too generic. How do you feel about secure_header_options_for? I don't fully understand what you mean by This could also be extended to the rest of the config, do you mind elaborating? I've created a branch which reflects my thoughts.

The 2 key things this helps with are:

  1. Overriding the options_for (or equivalent) method can use the default config and we can override specific keys instead of using ensure_security_headers which clobbers the default config.
  2. Overriding options_for in the controller allows us to use per instance data e.g. current_user

@oreoshake
Copy link
Contributor

secure_headers_options_for should really encompass the entire config (x-frame-options, hsts, etc), not just the csp-related stuff. The config is just a giant hash, and this should take that into consideration.

#override
def secure_headers_options_for
  options = super
  if current_user.is_totally_not_a_hacker?
    options[:csp][:script_src] = "*"
    options[:hsts][:include_subdomains] = false
  end
  options
end

secure_headers_csp_options_for would be good for updating JUST the csp options.

   def secure_headers_csp_options_for
     { script_src: 'self' } if current_user.is_totally_not_a_hacker?
   end

@rupurt
Copy link
Author

rupurt commented Jun 17, 2015

I may be missing something but I don't think that's how options_for currently works. It takes type & options and returns the base configuration for the given type when options is falsy.

::SecureHeaders::Configuration.configure do |config|
  config.csp = {
    :default_src => "https: self",
    :frame_src => "https: http:.twimg.com http://itunes.apple.com"
  }
  config.hpkp = {
    :max_age => 60.days.to_i,
    :include_subdomains => true
  }
end
opts = options_for(:csp, nil)
# {
#   :default_src => "https: self",
#   :frame_src => "https: http:.twimg.com http://itunes.apple.com"
# }

@oreoshake
Copy link
Contributor

Oh I'm sorry you're right. And therefore my earlier concern is off-base

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

No branches or pull requests

2 participants