Skip to content

Commit

Permalink
Merge pull request #346 from twitter/opt-out-overuse
Browse files Browse the repository at this point in the history
Prevent global cookie OPT_OUT from blowing up in middleware
  • Loading branch information
oreoshake committed Jul 25, 2017
2 parents 501270c + 5d046a9 commit 90b6b2c
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/secure_headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module SecureHeaders
class NoOpHeaderConfig
include Singleton

def boom(arg = nil)
def boom(*args)
raise "Illegal State: attempted to modify NoOpHeaderConfig. Create a new config instead."
end

Expand Down
2 changes: 1 addition & 1 deletion lib/secure_headers/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def flag_cookies!(headers, config)

# disable Secure cookies for non-https requests
def override_secure(env, config = {})
if scheme(env) != "https"
if scheme(env) != "https" && config != OPT_OUT
config[:secure] = OPT_OUT
end

Expand Down
11 changes: 11 additions & 0 deletions spec/lib/secure_headers/middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,17 @@ module SecureHeaders
end
end

it "allows opting out of cookie protection with OPT_OUT alone" do
Configuration.default { |config| config.cookies = OPT_OUT}

# do NOT make this request https. non-https requests modify a config,
# causing an exception when operating on OPT_OUT. This ensures we don't
# try to modify the config.
request = Rack::Request.new({})
_, env = cookie_middleware.call request.env
expect(env["Set-Cookie"]).to eq("foo=bar")
end

context "cookies should not be flagged" do
it "does not flags cookies as secure" do
Configuration.default { |config| config.cookies = {secure: OPT_OUT, httponly: OPT_OUT, samesite: OPT_OUT} }
Expand Down

0 comments on commit 90b6b2c

Please sign in to comment.