From 2c7d77111d915d0bce0a322e903e784c4cb456ae Mon Sep 17 00:00:00 2001 From: Giorgio Torres Date: Tue, 10 Feb 2026 15:48:56 +0100 Subject: [PATCH 1/4] Adds protection against CSRF on authorization flow --- lib/oidcc/plug/authorize.ex | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/oidcc/plug/authorize.ex b/lib/oidcc/plug/authorize.ex index d6a99f2..f87184c 100644 --- a/lib/oidcc/plug/authorize.ex +++ b/lib/oidcc/plug/authorize.ex @@ -17,11 +17,6 @@ defmodule Oidcc.Plug.Authorize do ] end ``` - - ## Query Params - - * `state` - State to relay to OpenID Provider. Commonly used for target redirect - URL after authorization. """ @moduledoc since: "0.1.0" @@ -99,11 +94,11 @@ defmodule Oidcc.Plug.Authorize do |> Utils.validate_client_context_opts!() @impl Plug - def call(%Plug.Conn{params: params} = conn, opts) do + def call(%Plug.Conn{} = conn, opts) 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 = :crypto.strong_rand_bytes(32) state_verifier = :erlang.phash2(state) nonce = 31 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) From e6b8d2544ad0564b936c98e06c01ef219c066dbb Mon Sep 17 00:00:00 2001 From: Giorgio Torres Date: Wed, 18 Feb 2026 11:55:16 +0100 Subject: [PATCH 2/4] allowing state as parameter with autheticity verification --- lib/oidcc/plug/authorize.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/oidcc/plug/authorize.ex b/lib/oidcc/plug/authorize.ex index f87184c..24b6003 100644 --- a/lib/oidcc/plug/authorize.ex +++ b/lib/oidcc/plug/authorize.ex @@ -94,11 +94,12 @@ defmodule Oidcc.Plug.Authorize do |> Utils.validate_client_context_opts!() @impl Plug - def call(%Plug.Conn{} = conn, opts) do + def call(%Plug.Conn{params: params} = conn, opts) do redirect_uri = opts |> Keyword.fetch!(:redirect_uri) |> evaluate_config() client_profile_opts = Keyword.get(opts, :client_profile_opts, %{profiles: []}) - state = :crypto.strong_rand_bytes(32) + state_authenticity = 31 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) + state = state_authenticity <> Map.get(params, "state", "") state_verifier = :erlang.phash2(state) nonce = 31 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) From 0bd777ae9503abd7f1164766a8990d29a9cba749 Mon Sep 17 00:00:00 2001 From: Giorgio Torres Date: Fri, 3 Apr 2026 16:19:58 +0200 Subject: [PATCH 3/4] Adds csrf protected state to conn.private --- lib/oidcc/plug/authorization_callback.ex | 7 ++ lib/oidcc/plug/authorize.ex | 13 ++- lib/oidcc/plug/utils.ex | 19 ++++ .../plug/authorization_callback_test.exs | 96 +++++++++++++++++++ 4 files changed, 133 insertions(+), 2 deletions(-) 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 24b6003..634a8c9 100644 --- a/lib/oidcc/plug/authorize.ex +++ b/lib/oidcc/plug/authorize.ex @@ -17,6 +17,12 @@ defmodule Oidcc.Plug.Authorize do ] end ``` + + ## Query Params + + * `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" @@ -98,8 +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_authenticity = 31 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) - state = state_authenticity <> Map.get(params, "state", "") + 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..7e4a5e4 100644 --- a/lib/oidcc/plug/utils.ex +++ b/lib/oidcc/plug/utils.ex @@ -70,4 +70,23 @@ 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( From 1041e7b0d1addb48a3e90b31c54ae89e18180edf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20M=C3=A4nnchen?= Date: Thu, 9 Apr 2026 16:05:34 +0200 Subject: [PATCH 4/4] Apply suggestion from @maennchen --- lib/oidcc/plug/utils.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/oidcc/plug/utils.ex b/lib/oidcc/plug/utils.ex index 7e4a5e4..da3a198 100644 --- a/lib/oidcc/plug/utils.ex +++ b/lib/oidcc/plug/utils.ex @@ -87,6 +87,5 @@ defmodule Oidcc.Plug.Utils do end @spec state_authenticity_separator() :: String.t() - defp state_authenticity_separator(), - do: "<>" + defp state_authenticity_separator, do: "<>" end