-
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
[WIP] New API Implementation #181
Conversation
HEADER_NAME = "Public-Key-Pins".freeze | ||
HASH_ALGORITHMS = [:sha256].freeze | ||
DIRECTIVES = [:max_age].freeze | ||
CONFIG_KEY = :hpkp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're using these in the request.env
, it seems like they should be more unique to avoid conflicts. Maybe SECURE_HEADERS_HPKP
, for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these CONFIG_KEYS will be scoped to https://github.com/twitter/secureheaders/pull/181/files#diff-e7fcd10663004caed933b95e0ff2e643R23 in https://github.com/twitter/secureheaders/pull/181/files#diff-e7fcd10663004caed933b95e0ff2e643R160. That will be "unsupported" when I remove the line from your comment about pulling the config from two different places in request.env
.
Unfortunately that's a left over from when I thought passing a hash directly to header_hash_for
would be a thing. 💩
This PR is entirely too large. Having responded to @mastahyeti's comments I'd say it's pretty unusable. I'm going to create a 3.x branch, commit the mass deletion of the fixtures directory, and create a new PR based on the 3.x branch. It will somewhat shorten this (although I still think it's better to look at the code as new and ignore the diff in general) |
Configure a global default and named overrides Use helper methods to set/modify configurations at runtime Set the headers in middleware based on the configuration saved to request.env Configuration changes: All headers require string values except for CSP and HPKP CSP directives must be arrays of strings, no more support for space-delimited strings or procs
1ff4bd6
to
32bb3f5
Compare
clsoing in favor of #191 |
This diff is unnecessarily large, probably better to just checkout the source and view it without the diff.
major changes
request.env
(isThread.local
better?), values are determined and set in rack middleware.append_content_security_policy_exception, override_content_security_policy_directives
,content_security_policy_nonce
), XFO (override_x_frame_options
), and HPKP (override_hpkp
) per request, unofficial API for overriding the rest. All are instance methods on a controller object and haveSecureHeaders
class methods too.SecureHeaders::opt_out_of(SecureHeaders::<HEADERNAME>::CONFIG_KEY)
Under the hood changes
Config changes
false
does not mean opt-out.SecureHeaders::OPT_OUT
does.self
none
inline
eval
will raise exceptions when validated. They are no longer valid and the standard values should be used'none'
,'self'
,'unsafe-inline'
,'unsafe-eval'
TODO
/cc @mastahyeti @ptoomey3 @gregose