fix(auth): trigger RP-initiated logout at OIDC provider
This commit is contained in:
parent
22955bdd9e
commit
ba66bc15db
4 changed files with 192 additions and 6 deletions
88
lib/mv/oidc/discovery.ex
Normal file
88
lib/mv/oidc/discovery.ex
Normal file
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"],
|
||||
|
|
|
|||
|
|
@ -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] ->
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue