feat: add oidc cycle breaker
This commit is contained in:
parent
92e6f07572
commit
25f3b19f50
7 changed files with 74 additions and 34 deletions
|
|
@ -38,6 +38,7 @@
|
||||||
### Sign-in page (OIDC-only mode)
|
### Sign-in page (OIDC-only mode)
|
||||||
|
|
||||||
- `OIDC_ONLY` (or Settings → OIDC → "Only OIDC sign-in") – When set to true/1/yes and OIDC is configured, the sign-in page shows only the Single Sign-On button (password login is hidden). ENV takes precedence over Settings.
|
- `OIDC_ONLY` (or Settings → OIDC → "Only OIDC sign-in") – When set to true/1/yes and OIDC is configured, the sign-in page shows only the Single Sign-On button (password login is hidden). ENV takes precedence over Settings.
|
||||||
|
- **Redirect loop fix:** After an OIDC failure (e.g. provider down), the app redirects to `/sign-in?oidc_failed=1`. The plug `OidcOnlySignInRedirect` does not redirect that request back to OIDC, so the sign-in page is shown with the error (no endless redirect).
|
||||||
|
|
||||||
### Sync Logic
|
### Sync Logic
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -21,7 +21,7 @@ defmodule MvWeb.AuthController do
|
||||||
if Config.oidc_only?() do
|
if Config.oidc_only?() do
|
||||||
conn
|
conn
|
||||||
|> put_flash(:error, gettext("Only sign-in via Single Sign-On (SSO) is allowed."))
|
|> put_flash(:error, gettext("Only sign-in via Single Sign-On (SSO) is allowed."))
|
||||||
|> redirect(to: ~p"/sign-in")
|
|> redirect(to: sign_in_path_after_oidc_failure())
|
||||||
else
|
else
|
||||||
success_continue(conn, {:password, :sign_in}, user, token)
|
success_continue(conn, {:password, :sign_in}, user, token)
|
||||||
end
|
end
|
||||||
|
|
@ -149,7 +149,7 @@ defmodule MvWeb.AuthController do
|
||||||
_ ->
|
_ ->
|
||||||
conn
|
conn
|
||||||
|> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again."))
|
|> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again."))
|
||||||
|> redirect(to: ~p"/sign-in")
|
|> redirect(to: sign_in_path_after_oidc_failure())
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -163,7 +163,7 @@ defmodule MvWeb.AuthController do
|
||||||
:error,
|
:error,
|
||||||
gettext("The authentication server is currently unavailable. Please try again later.")
|
gettext("The authentication server is currently unavailable. Please try again later.")
|
||||||
)
|
)
|
||||||
|> redirect(to: ~p"/sign-in")
|
|> redirect(to: sign_in_path_after_oidc_failure())
|
||||||
end
|
end
|
||||||
|
|
||||||
# Handle Assent invalid response errors (configuration or malformed responses)
|
# Handle Assent invalid response errors (configuration or malformed responses)
|
||||||
|
|
@ -176,7 +176,7 @@ defmodule MvWeb.AuthController do
|
||||||
:error,
|
:error,
|
||||||
gettext("Authentication configuration error. Please contact the administrator.")
|
gettext("Authentication configuration error. Please contact the administrator.")
|
||||||
)
|
)
|
||||||
|> redirect(to: ~p"/sign-in")
|
|> redirect(to: sign_in_path_after_oidc_failure())
|
||||||
end
|
end
|
||||||
|
|
||||||
# Catch-all clause for any other error types
|
# Catch-all clause for any other error types
|
||||||
|
|
@ -186,7 +186,7 @@ defmodule MvWeb.AuthController do
|
||||||
|
|
||||||
conn
|
conn
|
||||||
|> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again."))
|
|> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again."))
|
||||||
|> redirect(to: ~p"/sign-in")
|
|> redirect(to: sign_in_path_after_oidc_failure())
|
||||||
end
|
end
|
||||||
|
|
||||||
# Handle generic AuthenticationFailed errors
|
# Handle generic AuthenticationFailed errors
|
||||||
|
|
@ -226,10 +226,14 @@ defmodule MvWeb.AuthController do
|
||||||
|
|
||||||
conn
|
conn
|
||||||
|> put_flash(:error, error_message)
|
|> put_flash(:error, error_message)
|
||||||
|> redirect(to: ~p"/sign-in")
|
|> redirect(to: sign_in_path_after_oidc_failure())
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Path used when redirecting to sign-in after an OIDC failure. The query param tells
|
||||||
|
# OidcOnlySignInRedirect to show the sign-in page instead of redirecting back to OIDC (avoids loop).
|
||||||
|
defp sign_in_path_after_oidc_failure, do: "/sign-in?oidc_failed=1"
|
||||||
|
|
||||||
# Extract meaningful error message from Ash errors
|
# Extract meaningful error message from Ash errors
|
||||||
defp extract_meaningful_error_message(errors) do
|
defp extract_meaningful_error_message(errors) do
|
||||||
# Look for specific error messages in InvalidAttribute errors
|
# Look for specific error messages in InvalidAttribute errors
|
||||||
|
|
|
||||||
|
|
@ -900,17 +900,17 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
saves_vereinfacht = vereinfacht_params?(setting_params_clean)
|
saves_vereinfacht = vereinfacht_params?(setting_params_clean)
|
||||||
|
|
||||||
case MvWeb.LiveHelpers.submit_form(socket.assigns.form, setting_params_clean, actor) do
|
case MvWeb.LiveHelpers.submit_form(socket.assigns.form, setting_params_clean, actor) do
|
||||||
{:ok, _updated_settings} ->
|
{:ok, updated_settings} ->
|
||||||
{:ok, fresh_settings} = Membership.get_settings()
|
# Use the returned record for the form so saved values show immediately;
|
||||||
|
# get_settings() can return cached data without the new attribute until reload.
|
||||||
test_result =
|
test_result =
|
||||||
if saves_vereinfacht, do: Mv.Vereinfacht.test_connection(), else: nil
|
if saves_vereinfacht, do: Mv.Vereinfacht.test_connection(), else: nil
|
||||||
|
|
||||||
socket =
|
socket =
|
||||||
socket
|
socket
|
||||||
|> assign(:settings, fresh_settings)
|
|> assign(:settings, updated_settings)
|
||||||
|> assign(:registration_enabled, fresh_settings.registration_enabled != false)
|
|> assign(:registration_enabled, updated_settings.registration_enabled != false)
|
||||||
|> assign(:vereinfacht_api_key_set, present?(fresh_settings.vereinfacht_api_key))
|
|> assign(:vereinfacht_api_key_set, present?(updated_settings.vereinfacht_api_key))
|
||||||
|> assign(:oidc_client_secret_set, Mv.Config.oidc_client_secret_set?())
|
|> assign(:oidc_client_secret_set, Mv.Config.oidc_client_secret_set?())
|
||||||
|> assign(:oidc_only, Mv.Config.oidc_only?())
|
|> assign(:oidc_only, Mv.Config.oidc_only?())
|
||||||
|> assign(:oidc_configured, Mv.Config.oidc_configured?())
|
|> assign(:oidc_configured, Mv.Config.oidc_configured?())
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,8 @@ defmodule MvWeb.Plugs.OidcOnlySignInRedirect do
|
||||||
@moduledoc """
|
@moduledoc """
|
||||||
When OIDC-only mode is active:
|
When OIDC-only mode is active:
|
||||||
- GET /sign-in redirects to the OIDC flow when OIDC is configured (sign-in page skipped).
|
- GET /sign-in redirects to the OIDC flow when OIDC is configured (sign-in page skipped).
|
||||||
|
- GET /sign-in?oidc_failed=1 is not redirected, so the sign-in page is shown after an OIDC
|
||||||
|
failure (avoids redirect loop when the provider is down or misconfigured).
|
||||||
- GET /auth/user/password/sign_in_with_token is rejected (redirect to /sign-in with error)
|
- GET /auth/user/password/sign_in_with_token is rejected (redirect to /sign-in with error)
|
||||||
so password sign-in cannot complete.
|
so password sign-in cannot complete.
|
||||||
"""
|
"""
|
||||||
|
|
@ -19,19 +21,29 @@ defmodule MvWeb.Plugs.OidcOnlySignInRedirect do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp maybe_redirect_sign_in_to_oidc(conn) do
|
defp maybe_redirect_sign_in_to_oidc(conn) do
|
||||||
if conn.request_path == "/sign-in" and conn.method == "GET" do
|
if conn.request_path != "/sign-in" or conn.method != "GET" do
|
||||||
if Config.oidc_only?() and Config.oidc_configured?() do
|
|
||||||
conn
|
|
||||||
|> redirect(to: "/auth/user/oidc")
|
|
||||||
|> halt()
|
|
||||||
else
|
|
||||||
conn
|
|
||||||
end
|
|
||||||
else
|
|
||||||
conn
|
conn
|
||||||
|
else
|
||||||
|
conn = fetch_query_params(conn)
|
||||||
|
maybe_redirect_sign_in_to_oidc_checked(conn)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp maybe_redirect_sign_in_to_oidc_checked(conn) do
|
||||||
|
cond do
|
||||||
|
# Show sign-in page when returning from OIDC failure to avoid redirect loop.
|
||||||
|
conn.query_params["oidc_failed"] -> conn
|
||||||
|
Config.oidc_only?() and Config.oidc_configured?() -> redirect_and_halt(conn)
|
||||||
|
true -> conn
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp redirect_and_halt(conn) do
|
||||||
|
conn
|
||||||
|
|> redirect(to: "/auth/user/oidc")
|
||||||
|
|> halt()
|
||||||
|
end
|
||||||
|
|
||||||
defp maybe_reject_password_token_sign_in(conn) do
|
defp maybe_reject_password_token_sign_in(conn) do
|
||||||
if conn.halted, do: conn, else: reject_password_token_sign_in_if_applicable(conn)
|
if conn.halted, do: conn, else: reject_password_token_sign_in_if_applicable(conn)
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -3905,8 +3905,3 @@ msgstr "Nur OIDC-Anmeldung ist aktiv. Diese Option ist deaktiviert."
|
||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Only sign-in via Single Sign-On (SSO) is allowed."
|
msgid "Only sign-in via Single Sign-On (SSO) is allowed."
|
||||||
msgstr "Nur Anmeldung per Single Sign-On (SSO) ist erlaubt."
|
msgstr "Nur Anmeldung per Single Sign-On (SSO) ist erlaubt."
|
||||||
|
|
||||||
#~ #: lib/accounts/user/validations/oidc_only_blocks_password_registration.ex
|
|
||||||
#~ #, elixir-autogen, elixir-format
|
|
||||||
#~ msgid "Registration with password is disabled when only OIDC sign-in is active."
|
|
||||||
#~ msgstr "Registrierung mit Passwort ist deaktiviert, wenn nur OIDC-Anmeldung aktiv ist."
|
|
||||||
|
|
|
||||||
|
|
@ -3905,8 +3905,3 @@ msgstr ""
|
||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Only sign-in via Single Sign-On (SSO) is allowed."
|
msgid "Only sign-in via Single Sign-On (SSO) is allowed."
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
#~ #: lib/accounts/user/validations/oidc_only_blocks_password_registration.ex
|
|
||||||
#~ #, elixir-autogen, elixir-format
|
|
||||||
#~ msgid "Registration with password is disabled when only OIDC sign-in is active."
|
|
||||||
#~ msgstr "Registration with password is disabled when only OIDC sign-in is active."
|
|
||||||
|
|
|
||||||
|
|
@ -363,6 +363,39 @@ defmodule MvWeb.AuthControllerTest do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "returns 200 when OIDC-only but oidc_failed=1 (avoids redirect loop)", %{
|
||||||
|
conn: authenticated_conn
|
||||||
|
} do
|
||||||
|
{:ok, settings} = Membership.get_settings()
|
||||||
|
|
||||||
|
prev = %{
|
||||||
|
oidc_only: settings.oidc_only,
|
||||||
|
oidc_client_id: settings.oidc_client_id,
|
||||||
|
oidc_base_url: settings.oidc_base_url,
|
||||||
|
oidc_redirect_uri: settings.oidc_redirect_uri
|
||||||
|
}
|
||||||
|
|
||||||
|
{:ok, _} =
|
||||||
|
Membership.update_settings(settings, %{
|
||||||
|
oidc_only: true,
|
||||||
|
oidc_client_id: "test-client",
|
||||||
|
oidc_base_url: "https://idp.example.com",
|
||||||
|
oidc_redirect_uri: "http://localhost:4000/auth/user/oidc/callback",
|
||||||
|
oidc_client_secret: "test-secret"
|
||||||
|
})
|
||||||
|
|
||||||
|
try do
|
||||||
|
conn = build_unauthenticated_conn(authenticated_conn)
|
||||||
|
conn = get(conn, "/sign-in?oidc_failed=1")
|
||||||
|
assert conn.status == 200
|
||||||
|
# Sign-in page is shown, not redirect to OIDC
|
||||||
|
assert conn.resp_body =~ "Sign in" or conn.resp_body =~ "sign-in"
|
||||||
|
after
|
||||||
|
{:ok, s} = Membership.get_settings()
|
||||||
|
Membership.update_settings(s, prev)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
test "returns 200 when OIDC-only but OIDC not configured", %{conn: authenticated_conn} do
|
test "returns 200 when OIDC-only but OIDC not configured", %{conn: authenticated_conn} do
|
||||||
{:ok, settings} = Membership.get_settings()
|
{:ok, settings} = Membership.get_settings()
|
||||||
original_oidc_only = Map.get(settings, :oidc_only, false)
|
original_oidc_only = Map.get(settings, :oidc_only, false)
|
||||||
|
|
@ -400,7 +433,7 @@ defmodule MvWeb.AuthControllerTest do
|
||||||
|
|
||||||
conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, error)
|
conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, error)
|
||||||
|
|
||||||
assert redirected_to(conn) == ~p"/sign-in"
|
assert redirected_to(conn) == "/sign-in?oidc_failed=1"
|
||||||
|
|
||||||
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
|
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
|
||||||
"The authentication server is currently unavailable. Please try again later."
|
"The authentication server is currently unavailable. Please try again later."
|
||||||
|
|
@ -422,7 +455,7 @@ defmodule MvWeb.AuthControllerTest do
|
||||||
|
|
||||||
conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, error)
|
conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, error)
|
||||||
|
|
||||||
assert redirected_to(conn) == ~p"/sign-in"
|
assert redirected_to(conn) == "/sign-in?oidc_failed=1"
|
||||||
|
|
||||||
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
|
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
|
||||||
"Authentication configuration error. Please contact the administrator."
|
"Authentication configuration error. Please contact the administrator."
|
||||||
|
|
@ -436,7 +469,7 @@ defmodule MvWeb.AuthControllerTest do
|
||||||
|
|
||||||
conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, unknown_reason)
|
conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, unknown_reason)
|
||||||
|
|
||||||
assert redirected_to(conn) == ~p"/sign-in"
|
assert redirected_to(conn) == "/sign-in?oidc_failed=1"
|
||||||
|
|
||||||
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
|
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
|
||||||
"Unable to authenticate with OIDC. Please try again."
|
"Unable to authenticate with OIDC. Please try again."
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue