Skip to content

Commit

Permalink
Merge pull request #164 from pow-auth/pkce
Browse files Browse the repository at this point in the history
Add PKCE support
  • Loading branch information
danschultzer authored Dec 27, 2024
2 parents 418eb93 + c7052a0 commit 2de8d38
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* `Assent.Strategy.OIDC` now supports `none` authentication method
* `Assent.Strategy.Bitbucket` added
* `Assent.Strategy.Twitch` added
* `Assent.Strategy.OAuth2` now supports PKCE

## v0.2.10 (2024-04-11)

Expand Down
53 changes: 46 additions & 7 deletions lib/assent/strategies/oauth2.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ defmodule Assent.Strategy.OAuth2 do
`:session_params` should be stored and passed back into `callback/3` as part
of config when the user returns. The `:session_params` carries a `:state`
value for the request [to prevent
CSRF](https://tools.ietf.org/html/rfc6749#section-4.1.1).
CSRF](https://tools.ietf.org/html/rfc6749#section-4.1.1). If `:code_verifier`
is set to true, the `:session_params` will also carry PKCE [code verification
parameters](https://datatracker.ietf.org/doc/html/rfc7636#section-4).
This library also supports JWT tokens for client authentication as per
[RFC 7523](https://tools.ietf.org/html/rfc7523).
Expand Down Expand Up @@ -43,6 +45,10 @@ defmodule Assent.Strategy.OAuth2 do
- `:state` - A boolean or a string with the value of the state, optional,
defaults to `true`. When set to `true` a random 32 byte long url safe
string is generated. When set to `false` state will not be verified.
- `:code_verifier` - Boolean to generate and use a random 128 byte long
url safe code verifier for PKCE flow, optional, defaults to `false`. When
set to `true` the session params will contain `:code_verifier`,
`:code_challenge`, and `:code_challenge_method` params.
## Usage
Expand Down Expand Up @@ -84,7 +90,10 @@ defmodule Assent.Strategy.OAuth2 do
}

@type session_params :: %{
optional(:state) => binary()
optional(:state) => binary(),
optional(:code_verifier) => binary(),
optional(:code_challenge) => binary(),
optional(:code_challenge_method) => binary()
}

@doc """
Expand Down Expand Up @@ -136,6 +145,10 @@ defmodule Assent.Strategy.OAuth2 do
end

defp session_params(config) do
state_params(config) ++ code_verifier_params(config)
end

defp state_params(config) do
case Config.get(config, :state, true) do
state when is_binary(state) -> [state: state]
true -> [state: gen_url_encoded_base64(32)]
Expand All @@ -151,6 +164,22 @@ defmodule Assent.Strategy.OAuth2 do
|> Base.url_encode64(padding: false)
end

defp code_verifier_params(config) do
case Config.get(config, :code_verifier, false) do
true ->
code_verifier = gen_url_encoded_base64(128)

[
code_verifier: code_verifier,
code_challenge: Base.url_encode64(:crypto.hash(:sha256, code_verifier), padding: false),
code_challenge_method: "S256"
]

false ->
[]
end
end

defp authorization_params(config, client_id, redirect_uri, session_params) do
params = Config.get(config, :authorization_params, [])

Expand All @@ -160,7 +189,9 @@ defmodule Assent.Strategy.OAuth2 do
redirect_uri: redirect_uri
]
|> Keyword.merge(params)
|> Keyword.merge(Keyword.take(session_params, [:state]))
|> Keyword.merge(
Keyword.take(session_params, [:state, :code_challenge, :code_challenge_method])
)
|> List.keysort(0)
end

Expand All @@ -184,7 +215,7 @@ defmodule Assent.Strategy.OAuth2 do
with {:ok, session_params} <- Config.fetch(config, :session_params),
:ok <- check_error_params(params),
:ok <- verify_state(config, session_params, params),
{:ok, grant_params} <- fetch_grant_access_token_params(config, params),
{:ok, grant_params} <- fetch_grant_access_token_params(config, params, session_params),
{:ok, token} <- grant_access_token(config, "authorization_code", grant_params) do
fetch_user_with_strategy(config, token, strategy)
end
Expand Down Expand Up @@ -218,10 +249,11 @@ defmodule Assent.Strategy.OAuth2 do
{:error, MissingParamError.exception(expected_key: "state", params: params)}
end

defp fetch_grant_access_token_params(config, params) do
defp fetch_grant_access_token_params(config, params, session_params) do
with {:ok, code} <- fetch_code_param(params),
{:ok, redirect_uri} <- Config.fetch(config, :redirect_uri) do
{:ok, [code: code, redirect_uri: redirect_uri]}
{:ok, redirect_uri} <- Config.fetch(config, :redirect_uri),
{:ok, code_verifier_params} <- fetch_code_verifer_params(config, session_params) do
{:ok, [code: code, redirect_uri: redirect_uri] ++ code_verifier_params}
end
end

Expand All @@ -230,6 +262,13 @@ defmodule Assent.Strategy.OAuth2 do
defp fetch_code_param(params),
do: {:error, MissingParamError.exception(expected_key: "code", params: params)}

defp fetch_code_verifer_params(config, session_params) do
case Config.get(config, :code_verifier, false) do
true -> {:ok, [code_verifier: Map.fetch!(session_params, :code_verifier)]}
false -> {:ok, []}
end
end

defp authentication_params(nil, config) do
with {:ok, client_id} <- Config.fetch(config, :client_id) do
headers = []
Expand Down
53 changes: 53 additions & 0 deletions test/assent/strategies/oauth2_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,29 @@ defmodule Assent.Strategy.OAuth2Test do
assert session_params == %{}
end

test "authorize_url/2 with `code_verifier: true`", %{config: config} do
assert {:ok, %{url: url, session_params: session_params}} =
config
|> Keyword.put(:code_verifier, true)
|> OAuth2.authorize_url()

assert session_params.code_verifier
assert String.length(session_params.code_verifier) == 128

assert session_params.code_challenge ==
Base.url_encode64(:crypto.hash(:sha256, session_params.code_verifier),
padding: false
)

assert session_params.code_challenge_method == "S256"

query_params = url |> URI.parse() |> Map.fetch!(:query) |> URI.decode_query()

assert query_params["code_challenge"] == session_params.code_challenge
assert query_params["code_challenge_method"] == session_params.code_challenge_method
refute query_params["code_verifier"]
end

describe "callback/2" do
setup %{config: config} do
config =
Expand Down Expand Up @@ -194,6 +217,36 @@ defmodule Assent.Strategy.OAuth2Test do
assert {:ok, _any} = OAuth2.callback(config, params)
end

test "with `code_verifier: true` with missing code_verifier in session params", %{
config: config,
callback_params: params
} do
config = Keyword.put(config, :code_verifier, true)

assert_raise KeyError, fn ->
OAuth2.callback(config, params)
end
end

test "with `code_verifier: true`", %{config: config, callback_params: params} do
session_params = Map.put(config[:session_params], :code_verifier, "code_verifier_value")

config =
config
|> Keyword.put(:code_verifier, true)
|> Keyword.put(:session_params, session_params)

expect_oauth2_access_token_request([], fn _conn, params ->
assert params["code_verifier"] == "code_verifier_value"
refute params["code_challenge"]
refute params["code_challenge_method"]
end)

expect_oauth2_user_request(%{})

assert {:ok, _any} = OAuth2.callback(config, params)
end

test "with unreachable token url", %{config: config, callback_params: params} do
oauth_token_url = TestServer.url("/oauth/token")
TestServer.stop()
Expand Down

0 comments on commit 2de8d38

Please sign in to comment.