From 4d6c4bb92e477d42bfb564718d799ef461605da2 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sat, 8 Aug 2026 20:31:19 +0200 Subject: [PATCH] feat(oidc): improve error handling --- frontend/src/routes/login/+page.svelte | 962 +++++++++++--------- src/interfaces/api/handlers/auth_handler.rs | 65 +- tests/oidc/link_unlink.hurl | 22 +- 3 files changed, 587 insertions(+), 462 deletions(-) diff --git a/frontend/src/routes/login/+page.svelte b/frontend/src/routes/login/+page.svelte index 80bc738b..81450502 100644 --- a/frontend/src/routes/login/+page.svelte +++ b/frontend/src/routes/login/+page.svelte @@ -97,6 +97,13 @@ // immediately after so revisits / manual logouts don't re-show // the stale message. let sessionExpiredNotice = $state(false); + // One-shot notice populated from ?login_error= on mount. + // Set by the OIDC callback's AutoLinkRefused redirect when the + // IdP-returned email matches an existing local account but the + // auto-link decision tree refused (verified=false, disabled by + // config, or the local account is already linked to a different + // identity). See docs/plan/oidc-account-linking.md § Auto-link. + let loginErrorNotice = $state(null); // Refs used by the mode-driven auto-focus effect. Bound with // `bind:this` on the first input of each mode's form so the effect // can focus the "primary" field each time the mode changes without @@ -290,6 +297,43 @@ } } + // Reason keys mirror the OIDC callback's redirect arms in + // auth_handler.rs — snake_case, matching the URL param shape used + // by the sibling /profile?link_error= flow. Any unknown + // key falls back to the generic copy so a new backend reason never + // blanks out the notice. + function loginErrorMessage(key: string): string { + switch (key) { + case 'auto_link_disabled': + return t( + 'auth.login_error_auto_link_disabled', + 'This server does not auto-link SSO accounts. Sign in with your existing credentials, then connect SSO from your profile.' + ); + case 'auto_link_email_not_verified': + return t( + 'auth.login_error_auto_link_email_not_verified', + 'Your SSO provider did not confirm your email address. Verify your email at your identity provider, then try again.' + ); + case 'already_linked_elsewhere': + return t( + 'auth.login_error_already_linked_elsewhere', + 'A local account with this email already exists and is linked to a different SSO identity. Contact your administrator.' + ); + case 'callback_denied': + return t( + 'auth.login_error_callback_denied', + 'Your sign-in link expired or was already used. Please try signing in again.' + ); + case 'callback_failed': + return t( + 'auth.login_error_callback_failed', + "SSO sign-in couldn't complete. Please try again." + ); + default: + return t('auth.login_error_generic', 'SSO sign-in was refused. Please try again.'); + } + } + onMount(async () => { // 0) Consume the one-shot `?source=session_expired` flag, if any. // Strip it from the URL so the banner never re-appears on @@ -306,6 +350,22 @@ ); } + // One-shot auto-link refusal notice. Reason key is a stable + // snake_case identifier the OIDC callback emitted; map each to + // localized copy and strip the param so a reload doesn't + // re-surface the same notice. + const loginErrorKey = page.url.searchParams.get('login_error'); + if (loginErrorKey) { + loginErrorNotice = loginErrorMessage(loginErrorKey); + const stripped = new URL(page.url); + stripped.searchParams.delete('login_error'); + window.history.replaceState( + window.history.state, + '', + stripped.pathname + stripped.search + stripped.hash + ); + } + // 1) OIDC code-exchange fallback: the IdP round-trip may land back here // with ?oidc_code=. Exchange it for a session and redirect into the app. const oidcCode = page.url.searchParams.get('oidc_code'); @@ -386,481 +446,509 @@ magic-link toggle) in place. Guarding the whole form behind `booting` caused a "logo only, then form" flash on first paint. --> -

- {#if mode === 'login'} - {t('auth.sign_in', 'Sign in')} - {:else if mode === 'register'} - {t('auth.register', 'Create account')} - {:else} - {t('auth.setup_title', 'Initial setup')} + {#if loginErrorNotice} + + + {:else} +

+ {#if mode === 'login'} + {t('auth.sign_in', 'Sign in')} + {:else if mode === 'register'} + {t('auth.register', 'Create account')} + {:else} + {t('auth.setup_title', 'Initial setup')} + {/if} +

+ + {#if sessionExpiredNotice} + {/if} - - {#if sessionExpiredNotice} - - {/if} + {postRegisterNotice} + + + {/if} - {#if postRegisterNotice && mode === 'login'} -
- {postRegisterNotice} - -
- {/if} - - {#if mode === 'login'} - - {#if passwordLoginEnabled || magicLinkLoginEnabled} - {#if error} - + {/if} + {#if magicStatus} +
+ {magicStatus.text} +
+ {/if} +
+
+ +
+ +
+
+ + {#if passwordLoginEnabled} +
+ +
+ + +
+ {#if capsOn} +
{t('auth.caps_lock', 'Caps Lock is on')}
+ {/if} +
+ {/if} + + +
+ {/if} + + {#if oidc.enabled} + {#if passwordLoginEnabled} +
{t('auth.or', 'or')}
+ {/if} + + - {error} + {t( + 'auth.sso_login_provider', + { provider: oidc.provider_name ?? 'SSO' }, + 'Sign in with {{provider}}' + )} + + {/if} + + {#if passwordLoginEnabled} +
+ {t('auth.no_account', 'No account?')} +
{/if} - {#if magicStatus} -
- {magicStatus.text} + + {#if setupAvailable} +
+ {t('auth.admin_setup', 'First time?')} +
{/if} -
+ {:else if mode === 'register'} + {#if regError}{/if} + +
-
+
+ + +
+ + {#if passwordLoginEnabled} +
+ +
+ + +
+ {#if regCapsOn} +
{t('auth.caps_lock', 'Caps Lock is on')}
+ {/if} +
+ {#if !regEmailOnly} +
+ +
+ + +
+ {#if matchState} +
+ {matchState === 'ok' + ? t('auth.passwords_match', 'Passwords match') + : t('auth.passwords_mismatch', "Passwords don't match")} +
+ {/if} +
+ {/if} + {/if} + +
+
+ {t('auth.have_account', 'Already have an account?')} + +
+ {:else} +
+
+
1
+
{t('auth.setup_step1', 'Admin')}
+
+
+
2
+
{t('auth.setup_step2', 'System')}
+
+
+
3
+
{t('auth.setup_step3', 'Completed')}
+
+
+ + {#if setupError}{/if} + {#if setupSuccess}
{setupSuccess}
{/if} + +
+
+
+
+
+ +
+ +
+
- {#if passwordLoginEnabled} -
- -
- - -
- {#if capsOn} -
{t('auth.caps_lock', 'Caps Lock is on')}
- {/if} -
- {/if} - - -
- {/if} - - {#if oidc.enabled} - {#if passwordLoginEnabled} -
{t('auth.or', 'or')}
- {/if} - - - {t( - 'auth.sso_login_provider', - { provider: oidc.provider_name ?? 'SSO' }, - 'Sign in with {{provider}}' - )} - - {/if} - - {#if passwordLoginEnabled} -
- {t('auth.no_account', 'No account?')} - -
- {/if} - - {#if setupAvailable} -
- {t('auth.admin_setup', 'First time?')} - -
- {/if} - {:else if mode === 'register'} - {#if regError}{/if} -
- -
- - -
-
- - -
- - {#if passwordLoginEnabled}
-
- {#if !regEmailOnly} -
- + +
+ + +
+ {#if setupMatchState} +
-
- - + {setupMatchState === 'ok' + ? t('auth.passwords_match', 'Passwords match') + : t('auth.passwords_mismatch', "Passwords don't match")}
- {#if matchState} -
- {matchState === 'ok' - ? t('auth.passwords_match', 'Passwords match') - : t('auth.passwords_mismatch', "Passwords don't match")} -
- {/if} -
- {/if} - {/if} - - -
- {t('auth.have_account', 'Already have an account?')} - -
- {:else} -
-
-
1
-
{t('auth.setup_step1', 'Admin')}
-
-
-
2
-
{t('auth.setup_step2', 'System')}
-
-
-
3
-
{t('auth.setup_step3', 'Completed')}
-
-
- - {#if setupError}{/if} - {#if setupSuccess}
{setupSuccess}
{/if} - -
-
- -
- + {/if}
+ + + + +
+ {t('auth.back_to_login', 'Already configured?')} +
- -
- -
- -
-
- -
- -
- - -
- {#if setupCapsOn} -
{t('auth.caps_lock', 'Caps Lock is on')}
- {/if} -
- -
- -
- - -
- {#if setupMatchState} -
- {setupMatchState === 'ok' - ? t('auth.passwords_match', 'Passwords match') - : t('auth.passwords_mismatch', "Passwords don't match")} -
- {/if} -
- - - - -
- {t('auth.back_to_login', 'Already configured?')} - -
+ {/if} {/if}
@@ -911,4 +999,22 @@ .auth-notice-dismiss:hover { opacity: 0.7; } + + /* Dedicated error view — centered in the auth-panel with the + message padded off the title/button. `.auth-button` (from + ported/auth.css) already carries the full-width primary + styling used by the login submit, so the back button lands + right on the existing theme. */ + .auth-error-view { + display: flex; + flex-direction: column; + align-items: center; + gap: var(--space-4); + text-align: center; + } + + .auth-error-view__message { + color: var(--color-text-secondary); + margin: 0; + } diff --git a/src/interfaces/api/handlers/auth_handler.rs b/src/interfaces/api/handlers/auth_handler.rs index db1ad883..d5f6173d 100644 --- a/src/interfaces/api/handlers/auth_handler.rs +++ b/src/interfaces/api/handlers/auth_handler.rs @@ -1509,14 +1509,35 @@ pub async fn oidc_callback( tracing::info!("OIDC callback received with code"); - // Exchange code, validate state/nonce/PKCE, authenticate user - let result = auth_app + // Exchange code, validate state/nonce/PKCE, authenticate user. + // 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= — 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) .await - .map_err(|e| { + { + Ok(r) => r, + Err(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 { OidcCallbackResult::WebLogin { exchange_code } => { @@ -1582,26 +1603,22 @@ pub async fn oidc_callback( ); Ok(Redirect::temporary(&redirect_url).into_response()) } - // Map each auto-link refusal reason to a distinct stable - // CamelCase `error_type`. The SPA switches on this to render - // targeted copy (contact-admin vs. verify-email-at-IdP vs. - // already-linked-elsewhere) rather than a generic error toast. - // Status stays 409 (CONFLICT) — semantically an existing user - // blocks the auto-provision path. + // Redirect the browser back to the login page with a + // machine-readable reason on the query string, mirroring the + // LinkRefused → `/profile?link_error=` pattern above. + // The browser is mid-redirect from the IdP; returning a 409 + // JSON body would leave the user staring at raw JSON. The SPA + // login page reads `?login_error=` on mount, renders a + // localized notice, and strips the param via history.replaceState. OidcCallbackResult::AutoLinkRefused { reason } => { - let error_type = match reason { - "auto_link_disabled" => "AutoLinkDisabled", - "auto_link_email_not_verified" => "AutoLinkEmailNotVerified", - "already_linked_elsewhere" => "AutoLinkAlreadyLinkedElsewhere", - _ => "AutoLinkRefused", - }; - Err(AppError::new( - StatusCode::CONFLICT, - "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, - )) + let config = auth_app.oidc_config().unwrap(); + let frontend_url = config.frontend_url.trim_end_matches('/'); + let redirect_url = format!("{}/login?login_error={}", frontend_url, reason); + tracing::info!( + reason = reason, + "OIDC auto-link refused, redirecting to /login?login_error" + ); + Ok(Redirect::temporary(&redirect_url).into_response()) } } } diff --git a/tests/oidc/link_unlink.hurl b/tests/oidc/link_unlink.hurl index d42cba9d..31e41bb7 100644 --- a/tests/oidc/link_unlink.hurl +++ b/tests/oidc/link_unlink.hurl @@ -40,9 +40,12 @@ # (iss, sub) miss but email matches admin, so it auto- # links + logs admin in. # 2. Auto-link refused — email_verified=false. Callback -# returns HTTP 409 with error_type "AutoLinkEmailNotVerified" -# (one of three distinct auto-link refusal error_types — -# see auth_handler.rs AutoLinkRefused arm). +# redirects the browser to +# /login?login_error=auto_link_email_not_verified so the +# SPA login page can render a localized notice. Sibling +# of the /profile?link_error= redirect used by +# the self-service link flow. See auth_handler.rs +# AutoLinkRefused arm. # # [OIDC-only user] # 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 -# up to the callback; the callback returns 409 (non-3xx) and -# location follow stops. The final response is what we assert on. +# up to the callback; the callback redirects to /login with a +# 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 [Options] location: true location-trusted: true -HTTP 409 +HTTP 200 [Asserts] -# Distinct CamelCase key per auto-link refusal reason — the SPA -# switches on this to render "verify your email at the IdP" copy -# rather than the generic contact-admin fallback. -jsonpath "$.error_type" == "AutoLinkEmailNotVerified" +url matches "^http://localhost:8087/login\\?login_error=auto_link_email_not_verified$" # Belt-and-braces invariant: admin's row is still un-linked