Extract Plug.RewriteOn from Plug.SSL (elixir-plug#990)

* RewriteOn: Add a new plug with the rewrite on logic from Plug.SSL * SSL: Delegate the "rewrite_on" logic to `Plug.RewriteOn` * SSL: Adjust the moduledoc to refer to `Plug.RewriteOn`
blockfi · Oct 9, 2020 · d685f45 · d685f45
1 parent 325ee50
commit d685f45
Show file tree

Hide file tree

Showing 3 changed files with 156 additions and 71 deletions.
diff --git a/lib/plug/rewrite_on.ex b/lib/plug/rewrite_on.ex
@@ -0,0 +1,87 @@
+defmodule Plug.RewriteOn do
+  @moduledoc """
+  A plug to rewrite the request's host/port/protocol from `x-forwarded-*` headers.
+
+  If your Plug application is behind a proxy that handles HTTPS, you may
+  need to tell Plug to parse the proper protocol from the `x-forwarded-*`
+  header.
+
+      plug Plug.RewriteOn, [:x_forwarded_host, :x_forwarded_port, :x_forwarded_proto]
+
+  The supported values are:
+
+    * `:x_forwarded_host` - to override the host based on on the "x-forwarded-host" header
+    * `:x_forwarded_port` - to override the port based on on the "x-forwarded-port" header
+    * `:x_forwarded_proto` - to override the protocol based on on the "x-forwarded-proto" header
+
+  Since rewriting the scheme based on `x-forwarded-*` headers can open up
+  security vulnerabilities, only use this plug if:
+
+    * your app is behind a proxy
+    * your proxy strips the given `x-forwarded-*` headers from all incoming requests
+    * your proxy sets the `x-forwarded-*` headers and sends it to Plug
+  """
+  @behaviour Plug
+
+  import Plug.Conn, only: [get_req_header: 2]
+
+  @impl true
+  def init(header), do: List.wrap(header)
+
+  @impl true
+  def call(conn, [:x_forwarded_proto | rewrite_on]) do
+    conn
+    |> put_scheme(get_req_header(conn, "x-forwarded-proto"))
+    |> call(rewrite_on)
+  end
+
+  def call(conn, [:x_forwarded_port | rewrite_on]) do
+    conn
+    |> put_port(get_req_header(conn, "x-forwarded-port"))
+    |> call(rewrite_on)
+  end
+
+  def call(conn, [:x_forwarded_host | rewrite_on]) do
+    conn
+    |> put_host(get_req_header(conn, "x-forwarded-host"))
+    |> call(rewrite_on)
+  end
+
+  def call(_conn, [other | _rewrite_on]) do
+    raise "unknown rewrite: #{inspect(other)}"
+  end
+
+  def call(conn, []) do
+    conn
+  end
+
+  defp put_scheme(%{scheme: :http, port: 80} = conn, ["https"]),
+    do: %{conn | scheme: :https, port: 443}
+
+  defp put_scheme(conn, ["https"]),
+    do: %{conn | scheme: :https}
+
+  defp put_scheme(%{scheme: :https, port: 443} = conn, ["http"]),
+    do: %{conn | scheme: :http, port: 80}
+
+  defp put_scheme(conn, ["http"]),
+    do: %{conn | scheme: :http}
+
+  defp put_scheme(conn, _scheme),
+    do: conn
+
+  defp put_host(conn, [proper_host]),
+    do: %{conn | host: proper_host}
+
+  defp put_host(conn, _),
+    do: conn
+
+  defp put_port(conn, headers) do
+    with [header] <- headers,
+         {port, ""} <- Integer.parse(header) do
+      %{conn | port: port}
+    else
+      _ -> conn
+    end
+  end
+end
diff --git a/lib/plug/ssl.ex b/lib/plug/ssl.ex
@@ -19,20 +19,9 @@ defmodule Plug.SSL do
   need to tell Plug to parse the proper protocol from the `x-forwarded-*`
   header. This can be done using the `:rewrite_on` option:
 
-      plug Plug.SSL, rewrite_on: [:x_forwarded_proto, :x_forwarded_host, :x_forwarded_port]
+      plug Plug.SSL, rewrite_on: [:x_forwarded_host, :x_forwarded_port, :x_forwarded_proto]
 
-  The supported values are:
-
-    * `:x_forwarded_host` - to override the host based on on the "x-forwarded-host" header
-    * `:x_forwarded_port` - to override the port based on on the "x-forwarded-port" header
-    * `:x_forwarded_proto` - to override the protocol based on on the "x-forwarded-proto" header
-
-  Since rewriting the scheme based on `x-forwarded-*` headers can open up
-  security vulnerabilities, only provide the option above if:
-
-    * your app is behind a proxy
-    * your proxy strips the given `x-forwarded-*` headers from all incoming requests
-    * your proxy sets the `x-forwarded-*` headers and sends it to Plug
+  For further details refer to `Plug.RewriteOn`.
 
   ## Plug Options
 
@@ -316,15 +305,15 @@ defmodule Plug.SSL do
   @impl true
   def init(opts) do
     host = Keyword.get(opts, :host)
-    rewrite_on = List.wrap(Keyword.get(opts, :rewrite_on))
+    rewrite_on = Plug.RewriteOn.init(Keyword.get(opts, :rewrite_on))
     log = Keyword.get(opts, :log, :info)
     exclude = Keyword.get(opts, :exclude, ["localhost"])
     {hsts_header(opts), exclude, host, rewrite_on, log}
   end
 
   @impl true
   def call(conn, {hsts, exclude, host, rewrite_on, log_level}) do
-    conn = rewrite_on(conn, rewrite_on)
+    conn = Plug.RewriteOn.call(conn, rewrite_on)
 
     cond do
       excluded?(conn.host, exclude) -> conn
@@ -336,62 +325,6 @@ defmodule Plug.SSL do
   defp excluded?(host, list) when is_list(list), do: :lists.member(host, list)
   defp excluded?(host, {mod, fun, args}), do: apply(mod, fun, [host | args])
 
-  defp rewrite_on(conn, [:x_forwarded_proto | rewrite_on]) do
-    conn
-    |> put_scheme(get_req_header(conn, "x-forwarded-proto"))
-    |> rewrite_on(rewrite_on)
-  end
-
-  defp rewrite_on(conn, [:x_forwarded_port | rewrite_on]) do
-    conn
-    |> put_port(get_req_header(conn, "x-forwarded-port"))
-    |> rewrite_on(rewrite_on)
-  end
-
-  defp rewrite_on(conn, [:x_forwarded_host | rewrite_on]) do
-    conn
-    |> put_host(get_req_header(conn, "x-forwarded-host"))
-    |> rewrite_on(rewrite_on)
-  end
-
-  defp rewrite_on(_conn, [other | _rewrite_on]) do
-    raise "unknown rewrite: #{inspect(other)}"
-  end
-
-  defp rewrite_on(conn, []) do
-    conn
-  end
-
-  defp put_scheme(%{scheme: :http, port: 80} = conn, ["https"]),
-    do: %{conn | scheme: :https, port: 443}
-
-  defp put_scheme(conn, ["https"]),
-    do: %{conn | scheme: :https}
-
-  defp put_scheme(%{scheme: :https, port: 443} = conn, ["http"]),
-    do: %{conn | scheme: :http, port: 80}
-
-  defp put_scheme(conn, ["http"]),
-    do: %{conn | scheme: :http}
-
-  defp put_scheme(conn, _scheme),
-    do: conn
-
-  defp put_host(conn, [proper_host]),
-    do: %{conn | host: proper_host}
-
-  defp put_host(conn, _),
-    do: conn
-
-  defp put_port(conn, headers) do
-    with [header] <- headers,
-         {port, ""} <- Integer.parse(header) do
-      %{conn | port: port}
-    else
-      _ -> conn
-    end
-  end
-
   # http://tools.ietf.org/html/draft-hodges-strict-transport-sec-02
   defp hsts_header(opts) do
     if Keyword.get(opts, :hsts, true) do

diff --git a/test/plug/rewrite_on_test.exs b/test/plug/rewrite_on_test.exs
@@ -0,0 +1,65 @@
+defmodule Plug.RewriteOnTest do
+  use ExUnit.Case, async: true
+  use Plug.Test
+
+  defp call(conn, rewrite) do
+    Plug.RewriteOn.call(conn, Plug.RewriteOn.init(rewrite))
+  end
+
+  test "rewrites http to https based on x-forwarded-proto" do
+    conn =
+      conn(:get, "http://example.com/")
+      |> put_req_header("x-forwarded-proto", "https")
+      |> call(:x_forwarded_proto)
+
+    assert conn.scheme == :https
+    assert conn.port == 443
+  end
+
+  test "doesn't change the port when it doesn't match the scheme" do
+    conn =
+      conn(:get, "http://example.com:1234/")
+      |> put_req_header("x-forwarded-proto", "https")
+      |> call(:x_forwarded_proto)
+
+    assert conn.scheme == :https
+    assert conn.port == 1234
+  end
+
+  test "rewrites host with a x-forwarder-host header" do
+    conn =
+      conn(:get, "http://example.com/")
+      |> put_req_header("x-forwarded-host", "truessl.example.com")
+      |> call(:x_forwarded_host)
+
+    assert conn.host == "truessl.example.com"
+  end
+
+  test "rewrites port with a x-forwarder-port header" do
+    conn =
+      conn(:get, "http://example.com/")
+      |> put_req_header("x-forwarded-port", "3030")
+      |> call(:x_forwarded_port)
+
+    assert conn.port == 3030
+  end
+
+  test "rewrites the host, the port, and the protocol" do
+    conn =
+      conn(:get, "http://example.com/")
+      |> put_req_header("x-forwarded-host", "truessl.example.com")
+      |> put_req_header("x-forwarded-port", "3030")
+      |> put_req_header("x-forwarded-proto", "https")
+      |> call([:x_forwarded_host, :x_forwarded_port, :x_forwarded_proto])
+
+    assert conn.host == "truessl.example.com"
+    assert conn.port == 3030
+    assert conn.scheme == :https
+  end
+
+  test "raises when receiving an unknown rewrite" do
+    assert_raise RuntimeError, "unknown rewrite: :x_forwarded_other", fn ->
+      call(conn(:get, "http://example.com/"), :x_forwarded_other)
+    end
+  end
+end