Fix Authorization Callback PKCE Verifier :none (#30)

Fixes #29
erlef · Dec 22, 2024 · 56255b0 · 56255b0
1 parent c195e9d
commit 56255b0
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 11 deletions.
diff --git a/lib/oidcc/plug/authorization_callback.ex b/lib/oidcc/plug/authorization_callback.ex
@@ -201,17 +201,7 @@ defmodule Oidcc.Plug.AuthorizationCallback do
            :ok <- check_issuer_request_param(params, client_context),
            {:ok, code} <- fetch_request_param(params, "code"),
            scope = Map.get(params, "scope", "openid"),
-           scopes = :oidcc_scope.parse(scope),
-           token_opts =
-             opts
-             |> Keyword.take([:request_opts, :preferred_auth_methods])
-             |> Map.new()
-             |> Map.merge(%{
-               nonce: nonce,
-               scope: scopes,
-               redirect_uri: redirect_uri,
-               pkce_verifier: pkce_verifier
-             }),
+           token_opts = prepare_retrieve_opts(opts, scope, nonce, redirect_uri, pkce_verifier),
            {:ok, token} <-
              retrieve_token(
                code,
@@ -229,6 +219,31 @@ defmodule Oidcc.Plug.AuthorizationCallback do
     |> put_private(__MODULE__, result)
   end
 
+  @spec prepare_retrieve_opts(
+          opts :: opts(),
+          scope :: String.t(),
+          nonce :: String.t() | :any,
+          redirect_uri :: String.t(),
+          pkce_verifier :: String.t() | :none
+        ) :: :oidcc_token.retrieve_opts()
+  defp prepare_retrieve_opts(opts, scope, nonce, redirect_uri, pkce_verifier) do
+    scopes = :oidcc_scope.parse(scope)
+
+    opts
+    |> Keyword.take([:request_opts, :preferred_auth_methods])
+    |> Map.new()
+    |> Map.merge(%{
+      nonce: nonce,
+      scope: scopes,
+      redirect_uri: redirect_uri,
+      pkce_verifier: pkce_verifier
+    })
+    |> case do
+      %{pkce_verifier: :none} = opts -> Map.drop(opts, [:pkce_verifier])
+      opts -> opts
+    end
+  end
+
   @spec check_peer_ip(
           conn :: Plug.Conn.t(),
           peer_ip :: :inet.ip_address() | nil,

diff --git a/test/oidcc/plug/authorization_callback_test.exs b/test/oidcc/plug/authorization_callback_test.exs
@@ -434,4 +434,40 @@ defmodule Oidcc.Plug.AuthorizationCallbackTest do
              |> put_req_header("user-agent", "useragent")
              |> AuthorizationCallback.call(opts)
   end
+
+  test "no session" do
+    with_mocks [
+      {Oidcc.Token, [],
+       retrieve: fn
+         "code", _client_context, opts ->
+           refute Map.has_key?(opts, :pkce_verifier)
+           {:ok, :token}
+           {:ok, :token}
+       end},
+      {Oidcc.Userinfo, [],
+       retrieve: fn :token, _client_context, %{} ->
+         {:ok, %{"sub" => "sub"}}
+       end}
+    ] do
+      opts =
+        AuthorizationCallback.init(
+          provider: ProviderName,
+          client_id: "client_id",
+          client_secret: "client_secret",
+          redirect_uri: "http://localhost:8080/oidc/return"
+        )
+
+      assert %{
+               halted: false,
+               private: %{
+                 Oidcc.Plug.AuthorizationCallback => {:ok, {:token, %{"sub" => "sub"}}}
+               }
+             } =
+               "get"
+               |> conn("/", %{"code" => "code"})
+               |> Plug.Test.init_test_session(%{})
+               |> put_req_header("user-agent", "useragent")
+               |> AuthorizationCallback.call(opts)
+    end
+  end
 end