Repository: wojtekmach/req (elixir)

Issue:
`follow_redirects`: Do not send authorization header on redirects to different hosts
See `curl --locattion` and `curl --location-trusted` for more information.

Pull request description:
`follow_redirects`: Don't send auth headers on redirect to different host
Closes #13.

Patch:
diff --git a/README.md b/README.md
index 73fff43..d9ba754 100644
--- a/README.md
+++ b/README.md
@@ -25,7 +25,7 @@ Req is an HTTP client with a focus on ease of use and composability, built on to
 
   * Range requests (via [`put_range`](`Req.Steps.put_range/2`) step)
 
-  * Follows redirects (via [`follow_redirects`](`Req.Steps.follow_redirects/1`) step)
+  * Follows redirects (via [`follow_redirects`](`Req.Steps.follow_redirects/2`) step)
 
   * Retries on errors (via [`retry`](`Req.Steps.retry/2`) step)
 
diff --git a/lib/req/request.ex b/lib/req/request.ex
index 84c49da..bf3b7e8 100644
--- a/lib/req/request.ex
+++ b/lib/req/request.ex
@@ -124,7 +124,8 @@ defmodule Req.Request do
     request_steps: [],
     response_steps: [],
     error_steps: [],
-    private: %{}
+    private: %{},
+    location_trusted: false
   ]
 
   @doc """
diff --git a/lib/req/steps.ex b/lib/req/steps.ex
index 1b07723..2ea6e05 100644
--- a/lib/req/steps.ex
+++ b/lib/req/steps.ex
@@ -36,7 +36,7 @@ defmodule Req.Steps do
     * [`&retry(&1, options[:retry])`](`retry/2`) (if `options[:retry]` is set to
       an atom true or a options keywords list)
 
-    * `follow_redirects/1`
+    * [`&follow_redirects(&1, options[:follow_redirects])`](`follow_redirects/2`)
 
     * `decompress/1`
 
@@ -67,6 +67,9 @@ defmodule Req.Steps do
 
     * `:steps` - if set, runs the `run_steps/2` step with the given steps
 
+    * `:follow_redirects` - if set, runs the `follow_redirect/2` step with the given list
+       of options
+
   """
   @doc step: :request
   def put_default_steps(request, options \\ []) do
@@ -91,7 +94,7 @@ defmodule Req.Steps do
 
     response_steps =
       maybe_steps(retry, [{Req.Steps, :retry, [retry]}]) ++
-        [{Req.Steps, :follow_redirects, []}] ++
+        [{Req.Steps, :follow_redirects, [options[:follow_redirects]]}] ++
         maybe_steps(not raw?, [
           {Req.Steps, :decompress, []},
           {Req.Steps, :decode_body, []}
@@ -621,6 +624,12 @@ defmodule Req.Steps do
   @doc """
   Follows redirects.
 
+  ## Options
+
+    * `:location_trusted` - by default, authorization credentials are only sent
+      on redirects to the same host. If `:location_trusted` is set to `true`, credentials
+      will be sent to any host.
+
   ## Examples
 
       iex> Req.get!("http://api.github.com").status
@@ -629,24 +638,26 @@ defmodule Req.Steps do
 
   """
   @doc step: :response
-  def follow_redirects({request, %{status: status} = response}) when status in 301..302 do
+  def follow_redirects(request, options \\ [])
+
+  def follow_redirects({request, %{status: status} = response}, options)
+      when status in 301..302 do
     {_, location} = List.keyfind(response.headers, "location", 0)
     Logger.debug(["Req.follow_redirects/2: Redirecting to ", location])
 
+    location_trusted = options[:location_trusted]
+    location_url = URI.parse(location)
+
     request =
-      if String.starts_with?(location, "/") do
-        url = URI.parse(location)
-        update_in(request.url, &%{&1 | path: url.path, query: url.query})
-      else
-        url = URI.parse(location)
-        put_in(request.url, url)
-      end
+      request
+      |> remove_credentials_if_untrusted(location_trusted, location_url)
+      |> put_redirect_location(location_url)
 
     {_, result} = Req.Request.run(request)
     {Req.Request.halt(request), result}
   end
 
-  def follow_redirects(other) do
+  def follow_redirects(other, _) do
     other
   end
 
@@ -829,4 +840,29 @@ defmodule Req.Steps do
   defp parse_netrc([], nil, acc) do
     acc
   end
+
+  defp put_redirect_location(request, location_url) do
+    if location_url.host do
+      put_in(request.url, location_url)
+    else
+      update_in(request.url, &%{&1 | path: location_url.path, query: location_url.query})
+    end
+  end
+
+  defp remove_credentials_if_untrusted(request, true, _), do: request
+
+  defp remove_credentials_if_untrusted(request, _, location_url) do
+    if location_url.host == request.url.host do
+      request
+    else
+      remove_credentials(request)
+    end
+  end
+
+  defp remove_credentials(request) do
+    headers = List.keydelete(request.headers, "authorization", 0)
+    request_steps = Enum.reject(request.request_steps, fn {_, step, _} -> step == :auth end)
+
+    %{request | headers: headers, request_steps: request_steps}
+  end
 end


Test patch:
diff --git a/test/req/steps_test.exs b/test/req/steps_test.exs
index 733f986..48eac69 100644
--- a/test/req/steps_test.exs
+++ b/test/req/steps_test.exs
@@ -353,6 +353,100 @@ defmodule Req.StepsTest do
            end) =~ "[debug] Req.follow_redirects/2: Redirecting to /ok?a=1"
   end
 
+  test "follow_redirects/2: auth - same host", c do
+    auth_header = {"authorization", "Basic " <> Base.encode64("foo:bar")}
+
+    Bypass.expect(c.bypass, "GET", "/redirect", fn conn ->
+      location = c.url <> "/auth"
+
+      assert auth_header in conn.req_headers
+
+      conn
+      |> Plug.Conn.put_resp_header("location", location)
+      |> Plug.Conn.send_resp(302, "redirecting to #{location}")
+    end)
+
+    Bypass.expect(c.bypass, "GET", "/auth", fn conn ->
+      assert auth_header in conn.req_headers
+      Plug.Conn.send_resp(conn, 200, "ok")
+    end)
+
+    assert ExUnit.CaptureLog.capture_log(fn ->
+             assert Req.get!(c.url <> "/redirect", auth: {"foo", "bar"}).status == 200
+           end) =~ "[debug] Req.follow_redirects/2: Redirecting to #{c.url}/auth"
+  end
+
+  test "follow_redirects/2: auth - location trusted" do
+    adapter = fn request ->
+      case request.url.host do
+        "original" ->
+          assert List.keyfind(request.headers, "authorization", 0)
+
+          response = %Req.Response{
+            status: 301,
+            headers: [{"location", "http://untrusted"}],
+            body: "redirecting"
+          }
+
+          {request, response}
+
+        "untrusted" ->
+          assert List.keyfind(request.headers, "authorization", 0)
+
+          response = %Req.Response{
+            status: 200,
+            headers: [],
+            body: "bad things"
+          }
+
+          {request, response}
+      end
+    end
+
+    assert ExUnit.CaptureLog.capture_log(fn ->
+             assert Req.get!("http://original",
+                      steps: [adapter],
+                      auth: {"authorization", "credentials"},
+                      follow_redirects: [location_trusted: true]
+                    ).status == 200
+           end) =~ "[debug] Req.follow_redirects/2: Redirecting to http://untrusted"
+  end
+
+  test "follow_redirects/2: auth - different host" do
+    adapter = fn request ->
+      case request.url.host do
+        "original" ->
+          assert List.keyfind(request.headers, "authorization", 0)
+
+          response = %Req.Response{
+            status: 301,
+            headers: [{"location", "http://untrusted"}],
+            body: "redirecting"
+          }
+
+          {request, response}
+
+        "untrusted" ->
+          refute List.keyfind(request.headers, "authorization", 0)
+
+          response = %Req.Response{
+            status: 200,
+            headers: [],
+            body: "bad things"
+          }
+
+          {request, response}
+      end
+    end
+
+    assert ExUnit.CaptureLog.capture_log(fn ->
+             assert Req.get!("http://original",
+                      steps: [adapter],
+                      auth: {"authorization", "credentials"}
+                    ).status == 200
+           end) =~ "[debug] Req.follow_redirects/2: Redirecting to http://untrusted"
+  end
+
   ## Error steps
 
   @tag :capture_log