diff --git a/src/app.rs b/src/app.rs index 293534e..0e57b2c 100644 --- a/src/app.rs +++ b/src/app.rs @@ -138,7 +138,11 @@ fn QuestionView( .into_any() } else { view! { - + "Sign in" } diff --git a/src/server/oidc.rs b/src/server/oidc.rs index 5ba4339..043429f 100644 --- a/src/server/oidc.rs +++ b/src/server/oidc.rs @@ -40,6 +40,7 @@ pub struct Oidc { const PKCE_KEY: &str = "oidc_pkce_verifier"; const CSRF_KEY: &str = "oidc_csrf_state"; const NONCE_KEY: &str = "oidc_nonce"; +const REDIRECT_KEY: &str = "oidc_post_login_redirect"; impl Oidc { /// Discovers the provider and builds the client from environment: @@ -92,8 +93,31 @@ fn internal(err: impl std::fmt::Display) -> HandlerError { ) } -/// GET /auth/login — stash PKCE/state/nonce in the session and bounce to Kanidm. -pub async fn login(State(state): State, session: Session) -> Result { +#[derive(Deserialize)] +pub struct LoginParams { + redirect: Option, +} + +/// A same-site, single-segment-rooted path only - never an absolute +/// URL or scheme-relative one (`//evil.com/...` is a valid "path" to a +/// browser but resolves to an external host), since this value comes +/// straight from a query param an attacker could craft into a phishing +/// link that still passes through this app's own real login flow. +fn is_safe_redirect(path: &str) -> bool { + path.starts_with('/') && !path.starts_with("//") && !path.contains("://") +} + +/// GET /auth/login — stash PKCE/state/nonce (and where to return to +/// after) in the session and bounce to Kanidm. +pub async fn login( + State(state): State, + session: Session, + Query(params): Query, +) -> Result { + if let Some(redirect) = params.redirect.filter(|r| is_safe_redirect(r)) { + session.insert(REDIRECT_KEY, redirect).await.map_err(internal)?; + } + let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256(); let (auth_url, csrf_state, nonce) = state @@ -209,8 +233,11 @@ pub async fn callback( .await .map_err(internal)?; - tracing::info!(user = %user.username, "signed in"); - Ok(Redirect::to("/")) + let redirect: Option = session.remove(REDIRECT_KEY).await.map_err(internal)?; + let target = redirect.filter(|r| is_safe_redirect(r)).unwrap_or_else(|| "/".to_string()); + + tracing::info!(user = %user.username, redirect = %target, "signed in"); + Ok(Redirect::to(&target)) } /// GET /auth/logout — drop the session.