Skip to content

Commit

Permalink
Merge pull request rails#48830 from adrianna-chang-shopify/ac-ssl-mid…
Browse files Browse the repository at this point in the history 
…dleware-rack-lint

Make `ActionDispatch::SSL` compatible with Rack 3.0
  • Loading branch information
@guilleiguaran
guilleiguaran authored Jul 28, 2023
2 parents 43be5c4 + 9d840a1 commit 596c2a0
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 48 deletions.
2 changes: 2 additions & 0 deletions actionpack/lib/action_dispatch/constants.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module Constants
FEATURE_POLICY = "Feature-Policy"
X_REQUEST_ID = "X-Request-Id"
SERVER_TIMING = "Server-Timing"
STRICT_TRANSPORT_SECURITY = "Strict-Transport-Security"
else
VARY = "vary"
CONTENT_ENCODING = "content-encoding"
Expand All @@ -23,6 +24,7 @@ module Constants
FEATURE_POLICY = "feature-policy"
X_REQUEST_ID = "x-request-id"
SERVER_TIMING = "server-timing"
STRICT_TRANSPORT_SECURITY = "strict-transport-security"
end
end
end
24 changes: 16 additions & 8 deletions actionpack/lib/action_dispatch/middleware/ssl.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def call(env)

private
def set_hsts_header!(headers)
headers["Strict-Transport-Security"] ||= @hsts_header
headers[Constants::STRICT_TRANSPORT_SECURITY] ||= @hsts_header
end

def normalize_hsts_options(options)
Expand All @@ -114,25 +114,33 @@ def build_hsts_header(hsts)
end

def flag_cookies_as_secure!(headers)
if cookies = headers["Set-Cookie"]
if cookies.is_a?(String)
cookies = cookies.split("\n")
end
cookies = headers[Rack::SET_COOKIE]
return unless cookies

headers["Set-Cookie"] = cookies.map { |cookie|
if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
cookies = cookies.split("\n")
headers[Rack::SET_COOKIE] = cookies.map { |cookie|
if !/;\s*secure\s*(;|$)/i.match?(cookie)
"#{cookie}; secure"
else
cookie
end
}.join("\n")
else
headers[Rack::SET_COOKIE] = Array(cookies).map do |cookie|
if !/;\s*secure\s*(;|$)/i.match?(cookie)
"#{cookie}; secure"
else
cookie
end
end
end
end

def redirect_to_https(request)
[ @redirect.fetch(:status, redirection_status(request)),
{ "Content-Type" => "text/html; charset=utf-8",
"Location" => https_location_for(request) },
{ Rack::CONTENT_TYPE => "text/html; charset=utf-8",
Constants::LOCATION => https_location_for(request) },
(@redirect[:body] || []) ]
end

Expand Down
96 changes: 56 additions & 40 deletions actionpack/test/dispatch/ssl_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
require "abstract_unit"

class SSLTest < ActionDispatch::IntegrationTest
HEADERS = { "Content-Type" => "text/html" }

attr_accessor :app

def build_app(headers: {}, ssl_options: {})
headers = HEADERS.merge(headers)
ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, **ssl_options.reverse_merge(hsts: { subdomains: true })
app = lambda { |env| [200, headers, []] }
Rack::Lint.new(
ActionDispatch::SSL.new(
Rack::Lint.new(app),
**ssl_options.reverse_merge(hsts: { subdomains: true }),
)
)
end
end

Expand Down Expand Up @@ -128,9 +131,9 @@ def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }
self.app = build_app ssl_options: { hsts: hsts }, headers: headers
get url
if expected.nil?
assert_nil response.headers["Strict-Transport-Security"]
assert_nil response.headers["strict-transport-security"]
else
assert_equal expected, response.headers["Strict-Transport-Security"]
assert_equal expected, response.headers["strict-transport-security"]
end
end

Expand All @@ -143,7 +146,8 @@ def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }
end

test "defers to app-provided header" do
assert_hsts "app-provided", headers: { "Strict-Transport-Security" => "app-provided" }
headers = { ActionDispatch::Constants::STRICT_TRANSPORT_SECURITY => "app-provided" }
assert_hsts "app-provided", headers: headers
end

test "hsts: true enables default settings" do
Expand Down Expand Up @@ -180,82 +184,94 @@ def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }
end

class SecureCookiesTest < SSLTest
DEFAULT = %(id=1; path=/\ntoken=abc; path=/; secure; HttpOnly)

def get(**options)
self.app = build_app(**options)
super "https://example.org"
end

def assert_cookies(*expected)
assert_equal expected, response.headers["Set-Cookie"].split("\n")
DEFAULT = if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
%(id=1; path=/\ntoken=abc; path=/; secure; HttpOnly)
else
["id=1; path=/", "token=abc; path=/; secure; HttpOnly"]
end

def test_flag_cookies_as_secure
get headers: { "Set-Cookie" => DEFAULT }
get headers: { Rack::SET_COOKIE => DEFAULT }
assert_cookies "id=1; path=/; secure", "token=abc; path=/; secure; HttpOnly"
end

def test_flag_cookies_as_secure_with_single_cookie_in_array
get headers: { "Set-Cookie" => ["id=1"] }
assert_cookies "id=1; secure"
end

def test_flag_cookies_as_secure_with_multiple_cookies_in_array
get headers: { "Set-Cookie" => ["id=1", "problem=def"] }
assert_cookies "id=1; secure", "problem=def; secure"
end

def test_flag_cookies_as_secure_at_end_of_line
get headers: { "Set-Cookie" => "problem=def; path=/; HttpOnly; secure" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; HttpOnly; secure" }
assert_cookies "problem=def; path=/; HttpOnly; secure"
end

def test_flag_cookies_as_secure_with_more_spaces_before
get headers: { "Set-Cookie" => "problem=def; path=/; HttpOnly; secure" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; HttpOnly; secure" }
assert_cookies "problem=def; path=/; HttpOnly; secure"
end

def test_flag_cookies_as_secure_with_more_spaces_after
get headers: { "Set-Cookie" => "problem=def; path=/; secure; HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; secure; HttpOnly" }
assert_cookies "problem=def; path=/; secure; HttpOnly"
end

def test_flag_cookies_as_secure_with_has_not_spaces_before
get headers: { "Set-Cookie" => "problem=def; path=/;secure; HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/;secure; HttpOnly" }
assert_cookies "problem=def; path=/;secure; HttpOnly"
end

def test_flag_cookies_as_secure_with_has_not_spaces_after
get headers: { "Set-Cookie" => "problem=def; path=/; secure;HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; secure;HttpOnly" }
assert_cookies "problem=def; path=/; secure;HttpOnly"
end

def test_flag_cookies_as_secure_with_ignore_case
get headers: { "Set-Cookie" => "problem=def; path=/; Secure; HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; Secure; HttpOnly" }
assert_cookies "problem=def; path=/; Secure; HttpOnly"
end

def test_cookies_as_not_secure_with_secure_cookies_disabled
get headers: { "Set-Cookie" => DEFAULT }, ssl_options: { secure_cookies: false }
assert_cookies(*DEFAULT.split("\n"))
get headers: { Rack::SET_COOKIE => DEFAULT }, ssl_options: { secure_cookies: false }
assert_cookies("id=1; path=/", "token=abc; path=/; secure; HttpOnly")
end

def test_cookies_as_not_secure_with_exclude
excluding = { exclude: -> request { /example/.match?(request.domain) } }
get headers: { "Set-Cookie" => DEFAULT }, ssl_options: { redirect: excluding }
get headers: { Rack::SET_COOKIE => DEFAULT }, ssl_options: { redirect: excluding }

assert_cookies(*DEFAULT.split("\n"))
assert_cookies("id=1; path=/", "token=abc; path=/; secure; HttpOnly")
assert_response :ok
end

def test_no_cookies
get
assert_nil response.headers["Set-Cookie"]
assert_nil response.headers[Rack::SET_COOKIE]
end

def test_keeps_original_headers_behavior
get headers: { "Connection" => "close" }
assert_equal "close", response.headers["Connection"]
get headers: { "connection" => "close" }
assert_equal "close", response.headers["connection"]
end

# Array-based headers are only supported in Rack 3+
if Gem::Version.new(Rack::RELEASE) >= Gem::Version.new("3")
def test_flag_cookies_as_secure_with_single_cookie_in_array
get headers: { Rack::SET_COOKIE => ["id=1"] }
assert_cookies "id=1; secure"
end

def test_flag_cookies_as_secure_with_multiple_cookies_in_array
get headers: { Rack::SET_COOKIE => ["id=1", "problem=def"] }
assert_cookies "id=1; secure", "problem=def; secure"
end
end

private
def get(**options)
self.app = build_app(**options)
super "https://example.org"
end

def assert_cookies(*expected)
cookies = response.headers[Rack::SET_COOKIE]
if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
cookies = cookies.split("\n")
end
assert_equal expected, cookies
end
end

0 comments on commit 596c2a0

Please sign in to comment.