diff --git a/CHANGELOG.md b/CHANGELOG.md index b7d7d80b..26aacb28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ ## unreleased - fix: `sqlpage.variables()` now does not return json objects with duplicate keys when post, get and set variables of the same name are present. The semantics of the returned values remains the same (precedence: set > post > get). - add support for some duckdb-specific syntax like `select {'a': 1, 'b': 2}` when connected to duckdb through odbc. +- better oidc support. Single-sign-on now works with sites: + - using a non-default `site_prefix` + - hosted behind an ssl-terminating reverse proxy ## 0.41.0 (2025-12-28) - **New Function**: `sqlpage.oidc_logout_url(redirect_uri)` - Generates a secure logout URL for OIDC-authenticated users with support for [RP-Initiated Logout](https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout) 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/database/sqlpage_functions/functions.rs b/src/webserver/database/sqlpage_functions/functions.rs index 084cbac7..16737740 100644 --- a/src/webserver/database/sqlpage_functions/functions.rs +++ b/src/webserver/database/sqlpage_functions/functions.rs @@ -882,11 +882,7 @@ async fn oidc_logout_url<'a>( ); } - let logout_url = crate::webserver::oidc::create_logout_url( - redirect_uri, - &request.app_state.config.site_prefix, - &oidc_state.config.client_secret, - ); + let logout_url = oidc_state.config.create_logout_url(redirect_uri); Ok(Some(logout_url)) } diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index db7e180f..784420d5 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -79,6 +79,9 @@ pub struct OidcConfig { pub app_host: String, pub scopes: Vec, pub additional_audience_verifier: AudienceVerifier, + pub site_prefix: String, + pub redirect_uri: String, + pub logout_uri: String, } impl TryFrom<&AppConfig> for OidcConfig { @@ -94,6 +97,10 @@ impl TryFrom<&AppConfig> for OidcConfig { let app_host = get_app_host(config); + let site_prefix_trimmed = config.site_prefix.trim_end_matches('/'); + let redirect_uri = format!("{site_prefix_trimmed}{SQLPAGE_REDIRECT_URI}"); + let logout_uri = format!("{site_prefix_trimmed}{SQLPAGE_LOGOUT_URI}"); + Ok(Self { issuer_url: issuer_url.clone(), client_id: config.oidc_client_id.clone(), @@ -109,6 +116,9 @@ impl TryFrom<&AppConfig> for OidcConfig { additional_audience_verifier: AudienceVerifier::new( config.oidc_additional_trusted_audiences.clone(), ), + site_prefix: config.site_prefix.clone(), + redirect_uri, + logout_uri, }) } } @@ -129,6 +139,19 @@ impl OidcConfig { .id_token_verifier() .set_other_audience_verifier_fn(self.additional_audience_verifier.as_fn()) } + + /// Creates a logout URL with the given redirect URI + #[must_use] + pub fn create_logout_url(&self, redirect_uri: &str) -> String { + let timestamp = chrono::Utc::now().timestamp(); + let signature = compute_logout_signature(redirect_uri, timestamp, &self.client_secret); + let query = form_urlencoded::Serializer::new(String::new()) + .append_pair("redirect_uri", redirect_uri) + .append_pair("timestamp", ×tamp.to_string()) + .append_pair("signature", &signature) + .finish(); + format!("{}?{}", self.logout_uri, query) + } } fn get_app_host(config: &AppConfig) -> String { @@ -375,12 +398,12 @@ 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 { + if request.path() == oidc_state.config.redirect_uri { let response = handle_oidc_callback(oidc_state, request).await; return MiddlewareResponse::Respond(response); } - if request.path() == SQLPAGE_LOGOUT_URI { + if request.path() == oidc_state.config.logout_uri { let response = handle_oidc_logout(oidc_state, request).await; return MiddlewareResponse::Respond(response); } @@ -597,23 +620,6 @@ fn verify_logout_params(params: &LogoutParams, client_secret: &str) -> anyhow::R Ok(()) } -#[must_use] -pub fn create_logout_url(redirect_uri: &str, site_prefix: &str, client_secret: &str) -> String { - let timestamp = chrono::Utc::now().timestamp(); - let signature = compute_logout_signature(redirect_uri, timestamp, client_secret); - let query = form_urlencoded::Serializer::new(String::new()) - .append_pair("redirect_uri", redirect_uri) - .append_pair("timestamp", ×tamp.to_string()) - .append_pair("signature", &signature) - .finish(); - format!( - "{}{}?{}", - site_prefix.trim_end_matches('/'), - SQLPAGE_LOGOUT_URI, - query - ) -} - impl Service for OidcService where S: Service, Error = Error> + 'static, @@ -654,7 +660,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()); + let redirect_target = + validate_redirect_url(redirect_target.to_string(), &oidc_state.config.redirect_uri); log::info!("Redirecting to {redirect_target} after a successful login"); let mut response = build_redirect_response(redirect_target); @@ -900,7 +907,7 @@ fn make_oidc_client( let mut redirect_url = RedirectUrl::new(format!( "https://{}{}", - config.app_host, SQLPAGE_REDIRECT_URI, + config.app_host, config.redirect_uri, )) .with_context(|| { format!( @@ -915,10 +922,8 @@ 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, SQLPAGE_REDIRECT_URI, - ))?; + redirect_url = + RedirectUrl::new(format!("http://{}{}", config.app_host, config.redirect_uri,))?; } log::info!("OIDC redirect URL for {}: {redirect_url}", config.client_id); let client = @@ -1091,8 +1096,8 @@ 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, redirect_uri: &str) -> String { + if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(redirect_uri) { return url; } log::warn!("Refusing to redirect to {url}"); @@ -1136,7 +1141,20 @@ mod tests { #[test] fn logout_url_generation_and_parsing_are_compatible() { let secret = "super_secret_key"; - let generated = create_logout_url("/after", "https://example.com", secret); + let config = OidcConfig { + issuer_url: IssuerUrl::new("https://example.com".to_string()).unwrap(), + client_id: "test_client".to_string(), + client_secret: secret.to_string(), + protected_paths: vec![], + public_paths: vec![], + app_host: "example.com".to_string(), + scopes: vec![], + additional_audience_verifier: AudienceVerifier::new(None), + site_prefix: "https://example.com".to_string(), + redirect_uri: format!("https://example.com{SQLPAGE_REDIRECT_URI}"), + logout_uri: format!("https://example.com{SQLPAGE_LOGOUT_URI}"), + }; + let generated = config.create_logout_url("/after"); let parsed = Url::parse(&generated).expect("generated URL should be valid"); assert_eq!(parsed.path(), SQLPAGE_LOGOUT_URI); diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index 2972c126..7cf128c1 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -438,11 +438,67 @@ 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}", + "oidc_issuer_url": "{}", + "oidc_client_id": "{}", + "oidc_client_secret": "{}", + "oidc_protected_paths": ["/"], + "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 + ); +} + #[actix_web::test] async fn test_oidc_logout_uses_correct_scheme() { use sqlpage::{ app_config::{test_database_url, AppConfig}, - webserver::oidc::create_logout_url, AppState, }; @@ -463,9 +519,13 @@ async fn test_oidc_logout_uses_correct_scheme() { let config: AppConfig = serde_json::from_str(&config_json).unwrap(); let app_state = AppState::init(&config).await.unwrap(); + let logout_path = app_state + .oidc_state + .as_ref() + .unwrap() + .config + .create_logout_url("/logged_out"); let app = test::init_service(create_app(Data::new(app_state))).await; - - let logout_path = create_logout_url("/logged_out", "", &provider.client_secret); // make sure the logout path includes the configured domain assert!(logout_path.starts_with("/sqlpage/oidc_logout"));