diff --git a/lib/claper_web/controllers/user_oidc_auth.ex b/lib/claper_web/controllers/user_oidc_auth.ex index 6cb495d..9dce491 100644 --- a/lib/claper_web/controllers/user_oidc_auth.ex +++ b/lib/claper_web/controllers/user_oidc_auth.ex @@ -7,29 +7,31 @@ defmodule ClaperWeb.UserOidcAuth do import Phoenix.Controller - # Add PKCE-related functions + require Logger + defp generate_pkce_verifier do :crypto.strong_rand_bytes(32) |> Base.url_encode64(padding: false) end - defp generate_pkce_challenge(verifier) do - :crypto.hash(:sha256, verifier) - |> Base.url_encode64(padding: false) - end - @doc false def new(conn, _params) do - # Generate PKCE verifier and store it in session pkce_verifier = generate_pkce_verifier() - conn = put_session(conn, :pkce_verifier, pkce_verifier) + state = :crypto.strong_rand_bytes(16) |> Base.url_encode64(padding: false) + nonce = :crypto.strong_rand_bytes(16) |> Base.url_encode64(padding: false) + + conn = + conn + |> put_session(:pkce_verifier, pkce_verifier) + |> put_session(:oidc_state, state) + |> put_session(:oidc_nonce, nonce) + + client_context = patched_client_context!() {:ok, redirect_uri} = - Oidcc.create_redirect_url( - Claper.OidcProviderConfig, - client_id(), - client_secret(), - opts(pkce_verifier) + Oidcc.Authorization.create_redirect_url( + client_context, + auth_opts(pkce_verifier) |> Map.merge(%{state: state, nonce: nonce}) ) uri = Enum.join(redirect_uri, "") @@ -37,48 +39,120 @@ defmodule ClaperWeb.UserOidcAuth do redirect(conn, external: uri) end - def callback(conn, %{"code" => code} = _params) do - # Get PKCE verifier from session + def callback(conn, %{"code" => code, "state" => state} = _params) do pkce_verifier = get_session(conn, :pkce_verifier) + saved_state = get_session(conn, :oidc_state) + nonce = get_session(conn, :oidc_nonce) + + if state != saved_state do + Logger.error( + "OIDC state mismatch: received=#{inspect(state)}, saved=#{inspect(saved_state)}" + ) - with {:ok, - %Oidcc.Token{ - id: %Oidcc.Token.Id{token: id_token, claims: claims}, - access: %Oidcc.Token.Access{token: access_token}, - refresh: refresh_token - }} <- - Oidcc.retrieve_token( - code, - Claper.OidcProviderConfig, - client_id(), - client_secret(), - opts(pkce_verifier) - ), - {:ok, oidc_user} <- validate_user(id_token, access_token, refresh_token, claims) do conn - # Clean up the verifier - |> delete_session(:pkce_verifier) - |> UserAuth.log_in_user(oidc_user.user) + |> clear_oidc_session() + |> put_status(:unauthorized) + |> put_view(ClaperWeb.ErrorView) + |> render("csrf_error.html", %{error: "Authentication failed: state mismatch"}) else - {:error, reason} -> + client_context = patched_client_context!() + + with {:ok, + %Oidcc.Token{ + id: %Oidcc.Token.Id{token: id_token, claims: claims}, + access: %Oidcc.Token.Access{token: access_token}, + refresh: refresh_token + } = token} <- + Oidcc.Token.retrieve( + code, + client_context, + token_opts(pkce_verifier, nonce) + ), + {:ok, claims} <- maybe_enrich_claims(claims, token, client_context), + {:ok, oidc_user} <- validate_user(id_token, access_token, refresh_token, claims) do conn - # Clean up the verifier even on error - |> delete_session(:pkce_verifier) - |> put_status(:unauthorized) - |> put_view(ClaperWeb.ErrorView) - |> render("csrf_error.html", %{error: "Authentication failed: #{inspect(reason)}"}) + |> clear_oidc_session() + |> UserAuth.log_in_user(oidc_user.user) + else + {:error, reason} -> + Logger.error("OIDC token retrieval/validation failed: #{inspect(reason)}") + + conn + |> clear_oidc_session() + |> put_status(:unauthorized) + |> put_view(ClaperWeb.ErrorView) + |> render("csrf_error.html", %{error: "Authentication failed: #{inspect(reason)}"}) + end end end + def callback(conn, %{"code" => _code} = _params) do + conn + |> clear_oidc_session() + |> put_status(:unauthorized) + |> put_view(ClaperWeb.ErrorView) + |> render("csrf_error.html", %{error: "Authentication failed: missing state parameter"}) + end + def callback(conn, %{"error" => error} = _params) do conn - # Clean up the verifier even on error - |> delete_session(:pkce_verifier) + |> clear_oidc_session() |> put_status(:unauthorized) |> put_view(ClaperWeb.ErrorView) |> render("csrf_error.html", %{error: "Authentication failed: #{error}"}) end + # Fetch userinfo to fill in claims missing from the ID token (e.g. email on Authelia) + defp maybe_enrich_claims(%{"email" => email} = claims, _token, _client_context) + when is_binary(email) do + {:ok, claims} + end + + defp maybe_enrich_claims(claims, token, client_context) do + case Oidcc.Userinfo.retrieve( + token, + client_context, + %{preferred_auth_methods: [:client_secret_basic, :client_secret_post]} + ) do + {:ok, userinfo} -> + {:ok, Map.merge(userinfo, claims)} + + {:error, reason} -> + Logger.error("OIDC userinfo retrieval failed: #{inspect(reason)}") + {:ok, claims} + end + end + + # Build a client context with provider config overrides for broad compatibility: + # - Disable request objects and PAR (Authelia compatibility) + # - Ensure S256 PKCE is listed as supported (Entra ID compatibility) + defp patched_client_context! do + {:ok, client_context} = + Oidcc.ClientContext.from_configuration_worker( + Claper.OidcProviderConfig, + client_id(), + client_secret() + ) + + provider_config = %{ + client_context.provider_configuration + | request_parameter_supported: false, + require_signed_request_object: false, + pushed_authorization_request_endpoint: :undefined, + require_pushed_authorization_requests: false, + code_challenge_methods_supported: ["S256"] + } + + %{client_context | provider_configuration: provider_config} + end + + defp clear_oidc_session(conn) do + conn + |> delete_session(:pkce_verifier) + |> delete_session(:oidc_state) + |> delete_session(:oidc_nonce) + end + defp config do Application.get_env(:claper, :oidc) end @@ -103,25 +177,30 @@ defmodule ClaperWeb.UserOidcAuth do Application.get_env(:claper, ClaperWeb.Endpoint)[:base_url] end - defp opts(pkce_verifier) do - url = base_url() + defp redirect_uri do + "#{base_url()}/users/oidc/callback" + end - base_opts = %{ - redirect_uri: "#{url}/users/oidc/callback", + # Options for Oidcc.Authorization.create_redirect_url/2 (uses `scopes`) + defp auth_opts(pkce_verifier) do + %{ + redirect_uri: redirect_uri(), scopes: scopes(), - preferred_auth_methods: [:client_secret_basic, :client_secret_post], - require_pkce: true + require_pkce: true, + pkce_verifier: pkce_verifier } + end - if pkce_verifier do - Map.merge(base_opts, %{ - pkce_verifier: pkce_verifier, - code_challenge: generate_pkce_challenge(pkce_verifier), - code_challenge_method: "S256" - }) - else - base_opts - end + # Options for :oidcc_token.retrieve/3 (uses `scope`) + defp token_opts(pkce_verifier, nonce) do + %{ + redirect_uri: redirect_uri(), + scope: scopes(), + require_pkce: true, + pkce_verifier: pkce_verifier, + nonce: nonce, + preferred_auth_methods: [:client_secret_basic, :client_secret_post] + } end defp format_refresh_token(%Oidcc.Token.Refresh{token: token}) do