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

Deprecate assigning single trusted_proxies value #40789

Merged
merged 1 commit into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
## Unreleased

* Deprecate the ability to assign a single value to `config.action_dispatch.trusted_proxies`
as `RemoteIp` middleware behaves inconsistently depending on whether this is configured
with a single value or an enumerable.

Fixes #40772

*Christian Sutter*

* Add `redirect_back_or_to(fallback_location, **)` as a more aesthetically pleasing version of `redirect_back fallback_location:, **`.
The old method name is retained without explicit deprecation.

Expand Down
20 changes: 16 additions & 4 deletions actionpack/lib/action_dispatch/middleware/remote_ip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,8 @@ class IpSpoofAttackError < StandardError; end
# clients (like WAP devices), or behind proxies that set headers in an
# incorrect or confusing way (like AWS ELB).
#
# The +custom_proxies+ argument can take an Array of string, IPAddr, or
# Regexp objects which will be used instead of +TRUSTED_PROXIES+. If a
# single string, IPAddr, or Regexp object is provided, it will be used in
# addition to +TRUSTED_PROXIES+. Any proxy setup will put the value you
# The +custom_proxies+ argument can take an enumerable which will be used
# instead of +TRUSTED_PROXIES+. Any proxy setup will put the value you
# want in the middle (or at the beginning) of the X-Forwarded-For list,
# with your proxy servers after it. If your proxies aren't removed, pass
# them in via the +custom_proxies+ parameter. That way, the middleware will
Expand All @@ -67,6 +65,20 @@ def initialize(app, ip_spoofing_check = true, custom_proxies = nil)
elsif custom_proxies.respond_to?(:any?)
custom_proxies
else
ActiveSupport::Deprecation.warn(<<~EOM)
Setting config.action_dispatch.trusted_proxies to a single value has
been deprecated. Please set this to an enumerable instead. For
example, instead of:

config.action_dispatch.trusted_proxies = IPAddr.new("10.0.0.0/8")

Wrap the value in an Array:

config.action_dispatch.trusted_proxies = [IPAddr.new("10.0.0.0/8")]

Note that unlike passing a single argument, passing an enumerable
will *replace* the default set of trusted proxies.
EOM
Array(custom_proxies) + TRUSTED_PROXIES
end
end
Expand Down
25 changes: 20 additions & 5 deletions railties/test/application/middleware/remote_ip_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,35 @@ def remote_ip(env = {})
assert_equal "4.2.42.42", remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "4.2.42.42")
end

test "the user can set trusted proxies" do
test "the user can set trusted proxies with an enumerable of case equality comparable values" do
make_basic_app do |app|
app.config.action_dispatch.trusted_proxies = /^4\.2\.42\.42$/
app.config.action_dispatch.trusted_proxies = [IPAddr.new("4.2.42.0/24"), /^4\.2\.42\.43$/, "4.2.42.44"]
end

assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "4.2.42.42")
assert_not_deprecated do
assert_equal "1.1.1.1",
remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "4.2.42.42,4.2.42.43,4.2.42.44")
end
end

test "the user can set trusted proxies with an IPAddr argument" do
test "setting trusted proxies replaces the default set of trusted proxies" do
make_basic_app do |app|
app.config.action_dispatch.trusted_proxies = [IPAddr.new("4.2.42.0/24")]
end

assert_equal "10.0.0.0",
remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "10.0.0.0,4.2.42.42")
end

test "setting trusted proxies to a single value (deprecated) adds value to default trusted proxies" do
make_basic_app do |app|
app.config.action_dispatch.trusted_proxies = IPAddr.new("4.2.42.0/24")
end

assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "10.0.0.0,4.2.42.42")
assert_deprecated(/Setting config\.action_dispatch\.trusted_proxies to a single value/) do
assert_equal "1.1.1.1",
remote_ip("REMOTE_ADDR" => "1.1.1.1", "HTTP_X_FORWARDED_FOR" => "10.0.0.0,4.2.42.42")
end
end
end
end