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`.

Author: cursoragent

src/webserver/oidc.rs

@@ -79,6 +79,7 @@ pub struct OidcConfig {
     pub app_host: String,
     pub scopes: Vec<Scope>,
     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}");

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<impl actix_web::body::MessageBody>,
+        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<Cookie<'static>> = 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);
+}