diff --git a/lib/mv/oidc/discovery.ex b/lib/mv/oidc/discovery.ex new file mode 100644 index 0000000..a3a373a --- /dev/null +++ b/lib/mv/oidc/discovery.ex @@ -0,0 +1,88 @@ +defmodule Mv.Oidc.Discovery do + @moduledoc """ + Fetches and caches the OIDC provider's discovery document + (`/.well-known/openid-configuration`). + + Currently only `end_session_endpoint` is exposed — used by the logout flow to + trigger RP-initiated logout at the IdP so the user's SSO session is cleared + and they don't get auto-re-logged-in. + + Cache lives in `:persistent_term`, keyed by base URL, for the lifetime of the + BEAM. Re-fetch on next call after `clear_cache/0`. + """ + + require Logger + + @persistent_term_key {__MODULE__, :discovery} + @request_timeout 5_000 + + @doc """ + Returns the IdP's `end_session_endpoint` URL. + + - `{:ok, url}` if discovery succeeds (and is cached for future calls) + - `{:error, reason}` if the IdP is unreachable, the document is malformed, + or the field is missing + """ + @spec end_session_endpoint(String.t()) :: {:ok, String.t()} | {:error, term()} + def end_session_endpoint(base_url) when is_binary(base_url) do + case fetch_cached(base_url) do + {:ok, %{"end_session_endpoint" => url}} when is_binary(url) -> {:ok, url} + {:ok, _config} -> {:error, :no_end_session_endpoint} + {:error, _} = err -> err + end + end + + @doc """ + Clears the cached discovery documents. Intended for tests. + """ + @spec clear_cache() :: :ok + def clear_cache do + :persistent_term.erase(@persistent_term_key) + :ok + end + + @doc """ + Seeds the cache with a fixed result for a base URL. Intended for tests so the + HTTP fetch is skipped. + """ + @spec put_cache(String.t(), {:ok, map()} | {:error, term()}) :: :ok + def put_cache(base_url, result) when is_binary(base_url) do + cache = :persistent_term.get(@persistent_term_key, %{}) + :persistent_term.put(@persistent_term_key, Map.put(cache, base_url, result)) + :ok + end + + defp fetch_cached(base_url) do + cache = :persistent_term.get(@persistent_term_key, %{}) + + case Map.fetch(cache, base_url) do + {:ok, result} -> + result + + :error -> + result = fetch(base_url) + :persistent_term.put(@persistent_term_key, Map.put(cache, base_url, result)) + result + end + end + + defp fetch(base_url) do + url = String.trim_trailing(base_url, "/") <> "/.well-known/openid-configuration" + + case Req.get(url, + receive_timeout: @request_timeout, + connect_options: [timeout: @request_timeout] + ) do + {:ok, %Req.Response{status: 200, body: body}} when is_map(body) -> + {:ok, body} + + {:ok, %Req.Response{status: status}} -> + Logger.warning("OIDC discovery returned HTTP #{status} for #{url}") + {:error, {:http_status, status}} + + {:error, reason} -> + Logger.warning("OIDC discovery request failed for #{url}: #{inspect(reason)}") + {:error, reason} + end + end +end diff --git a/lib/mv_web/controllers/auth_controller.ex b/lib/mv_web/controllers/auth_controller.ex index adde4e8..120c245 100644 --- a/lib/mv_web/controllers/auth_controller.ex +++ b/lib/mv_web/controllers/auth_controller.ex @@ -16,6 +16,7 @@ defmodule MvWeb.AuthController do alias Mv.Accounts.User.Errors.PasswordVerificationRequired alias Mv.Config + alias Mv.Oidc.Discovery def success(conn, {:password, :sign_in} = _activity, user, token) do if Config.oidc_only?() do @@ -337,11 +338,28 @@ defmodule MvWeb.AuthController do defp redact_url(_), do: "[redacted]" def sign_out(conn, _params) do - return_to = get_session(conn, :return_to) || ~p"/" + conn = clear_session(conn, :mv) |> put_flash(:success, gettext("You are now signed out")) - conn - |> clear_session(:mv) - |> put_flash(:success, gettext("You are now signed out")) - |> redirect(to: return_to) + case oidc_end_session_url() do + {:ok, url} -> + redirect(conn, external: url) + + :no_oidc -> + redirect(conn, to: get_session(conn, :return_to) || ~p"/") + + {:error, _reason} -> + # IdP discovery failed — fall back to local logout. The user's IdP session + # is still active, so OIDC_ONLY setups may auto-re-login. Better than + # blocking logout entirely. + redirect(conn, to: ~p"/sign-in?oidc_failed=1") + end + end + + defp oidc_end_session_url do + if Config.oidc_configured?() do + Discovery.end_session_endpoint(Config.oidc_base_url()) + else + :no_oidc + end end end diff --git a/rauthy-bootstrap/clients.json b/rauthy-bootstrap/clients.json index e0f608a..6869410 100644 --- a/rauthy-bootstrap/clients.json +++ b/rauthy-bootstrap/clients.json @@ -4,7 +4,6 @@ "name": "Mila dev", "secret": { "Plain": "mv-dev-shared-secret-not-for-production-do-not-use-anywhere-else" }, "redirect_uris": ["http://localhost:4000/auth/user/oidc/callback"], - "post_logout_redirect_uris": ["http://localhost:4000/"], "allowed_origins": ["http://localhost:4000"], "enabled": true, "flows_enabled": ["authorization_code", "refresh_token"], diff --git a/test/mv_web/controllers/auth_controller_test.exs b/test/mv_web/controllers/auth_controller_test.exs index 9282144..53c41c6 100644 --- a/test/mv_web/controllers/auth_controller_test.exs +++ b/test/mv_web/controllers/auth_controller_test.exs @@ -62,6 +62,87 @@ defmodule MvWeb.AuthControllerTest do assert redirected_to(conn) == ~p"/" end + describe "DELETE /sign-out with OIDC configured" do + @base_url "https://idp.example.com" + + defp with_oidc_settings(fun) do + {:ok, settings} = Membership.get_settings() + + prev = %{ + oidc_client_id: settings.oidc_client_id, + oidc_base_url: settings.oidc_base_url, + oidc_redirect_uri: settings.oidc_redirect_uri, + oidc_client_secret: settings.oidc_client_secret + } + + {:ok, _} = + Membership.update_settings(settings, %{ + oidc_client_id: "test-client", + oidc_base_url: @base_url, + oidc_redirect_uri: "http://localhost:4000/auth/user/oidc/callback", + oidc_client_secret: "test-secret" + }) + + try do + fun.() + after + Mv.Oidc.Discovery.clear_cache() + {:ok, s} = Membership.get_settings() + Membership.update_settings(s, prev) + end + end + + test "redirects to end_session_endpoint when discovery succeeds", %{ + conn: authenticated_conn + } do + with_oidc_settings(fn -> + end_session_url = "https://idp.example.com/end-session" + + Mv.Oidc.Discovery.put_cache( + @base_url, + {:ok, %{"end_session_endpoint" => end_session_url}} + ) + + conn = + authenticated_conn + |> conn_with_oidc_user() + |> delete(~p"/sign-out") + + assert redirected_to(conn, 302) == end_session_url + end) + end + + test "falls back to /sign-in?oidc_failed=1 when discovery fails", %{ + conn: authenticated_conn + } do + with_oidc_settings(fn -> + Mv.Oidc.Discovery.put_cache(@base_url, {:error, :test_failure}) + + conn = + authenticated_conn + |> conn_with_oidc_user() + |> delete(~p"/sign-out") + + assert redirected_to(conn) == "/sign-in?oidc_failed=1" + end) + end + + test "falls back to /sign-in?oidc_failed=1 when end_session_endpoint is missing", %{ + conn: authenticated_conn + } do + with_oidc_settings(fn -> + Mv.Oidc.Discovery.put_cache(@base_url, {:ok, %{"issuer" => @base_url}}) + + conn = + authenticated_conn + |> conn_with_oidc_user() + |> delete(~p"/sign-out") + + assert redirected_to(conn) == "/sign-in?oidc_failed=1" + end) + end + end + defp csrf_token_from_sign_out_form(html) when is_binary(html) do case Regex.run(~r/name="_csrf_token"[^>]*value="([^"]+)"/, html) do [_, token] ->