diff --git a/lib/oidcc/plug/authorization_callback.ex b/lib/oidcc/plug/authorization_callback.ex index e3149bb..90f5a15 100644 --- a/lib/oidcc/plug/authorization_callback.ex +++ b/lib/oidcc/plug/authorization_callback.ex @@ -50,6 +50,7 @@ defmodule Oidcc.Plug.AuthorizationCallback do def handle_callback( %Plug.Conn{private: %{ Oidcc.Plug.AuthorizationCallback => {:ok, {token, userinfo}} + Oidcc.Plug.Authorize.State => "query_param_state" | nil }}, _params ) do @@ -215,9 +216,15 @@ defmodule Oidcc.Plug.AuthorizationCallback do {:ok, {token, userinfo}} end + authorize_state = + params + |> Map.get("state", "") + |> Utils.remove_csrf_payload() + conn |> delete_session(Authorize.get_session_name()) |> put_private(__MODULE__, result) + |> put_private(Authorize.State, authorize_state) end @spec prepare_retrieve_opts( diff --git a/lib/oidcc/plug/authorize.ex b/lib/oidcc/plug/authorize.ex index d6a99f2..634a8c9 100644 --- a/lib/oidcc/plug/authorize.ex +++ b/lib/oidcc/plug/authorize.ex @@ -22,6 +22,7 @@ defmodule Oidcc.Plug.Authorize do * `state` - State to relay to OpenID Provider. Commonly used for target redirect URL after authorization. + Accessible through `Plug.Conn.private[#{__MODULE__}.State]` after `Oidcc.Plug.AuthorizationCallback` """ @moduledoc since: "0.1.0" @@ -103,7 +104,11 @@ defmodule Oidcc.Plug.Authorize do redirect_uri = opts |> Keyword.fetch!(:redirect_uri) |> evaluate_config() client_profile_opts = Keyword.get(opts, :client_profile_opts, %{profiles: []}) - state = Map.get(params, "state", :undefined) + state = + params + |> Map.get("state") + |> Utils.add_csrf_payload() + state_verifier = :erlang.phash2(state) nonce = 31 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) diff --git a/lib/oidcc/plug/utils.ex b/lib/oidcc/plug/utils.ex index 7061a55..da3a198 100644 --- a/lib/oidcc/plug/utils.ex +++ b/lib/oidcc/plug/utils.ex @@ -70,4 +70,22 @@ defmodule Oidcc.Plug.Utils do opts end + + @spec add_csrf_payload(state :: String.t()) :: String.t() + def add_csrf_payload(state) do + authenticity_payload = 31 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) + state = if is_binary(state), do: "#{state_authenticity_separator()}#{state}", else: "" + "#{authenticity_payload}#{state}" + end + + @spec remove_csrf_payload(state :: String.t()) :: String.t() | nil + def remove_csrf_payload(authed_state) do + case String.split(authed_state, state_authenticity_separator(), parts: 2) do + [_auth_payload] -> nil + [_auth_payload, state] -> state + end + end + + @spec state_authenticity_separator() :: String.t() + defp state_authenticity_separator, do: "<>" end diff --git a/test/oidcc/plug/authorization_callback_test.exs b/test/oidcc/plug/authorization_callback_test.exs index d385190..f5b8757 100644 --- a/test/oidcc/plug/authorization_callback_test.exs +++ b/test/oidcc/plug/authorization_callback_test.exs @@ -124,6 +124,102 @@ defmodule Oidcc.Plug.AuthorizationCallbackTest do |> AuthorizationCallback.call(opts) end + test "successful retrieve with CSRF state" do + with_mocks [ + {Oidcc.Token, [], + retrieve: fn "code", + _client_context, + %{ + redirect_uri: "http://localhost:8080/oidc/return", + nonce: _nonce, + refresh_jwks: _refresh_fun + } -> + {:ok, :token} + end}, + {Oidcc.Userinfo, [], + retrieve: fn :token, _client_context, %{} -> + {:ok, %{"sub" => "sub"}} + end} + ] do + opts = + AuthorizationCallback.init( + provider: ProviderName, + client_id: fn -> "client_id" end, + client_secret: "client_secret", + redirect_uri: "http://localhost:8080/oidc/return" + ) + + assert %{ + halted: false, + private: %{ + AuthorizationCallback => {:ok, {:token, %{"sub" => "sub"}}}, + Authorize.State => "state" + } + } = + "get" + |> conn("/", %{"code" => "code", "state" => "1234<>state"}) + |> Plug.Test.init_test_session(%{ + Authorize.get_session_name() => %{ + nonce: "nonce", + peer_ip: {127, 0, 0, 1}, + useragent: "useragent", + pkce_verifier: "pkce_verifier", + state_verifier: :erlang.phash2("1234<>state") + } + }) + |> put_req_header("user-agent", "useragent") + |> AuthorizationCallback.call(opts) + end + end + + test "successful retrieve with CSRF only" do + with_mocks [ + {Oidcc.Token, [], + retrieve: fn "code", + _client_context, + %{ + redirect_uri: "http://localhost:8080/oidc/return", + nonce: _nonce, + refresh_jwks: _refresh_fun + } -> + {:ok, :token} + end}, + {Oidcc.Userinfo, [], + retrieve: fn :token, _client_context, %{} -> + {:ok, %{"sub" => "sub"}} + end} + ] do + opts = + AuthorizationCallback.init( + provider: ProviderName, + client_id: fn -> "client_id" end, + client_secret: "client_secret", + redirect_uri: "http://localhost:8080/oidc/return" + ) + + assert %{ + halted: false, + private: %{ + AuthorizationCallback => {:ok, {:token, %{"sub" => "sub"}}}, + Authorize.State => nil + } + } = + "get" + |> conn("/", %{"code" => "code", "state" => "1234"}) + |> Plug.Test.init_test_session(%{ + Authorize.get_session_name() => %{ + nonce: "nonce", + peer_ip: {127, 0, 0, 1}, + useragent: "useragent", + pkce_verifier: "pkce_verifier", + state_verifier: :erlang.phash2("1234") + } + }) + |> put_req_header("user-agent", "useragent") + |> AuthorizationCallback.call(opts) + end + end + test "useragent mismatch" do opts = AuthorizationCallback.init(