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

Improve errors #138

Merged
merged 1 commit into from
Nov 18, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading