From 17a21adea1accb8c96f7964b53c5d2d8e1b71afe Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Wed, 11 Mar 2026 17:57:16 +0100 Subject: [PATCH 1/3] Cap parallel OIDC login state cookies --- src/webserver/oidc.rs | 74 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 21a40e24..9bc6a637 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -51,6 +51,7 @@ const OIDC_CLIENT_MIN_REFRESH_INTERVAL: Duration = Duration::from_secs(5); const OIDC_HTTP_BODY_TIMEOUT: Duration = OIDC_CLIENT_MIN_REFRESH_INTERVAL; const SQLPAGE_OIDC_REDIRECT_COUNT_COOKIE: &str = "sqlpage_oidc_redirect_count"; const MAX_OIDC_REDIRECTS: u8 = 3; +const MAX_OIDC_PARALLEL_LOGIN_FLOWS: usize = 8; const AUTH_COOKIE_EXPIRATION: awc::cookie::time::Duration = actix_web::cookie::time::Duration::days(7); const LOGIN_FLOW_STATE_COOKIE_EXPIRATION: awc::cookie::time::Duration = @@ -455,7 +456,8 @@ fn handle_unauthenticated_request( let initial_url = request.uri().to_string(); let redirect_count = get_redirect_count(&request); - let response = build_auth_provider_redirect_response(oidc_state, &initial_url, redirect_count); + let response = + build_auth_provider_redirect_response(oidc_state, &request, &initial_url, redirect_count); MiddlewareResponse::Respond(request.into_response(response)) } @@ -487,7 +489,7 @@ fn handle_oidc_callback_error( if let Ok(http_client) = get_http_client_from_appdata(&request) { oidc_state.maybe_refresh(http_client, OIDC_CLIENT_MIN_REFRESH_INTERVAL); } - let resp = build_auth_provider_redirect_response(oidc_state, "/", redirect_count); + let resp = build_auth_provider_redirect_response(oidc_state, &request, "/", redirect_count); request.into_response(resp) } @@ -585,7 +587,6 @@ fn process_oidc_logout( .path("/") .finish(), )?; - log::debug!("User logged out successfully"); Ok(response) } @@ -736,6 +737,7 @@ fn set_auth_cookie(response: &mut HttpResponse, id_token: &OidcToken) { fn build_auth_provider_redirect_response( oidc_state: &OidcState, + request: &ServiceRequest, initial_url: &str, redirect_count: u8, ) -> HttpResponse { @@ -750,11 +752,15 @@ fn build_auth_provider_redirect_response( .same_site(actix_web::cookie::SameSite::Lax) .max_age(LOGIN_FLOW_STATE_COOKIE_EXPIRATION) .finish(); - HttpResponse::SeeOther() - .append_header((header::LOCATION, url.to_string())) - .cookie(tmp_login_flow_state_cookie) - .cookie(redirect_count_cookie) - .body("Redirecting...") + let mut response = HttpResponse::SeeOther(); + response.append_header((header::LOCATION, url.to_string())); + for mut cookie in get_tmp_login_flow_state_cookies_to_evict(request) { + cookie.make_removal(); + response.cookie(cookie); + } + response.cookie(tmp_login_flow_state_cookie); + response.cookie(redirect_count_cookie); + response.body("Redirecting...") } fn build_redirect_response(target_url: String) -> HttpResponse { @@ -1078,6 +1084,38 @@ fn get_tmp_login_flow_state_cookie( .with_context(|| format!("No {cookie_name} cookie found")) } +fn get_tmp_login_flow_state_cookies_to_evict(request: &ServiceRequest) -> Vec> { + request + .cookies() + .map(|cookies| { + let excess_cookie_count = cookies + .iter() + .filter(|cookie| { + cookie + .name() + .starts_with(SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX) + }) + .count() + .saturating_add(1) + .saturating_sub(MAX_OIDC_PARALLEL_LOGIN_FLOWS); + cookies + .iter() + .filter(|cookie| { + cookie + .name() + .starts_with(SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX) + }) + .take(excess_cookie_count) + .map(|cookie| { + let mut cookie = cookie.clone().into_owned(); + cookie.set_path("/"); + cookie + }) + .collect() + }) + .unwrap_or_default() +} + #[derive(Debug, Serialize, Deserialize, Clone)] struct LoginFlowState<'a> { #[serde(rename = "n")] @@ -1127,6 +1165,7 @@ fn validate_redirect_url(url: String, redirect_uri: &str) -> String { mod tests { use super::*; use actix_web::http::StatusCode; + use actix_web::{cookie::Cookie, test::TestRequest}; use openidconnect::url::Url; #[test] @@ -1182,4 +1221,23 @@ mod tests { .expect("generated URL should parse"); verify_logout_params(¶ms, secret).expect("generated URL should validate"); } + + #[test] + fn evicts_excess_tmp_login_flow_state_cookies() { + let request = (0..MAX_OIDC_PARALLEL_LOGIN_FLOWS) + .fold(TestRequest::default(), |request, i| { + request.cookie(Cookie::new( + format!("{SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX}{i}"), + format!("value-{i}"), + )) + }) + .to_srv_request(); + + let cookies_to_evict = get_tmp_login_flow_state_cookies_to_evict(&request); + + assert_eq!(cookies_to_evict.len(), 1); + assert!(cookies_to_evict[0] + .name() + .starts_with(SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX)); + } } From d40e58bf3d63ad5562b98cc8eafd7c420e67872d Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Wed, 11 Mar 2026 17:57:16 +0100 Subject: [PATCH 2/3] Cap parallel OIDC login state cookies --- src/webserver/oidc.rs | 71 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 21a40e24..40251023 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -51,6 +51,7 @@ const OIDC_CLIENT_MIN_REFRESH_INTERVAL: Duration = Duration::from_secs(5); const OIDC_HTTP_BODY_TIMEOUT: Duration = OIDC_CLIENT_MIN_REFRESH_INTERVAL; const SQLPAGE_OIDC_REDIRECT_COUNT_COOKIE: &str = "sqlpage_oidc_redirect_count"; const MAX_OIDC_REDIRECTS: u8 = 3; +const MAX_OIDC_PARALLEL_LOGIN_FLOWS: usize = 8; const AUTH_COOKIE_EXPIRATION: awc::cookie::time::Duration = actix_web::cookie::time::Duration::days(7); const LOGIN_FLOW_STATE_COOKIE_EXPIRATION: awc::cookie::time::Duration = @@ -455,7 +456,8 @@ fn handle_unauthenticated_request( let initial_url = request.uri().to_string(); let redirect_count = get_redirect_count(&request); - let response = build_auth_provider_redirect_response(oidc_state, &initial_url, redirect_count); + let response = + build_auth_provider_redirect_response(oidc_state, &request, &initial_url, redirect_count); MiddlewareResponse::Respond(request.into_response(response)) } @@ -487,7 +489,7 @@ fn handle_oidc_callback_error( if let Ok(http_client) = get_http_client_from_appdata(&request) { oidc_state.maybe_refresh(http_client, OIDC_CLIENT_MIN_REFRESH_INTERVAL); } - let resp = build_auth_provider_redirect_response(oidc_state, "/", redirect_count); + let resp = build_auth_provider_redirect_response(oidc_state, &request, "/", redirect_count); request.into_response(resp) } @@ -585,7 +587,6 @@ fn process_oidc_logout( .path("/") .finish(), )?; - log::debug!("User logged out successfully"); Ok(response) } @@ -736,6 +737,7 @@ fn set_auth_cookie(response: &mut HttpResponse, id_token: &OidcToken) { fn build_auth_provider_redirect_response( oidc_state: &OidcState, + request: &ServiceRequest, initial_url: &str, redirect_count: u8, ) -> HttpResponse { @@ -750,11 +752,15 @@ fn build_auth_provider_redirect_response( .same_site(actix_web::cookie::SameSite::Lax) .max_age(LOGIN_FLOW_STATE_COOKIE_EXPIRATION) .finish(); - HttpResponse::SeeOther() - .append_header((header::LOCATION, url.to_string())) - .cookie(tmp_login_flow_state_cookie) - .cookie(redirect_count_cookie) - .body("Redirecting...") + let mut response = HttpResponse::SeeOther(); + response.append_header((header::LOCATION, url.to_string())); + for mut cookie in get_tmp_login_flow_state_cookies_to_evict(request) { + cookie.make_removal(); + response.cookie(cookie); + } + response.cookie(tmp_login_flow_state_cookie); + response.cookie(redirect_count_cookie); + response.body("Redirecting...") } fn build_redirect_response(target_url: String) -> HttpResponse { @@ -1078,6 +1084,35 @@ fn get_tmp_login_flow_state_cookie( .with_context(|| format!("No {cookie_name} cookie found")) } +fn get_tmp_login_flow_state_cookies_to_evict(request: &ServiceRequest) -> Vec> { + request + .cookies() + .map(|cookies| { + let mut tmp_login_flow_state_cookies = cookies + .iter() + .filter(|cookie| { + cookie + .name() + .starts_with(SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX) + }) + .cloned() + .collect::>(); + let excess_cookie_count = tmp_login_flow_state_cookies + .len() + .saturating_add(1) + .saturating_sub(MAX_OIDC_PARALLEL_LOGIN_FLOWS); + tmp_login_flow_state_cookies + .drain(..excess_cookie_count) + .map(|cookie| { + let mut cookie = cookie.into_owned(); + cookie.set_path("/"); + cookie + }) + .collect() + }) + .unwrap_or_default() +} + #[derive(Debug, Serialize, Deserialize, Clone)] struct LoginFlowState<'a> { #[serde(rename = "n")] @@ -1127,6 +1162,7 @@ fn validate_redirect_url(url: String, redirect_uri: &str) -> String { mod tests { use super::*; use actix_web::http::StatusCode; + use actix_web::{cookie::Cookie, test::TestRequest}; use openidconnect::url::Url; #[test] @@ -1182,4 +1218,23 @@ mod tests { .expect("generated URL should parse"); verify_logout_params(¶ms, secret).expect("generated URL should validate"); } + + #[test] + fn evicts_excess_tmp_login_flow_state_cookies() { + let request = (0..MAX_OIDC_PARALLEL_LOGIN_FLOWS) + .fold(TestRequest::default(), |request, i| { + request.cookie(Cookie::new( + format!("{SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX}{i}"), + format!("value-{i}"), + )) + }) + .to_srv_request(); + + let cookies_to_evict = get_tmp_login_flow_state_cookies_to_evict(&request); + + assert_eq!(cookies_to_evict.len(), 1); + assert!(cookies_to_evict[0] + .name() + .starts_with(SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX)); + } } From 3dfa8b56dbd3b26e55376c5a8ca4eca4a8f57a5a Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Thu, 12 Mar 2026 11:23:18 +0100 Subject: [PATCH 3/3] simplify cookie eviction code --- src/webserver/oidc.rs | 46 ++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 40251023..da4f669b 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -754,9 +754,11 @@ fn build_auth_provider_redirect_response( .finish(); let mut response = HttpResponse::SeeOther(); response.append_header((header::LOCATION, url.to_string())); - for mut cookie in get_tmp_login_flow_state_cookies_to_evict(request) { - cookie.make_removal(); - response.cookie(cookie); + if let Ok(cookies) = request.cookies() { + for mut cookie in get_tmp_login_flow_state_cookies_to_evict(&cookies).cloned() { + cookie.make_removal(); + response.cookie(cookie); + } } response.cookie(tmp_login_flow_state_cookie); response.cookie(redirect_count_cookie); @@ -1084,33 +1086,13 @@ fn get_tmp_login_flow_state_cookie( .with_context(|| format!("No {cookie_name} cookie found")) } -fn get_tmp_login_flow_state_cookies_to_evict(request: &ServiceRequest) -> Vec> { - request - .cookies() - .map(|cookies| { - let mut tmp_login_flow_state_cookies = cookies - .iter() - .filter(|cookie| { - cookie - .name() - .starts_with(SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX) - }) - .cloned() - .collect::>(); - let excess_cookie_count = tmp_login_flow_state_cookies - .len() - .saturating_add(1) - .saturating_sub(MAX_OIDC_PARALLEL_LOGIN_FLOWS); - tmp_login_flow_state_cookies - .drain(..excess_cookie_count) - .map(|cookie| { - let mut cookie = cookie.into_owned(); - cookie.set_path("/"); - cookie - }) - .collect() - }) - .unwrap_or_default() +fn get_tmp_login_flow_state_cookies_to_evict<'a>( + cookies: &'a [Cookie<'static>], +) -> impl Iterator> + 'a { + let is_state = &|c: &Cookie<'_>| c.name().starts_with(SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX); + let login_state_count = cookies.iter().filter(|c| is_state(c)).count(); + let to_evict = login_state_count.saturating_sub(MAX_OIDC_PARALLEL_LOGIN_FLOWS - 1); + cookies.iter().filter(|c| is_state(c)).take(to_evict) } #[derive(Debug, Serialize, Deserialize, Clone)] @@ -1230,7 +1212,9 @@ mod tests { }) .to_srv_request(); - let cookies_to_evict = get_tmp_login_flow_state_cookies_to_evict(&request); + let cookies = request.cookies().unwrap(); + let cookies_to_evict: Vec<_> = + get_tmp_login_flow_state_cookies_to_evict(&cookies).collect(); assert_eq!(cookies_to_evict.len(), 1); assert!(cookies_to_evict[0]