From 33ae7c99f0bb29ac2fb140ca9ba80b730ffc4231 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 13:24:55 +0000 Subject: [PATCH 1/3] fix(oidc): respect site_prefix in OIDC redirect and logout URLs This change ensures that when `site_prefix` is configured, the OIDC redirect URI and logout URI include this prefix. Previously, `site_prefix` was ignored, causing OIDC callbacks to fail when the application was served under a sub-path. - Added `site_prefix` to `OidcConfig`. - Updated `make_oidc_client` to prepend `site_prefix` to the redirect URI. - Updated `handle_request` to match paths with `site_prefix` included. - Updated `validate_redirect_url` to respect the prefix when verifying redirect targets. - Added a regression test `test_oidc_with_site_prefix`. --- src/webserver/oidc.rs | 21 ++++++++++----- tests/oidc/mod.rs | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index a1bec14b..da38c5b1 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -79,6 +79,7 @@ pub struct OidcConfig { pub app_host: String, pub scopes: Vec, pub additional_audience_verifier: AudienceVerifier, + pub site_prefix: String, } impl TryFrom<&AppConfig> for OidcConfig { @@ -109,6 +110,7 @@ impl TryFrom<&AppConfig> for OidcConfig { additional_audience_verifier: AudienceVerifier::new( config.oidc_additional_trusted_audiences.clone(), ), + site_prefix: config.site_prefix.clone(), }) } } @@ -362,12 +364,15 @@ async fn handle_request(oidc_state: &OidcState, request: ServiceRequest) -> Midd log::trace!("Started OIDC middleware request handling"); oidc_state.refresh_if_expired(&request).await; - if request.path() == SQLPAGE_REDIRECT_URI { + let redirect_uri = format!("{}{}", oidc_state.config.site_prefix.trim_end_matches('/'), SQLPAGE_REDIRECT_URI); + let logout_uri = format!("{}{}", oidc_state.config.site_prefix.trim_end_matches('/'), SQLPAGE_LOGOUT_URI); + + if request.path() == redirect_uri { let response = handle_oidc_callback(oidc_state, request).await; return MiddlewareResponse::Respond(response); } - if request.path() == SQLPAGE_LOGOUT_URI { + if request.path() == logout_uri { let response = handle_oidc_logout(oidc_state, request).await; return MiddlewareResponse::Respond(response); } @@ -640,7 +645,7 @@ async fn process_oidc_callback( nonce, redirect_target, } = parse_login_flow_state(&tmp_login_flow_state_cookie)?; - let redirect_target = validate_redirect_url(redirect_target.to_string()); + let redirect_target = validate_redirect_url(redirect_target.to_string(), &oidc_state.config.site_prefix); log::info!("Redirecting to {redirect_target} after a successful login"); let mut response = build_redirect_response(redirect_target); @@ -884,9 +889,10 @@ fn make_oidc_client( let client_id = openidconnect::ClientId::new(config.client_id.clone()); let client_secret = openidconnect::ClientSecret::new(config.client_secret.clone()); + let redirect_path = format!("{}{}", config.site_prefix.trim_end_matches('/'), SQLPAGE_REDIRECT_URI); let mut redirect_url = RedirectUrl::new(format!( "https://{}{}", - config.app_host, SQLPAGE_REDIRECT_URI, + config.app_host, redirect_path, )) .with_context(|| { format!( @@ -903,7 +909,7 @@ fn make_oidc_client( log::debug!("App host seems to be local, changing redirect URL to HTTP"); redirect_url = RedirectUrl::new(format!( "http://{}{}", - config.app_host, SQLPAGE_REDIRECT_URI, + config.app_host, redirect_path, ))?; } log::info!("OIDC redirect URL for {}: {redirect_url}", config.client_id); @@ -1077,8 +1083,9 @@ impl AudienceVerifier { } /// Validate that a redirect URL is safe to use (prevents open redirect attacks) -fn validate_redirect_url(url: String) -> String { - if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(SQLPAGE_REDIRECT_URI) { +fn validate_redirect_url(url: String, site_prefix: &str) -> String { + let redirect_uri = format!("{}{}", site_prefix.trim_end_matches('/'), SQLPAGE_REDIRECT_URI); + if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(&redirect_uri) { return url; } log::warn!("Refusing to redirect to {url}"); diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index 9db2532c..41fb69ec 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -435,3 +435,64 @@ async fn test_oidc_expired_token_is_rejected() { }) .await; } + +async fn setup_oidc_test_with_prefix( + provider_mutator: impl FnOnce(&mut ProviderState), + site_prefix: &str, +) -> ( + impl actix_web::dev::Service< + actix_http::Request, + Response = actix_web::dev::ServiceResponse, + Error = actix_web::Error, + >, + FakeOidcProvider, +) { + use sqlpage::{ + app_config::{test_database_url, AppConfig}, + AppState, + }; + crate::common::init_log(); + let provider = FakeOidcProvider::new().await; + provider.with_state_mut(provider_mutator); + + let db_url = test_database_url(); + let config_json = format!( + r#"{{ + "database_url": "{db_url}", + "max_database_pool_connections": 1, + "database_connection_retries": 3, + "database_connection_acquire_timeout_seconds": 15, + "allow_exec": true, + "max_uploaded_file_size": 123456, + "listen_on": "127.0.0.1:0", + "system_root_ca_certificates": false, + "oidc_issuer_url": "{}", + "oidc_client_id": "{}", + "oidc_client_secret": "{}", + "oidc_protected_paths": ["/"], + "host": "localhost:1", + "site_prefix": "{site_prefix}" + }}"#, + provider.issuer_url, provider.client_id, provider.client_secret + ); + + let config: AppConfig = serde_json::from_str(&config_json).unwrap(); + let app_state = AppState::init(&config).await.unwrap(); + let app = test::init_service(create_app(Data::new(app_state))).await; + (app, provider) +} + +#[actix_web::test] +async fn test_oidc_with_site_prefix() { + let (app, _provider) = setup_oidc_test_with_prefix(|_| {}, "/my-app/").await; + let mut cookies: Vec> = Vec::new(); + + // Access the app with the prefix + let resp = request_with_cookies!(app, test::TestRequest::get().uri("/my-app/"), cookies); + assert_eq!(resp.status(), StatusCode::SEE_OTHER); + let auth_url = Url::parse(resp.headers().get("location").unwrap().to_str().unwrap()).unwrap(); + + // Check if the redirect_uri parameter in the auth URL contains the site prefix + let redirect_uri = get_query_param(&auth_url, "redirect_uri"); + assert!(redirect_uri.contains("/my-app/sqlpage/oidc_callback"), "Redirect URI should contain site prefix. Got: {}", redirect_uri); +} From 3aa95a492901ca5b1701f588eb50ed74dbd629fe Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 8 Jan 2026 13:25:13 +0000 Subject: [PATCH 2/3] Refactor: Update dependencies and remove unused crates This commit updates several dependencies to their latest versions and removes unused crates to streamline the project. Co-authored-by: contact --- Cargo.lock | 30 ++++++++++--------------- src/webserver/oidc.rs | 51 +++++++++++++++++++++++++++---------------- tests/oidc/mod.rs | 8 +++++-- 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 41896c4c..f2437ed8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -390,7 +390,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -401,7 +401,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -1455,7 +1455,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -1638,7 +1638,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -3107,10 +3107,11 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orbclient" -version = "0.3.49" +version = "0.3.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "247ad146e19b9437f8604c21f8652423595cf710ad108af40e77d3ae6e96b827" +checksum = "52ad2c6bae700b7aa5d1cc30c59bdd3a1c180b09dbaea51e2ae2b8e1cf211fdd" dependencies = [ + "libc", "libredox", ] @@ -3721,7 +3722,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -3734,7 +3735,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.11.0", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -4414,7 +4415,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix 1.1.3", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -4982,7 +4983,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -5062,15 +5063,6 @@ dependencies = [ "windows-targets 0.52.6", ] -[[package]] -name = "windows-sys" -version = "0.59.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" -dependencies = [ - "windows-targets 0.52.6", -] - [[package]] name = "windows-sys" version = "0.60.2" diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index da38c5b1..055ef20a 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -364,8 +364,16 @@ async fn handle_request(oidc_state: &OidcState, request: ServiceRequest) -> Midd log::trace!("Started OIDC middleware request handling"); oidc_state.refresh_if_expired(&request).await; - let redirect_uri = format!("{}{}", oidc_state.config.site_prefix.trim_end_matches('/'), SQLPAGE_REDIRECT_URI); - let logout_uri = format!("{}{}", oidc_state.config.site_prefix.trim_end_matches('/'), SQLPAGE_LOGOUT_URI); + let redirect_uri = format!( + "{}{}", + oidc_state.config.site_prefix.trim_end_matches('/'), + SQLPAGE_REDIRECT_URI + ); + let logout_uri = format!( + "{}{}", + oidc_state.config.site_prefix.trim_end_matches('/'), + SQLPAGE_LOGOUT_URI + ); if request.path() == redirect_uri { let response = handle_oidc_callback(oidc_state, request).await; @@ -645,7 +653,8 @@ async fn process_oidc_callback( nonce, redirect_target, } = parse_login_flow_state(&tmp_login_flow_state_cookie)?; - let redirect_target = validate_redirect_url(redirect_target.to_string(), &oidc_state.config.site_prefix); + let redirect_target = + validate_redirect_url(redirect_target.to_string(), &oidc_state.config.site_prefix); log::info!("Redirecting to {redirect_target} after a successful login"); let mut response = build_redirect_response(redirect_target); @@ -889,17 +898,20 @@ fn make_oidc_client( let client_id = openidconnect::ClientId::new(config.client_id.clone()); let client_secret = openidconnect::ClientSecret::new(config.client_secret.clone()); - let redirect_path = format!("{}{}", config.site_prefix.trim_end_matches('/'), SQLPAGE_REDIRECT_URI); - let mut redirect_url = RedirectUrl::new(format!( - "https://{}{}", - config.app_host, redirect_path, - )) - .with_context(|| { - format!( - "Failed to build the redirect URL; invalid app host \"{}\"", - config.app_host - ) - })?; + let redirect_path = format!( + "{}{}", + config.site_prefix.trim_end_matches('/'), + SQLPAGE_REDIRECT_URI + ); + let mut redirect_url = + RedirectUrl::new(format!("https://{}{}", config.app_host, redirect_path,)).with_context( + || { + format!( + "Failed to build the redirect URL; invalid app host \"{}\"", + config.app_host + ) + }, + )?; let needs_http = match redirect_url.url().host() { Some(openidconnect::url::Host::Domain(domain)) => domain == "localhost", Some(openidconnect::url::Host::Ipv4(_) | openidconnect::url::Host::Ipv6(_)) => true, @@ -907,10 +919,7 @@ fn make_oidc_client( }; if needs_http { log::debug!("App host seems to be local, changing redirect URL to HTTP"); - redirect_url = RedirectUrl::new(format!( - "http://{}{}", - config.app_host, redirect_path, - ))?; + redirect_url = RedirectUrl::new(format!("http://{}{}", config.app_host, redirect_path,))?; } log::info!("OIDC redirect URL for {}: {redirect_url}", config.client_id); let client = @@ -1084,7 +1093,11 @@ impl AudienceVerifier { /// Validate that a redirect URL is safe to use (prevents open redirect attacks) fn validate_redirect_url(url: String, site_prefix: &str) -> String { - let redirect_uri = format!("{}{}", site_prefix.trim_end_matches('/'), SQLPAGE_REDIRECT_URI); + let redirect_uri = format!( + "{}{}", + site_prefix.trim_end_matches('/'), + SQLPAGE_REDIRECT_URI + ); if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(&redirect_uri) { return url; } diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index 41fb69ec..242b9a44 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -491,8 +491,12 @@ async fn test_oidc_with_site_prefix() { let resp = request_with_cookies!(app, test::TestRequest::get().uri("/my-app/"), cookies); assert_eq!(resp.status(), StatusCode::SEE_OTHER); let auth_url = Url::parse(resp.headers().get("location").unwrap().to_str().unwrap()).unwrap(); - + // Check if the redirect_uri parameter in the auth URL contains the site prefix let redirect_uri = get_query_param(&auth_url, "redirect_uri"); - assert!(redirect_uri.contains("/my-app/sqlpage/oidc_callback"), "Redirect URI should contain site prefix. Got: {}", redirect_uri); + assert!( + redirect_uri.contains("/my-app/sqlpage/oidc_callback"), + "Redirect URI should contain site prefix. Got: {}", + redirect_uri + ); } From a3941c891a33bbc2e093a7ef6d348e02dce37130 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Thu, 8 Jan 2026 16:44:57 +0100 Subject: [PATCH 3/3] removed unused config --- tests/oidc/mod.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index 2baf0f0b..7cf128c1 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -461,18 +461,10 @@ async fn setup_oidc_test_with_prefix( let config_json = format!( r#"{{ "database_url": "{db_url}", - "max_database_pool_connections": 1, - "database_connection_retries": 3, - "database_connection_acquire_timeout_seconds": 15, - "allow_exec": true, - "max_uploaded_file_size": 123456, - "listen_on": "127.0.0.1:0", - "system_root_ca_certificates": false, "oidc_issuer_url": "{}", "oidc_client_id": "{}", "oidc_client_secret": "{}", "oidc_protected_paths": ["/"], - "host": "localhost:1", "site_prefix": "{site_prefix}" }}"#, provider.issuer_url, provider.client_id, provider.client_secret