diff --git a/src/admin.rs b/src/admin.rs index 184bcd4..da458bf 100644 --- a/src/admin.rs +++ b/src/admin.rs @@ -531,8 +531,6 @@ async fn logout(request: Request, session: Session) -> cot::Result { // OIDC Handlers // --------------------------------------------------------------------------- -const SESSION_OIDC_STATE: &str = "oidc_state"; - /// Read an OIDC-related setting from the DB, returning empty string if absent. async fn oidc_setting(db: &Database, name: &str) -> cot::Result { let k = name.to_string(); @@ -568,7 +566,7 @@ fn decode_jwt_payload(token: &str) -> Option { serde_json::from_slice(&bytes).ok() } -async fn oidc_start(request: Request, session: Session, db: Database) -> cot::Result { +async fn oidc_start(request: Request, db: Database) -> cot::Result { let lang = detect_lang(&request); let issuer_url = oidc_setting(&db, "oidc_issuer_url").await?; let client_id = oidc_setting(&db, "oidc_client_id").await?; @@ -588,7 +586,6 @@ async fn oidc_start(request: Request, session: Session, db: Database) -> cot::Re }; let state = rand_token(); - session.insert(SESSION_OIDC_STATE, state.clone()).await?; let redirect_uri = format!( "{}/admin/oidc/callback", @@ -603,13 +600,34 @@ async fn oidc_start(request: Request, session: Session, db: Database) -> cot::Re urlencoding::encode(&state), ); - Redirect::new(redirect_url).into_response() + let state_cookie = format!( + "oidc_state={}; Path=/admin/oidc; HttpOnly; Secure; SameSite=Lax; Max-Age=600", + state, + ); + + Redirect::new(redirect_url) + .into_response()? + .with_header("set-cookie", state_cookie) + .into_response() } async fn oidc_callback(request: Request, session: Session, db: Database) -> cot::Result { let lang = detect_lang(&request); let fail = format!("/admin/login?lang={}&error=sso", lang.code()); + // Read saved state from cookie + let saved_state = request + .headers() + .get("cookie") + .and_then(|v| v.to_str().ok()) + .and_then(|cookies| { + cookies.split(';').find_map(|part| { + let part = part.trim(); + part.strip_prefix("oidc_state=").map(|v| v.to_string()) + }) + }) + .unwrap_or_default(); + // Extract code and state from query string let query_str = request.uri().query().unwrap_or(""); let mut code = String::new(); @@ -622,14 +640,6 @@ async fn oidc_callback(request: Request, session: Session, db: Database) -> cot: } } - // Verify state - let saved_state = session - .get::(SESSION_OIDC_STATE) - .await - .ok() - .flatten() - .unwrap_or_default(); - if code.is_empty() || state.is_empty() || state != saved_state { tracing::warn!( "OIDC state mismatch: state={state:?}, saved={saved_state:?}, code_empty={}, state_empty={}", @@ -639,9 +649,6 @@ async fn oidc_callback(request: Request, session: Session, db: Database) -> cot: return Redirect::new(fail).into_response(); } - // Clear used state - let _ = session.remove::(SESSION_OIDC_STATE).await; - let issuer_url = oidc_setting(&db, "oidc_issuer_url").await?; let client_id = oidc_setting(&db, "oidc_client_id").await?; let client_secret = oidc_setting(&db, "oidc_client_secret").await?; @@ -753,7 +760,12 @@ async fn oidc_callback(request: Request, session: Session, db: Database) -> cot: session.insert(SESSION_USER_ID, user.id.unwrap()).await?; session.insert(SESSION_USER_NAME, display).await?; - Redirect::new(format!("/admin/?lang={}", lang.code())).into_response() + // Clear the oidc_state cookie + let clear_cookie = "oidc_state=; Path=/admin/oidc; HttpOnly; Secure; SameSite=Lax; Max-Age=0"; + Redirect::new(format!("/admin/?lang={}", lang.code())) + .into_response()? + .with_header("set-cookie", clear_cookie) + .into_response() } // --------------------------------------------------------------------------- diff --git a/src/main.rs b/src/main.rs index bf7a055..addbc01 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,8 +11,8 @@ use tracing_subscriber; use cot::cli::CliMetadata; use cot::config::{ - DatabaseConfig, MiddlewareConfig, ProjectConfig, SessionMiddlewareConfig, SessionStoreConfig, - SessionStoreTypeConfig, + DatabaseConfig, MiddlewareConfig, ProjectConfig, SameSite, SessionMiddlewareConfig, + SessionStoreConfig, SessionStoreTypeConfig, }; use cot::db::migrations::SyncDynMigration; use cot::middleware::SessionMiddleware; @@ -69,6 +69,7 @@ impl Project for PettingProject { .session( SessionMiddlewareConfig::builder() .secure(false) + .same_site(SameSite::Lax) .store( SessionStoreConfig::builder() .store_type(SessionStoreTypeConfig::Database)