Skip to content

Commit

Permalink
Improve errors
Browse files Browse the repository at this point in the history
  • Loading branch information
danschultzer committed Nov 18, 2023
1 parent 49fcba6 commit 15608fb
Show file tree
Hide file tree
Showing 19 changed files with 291 additions and 239 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## v0.2.8 (TBA)

- More expressive errors now including the whole HTTP response where applicable

## v0.2.7 (2023-09-12)

* `Assent.Strategy.Strava` added
Expand Down
103 changes: 55 additions & 48 deletions lib/assent.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,74 +6,81 @@ defmodule Assent do
end

defmodule CallbackCSRFError do
defexception [:message]
defexception [:key]

@spec new(binary()) :: %__MODULE__{}
def new(key) do
%__MODULE__{message: "CSRF detected with param key #{inspect key}"}
def message(exception) do
"CSRF detected with param key #{inspect exception.key}"
end
end

defmodule MissingParamError do
defexception [:message, :params]

@spec new(binary(), map()) :: %__MODULE__{}
def new(key, params) do
%__MODULE__{
message: "Expected #{inspect key} to exist in params, but only found the following keys: #{inspect Map.keys(params)}",
params: params
}
defexception [:expected_key, :params]

def message(exception) do
expected_key = inspect exception.expected_key
params = inspect Map.keys(exception.params)

"Expected #{expected_key} in params, got: #{params}"
end
end

defmodule RequestError do
defexception [:message, :error]
defexception [:message, :response]

alias Assent.HTTPAdapter.HTTPResponse

def message(exception) do
"""
#{exception.message}
#{HTTPResponse.format(exception.response)}
"""
end
end

defmodule InvalidResponseError do
defexception [:response]

alias Assent.HTTPAdapter.HTTPResponse

@spec unexpected(HTTPResponse.t()) :: %__MODULE__{}
def unexpected(response) do
%__MODULE__{
message:
"""
An unexpected success response was received:
#{inspect response.body}
""",
error: :unexpected_response
}
def message(exception) do
"""
An invalid response was received.
#{HTTPResponse.format(exception.response)}
"""
end
end

defmodule UnexpectedResponseError do
defexception [:response]

@spec invalid(HTTPResponse.t()) :: %__MODULE__{}
def invalid(response) do
%__MODULE__{
message:
"""
Server responded with status: #{response.status}
alias Assent.HTTPAdapter.HTTPResponse

Headers:#{Enum.reduce(response.headers, "", fn {k, v}, acc -> acc <> "\n#{k}: #{v}" end)}
def message(exception) do
"""
An unexpected response was received.
Body:
#{inspect response.body}
""",
error: :invalid_server_response
}
#{HTTPResponse.format(exception.response)}
"""
end
end

defmodule ServerUnreachableError do
defexception [:http_adapter, :request_url, :reason]

def message(exception) do
[url | _rest] = String.split(exception.request_url, "?", parts: 2)

@spec unreachable(atom(), binary(), term()) :: %__MODULE__{}
def unreachable(adapter, url, error) do
%__MODULE__{
message:
"""
Server was unreachable with #{inspect adapter}.
"""
The server was unreachable.
Failed with the following error:
#{inspect error}
HTTP Adapter: #{inspect exception.http_adapter}
Request URL: #{url}
URL: #{url}
""",
error: :unreachable
}
Reason:
#{inspect exception.reason}
"""
end
end

Expand Down
8 changes: 6 additions & 2 deletions lib/assent/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ defmodule Assent.Config do
defmodule MissingKeyError do
@type t :: %__MODULE__{}

defexception [:message]
defexception [:key]

def message(exception) do
"Key #{inspect exception.key} not found in config"
end
end

@type t :: Keyword.t()
Expand All @@ -18,7 +22,7 @@ defmodule Assent.Config do
def fetch(config, key) do
case Keyword.fetch(config, key) do
{:ok, value} -> {:ok, value}
:error -> {:error, MissingKeyError.exception("Key `:#{key}` not found in config")}
:error -> {:error, MissingKeyError.exception(key: key)}
end
end

Expand Down
21 changes: 20 additions & 1 deletion lib/assent/http_adapter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,31 @@ defmodule Assent.HTTPAdapter do

@type header :: {binary(), binary()}
@type t :: %__MODULE__{
http_adapter: atom(),
request_url: binary(),
status: integer(),
headers: [header()],
body: binary() | term()
}

defstruct status: 200, headers: [], body: ""
defstruct http_adapter: nil, request_url: nil, status: 200, headers: [], body: ""

def format(response) do
[request_url | _rest] = String.split(response.request_url, "?", parts: 2)

"""
HTTP Adapter: #{inspect response.http_adapter}
Request URL: #{request_url}
Response status: #{response.status}
Response headers:
#{Enum.reduce(response.headers, "", fn {k, v}, acc -> acc <> "\n#{k}: #{v}" end)}
Response body:
#{inspect response.body}
"""
end
end

@type method :: :get | :post
Expand Down
16 changes: 8 additions & 8 deletions lib/assent/jwt_adapter/assent_jwt.ex
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ defmodule Assent.JWTAdapter.AssentJWT do

case encode_json_base64(header, opts) do
{:ok, encoded_header} -> {:ok, encoded_header}
{:error, error} -> {:error, %Error{message: "Failed to encode header", reason: error, data: header}}
{:error, error} -> {:error, Error.exception(message: "Failed to encode header", reason: error, data: header)}
end
end

Expand All @@ -43,7 +43,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
defp encode_claims(claims, opts) do
case encode_json_base64(claims, opts) do
{:ok, encoded_claims} -> {:ok, encoded_claims}
{:error, error} -> {:error, %Error{message: "Failed to encode claims", reason: error, data: claims}}
{:error, error} -> {:error, Error.exception(message: "Failed to encode claims", reason: error, data: claims)}
end
end

Expand All @@ -52,7 +52,7 @@ defmodule Assent.JWTAdapter.AssentJWT do

case sign_message(message, alg, secret_or_private_key) do
{:ok, signature} -> {:ok, "#{message}.#{Base.url_encode64(signature, padding: false)}"}
{:error, error} -> {:error, %Error{message: "Failed to sign JWT", reason: error, data: {message, alg}}}
{:error, error} -> {:error, Error.exception(message: "Failed to sign JWT", reason: error, data: {message, alg})}
end
end

Expand Down Expand Up @@ -139,7 +139,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
defp split(token) do
case String.split(token, ".") do
[header, claims, signature] -> {:ok, %{header: header, claims: claims, signature: signature}}
parts -> {:error, %Error{message: "JWT must have exactly three parts", reason: :invalid_format, data: parts}}
parts -> {:error, Error.exception(message: "JWT must have exactly three parts", reason: :invalid_format, data: parts)}
end
end

Expand All @@ -150,7 +150,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
{:ok, alg} <- fetch_alg(header) do
{:ok, alg, header}
else
{:error, error} -> {:error, %Error{message: "Failed to decode header", reason: error, data: header}}
{:error, error} -> {:error, Error.exception(message: "Failed to decode header", reason: error, data: header)}
end
end

Expand All @@ -177,14 +177,14 @@ defmodule Assent.JWTAdapter.AssentJWT do
{:ok, claims} <- decode_json(claims, json_library) do
{:ok, claims}
else
{:error, error} -> {:error, %Error{message: "Failed to decode claims", reason: error, data: claims}}
{:error, error} -> {:error, Error.exception(message: "Failed to decode claims", reason: error, data: claims)}
end
end

defp decode_signature(signature) do
case decode_base64_url(signature) do
{:ok, signature} -> {:ok, signature}
{:error, error} -> {:error, %Error{message: "Failed to decode signature", reason: error, data: signature}}
{:error, error} -> {:error, Error.exception(message: "Failed to decode signature", reason: error, data: signature)}
end
end

Expand All @@ -193,7 +193,7 @@ defmodule Assent.JWTAdapter.AssentJWT do

case verify_message(message, signature, alg, secret_or_public_key) do
{:ok, verified} -> {:ok, verified}
{:error, error} -> {:error, %Error{message: "Failed to verify signature", reason: error, data: {message, signature, alg}}}
{:error, error} -> {:error, Error.exception(message: "Failed to verify signature", reason: error, data: {message, signature, alg})}
end
end

Expand Down
12 changes: 6 additions & 6 deletions lib/assent/strategies/oauth.ex
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ defmodule Assent.Strategy.OAuth do
@behaviour Assent.Strategy

alias Assent.Strategy, as: Helpers
alias Assent.{Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, RequestError}
alias Assent.{Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, InvalidResponseError, RequestError, UnexpectedResponseError}

@doc """
Generate authorization URL for request phase.
Expand Down Expand Up @@ -244,8 +244,8 @@ defmodule Assent.Strategy.OAuth do
defp process_token_response({:ok, %HTTPResponse{status: 200, body: %{"oauth_token" => _, "oauth_token_secret" => _} = token}}), do: {:ok, token}
defp process_token_response(any), do: process_response(any)

defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, RequestError.unexpected(response)}
defp process_response({:error, %HTTPResponse{} = response}), do: {:error, RequestError.invalid(response)}
defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, UnexpectedResponseError.exception(response: response)}
defp process_response({:error, %HTTPResponse{} = response}), do: {:error, InvalidResponseError.exception(response: response)}
defp process_response({:error, error}), do: {:error, error}

defp build_authorize_url({:ok, token}, config) do
Expand Down Expand Up @@ -298,10 +298,10 @@ defmodule Assent.Strategy.OAuth do
end

defp fetch_oauth_token(%{"oauth_token" => code}), do: {:ok, code}
defp fetch_oauth_token(params), do: {:error, MissingParamError.new("oauth_token", params)}
defp fetch_oauth_token(params), do: {:error, MissingParamError.exception(expected_key: "oauth_token", params: params)}

defp fetch_oauth_verifier(%{"oauth_verifier" => code}), do: {:ok, code}
defp fetch_oauth_verifier(params), do: {:error, MissingParamError.new("oauth_verifier", params)}
defp fetch_oauth_verifier(params), do: {:error, MissingParamError.exception(expected_key: "oauth_verifier", params: params)}

defp get_access_token(config, oauth_token, oauth_verifier) do
with {:ok, site} <- Config.fetch(config, :site) do
Expand Down Expand Up @@ -340,6 +340,6 @@ defmodule Assent.Strategy.OAuth do
end

defp process_user_response({:ok, %HTTPResponse{status: 200, body: user}}), do: {:ok, user}
defp process_user_response({:error, %HTTPResponse{status: 401}}), do: {:error, %RequestError{message: "Unauthorized token"}}
defp process_user_response({:error, %HTTPResponse{status: 401} = response}), do: {:error, RequestError.exception(message: "Unauthorized token", response: response)}
defp process_user_response(any), do: process_response(any)
end
16 changes: 8 additions & 8 deletions lib/assent/strategies/oauth2.ex
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ defmodule Assent.Strategy.OAuth2 do
@behaviour Assent.Strategy

alias Assent.Strategy, as: Helpers
alias Assent.{CallbackCSRFError, CallbackError, Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, RequestError}
alias Assent.{CallbackCSRFError, CallbackError, Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, InvalidResponseError, RequestError, UnexpectedResponseError}

@doc """
Generate authorization URL for request phase.
Expand Down Expand Up @@ -144,21 +144,21 @@ defmodule Assent.Strategy.OAuth2 do
error = params["error"]
error_uri = params["error_uri"]

{:error, %CallbackError{message: message, error: error, error_uri: error_uri}}
{:error, CallbackError.exception(message: message, error: error, error_uri: error_uri)}
end
defp check_error_params(_params), do: :ok

defp fetch_code_param(%{"code" => code}), do: {:ok, code}
defp fetch_code_param(params), do: {:error, MissingParamError.new("code", params)}
defp fetch_code_param(params), do: {:error, MissingParamError.exception(expected_key: "code", params: params)}

defp maybe_check_state(%{state: stored_state}, %{"state" => provided_state}) do
case Assent.constant_time_compare(stored_state, provided_state) do
true -> :ok
false -> {:error, CallbackCSRFError.new("state")}
false -> {:error, CallbackCSRFError.exception(key: "state")}
end
end
defp maybe_check_state(%{state: _state}, params) do
{:error, MissingParamError.new("state", params)}
{:error, MissingParamError.exception(expected_key: "state", params: params)}
end
defp maybe_check_state(_session_params, _params), do: :ok

Expand Down Expand Up @@ -262,8 +262,8 @@ defmodule Assent.Strategy.OAuth2 do
defp process_access_token_response({:ok, %HTTPResponse{status: status, body: %{"access_token" => _} = token}}) when status in [200, 201], do: {:ok, token}
defp process_access_token_response(any), do: process_response(any)

defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, RequestError.unexpected(response)}
defp process_response({:error, %HTTPResponse{} = response}), do: {:error, RequestError.invalid(response)}
defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, UnexpectedResponseError.exception(response: response)}
defp process_response({:error, %HTTPResponse{} = response}), do: {:error, InvalidResponseError.exception(response: response)}
defp process_response({:error, error}), do: {:error, error}

defp fetch_user_with_strategy(config, token, strategy) do
Expand Down Expand Up @@ -350,6 +350,6 @@ defmodule Assent.Strategy.OAuth2 do
end

defp process_user_response({:ok, %HTTPResponse{status: 200, body: user}}), do: {:ok, user}
defp process_user_response({:error, %HTTPResponse{status: 401}}), do: {:error, %RequestError{message: "Unauthorized token"}}
defp process_user_response({:error, %HTTPResponse{status: 401} = response}), do: {:error, RequestError.exception(message: "Unauthorized token", response: response)}
defp process_user_response(any), do: process_response(any)
end
8 changes: 4 additions & 4 deletions lib/assent/strategies/oidc.ex
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ defmodule Assent.Strategy.OIDC do
@behaviour Assent.Strategy

alias Assent.Strategy, as: Helpers
alias Assent.{Config, HTTPAdapter.HTTPResponse, RequestError, Strategy.OAuth2}
alias Assent.{Config, HTTPAdapter.HTTPResponse, RequestError, UnexpectedResponseError, InvalidResponseError, Strategy.OAuth2}

@doc """
Generates an authorization URL for request phase.
Expand Down Expand Up @@ -119,8 +119,8 @@ defmodule Assent.Strategy.OIDC do
defp process_openid_configuration_response({:ok, %HTTPResponse{status: 200, body: configuration}}), do: {:ok, configuration}
defp process_openid_configuration_response(any), do: process_response(any)

defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, RequestError.unexpected(response)}
defp process_response({:error, %HTTPResponse{} = response}), do: {:error, RequestError.invalid(response)}
defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, UnexpectedResponseError.exception(response: response)}
defp process_response({:error, %HTTPResponse{} = response}), do: {:error, InvalidResponseError.exception(response: response)}
defp process_response({:error, error}), do: {:error, error}

defp fetch_from_openid_config(config, key) do
Expand Down Expand Up @@ -439,7 +439,7 @@ defmodule Assent.Strategy.OIDC do
_any -> {:ok, body}
end
end
defp process_userinfo_response({:error, %HTTPResponse{status: 401}}, _openid_config, _config), do: {:error, %RequestError{message: "Unauthorized token"}}
defp process_userinfo_response({:error, %HTTPResponse{status: 401} = response}, _openid_config, _config), do: {:error, RequestError.exception(message: "Unauthorized token", response: response)}
defp process_userinfo_response(any, _openid_config, _config), do: process_response(any)

defp process_jwt(body, openid_config, config) do
Expand Down
2 changes: 1 addition & 1 deletion lib/assent/strategies/twitter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ defmodule Assent.Strategy.Twitter do
@impl true
def callback(config, params) do
case Map.has_key?(params, "denied") do
true -> {:error, %CallbackError{message: "The user denied the authorization request"}}
true -> {:error, CallbackError.exception(message: "The user denied the authorization request")}
false -> Base.callback(config, params, __MODULE__)
end
end
Expand Down
Loading

0 comments on commit 15608fb

Please sign in to comment.