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

Secure Cookies #231

Merged
merged 3 commits into from
Mar 16, 2016
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/secure_headers/configuration.rb
Original file line number Diff line number Diff line change
@@ -90,7 +90,7 @@ def add_noop_configuration

attr_accessor :hsts, :x_frame_options, :x_content_type_options,
:x_xss_protection, :csp, :x_download_options, :x_permitted_cross_domain_policies,
:hpkp
:hpkp, :secure_cookies
attr_reader :cached_headers

def initialize(&block)
26 changes: 26 additions & 0 deletions lib/secure_headers/middleware.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module SecureHeaders
class Middleware
SECURE_COOKIE_REGEXP = /;\s*secure\s*(;|$)/i.freeze

def initialize(app)
@app = app
end
@@ -8,8 +10,32 @@ def initialize(app)
def call(env)
req = Rack::Request.new(env)
status, headers, response = @app.call(env)

flag_cookies_as_secure!(headers) if config(req).secure_cookies
headers.merge!(SecureHeaders.header_hash_for(req))
[status, headers, response]
end

private

# inspired by https://github.com/tobmatth/rack-ssl-enforcer/blob/6c014/lib/rack/ssl-enforcer.rb#L183-L194
def flag_cookies_as_secure!(headers)
if cookies = headers['Set-Cookie']
# Support Rails 2.3 / Rack 1.1 arrays as headers
cookies = cookies.split("\n") unless cookies.is_a?(Array)

headers['Set-Cookie'] = cookies.map do |cookie|
if cookie !~ SECURE_COOKIE_REGEXP
"#{cookie}; secure"
else
cookie
end
end.join("\n")
end
end

def config(req)
req.env[SECURE_HEADERS_CONFIG] || Configuration.get(Configuration::DEFAULT_CONFIG)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oreoshake is there an easier way to access the config from this context?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

req.env[SECURE_HEADERS_CONFIG] should suffice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, will help keep things dry

end
end
end
2 changes: 1 addition & 1 deletion lib/secure_headers/railtie.rb
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ class Railtie < Rails::Railtie
'Public-Key-Pins', 'Public-Key-Pins-Report-Only']

initializer "secure_headers.middleware" do
Rails.application.config.middleware.use SecureHeaders::Middleware
Rails.application.config.middleware.insert_before 0, SecureHeaders::Middleware
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this feature to work correctly SecureHeaders::Middleware needs to come before the session middleware (or simply at the top of the rack stack).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine.

end

initializer "secure_headers.action_controller" do
2 changes: 1 addition & 1 deletion spec/lib/secure_headers/configuration_spec.rb
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ module SecureHeaders
config = Configuration.get(:test_override)
noop = Configuration.get(Configuration::NOOP_CONFIGURATION)
[:hsts, :x_frame_options, :x_content_type_options, :x_xss_protection,
:x_download_options, :x_permitted_cross_domain_policies, :hpkp, :csp].each do |key|
:x_download_options, :x_permitted_cross_domain_policies, :hpkp, :csp, :secure_cookies].each do |key|

expect(config.send(key)).to eq(noop.send(key)), "Value not copied: #{key}."
end
24 changes: 21 additions & 3 deletions spec/lib/secure_headers/middleware_spec.rb
Original file line number Diff line number Diff line change
@@ -3,10 +3,10 @@
module SecureHeaders
describe Middleware do
let(:app) { ->(env) { [200, env, "app"] } }
let(:cookie_app) { -> (env) { [200, env.merge("Set-Cookie" => "foo=bar"), "app"] } }

let :middleware do
Middleware.new(app)
end
let(:middleware) { Middleware.new(app) }
let(:cookie_middleware) { Middleware.new(cookie_app) }

before(:each) do
reset_config
@@ -36,5 +36,23 @@ module SecureHeaders
_, env = middleware.call request.env
expect(env[CSP::HEADER_NAME]).to match("example.org")
end

context "cookies should be flagged" do
it "flags cookies as secure" do
Configuration.default { |config| config.secure_cookies = true }
request = Rack::MockRequest.new(cookie_middleware)
response = request.get '/'
expect(response.headers['Set-Cookie']).to match(Middleware::SECURE_COOKIE_REGEXP)
end
end

context "cookies should not be flagged" do
it "does not flags cookies as secure" do
Configuration.default { |config| config.secure_cookies = false }
request = Rack::MockRequest.new(cookie_middleware)
response = request.get '/'
expect(response.headers['Set-Cookie']).not_to match(Middleware::SECURE_COOKIE_REGEXP)
end
end
end
end