feat(oidc): improve error handling

This commit is contained in:
Edouard Vanbelle
2026-08-08 20:31:19 +02:00
parent 93bb114e21
commit 4d6c4bb92e
3 changed files with 587 additions and 462 deletions
File diff suppressed because it is too large Load Diff
+41 -24
View File
@@ -1509,14 +1509,35 @@ pub async fn oidc_callback(
tracing::info!("OIDC callback received with code"); tracing::info!("OIDC callback received with code");
// Exchange code, validate state/nonce/PKCE, authenticate user // Exchange code, validate state/nonce/PKCE, authenticate user.
let result = auth_app // Any Err path (expired state on refresh, consumed code on replay,
// anti-takeover email refusal, etc.) is caught below and turned
// into a redirect to /login?login_error=<key> — a JSON 4xx here
// would render as raw JSON in the browser since the caller is
// mid-navigation from the IdP, not the SPA. The SPA login page
// renders localized copy per key.
let result = match auth_app
.oidc_callback(&query.code, &query.state, &state.locale_registry) .oidc_callback(&query.code, &query.state, &state.locale_registry)
.await .await
.map_err(|e| { {
Ok(r) => r,
Err(e) => {
tracing::error!("OIDC callback failed: {}", e); tracing::error!("OIDC callback failed: {}", e);
AppError::from(e) let config = auth_app.oidc_config().unwrap();
})?; let frontend_url = config.frontend_url.trim_end_matches('/');
// AccessDenied covers the CSRF/state/code/nonce validation
// failures (the common "refresh replayed a consumed state"
// case). Everything else is bucketed as a generic callback
// failure — operators dig into the log line above for the
// specifics; end-users only need "try again" guidance.
let reason = match e.kind {
crate::domain::errors::ErrorKind::AccessDenied => "callback_denied",
_ => "callback_failed",
};
let redirect_url = format!("{}/login?login_error={}", frontend_url, reason);
return Ok(Redirect::temporary(&redirect_url).into_response());
}
};
match result { match result {
OidcCallbackResult::WebLogin { exchange_code } => { OidcCallbackResult::WebLogin { exchange_code } => {
@@ -1582,26 +1603,22 @@ pub async fn oidc_callback(
); );
Ok(Redirect::temporary(&redirect_url).into_response()) Ok(Redirect::temporary(&redirect_url).into_response())
} }
// Map each auto-link refusal reason to a distinct stable // Redirect the browser back to the login page with a
// CamelCase `error_type`. The SPA switches on this to render // machine-readable reason on the query string, mirroring the
// targeted copy (contact-admin vs. verify-email-at-IdP vs. // LinkRefused → `/profile?link_error=<reason>` pattern above.
// already-linked-elsewhere) rather than a generic error toast. // The browser is mid-redirect from the IdP; returning a 409
// Status stays 409 (CONFLICT) — semantically an existing user // JSON body would leave the user staring at raw JSON. The SPA
// blocks the auto-provision path. // login page reads `?login_error=<reason>` on mount, renders a
// localized notice, and strips the param via history.replaceState.
OidcCallbackResult::AutoLinkRefused { reason } => { OidcCallbackResult::AutoLinkRefused { reason } => {
let error_type = match reason { let config = auth_app.oidc_config().unwrap();
"auto_link_disabled" => "AutoLinkDisabled", let frontend_url = config.frontend_url.trim_end_matches('/');
"auto_link_email_not_verified" => "AutoLinkEmailNotVerified", let redirect_url = format!("{}/login?login_error={}", frontend_url, reason);
"already_linked_elsewhere" => "AutoLinkAlreadyLinkedElsewhere", tracing::info!(
_ => "AutoLinkRefused", reason = reason,
}; "OIDC auto-link refused, redirecting to /login?login_error"
Err(AppError::new( );
StatusCode::CONFLICT, Ok(Redirect::temporary(&redirect_url).into_response())
"OIDC login blocked — a local account with this email already exists. \
Contact your administrator, or sign in with your existing credentials \
and connect SSO from your profile.",
error_type,
))
} }
} }
} }
+12 -10
View File
@@ -40,9 +40,12 @@
# (iss, sub) miss but email matches admin, so it auto- # (iss, sub) miss but email matches admin, so it auto-
# links + logs admin in. # links + logs admin in.
# 2. Auto-link refused — email_verified=false. Callback # 2. Auto-link refused — email_verified=false. Callback
# returns HTTP 409 with error_type "AutoLinkEmailNotVerified" # redirects the browser to
# (one of three distinct auto-link refusal error_types — # /login?login_error=auto_link_email_not_verified so the
# see auth_handler.rs AutoLinkRefused arm). # SPA login page can render a localized notice. Sibling
# of the /profile?link_error=<reason> redirect used by
# the self-service link flow. See auth_handler.rs
# AutoLinkRefused arm.
# #
# [OIDC-only user] # [OIDC-only user]
# 10. `oidc_user` unlink refused (would lock them out) with # 10. `oidc_user` unlink refused (would lock them out) with
@@ -512,19 +515,18 @@ HTTP 200
# Follow the whole OIDC dance. Hurl's location: true follows 3xx # Follow the whole OIDC dance. Hurl's location: true follows 3xx
# up to the callback; the callback returns 409 (non-3xx) and # up to the callback; the callback redirects to /login with a
# location follow stops. The final response is what we assert on. # machine-readable reason on the query string, and the SPA login
# page lands at 200 (index.html fallback). Assert on URL, since
# that's the load-bearing wire contract the SPA reads on mount.
GET {{base_url}}/api/auth/oidc/authorize GET {{base_url}}/api/auth/oidc/authorize
[Options] [Options]
location: true location: true
location-trusted: true location-trusted: true
HTTP 409 HTTP 200
[Asserts] [Asserts]
# Distinct CamelCase key per auto-link refusal reason — the SPA url matches "^http://localhost:8087/login\\?login_error=auto_link_email_not_verified$"
# switches on this to render "verify your email at the IdP" copy
# rather than the generic contact-admin fallback.
jsonpath "$.error_type" == "AutoLinkEmailNotVerified"
# Belt-and-braces invariant: admin's row is still un-linked # Belt-and-braces invariant: admin's row is still un-linked