-
Notifications
You must be signed in to change notification settings - Fork 252
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
Proposal: New API for dynamic settings, moving closer to rack #180
Comments
I like the approach a lot. Does SecureHeaders store it's config as a Hash? If so, could we just deep-dup the config and put it straight in the ENV? If would simplify the API a bit if we could just do something like: class ApplicationController
def secure_headers_config
env["SECURE_HEADERS_CONFIG"]
end
end
class MyCoolController
def my_cool_action
secure_headers_config["CSP"]["SCRIPT_SRC"] << "somedomain.com"
secure_headers_config["HPKP"] = {
:max_age => 60.days.to_i,
:include_subdomains => true,
:report_uri => '//example.com/uri-directive',
:pins => [
{:sha256 => 'abc'},
{:sha256 => '123'}
]
}
end
end One advantage of sticking with methods for updating the headers is that we could pre-compute the default header strings at boot time and only regenerate them if the ::SecureHeaders::Configuration.configure do |config|
config.csp = {
default_src: %w('self'),
script_src: %w('self')
}
end
class MyCoolController
SecureHeaders::Configuration.override("script_from_otherdomain_com") do |config|
config.csp["script_src"] << "otherdomain.com"
end
def my_cool_action
secure_headers_use_override("script_from_otherdomain_com")
end
end I'm not sure how costly regenerating the headers for each request is though, or how much cheaper a static approach is. |
Yeah, I'm not sure if matters much either way. I bet most smaller applications won't use a dynamic policy. But, those smaller applications probably won't care about the small performance hit of a per-request approach either. It seems like it would be pretty simple to support the "static by default" approach, but I don't think it needs to be a must have if we notice anything vaguely tricky/annoying about implementing it. |
Every policy served with This approach however, wouldn't even need a |
Do you know if Twitter or any other mega Rails apps are using this Gem? I don't imagine it would be noticeable on smaller apps, but a ton of procs and before filters could add up for us.
Makes sense. |
Heh twitter was mostly scala by the time this came out. I don't know what the largest site using it was, but yeah, that's really not important here since we have a better approach sans procs and filters. |
I'm beginning to realllly like this, so much so I'm considering removing support for all other methods. It would drastically clean up the code and should work for everyone. |
This has been merged. |
Thought: only
x-frame-options
,content-security-policy
, andhpkp
generally cannot be applied site wide without exception. So provide an API for managing these three directly and as an added bonus we will set all headers in Rack.This is backwards compatible with the existing APIs for setting per-request values [1] [2], include script hashes and nonce.
These methods can be called statically from anywhere code path that runs per request. The methods will manipulate a configuration object stored in
ENV
. The provided Rack middleware will mergeSecureHeader::header_hash
into the rack headers. By default, the library will read the values out fromENV
and fall back to globally configured options. This includes headers not mentioned as having dedicated APIs for changing the settings (i.e. if you really want to, modify the ENV for that config, but you really shouldn't)./cc @gregose @mastahyeti @ptoomey3
The text was updated successfully, but these errors were encountered: