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

Filter sensitive data from inspect(Tesla.Client.t) #534

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions lib/tesla/client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,46 @@ defmodule Tesla.Client do
defp unruntime({module, :call, [[]]}) when is_atom(module), do: module
defp unruntime({module, :call, [opts]}) when is_atom(module), do: {module, opts}
defp unruntime({:fn, fun}) when is_function(fun), do: fun

# TODO: Remove once we are done with all the middlewares
# defimpl Inspect do
# @filtered "[FILTERED]"
#
# @sensitive_opts %{
# Tesla.Middleware.BasicAuth => [:username, :password],
# Tesla.Middleware.BearerAuth => [:token],
# Tesla.Middleware.DigestAuth => [:username, :password]
# }
#
# def inspect(%Tesla.Client{} = client, opts) do
# client
# |> Map.update!(:pre, &filter_sensitive_opts/1)
# |> Inspect.Any.inspect(opts)
# end
#
# defp filter_sensitive_opts(middlewares) do
# Enum.map(middlewares, fn
# {middleware, :call, [opts]} ->
# sensitive_opts = Map.get(@sensitive_opts, middleware, [])
# filtered_opts = Enum.reduce(sensitive_opts, opts, &maybe_redact(&2, &1))
# {middleware, :call, [filtered_opts]}
#
# middleware ->
# middleware
# end)
# end
#
# defp maybe_redact(opts, key) do
# cond do
# is_map(opts) and Map.has_key?(opts, key) ->
# Map.put(opts, key, @filtered)
#
# is_list(opts) and Keyword.has_key?(opts, key) ->
# Keyword.put(opts, key, @filtered)
#
# true ->
# opts
# end
# end
# end
end
61 changes: 43 additions & 18 deletions lib/tesla/middleware/basic_auth.ex
Original file line number Diff line number Diff line change
@@ -1,38 +1,71 @@
defmodule Tesla.Middleware.BasicAuth do
@moduledoc """
@moduledoc ~S"""
Basic authentication middleware.

[Wiki on the topic](https://en.wikipedia.org/wiki/Basic_access_authentication)

## Examples

```
```elixir
defmodule MyClient do
use Tesla

# static configuration
plug Tesla.Middleware.BasicAuth, username: "user", password: "pass"
plug Tesla.Middleware.BasicAuth, Tesla.Middleware.BasicAuth.Options.new!(username: "user", password: "pass")

# dynamic user & pass
def new(username, password, opts \\\\ %{}) do
def new(username, password, opts \\ %{}) do
Tesla.client [
{Tesla.Middleware.BasicAuth, Map.merge(%{username: username, password: password}, opts)}
{Tesla.Middleware.BasicAuth, Tesla.Middleware.BasicAuth.Options.new!(Map.merge(%{username: username, password: password}, opts))}
]
end
end
```

## Options

- `:username` - username (defaults to `""`)
- `:password` - password (defaults to `""`)
Visit `t:Tesla.Middleware.BasicAuth.Options.t/0` to read more about the options.

> ### Using Map or Keyword List as Options {: .warning}
>
> It is possible to use `Map` or `Keyword` list as options, it is not recommended for security reasons.
> The `inspect/2` implementation of `Tesla.Middleware.BasicAuth.Options` will redact the `username` and `password`
> fields when you inspect the client.
"""

@behaviour Tesla.Middleware

defmodule Options do
@moduledoc """
Options for `Tesla.Middleware.BasicAuth`.
"""

@typedoc """
- `:username` - username (defaults to `""`)
- `:password` - password (defaults to `""`)
"""
@type t :: %__MODULE__{username: String.t(), password: String.t()}

@derive {Inspect, except: [:username, :password]}
defstruct username: "", password: ""

@doc """
Creates new `t:t/0` struct.
"""
@spec new!(%__MODULE__{} | Keyword.t() | map) :: t()
def new!(%__MODULE__{} = opts) do
opts
end

def new!(attrs) do
attrs = Map.merge(%{username: "", password: ""}, Enum.into(attrs, %{}))
struct!(__MODULE__, attrs)
end
end

@impl Tesla.Middleware
def call(env, next, opts) do
opts = opts || %{}
opts = Options.new!(opts)

env
|> Tesla.put_headers(authorization_header(opts))
Expand All @@ -41,23 +74,15 @@ defmodule Tesla.Middleware.BasicAuth do

defp authorization_header(opts) do
opts
|> authorization_vars()
|> encode()
|> create_header()
end

defp authorization_vars(opts) do
%{
username: opts[:username] || "",
password: opts[:password] || ""
}
end

defp create_header(auth) do
[{"authorization", "Basic #{auth}"}]
end

defp encode(%{username: username, password: password}) do
Base.encode64("#{username}:#{password}")
defp encode(%Options{} = opts) do
Base.encode64("#{opts.username}:#{opts.password}")
end
end
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ defmodule Tesla.Mixfile do
],
Middlewares: [
Tesla.Middleware.BaseUrl,
Tesla.Middleware.BasicAuth,
~r/Tesla.Middleware.BasicAuth/,
Tesla.Middleware.BearerAuth,
Tesla.Middleware.Compression,
Tesla.Middleware.CompressRequest,
Expand Down
17 changes: 17 additions & 0 deletions test/tesla/client_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,21 @@ defmodule Tesla.ClientTest do
assert middlewares == Tesla.Client.middleware(client)
end
end

describe "Inspect.Tesla.Client" do
test "ensures that no secrets are leaked in logs" do
middlewares = [
{Tesla.Middleware.BasicAuth, username: "secret", password: "secret", other: "OK"},
{Tesla.Middleware.BearerAuth, token: "secret", other: "OK"},
{Tesla.Middleware.DigestAuth, %{username: "secret", password: "secret", other: "OK"}}
]

inspected = middlewares |> Tesla.client() |> inspect()

refute String.contains?(inspected, "secret")
assert String.contains?(inspected, ~s(password: "[FILTERED]"))
assert String.contains?(inspected, ~s(username: "[FILTERED]"))
assert String.contains?(inspected, ~s(token: "[FILTERED]"))
end
end
end
28 changes: 22 additions & 6 deletions test/tesla/middleware/basic_auth_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ defmodule Tesla.Middleware.BasicAuthTest do
Tesla.client([
{
Tesla.Middleware.BasicAuth,
Map.merge(
%{
username: username,
password: password
},
opts
Tesla.Middleware.BasicAuth.Options.new!(
Map.merge(
%{
username: username,
password: password
},
opts
)
)
}
])
Expand All @@ -41,6 +43,20 @@ defmodule Tesla.Middleware.BasicAuthTest do
end
end

test "ensures that no secrets are leaked in logs" do
inspected =
Tesla.Middleware.BasicAuth.Options.new!(%{username: "admin", password: "secret"})
|> build_client()
|> inspect()

refute String.contains?(inspected, "admin")
refute String.contains?(inspected, "secret")
end

defp build_client(opts) do
Tesla.client([{Tesla.Middleware.BasicAuth, opts}])
end

test "sends request with proper authorization header" do
username = "Aladdin"
password = "OpenSesame"
Expand Down