Skip to content

Commit 8d106fb

Browse files
lovasoacursoragent
andauthored
Oidc site prefix handling (#1179)
* 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`. * 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 <contact@ophir.dev> * removed unused config --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 72eff80 commit 8d106fb

5 files changed

Lines changed: 124 additions & 55 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
## unreleased
44
- 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).
55
- add support for some duckdb-specific syntax like `select {'a': 1, 'b': 2}` when connected to duckdb through odbc.
6+
- better oidc support. Single-sign-on now works with sites:
7+
- using a non-default `site_prefix`
8+
- hosted behind an ssl-terminating reverse proxy
69

710
## 0.41.0 (2025-12-28)
811
- **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)

Cargo.lock

Lines changed: 11 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/webserver/database/sqlpage_functions/functions.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -882,11 +882,7 @@ async fn oidc_logout_url<'a>(
882882
);
883883
}
884884

885-
let logout_url = crate::webserver::oidc::create_logout_url(
886-
redirect_uri,
887-
&request.app_state.config.site_prefix,
888-
&oidc_state.config.client_secret,
889-
);
885+
let logout_url = oidc_state.config.create_logout_url(redirect_uri);
890886

891887
Ok(Some(logout_url))
892888
}

src/webserver/oidc.rs

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ pub struct OidcConfig {
7979
pub app_host: String,
8080
pub scopes: Vec<Scope>,
8181
pub additional_audience_verifier: AudienceVerifier,
82+
pub site_prefix: String,
83+
pub redirect_uri: String,
84+
pub logout_uri: String,
8285
}
8386

8487
impl TryFrom<&AppConfig> for OidcConfig {
@@ -94,6 +97,10 @@ impl TryFrom<&AppConfig> for OidcConfig {
9497

9598
let app_host = get_app_host(config);
9699

100+
let site_prefix_trimmed = config.site_prefix.trim_end_matches('/');
101+
let redirect_uri = format!("{site_prefix_trimmed}{SQLPAGE_REDIRECT_URI}");
102+
let logout_uri = format!("{site_prefix_trimmed}{SQLPAGE_LOGOUT_URI}");
103+
97104
Ok(Self {
98105
issuer_url: issuer_url.clone(),
99106
client_id: config.oidc_client_id.clone(),
@@ -109,6 +116,9 @@ impl TryFrom<&AppConfig> for OidcConfig {
109116
additional_audience_verifier: AudienceVerifier::new(
110117
config.oidc_additional_trusted_audiences.clone(),
111118
),
119+
site_prefix: config.site_prefix.clone(),
120+
redirect_uri,
121+
logout_uri,
112122
})
113123
}
114124
}
@@ -129,6 +139,19 @@ impl OidcConfig {
129139
.id_token_verifier()
130140
.set_other_audience_verifier_fn(self.additional_audience_verifier.as_fn())
131141
}
142+
143+
/// Creates a logout URL with the given redirect URI
144+
#[must_use]
145+
pub fn create_logout_url(&self, redirect_uri: &str) -> String {
146+
let timestamp = chrono::Utc::now().timestamp();
147+
let signature = compute_logout_signature(redirect_uri, timestamp, &self.client_secret);
148+
let query = form_urlencoded::Serializer::new(String::new())
149+
.append_pair("redirect_uri", redirect_uri)
150+
.append_pair("timestamp", &timestamp.to_string())
151+
.append_pair("signature", &signature)
152+
.finish();
153+
format!("{}?{}", self.logout_uri, query)
154+
}
132155
}
133156

134157
fn get_app_host(config: &AppConfig) -> String {
@@ -375,12 +398,12 @@ async fn handle_request(oidc_state: &OidcState, request: ServiceRequest) -> Midd
375398
log::trace!("Started OIDC middleware request handling");
376399
oidc_state.refresh_if_expired(&request).await;
377400

378-
if request.path() == SQLPAGE_REDIRECT_URI {
401+
if request.path() == oidc_state.config.redirect_uri {
379402
let response = handle_oidc_callback(oidc_state, request).await;
380403
return MiddlewareResponse::Respond(response);
381404
}
382405

383-
if request.path() == SQLPAGE_LOGOUT_URI {
406+
if request.path() == oidc_state.config.logout_uri {
384407
let response = handle_oidc_logout(oidc_state, request).await;
385408
return MiddlewareResponse::Respond(response);
386409
}
@@ -597,23 +620,6 @@ fn verify_logout_params(params: &LogoutParams, client_secret: &str) -> anyhow::R
597620
Ok(())
598621
}
599622

600-
#[must_use]
601-
pub fn create_logout_url(redirect_uri: &str, site_prefix: &str, client_secret: &str) -> String {
602-
let timestamp = chrono::Utc::now().timestamp();
603-
let signature = compute_logout_signature(redirect_uri, timestamp, client_secret);
604-
let query = form_urlencoded::Serializer::new(String::new())
605-
.append_pair("redirect_uri", redirect_uri)
606-
.append_pair("timestamp", &timestamp.to_string())
607-
.append_pair("signature", &signature)
608-
.finish();
609-
format!(
610-
"{}{}?{}",
611-
site_prefix.trim_end_matches('/'),
612-
SQLPAGE_LOGOUT_URI,
613-
query
614-
)
615-
}
616-
617623
impl<S> Service<ServiceRequest> for OidcService<S>
618624
where
619625
S: Service<ServiceRequest, Response = ServiceResponse<BoxBody>, Error = Error> + 'static,
@@ -654,7 +660,8 @@ async fn process_oidc_callback(
654660
nonce,
655661
redirect_target,
656662
} = parse_login_flow_state(&tmp_login_flow_state_cookie)?;
657-
let redirect_target = validate_redirect_url(redirect_target.to_string());
663+
let redirect_target =
664+
validate_redirect_url(redirect_target.to_string(), &oidc_state.config.redirect_uri);
658665

659666
log::info!("Redirecting to {redirect_target} after a successful login");
660667
let mut response = build_redirect_response(redirect_target);
@@ -900,7 +907,7 @@ fn make_oidc_client(
900907

901908
let mut redirect_url = RedirectUrl::new(format!(
902909
"https://{}{}",
903-
config.app_host, SQLPAGE_REDIRECT_URI,
910+
config.app_host, config.redirect_uri,
904911
))
905912
.with_context(|| {
906913
format!(
@@ -915,10 +922,8 @@ fn make_oidc_client(
915922
};
916923
if needs_http {
917924
log::debug!("App host seems to be local, changing redirect URL to HTTP");
918-
redirect_url = RedirectUrl::new(format!(
919-
"http://{}{}",
920-
config.app_host, SQLPAGE_REDIRECT_URI,
921-
))?;
925+
redirect_url =
926+
RedirectUrl::new(format!("http://{}{}", config.app_host, config.redirect_uri,))?;
922927
}
923928
log::info!("OIDC redirect URL for {}: {redirect_url}", config.client_id);
924929
let client =
@@ -1091,8 +1096,8 @@ impl AudienceVerifier {
10911096
}
10921097

10931098
/// Validate that a redirect URL is safe to use (prevents open redirect attacks)
1094-
fn validate_redirect_url(url: String) -> String {
1095-
if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(SQLPAGE_REDIRECT_URI) {
1099+
fn validate_redirect_url(url: String, redirect_uri: &str) -> String {
1100+
if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(redirect_uri) {
10961101
return url;
10971102
}
10981103
log::warn!("Refusing to redirect to {url}");
@@ -1136,7 +1141,20 @@ mod tests {
11361141
#[test]
11371142
fn logout_url_generation_and_parsing_are_compatible() {
11381143
let secret = "super_secret_key";
1139-
let generated = create_logout_url("/after", "https://example.com", secret);
1144+
let config = OidcConfig {
1145+
issuer_url: IssuerUrl::new("https://example.com".to_string()).unwrap(),
1146+
client_id: "test_client".to_string(),
1147+
client_secret: secret.to_string(),
1148+
protected_paths: vec![],
1149+
public_paths: vec![],
1150+
app_host: "example.com".to_string(),
1151+
scopes: vec![],
1152+
additional_audience_verifier: AudienceVerifier::new(None),
1153+
site_prefix: "https://example.com".to_string(),
1154+
redirect_uri: format!("https://example.com{SQLPAGE_REDIRECT_URI}"),
1155+
logout_uri: format!("https://example.com{SQLPAGE_LOGOUT_URI}"),
1156+
};
1157+
let generated = config.create_logout_url("/after");
11401158

11411159
let parsed = Url::parse(&generated).expect("generated URL should be valid");
11421160
assert_eq!(parsed.path(), SQLPAGE_LOGOUT_URI);

tests/oidc/mod.rs

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -438,11 +438,67 @@ async fn test_oidc_expired_token_is_rejected() {
438438
.await;
439439
}
440440

441+
async fn setup_oidc_test_with_prefix(
442+
provider_mutator: impl FnOnce(&mut ProviderState),
443+
site_prefix: &str,
444+
) -> (
445+
impl actix_web::dev::Service<
446+
actix_http::Request,
447+
Response = actix_web::dev::ServiceResponse<impl actix_web::body::MessageBody>,
448+
Error = actix_web::Error,
449+
>,
450+
FakeOidcProvider,
451+
) {
452+
use sqlpage::{
453+
app_config::{test_database_url, AppConfig},
454+
AppState,
455+
};
456+
crate::common::init_log();
457+
let provider = FakeOidcProvider::new().await;
458+
provider.with_state_mut(provider_mutator);
459+
460+
let db_url = test_database_url();
461+
let config_json = format!(
462+
r#"{{
463+
"database_url": "{db_url}",
464+
"oidc_issuer_url": "{}",
465+
"oidc_client_id": "{}",
466+
"oidc_client_secret": "{}",
467+
"oidc_protected_paths": ["/"],
468+
"site_prefix": "{site_prefix}"
469+
}}"#,
470+
provider.issuer_url, provider.client_id, provider.client_secret
471+
);
472+
473+
let config: AppConfig = serde_json::from_str(&config_json).unwrap();
474+
let app_state = AppState::init(&config).await.unwrap();
475+
let app = test::init_service(create_app(Data::new(app_state))).await;
476+
(app, provider)
477+
}
478+
479+
#[actix_web::test]
480+
async fn test_oidc_with_site_prefix() {
481+
let (app, _provider) = setup_oidc_test_with_prefix(|_| {}, "/my-app/").await;
482+
let mut cookies: Vec<Cookie<'static>> = Vec::new();
483+
484+
// Access the app with the prefix
485+
let resp = request_with_cookies!(app, test::TestRequest::get().uri("/my-app/"), cookies);
486+
assert_eq!(resp.status(), StatusCode::SEE_OTHER);
487+
let auth_url = Url::parse(resp.headers().get("location").unwrap().to_str().unwrap()).unwrap();
488+
489+
// Check if the redirect_uri parameter in the auth URL contains the site prefix
490+
let redirect_uri = get_query_param(&auth_url, "redirect_uri");
491+
assert!(
492+
redirect_uri.contains("/my-app/sqlpage/oidc_callback"),
493+
"Redirect URI should contain site prefix. Got: {}",
494+
redirect_uri
495+
);
496+
}
497+
441498
#[actix_web::test]
442499
async fn test_oidc_logout_uses_correct_scheme() {
443500
use sqlpage::{
444501
app_config::{test_database_url, AppConfig},
445-
webserver::oidc::create_logout_url,
446502
AppState,
447503
};
448504

@@ -463,9 +519,13 @@ async fn test_oidc_logout_uses_correct_scheme() {
463519

464520
let config: AppConfig = serde_json::from_str(&config_json).unwrap();
465521
let app_state = AppState::init(&config).await.unwrap();
522+
let logout_path = app_state
523+
.oidc_state
524+
.as_ref()
525+
.unwrap()
526+
.config
527+
.create_logout_url("/logged_out");
466528
let app = test::init_service(create_app(Data::new(app_state))).await;
467-
468-
let logout_path = create_logout_url("/logged_out", "", &provider.client_secret);
469529
// make sure the logout path includes the configured domain
470530
assert!(logout_path.starts_with("/sqlpage/oidc_logout"));
471531

0 commit comments

Comments
 (0)