From 95c37c7007bd9801b229b1273910bccb2d84ade1 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 22 Apr 2025 01:24:27 +0200 Subject: [PATCH 01/30] add oidc config variables --- configuration.md | 41 +++++++++++++++++++++++++++++++++++++++++ src/app_config.rs | 23 +++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/configuration.md b/configuration.md index 344048d6..bb5884a6 100644 --- a/configuration.md +++ b/configuration.md @@ -24,6 +24,10 @@ Here are the available configuration options and their default values: | `configuration_directory` | `./sqlpage/` | The directory where the `sqlpage.json` file is located. This is used to find the path to [`templates/`](https://sql-page.com/custom_components.sql), [`migrations/`](https://sql-page.com/your-first-sql-website/migrations.sql), and `on_connect.sql`. Obviously, this configuration parameter can be set only through environment variables, not through the `sqlpage.json` file itself in order to find the `sqlpage.json` file. Be careful not to use a path that is accessible from the public WEB_ROOT | | `allow_exec` | false | Allow usage of the `sqlpage.exec` function. Do this only if all users with write access to sqlpage query files and to the optional `sqlpage_files` table on the database are trusted. | | `max_uploaded_file_size` | 5242880 | Maximum size of forms and uploaded files in bytes. Defaults to 5 MiB. | +| `oidc_issuer_url` | | The base URL of the [OpenID Connect provider](#openid-connect-oidc-authentication). Required for enabling Single Sign-On. | +| `oidc_client_id` | sqlpage | The ID that identifies your SQLPage application to the OIDC provider. You get this when registering your app with the provider. | +| `oidc_client_secret` | | The secret key for your SQLPage application. Keep this confidential as it allows your app to authenticate with the OIDC provider. | +| `oidc_scopes` | openid email profile | Space-separated list of [scopes](https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims) your app requests from the OIDC provider. | | `max_pending_rows` | 256 | Maximum number of rendered rows that can be queued up in memory when a client is slow to receive them. | | `compress_responses` | true | When the client supports it, compress the http response body. This can save bandwidth and speed up page loading on slow connections, but can also increase CPU usage and cause rendering delays on pages that take time to render (because streaming responses are buffered for longer than necessary). | | `https_domain` | | Domain name to request a certificate for. Setting this parameter will automatically make SQLPage listen on port 443 and request an SSL certificate. The server will take a little bit longer to start the first time it has to request a certificate. | @@ -83,6 +87,43 @@ If the `database_password` configuration parameter is set, it will override any It does not need to be percent-encoded. This allows you to keep the password separate from the connection string, which can be useful for security purposes, especially when storing configurations in version control systems. +### OpenID Connect (OIDC) Authentication + +OpenID Connect (OIDC) is a secure way to let users log in to your SQLPage application using their existing accounts from popular services. When OIDC is configured, all access to your SQLPage application will require users to log in through the chosen provider. This enables Single Sign-On (SSO), allowing you to restrict access to your application without having to handle authentication yourself. + +To set up OIDC, you'll need to: +1. Register your application with an OIDC provider +2. Configure the provider's details in SQLPage + +#### Cloud Identity Providers + +- **Google** + - Documentation: https://developers.google.com/identity/openid-connect/openid-connect + - Set *oidc_issuer_url* to `https://accounts.google.com` + +- **Microsoft Entra ID** (formerly Azure AD) + - Documentation: https://learn.microsoft.com/en-us/entra/identity-platform/quickstart-register-app + - Set *oidc_issuer_url* to `https://login.microsoftonline.com/{tenant}/v2.0` + - ([Find your tenant name](https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc#find-your-apps-openid-configuration-document-uri)) + +- **GitHub** + - Issuer URL: `https://github.com` + - Documentation: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps + +#### Self-Hosted Solutions + +- **Keycloak** + - Issuer URL: `https://your-keycloak-server/auth/realms/your-realm` + - [Setup Guide](https://www.keycloak.org/getting-started/getting-started-docker) + +- **Authentik** + - Issuer URL: `https://your-authentik-server/application/o/your-application` + - [Setup Guide](https://goauthentik.io/docs/providers/oauth2) + +After registering your application with the provider, you'll receive a client ID and client secret. These are used to configure SQLPage to work with your chosen provider. + +Note: OIDC is optional. If you don't configure it, your SQLPage application will be accessible without authentication. + ### Example `.env` file ```bash diff --git a/src/app_config.rs b/src/app_config.rs index 12bc1463..a2d4a20c 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -198,6 +198,21 @@ pub struct AppConfig { #[serde(default = "default_max_file_size")] pub max_uploaded_file_size: usize, + /// The base URL of the `OpenID` Connect provider. + /// Required when enabling Single Sign-On through an OIDC provider. + pub oidc_issuer_url: Option, + /// The client ID assigned to `SQLPage` when registering with the OIDC provider. + /// Defaults to `sqlpage`. + #[serde(default = "default_oidc_client_id")] + pub oidc_client_id: String, + /// The client secret for authenticating `SQLPage` to the OIDC provider. + /// Required when enabling Single Sign-On through an OIDC provider. + pub oidc_client_secret: Option, + /// Space-separated list of [scopes](https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims) to request during OIDC authentication. + /// Defaults to "openid email profile" + #[serde(default = "default_oidc_scopes")] + pub oidc_scopes: String, + /// A domain name to use for the HTTPS server. If this is set, the server will perform all the necessary /// steps to set up an HTTPS server automatically. All you need to do is point your domain name to the /// server's IP address. @@ -528,6 +543,14 @@ fn default_markdown_allow_dangerous_protocol() -> bool { false } +fn default_oidc_client_id() -> String { + "sqlpage".to_string() +} + +fn default_oidc_scopes() -> String { + "openid email profile".to_string() +} + #[derive(Debug, Deserialize, Serialize, PartialEq, Clone, Copy, Eq, Default)] #[serde(rename_all = "lowercase")] pub enum DevOrProd { From 6f585873b0eec2878d5d0bd939703579b794a8f3 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Wed, 23 Apr 2025 00:42:50 +0200 Subject: [PATCH 02/30] setup a basic middleware --- src/webserver/http.rs | 2 + src/webserver/mod.rs | 1 + src/webserver/oidc.rs | 117 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 src/webserver/oidc.rs diff --git a/src/webserver/http.rs b/src/webserver/http.rs index fc1ecea9..fc2bfdbf 100644 --- a/src/webserver/http.rs +++ b/src/webserver/http.rs @@ -20,6 +20,7 @@ use actix_web::{ use actix_web::{HttpResponseBuilder, ResponseError}; use super::https::make_auto_rustls_config; +use super::oidc::OidcMiddleware; use super::response_writer::ResponseWriter; use super::static_content; use crate::webserver::routing::RoutingAction::{ @@ -466,6 +467,7 @@ pub fn create_app( ) // when receiving a request outside of the prefix, redirect to the prefix .default_service(fn_service(default_prefix_redirect)) + .wrap(OidcMiddleware::new(&app_state.config)) .wrap(Logger::default()) .wrap(default_headers(&app_state)) .wrap(middleware::Condition::new( diff --git a/src/webserver/mod.rs b/src/webserver/mod.rs index 1393d9e6..41cf28d5 100644 --- a/src/webserver/mod.rs +++ b/src/webserver/mod.rs @@ -45,3 +45,4 @@ pub use database::migrations::apply; pub mod response_writer; pub mod routing; mod static_content; +mod oidc; \ No newline at end of file diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs new file mode 100644 index 00000000..ec132216 --- /dev/null +++ b/src/webserver/oidc.rs @@ -0,0 +1,117 @@ +use std::{ + future::{ready, Future, Ready}, + pin::Pin, + sync::Arc, +}; + +use crate::app_config::AppConfig; +use actix_web::{ + dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, + middleware::Condition, + Error, +}; + +#[derive(Clone, Debug)] +pub struct OidcConfig { + pub issuer_url: String, + pub client_id: String, + pub client_secret: String, + pub scopes: String, +} + +impl TryFrom<&AppConfig> for OidcConfig { + type Error = Option<&'static str>; + + fn try_from(config: &AppConfig) -> Result { + let issuer_url = config.oidc_issuer_url.as_ref().ok_or(None)?; + let client_secret = config + .oidc_client_secret + .as_ref() + .ok_or(Some("Missing oidc_client_secret"))?; + + Ok(Self { + issuer_url: issuer_url.clone(), + client_id: config.oidc_client_id.clone(), + client_secret: client_secret.clone(), + scopes: config.oidc_scopes.clone(), + }) + } +} + +pub struct OidcMiddleware { + pub config: Option>, +} + +impl OidcMiddleware { + pub fn new(config: &AppConfig) -> Condition { + let config = OidcConfig::try_from(config); + match &config { + Ok(config) => { + log::info!("Setting up OIDC with config: {config:?}"); + } + Err(Some(err)) => { + log::error!("Invalid OIDC configuration: {err}"); + } + Err(None) => { + log::debug!("No OIDC configuration provided, skipping middleware."); + } + } + let config = config.ok().map(Arc::new); + Condition::new(config.is_some(), Self { config }) + } +} + +impl Transform for OidcMiddleware +where + S: Service, Error = Error>, + S::Future: 'static, + B: 'static, +{ + type Response = ServiceResponse; + type Error = Error; + type InitError = (); + type Transform = OidcService; + type Future = Ready>; + + fn new_transform(&self, service: S) -> Self::Future { + ready( + self.config + .as_ref() + .map(|config| OidcService { + service, + config: Arc::clone(config), + }) + .ok_or(()), + ) + } +} + +pub struct OidcService { + service: S, + config: Arc, +} + +type LocalBoxFuture = Pin + 'static>>; + +impl Service for OidcService +where + S: Service, Error = Error>, + S::Future: 'static, + B: 'static, +{ + type Response = ServiceResponse; + type Error = Error; + type Future = LocalBoxFuture>; + + forward_ready!(service); + + fn call(&self, request: ServiceRequest) -> Self::Future { + log::info!("OIDC config: {:?}", self.config); + let future = self.service.call(request); + + Box::pin(async move { + let response = future.await?; + Ok(response) + }) + } +} From c21f89a8df8c9952dd2b9e90466c0227780f0b03 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 24 Apr 2025 00:21:19 +0200 Subject: [PATCH 03/30] implement an async http client that uses oidc --- Cargo.lock | 345 +++++++++++++++++++++++++++++++++++++++++- Cargo.toml | 1 + src/webserver/mod.rs | 2 +- src/webserver/oidc.rs | 53 +++++++ 4 files changed, 396 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02a71e4b..82d08296 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -664,6 +664,12 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "base16ct" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c7f02d4ea65f2c1853089ffd8d2787bdbc63de2f0d29dedbcf8ccdfa0ccd4cf" + [[package]] name = "base64" version = "0.13.1" @@ -824,6 +830,7 @@ dependencies = [ "iana-time-zone", "js-sys", "num-traits", + "serde", "wasm-bindgen", "windows-link", ] @@ -1033,6 +1040,18 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "43da5946c66ffcc7745f48db692ffbb10a83bfe0afd96235c5c2a4fb23994929" +[[package]] +name = "crypto-bigint" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0dc92fb57ca44df6db8059111ab3af99a63d5d0f8375d9972e319a379c6bab76" +dependencies = [ + "generic-array", + "rand_core 0.6.4", + "subtle", + "zeroize", +] + [[package]] name = "crypto-common" version = "0.1.6" @@ -1068,6 +1087,33 @@ dependencies = [ "memchr", ] +[[package]] +name = "curve25519-dalek" +version = "4.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97fb8b7c4503de7d6ae7b42ab72a5a59857b4c937ec27a3d4539dba95b5ab2be" +dependencies = [ + "cfg-if", + "cpufeatures", + "curve25519-dalek-derive", + "digest", + "fiat-crypto", + "rustc_version", + "subtle", + "zeroize", +] + +[[package]] +name = "curve25519-dalek-derive" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f46882e17999c6cc590af592290432be3bce0428cb0d5f8b6715e4dc7b383eb3" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.100", +] + [[package]] name = "darling" version = "0.20.11" @@ -1147,6 +1193,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c9e6a11ca8224451684bc0d7d5a7adbf8f2fd6887261a1cfc3c0432f9d4068e" dependencies = [ "powerfmt", + "serde", ] [[package]] @@ -1273,12 +1320,77 @@ version = "0.15.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1aaf95b3e5c8f23aa320147307562d361db0ae0d51242340f558153b4eb2439b" +[[package]] +name = "dyn-clone" +version = "1.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c7a8fb8a9fbf66c1f703fe16184d10ca0ee9d23be5b4436400408ba54a95005" + +[[package]] +name = "ecdsa" +version = "0.16.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee27f32b5c5292967d2d4a9d7f1e0b0aed2c15daded5a60300e4abb9d8020bca" +dependencies = [ + "der", + "digest", + "elliptic-curve", + "rfc6979", + "signature", + "spki", +] + +[[package]] +name = "ed25519" +version = "2.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "115531babc129696a58c64a4fef0a8bf9e9698629fb97e9e40767d235cfbcd53" +dependencies = [ + "pkcs8", + "signature", +] + +[[package]] +name = "ed25519-dalek" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4a3daa8e81a3963a60642bcc1f90a670680bd4a77535faa384e9d1c79d620871" +dependencies = [ + "curve25519-dalek", + "ed25519", + "serde", + "sha2", + "subtle", + "zeroize", +] + [[package]] name = "either" version = "1.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719" +[[package]] +name = "elliptic-curve" +version = "0.13.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5e6043086bf7973472e0c7dff2142ea0b680d30e18d9cc40f267efbf222bd47" +dependencies = [ + "base16ct", + "crypto-bigint", + "digest", + "ff", + "generic-array", + "group", + "hkdf", + "pem-rfc7468", + "pkcs8", + "rand_core 0.6.4", + "sec1", + "subtle", + "zeroize", +] + [[package]] name = "encoding_rs" version = "0.8.35" @@ -1365,6 +1477,22 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "ff" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0b50bfb653653f9ca9095b427bed08ab8d75a137839d9ad64eb11810d5b6393" +dependencies = [ + "rand_core 0.6.4", + "subtle", +] + +[[package]] +name = "fiat-crypto" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28dea519a9695b9977216879a3ebfddf92f1c08c05d984f8996aecd6ecdc811d" + [[package]] name = "flate2" version = "1.1.1" @@ -1539,6 +1667,7 @@ checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" dependencies = [ "typenum", "version_check", + "zeroize", ] [[package]] @@ -1548,8 +1677,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" dependencies = [ "cfg-if", + "js-sys", "libc", "wasi 0.11.0+wasi-snapshot-preview1", + "wasm-bindgen", ] [[package]] @@ -1595,6 +1726,17 @@ dependencies = [ "web-sys", ] +[[package]] +name = "group" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0f9ef7462f7c099f518d754361858f86d8a07af53ba9af0fe635bbccb151a63" +dependencies = [ + "ff", + "rand_core 0.6.4", + "subtle", +] + [[package]] name = "h2" version = "0.3.26" @@ -1607,7 +1749,7 @@ dependencies = [ "futures-sink", "futures-util", "http 0.2.12", - "indexmap", + "indexmap 2.9.0", "slab", "tokio", "tokio-util", @@ -1630,6 +1772,12 @@ dependencies = [ "thiserror 2.0.12", ] +[[package]] +name = "hashbrown" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" + [[package]] name = "hashbrown" version = "0.14.5" @@ -1965,6 +2113,17 @@ dependencies = [ "quote", ] +[[package]] +name = "indexmap" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" +dependencies = [ + "autocfg", + "hashbrown 0.12.3", + "serde", +] + [[package]] name = "indexmap" version = "2.9.0" @@ -1973,6 +2132,7 @@ checksum = "cea70ddb795996207ad57735b50c5982d8844f38ba9ee5f1aedcfb708a2aa11e" dependencies = [ "equivalent", "hashbrown 0.15.2", + "serde", ] [[package]] @@ -1981,6 +2141,15 @@ version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7943c866cc5cd64cbc25b2e01621d07fa8eb2a1a23160ee81ce38704e97b8ecf" +[[package]] +name = "itertools" +version = "0.10.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.15" @@ -2371,6 +2540,25 @@ dependencies = [ "libm", ] +[[package]] +name = "oauth2" +version = "5.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51e219e79014df21a225b1860a479e2dcd7cbd9130f4defd4bd0e191ea31d67d" +dependencies = [ + "base64 0.22.1", + "chrono", + "getrandom 0.2.15", + "http 1.3.1", + "rand 0.8.5", + "serde", + "serde_json", + "serde_path_to_error", + "sha2", + "thiserror 1.0.69", + "url", +] + [[package]] name = "object" version = "0.36.7" @@ -2395,6 +2583,37 @@ version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" +[[package]] +name = "openidconnect" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dd50d4a5e7730e754f94d977efe61f611aadd3131f6a2b464f6e3a4167e8ef7" +dependencies = [ + "base64 0.21.7", + "chrono", + "dyn-clone", + "ed25519-dalek", + "hmac", + "http 1.3.1", + "itertools", + "log", + "oauth2", + "p256", + "p384", + "rand 0.8.5", + "rsa", + "serde", + "serde-value", + "serde_json", + "serde_path_to_error", + "serde_plain", + "serde_with", + "sha2", + "subtle", + "thiserror 1.0.69", + "url", +] + [[package]] name = "openssl-probe" version = "0.1.6" @@ -2407,6 +2626,15 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" +[[package]] +name = "ordered-float" +version = "2.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68f19d67e5a2795c94e73e0bb1cc1a7edeb2e28efd39e2e1c9b7a40c1108b11c" +dependencies = [ + "num-traits", +] + [[package]] name = "ordered-multimap" version = "0.7.3" @@ -2417,6 +2645,30 @@ dependencies = [ "hashbrown 0.14.5", ] +[[package]] +name = "p256" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c9863ad85fa8f4460f9c48cb909d38a0d689dba1f6f6988a5e3e0d31071bcd4b" +dependencies = [ + "ecdsa", + "elliptic-curve", + "primeorder", + "sha2", +] + +[[package]] +name = "p384" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe42f1670a52a47d448f14b6a5c61dd78fce51856e68edaa38f7ae3a46b8d6b6" +dependencies = [ + "ecdsa", + "elliptic-curve", + "primeorder", + "sha2", +] + [[package]] name = "parking" version = "2.2.1" @@ -2660,6 +2912,15 @@ dependencies = [ "zerocopy 0.8.24", ] +[[package]] +name = "primeorder" +version = "0.13.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "353e1ca18966c16d9deb1c69278edbc5f194139612772bd9537af60ac231e1e6" +dependencies = [ + "elliptic-curve", +] + [[package]] name = "proc-macro2" version = "1.0.95" @@ -2810,6 +3071,16 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" +[[package]] +name = "rfc6979" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dd2a808d456c4a54e300a23e9f5a67e122c3024119acbfd73e3bf664491cb2" +dependencies = [ + "hmac", + "subtle", +] + [[package]] name = "ring" version = "0.17.14" @@ -3029,6 +3300,20 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "sec1" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3e97a565f76233a6003f9f5c54be1d9c5bdfa3eccfb189469f11ec4901c47dc" +dependencies = [ + "base16ct", + "der", + "generic-array", + "pkcs8", + "subtle", + "zeroize", +] + [[package]] name = "security-framework" version = "2.11.1" @@ -3067,6 +3352,16 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-value" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3a1a3341211875ef120e117ea7fd5228530ae7e7036a779fdc9117be6b3282c" +dependencies = [ + "ordered-float", + "serde", +] + [[package]] name = "serde_derive" version = "1.0.219" @@ -3084,13 +3379,23 @@ version = "1.0.140" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "20068b6e96dc6c9bd23e01df8827e6c7e1f2fddd43c21810382803c136b99373" dependencies = [ - "indexmap", + "indexmap 2.9.0", "itoa", "memchr", "ryu", "serde", ] +[[package]] +name = "serde_path_to_error" +version = "0.1.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59fab13f937fa393d08645bf3a84bdfe86e296747b506ada67bb15f10f218b2a" +dependencies = [ + "itoa", + "serde", +] + [[package]] name = "serde_plain" version = "1.0.2" @@ -3121,6 +3426,36 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_with" +version = "3.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6b6f7f2fcb69f747921f79f3926bd1e203fce4fef62c268dd3abfb6d86029aa" +dependencies = [ + "base64 0.22.1", + "chrono", + "hex", + "indexmap 1.9.3", + "indexmap 2.9.0", + "serde", + "serde_derive", + "serde_json", + "serde_with_macros", + "time", +] + +[[package]] +name = "serde_with_macros" +version = "3.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d00caa5193a3c8362ac2b73be6b9e768aa5a4b2f721d8f4b339600c3cb51f8e" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn 2.0.100", +] + [[package]] name = "sha1" version = "0.10.6" @@ -3251,6 +3586,7 @@ dependencies = [ "log", "markdown", "mime_guess", + "openidconnect", "password-hash", "percent-encoding", "rand 0.9.1", @@ -3318,7 +3654,7 @@ dependencies = [ "hex", "hkdf", "hmac", - "indexmap", + "indexmap 2.9.0", "itoa", "libc", "libsqlite3-sys", @@ -3676,7 +4012,7 @@ version = "0.22.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "17b4795ff5edd201c7cd6dca065ae59972ce77d1b80fa0a84d94950ece7d1474" dependencies = [ - "indexmap", + "indexmap 2.9.0", "serde", "serde_spanned", "toml_datetime", @@ -3838,6 +4174,7 @@ dependencies = [ "form_urlencoded", "idna", "percent-encoding", + "serde", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index c191791d..dc96f239 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,6 +63,7 @@ rustls-native-certs = "0.7.0" awc = { version = "3", features = ["rustls-0_22-webpki-roots"] } clap = { version = "4.5.17", features = ["derive"] } tokio-util = "0.7.12" +openidconnect = { version = "4.0.0", default-features = false } [build-dependencies] awc = { version = "3", features = ["rustls-0_22-webpki-roots"] } diff --git a/src/webserver/mod.rs b/src/webserver/mod.rs index 41cf28d5..42cf69d2 100644 --- a/src/webserver/mod.rs +++ b/src/webserver/mod.rs @@ -42,7 +42,7 @@ pub use error_with_status::ErrorWithStatus; pub use database::make_placeholder; pub use database::migrations::apply; +mod oidc; pub mod response_writer; pub mod routing; mod static_content; -mod oidc; \ No newline at end of file diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index ec132216..61d77934 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,6 +1,7 @@ use std::{ future::{ready, Future, Ready}, pin::Pin, + str::FromStr, sync::Arc, }; @@ -10,6 +11,9 @@ use actix_web::{ middleware::Condition, Error, }; +use awc::Client; +use openidconnect::AsyncHttpClient; +use std::error::Error as StdError; #[derive(Clone, Debug)] pub struct OidcConfig { @@ -115,3 +119,52 @@ where }) } } + +pub struct AwcHttpClient { + client: Client, +} + +impl AwcHttpClient { + pub fn new() -> Self { + Self { + client: Client::default(), + } + } +} + +impl<'c> AsyncHttpClient<'c> for AwcHttpClient { + type Error = awc::error::SendRequestError; + type Future = Pin< + Box>, Self::Error>> + 'c>, + >; + + fn call(&'c self, request: openidconnect::http::Request>) -> Self::Future { + let client = self.client.clone(); + Box::pin(async move { + let awc_method = awc::http::Method::from_bytes(request.method().as_str().as_bytes()) + .map_err(to_awc_error)?; + let awc_uri = + awc::http::Uri::from_str(&request.uri().to_string()).map_err(to_awc_error)?; + let mut req = client.request(awc_method, awc_uri); + for (name, value) in request.headers() { + req = req.insert_header((name.as_str(), value.to_str().map_err(to_awc_error)?)); + } + let mut response = req.send_body(request.into_body()).await?; + let head = response.headers(); + let mut resp_builder = + openidconnect::http::Response::builder().status(response.status().as_u16()); + for (name, value) in head { + resp_builder = + resp_builder.header(name.as_str(), value.to_str().map_err(to_awc_error)?); + } + let body = response.body().await.map_err(to_awc_error)?.to_vec(); + let resp = resp_builder.body(body).map_err(to_awc_error)?; + Ok(resp) + }) + } +} + +fn to_awc_error(err: T) -> awc::error::SendRequestError { + let err_str = err.to_string(); + awc::error::SendRequestError::Custom(Box::new(err), Box::new(err_str)) +} From 661c6d68b6d1712fd08597c90051c37a68a36bc7 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 24 Apr 2025 01:03:41 +0200 Subject: [PATCH 04/30] initialize provider_metadata in OidcService --- src/webserver/oidc.rs | 115 ++++++++++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 37 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 61d77934..eee5d6fc 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,5 +1,5 @@ use std::{ - future::{ready, Future, Ready}, + future::Future, pin::Pin, str::FromStr, sync::Arc, @@ -11,9 +11,9 @@ use actix_web::{ middleware::Condition, Error, }; +use anyhow::anyhow; use awc::Client; -use openidconnect::AsyncHttpClient; -use std::error::Error as StdError; +use openidconnect::{AsyncHttpClient, IssuerUrl}; #[derive(Clone, Debug)] pub struct OidcConfig { @@ -65,9 +65,19 @@ impl OidcMiddleware { } } +async fn discover_provider_metadata( + issuer_url: String, +) -> anyhow::Result { + let http_client = AwcHttpClient::new(); + let issuer_url = IssuerUrl::new(issuer_url)?; + let provider_metadata = + openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, &http_client).await?; + Ok(provider_metadata) +} + impl Transform for OidcMiddleware where - S: Service, Error = Error>, + S: Service, Error = Error> + 'static, S::Future: 'static, B: 'static, { @@ -75,24 +85,40 @@ where type Error = Error; type InitError = (); type Transform = OidcService; - type Future = Ready>; + type Future = Pin> + 'static>>; fn new_transform(&self, service: S) -> Self::Future { - ready( - self.config - .as_ref() - .map(|config| OidcService { - service, - config: Arc::clone(config), - }) - .ok_or(()), - ) + let config = self.config.clone(); + Box::pin(async move { + match config { + Some(config) => Ok(OidcService::new(service, Arc::clone(&config)) + .await + .map_err(|err| { + log::error!( + "Error creating OIDC service with issuer: {}: {err:?}", + config.issuer_url + ); + })?), + None => Err(()), + } + }) } } pub struct OidcService { service: S, config: Arc, + provider_metadata: openidconnect::core::CoreProviderMetadata, +} + +impl OidcService { + pub async fn new(service: S, config: Arc) -> anyhow::Result { + Ok(Self { + service, + config: Arc::clone(&config), + provider_metadata: discover_provider_metadata(config.issuer_url.clone()).await?, + }) + } } type LocalBoxFuture = Pin + 'static>>; @@ -133,7 +159,7 @@ impl AwcHttpClient { } impl<'c> AsyncHttpClient<'c> for AwcHttpClient { - type Error = awc::error::SendRequestError; + type Error = StringError; type Future = Pin< Box>, Self::Error>> + 'c>, >; @@ -141,30 +167,45 @@ impl<'c> AsyncHttpClient<'c> for AwcHttpClient { fn call(&'c self, request: openidconnect::http::Request>) -> Self::Future { let client = self.client.clone(); Box::pin(async move { - let awc_method = awc::http::Method::from_bytes(request.method().as_str().as_bytes()) - .map_err(to_awc_error)?; - let awc_uri = - awc::http::Uri::from_str(&request.uri().to_string()).map_err(to_awc_error)?; - let mut req = client.request(awc_method, awc_uri); - for (name, value) in request.headers() { - req = req.insert_header((name.as_str(), value.to_str().map_err(to_awc_error)?)); - } - let mut response = req.send_body(request.into_body()).await?; - let head = response.headers(); - let mut resp_builder = - openidconnect::http::Response::builder().status(response.status().as_u16()); - for (name, value) in head { - resp_builder = - resp_builder.header(name.as_str(), value.to_str().map_err(to_awc_error)?); - } - let body = response.body().await.map_err(to_awc_error)?.to_vec(); - let resp = resp_builder.body(body).map_err(to_awc_error)?; - Ok(resp) + execute_oidc_request_with_awc(client, request) + .await + .map_err(|err| StringError(format!("Failed to execute OIDC request: {err:?}"))) }) } } -fn to_awc_error(err: T) -> awc::error::SendRequestError { - let err_str = err.to_string(); - awc::error::SendRequestError::Custom(Box::new(err), Box::new(err_str)) +async fn execute_oidc_request_with_awc( + client: Client, + request: openidconnect::http::Request>, +) -> Result>, anyhow::Error> { + let awc_method = awc::http::Method::from_bytes(request.method().as_str().as_bytes())?; + let awc_uri = awc::http::Uri::from_str(&request.uri().to_string())?; + let mut req = client.request(awc_method, awc_uri); + for (name, value) in request.headers() { + req = req.insert_header((name.as_str(), value.to_str()?)); + } + let mut response = req + .send_body(request.into_body()) + .await + .map_err(|e| anyhow!("{:?}", e))?; + let head = response.headers(); + let mut resp_builder = + openidconnect::http::Response::builder().status(response.status().as_u16()); + for (name, value) in head { + resp_builder = resp_builder.header(name.as_str(), value.to_str()?); + } + let body = response.body().await?.to_vec(); + let resp = resp_builder.body(body)?; + Ok(resp) +} + +#[derive(Debug, PartialEq, Eq)] +pub struct StringError(String); + +impl std::fmt::Display for StringError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(&self.0, f) + } } + +impl std::error::Error for StringError {} From 5e54aab983d18cdf3437952f8d2f61222dd1d896 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 24 Apr 2025 01:22:13 +0200 Subject: [PATCH 05/30] better error handling in oidc config --- src/app_config.rs | 3 ++- src/webserver/oidc.rs | 23 ++++++++++++----------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/app_config.rs b/src/app_config.rs index a2d4a20c..cf215520 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -2,6 +2,7 @@ use crate::webserver::routing::RoutingConfig; use anyhow::Context; use clap::Parser; use config::Config; +use openidconnect::IssuerUrl; use percent_encoding::AsciiSet; use serde::de::Error; use serde::{Deserialize, Deserializer, Serialize}; @@ -200,7 +201,7 @@ pub struct AppConfig { /// The base URL of the `OpenID` Connect provider. /// Required when enabling Single Sign-On through an OIDC provider. - pub oidc_issuer_url: Option, + pub oidc_issuer_url: Option, /// The client ID assigned to `SQLPage` when registering with the OIDC provider. /// Defaults to `sqlpage`. #[serde(default = "default_oidc_client_id")] diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index eee5d6fc..ea06ba79 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,9 +1,4 @@ -use std::{ - future::Future, - pin::Pin, - str::FromStr, - sync::Arc, -}; +use std::{future::Future, pin::Pin, str::FromStr, sync::Arc}; use crate::app_config::AppConfig; use actix_web::{ @@ -17,7 +12,7 @@ use openidconnect::{AsyncHttpClient, IssuerUrl}; #[derive(Clone, Debug)] pub struct OidcConfig { - pub issuer_url: String, + pub issuer_url: IssuerUrl, pub client_id: String, pub client_secret: String, pub scopes: String, @@ -51,7 +46,8 @@ impl OidcMiddleware { let config = OidcConfig::try_from(config); match &config { Ok(config) => { - log::info!("Setting up OIDC with config: {config:?}"); + log::debug!("Setting up OIDC with issuer: {}", config.issuer_url); + // contains secrets } Err(Some(err)) => { log::error!("Invalid OIDC configuration: {err}"); @@ -66,10 +62,9 @@ impl OidcMiddleware { } async fn discover_provider_metadata( - issuer_url: String, + issuer_url: IssuerUrl, ) -> anyhow::Result { let http_client = AwcHttpClient::new(); - let issuer_url = IssuerUrl::new(issuer_url)?; let provider_metadata = openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, &http_client).await?; Ok(provider_metadata) @@ -136,7 +131,7 @@ where forward_ready!(service); fn call(&self, request: ServiceRequest) -> Self::Future { - log::info!("OIDC config: {:?}", self.config); + log::debug!("Started OIDC middleware with config: {:?}", self.config); let future = self.service.call(request); Box::pin(async move { @@ -180,6 +175,7 @@ async fn execute_oidc_request_with_awc( ) -> Result>, anyhow::Error> { let awc_method = awc::http::Method::from_bytes(request.method().as_str().as_bytes())?; let awc_uri = awc::http::Uri::from_str(&request.uri().to_string())?; + log::debug!("Executing OIDC request: {} {}", awc_method, awc_uri); let mut req = client.request(awc_method, awc_uri); for (name, value) in request.headers() { req = req.insert_header((name.as_str(), value.to_str()?)); @@ -195,6 +191,11 @@ async fn execute_oidc_request_with_awc( resp_builder = resp_builder.header(name.as_str(), value.to_str()?); } let body = response.body().await?.to_vec(); + log::debug!( + "Received OIDC response with status {}: {}", + response.status(), + String::from_utf8_lossy(&body) + ); let resp = resp_builder.body(body)?; Ok(resp) } From 845b39b4aa613e3a0b5e511f6dd53b68d85448fa Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 24 Apr 2025 01:43:08 +0200 Subject: [PATCH 06/30] HTTP client initialization in oidc now follows global config --- .../database/sqlpage_functions/functions.rs | 46 +----------------- src/webserver/http.rs | 2 +- src/webserver/http_client.rs | 45 +++++++++++++++++ src/webserver/mod.rs | 1 + src/webserver/oidc.rs | 48 +++++++++++++------ 5 files changed, 82 insertions(+), 60 deletions(-) create mode 100644 src/webserver/http_client.rs diff --git a/src/webserver/database/sqlpage_functions/functions.rs b/src/webserver/database/sqlpage_functions/functions.rs index 7142fee7..15eef93f 100644 --- a/src/webserver/database/sqlpage_functions/functions.rs +++ b/src/webserver/database/sqlpage_functions/functions.rs @@ -4,13 +4,14 @@ use crate::webserver::{ execute_queries::DbConn, sqlpage_functions::url_parameter_deserializer::URLParameters, }, http::SingleOrVec, + http_client::make_http_client, request_variables::ParamMap, ErrorWithStatus, }; use anyhow::{anyhow, Context}; use futures_util::StreamExt; use mime_guess::mime; -use std::{borrow::Cow, ffi::OsStr, str::FromStr, sync::OnceLock}; +use std::{borrow::Cow, ffi::OsStr, str::FromStr}; super::function_definition_macro::sqlpage_functions! { basic_auth_password((&RequestInfo)); @@ -312,49 +313,6 @@ async fn fetch_with_meta( Ok(return_value) } -static NATIVE_CERTS: OnceLock> = OnceLock::new(); - -fn make_http_client(config: &crate::app_config::AppConfig) -> anyhow::Result { - let connector = if config.system_root_ca_certificates { - let roots = NATIVE_CERTS - .get_or_init(|| { - log::debug!("Loading native certificates because system_root_ca_certificates is enabled"); - let certs = rustls_native_certs::load_native_certs() - .with_context(|| "Initial native certificates load failed")?; - log::info!("Loaded {} native certificates", certs.len()); - let mut roots = rustls::RootCertStore::empty(); - for cert in certs { - log::trace!("Adding native certificate to root store: {cert:?}"); - roots.add(cert.clone()).with_context(|| { - format!("Unable to add certificate to root store: {cert:?}") - })?; - } - Ok(roots) - }) - .as_ref() - .map_err(|e| anyhow!("Unable to load native certificates, make sure the system root CA certificates are available: {e}"))?; - - log::trace!("Creating HTTP client with custom TLS connector using native certificates. SSL_CERT_FILE={:?}, SSL_CERT_DIR={:?}", - std::env::var("SSL_CERT_FILE").unwrap_or_default(), - std::env::var("SSL_CERT_DIR").unwrap_or_default()); - - let tls_conf = rustls::ClientConfig::builder() - .with_root_certificates(roots.clone()) - .with_no_client_auth(); - - awc::Connector::new().rustls_0_22(std::sync::Arc::new(tls_conf)) - } else { - log::debug!("Using the default tls connector with builtin certs because system_root_ca_certificates is disabled"); - awc::Connector::new() - }; - let client = awc::Client::builder() - .connector(connector) - .add_default_header((awc::http::header::USER_AGENT, env!("CARGO_PKG_NAME"))) - .finish(); - log::debug!("Created HTTP client"); - Ok(client) -} - pub(crate) async fn hash_password(password: Option) -> anyhow::Result> { let Some(password) = password else { return Ok(None); diff --git a/src/webserver/http.rs b/src/webserver/http.rs index fc2bfdbf..7885eea6 100644 --- a/src/webserver/http.rs +++ b/src/webserver/http.rs @@ -467,7 +467,7 @@ pub fn create_app( ) // when receiving a request outside of the prefix, redirect to the prefix .default_service(fn_service(default_prefix_redirect)) - .wrap(OidcMiddleware::new(&app_state.config)) + .wrap(OidcMiddleware::new(&app_state)) .wrap(Logger::default()) .wrap(default_headers(&app_state)) .wrap(middleware::Condition::new( diff --git a/src/webserver/http_client.rs b/src/webserver/http_client.rs new file mode 100644 index 00000000..e2cee885 --- /dev/null +++ b/src/webserver/http_client.rs @@ -0,0 +1,45 @@ +use anyhow::{anyhow, Context}; +use std::sync::OnceLock; + +static NATIVE_CERTS: OnceLock> = OnceLock::new(); + +pub fn make_http_client(config: &crate::app_config::AppConfig) -> anyhow::Result { + let connector = if config.system_root_ca_certificates { + let roots = NATIVE_CERTS + .get_or_init(|| { + log::debug!("Loading native certificates because system_root_ca_certificates is enabled"); + let certs = rustls_native_certs::load_native_certs() + .with_context(|| "Initial native certificates load failed")?; + log::info!("Loaded {} native certificates", certs.len()); + let mut roots = rustls::RootCertStore::empty(); + for cert in certs { + log::trace!("Adding native certificate to root store: {cert:?}"); + roots.add(cert.clone()).with_context(|| { + format!("Unable to add certificate to root store: {cert:?}") + })?; + } + Ok(roots) + }) + .as_ref() + .map_err(|e| anyhow!("Unable to load native certificates, make sure the system root CA certificates are available: {e}"))?; + + log::trace!("Creating HTTP client with custom TLS connector using native certificates. SSL_CERT_FILE={:?}, SSL_CERT_DIR={:?}", + std::env::var("SSL_CERT_FILE").unwrap_or_default(), + std::env::var("SSL_CERT_DIR").unwrap_or_default()); + + let tls_conf = rustls::ClientConfig::builder() + .with_root_certificates(roots.clone()) + .with_no_client_auth(); + + awc::Connector::new().rustls_0_22(std::sync::Arc::new(tls_conf)) + } else { + log::debug!("Using the default tls connector with builtin certs because system_root_ca_certificates is disabled"); + awc::Connector::new() + }; + let client = awc::Client::builder() + .connector(connector) + .add_default_header((awc::http::header::USER_AGENT, env!("CARGO_PKG_NAME"))) + .finish(); + log::debug!("Created HTTP client"); + Ok(client) +} diff --git a/src/webserver/mod.rs b/src/webserver/mod.rs index 42cf69d2..67cdf25f 100644 --- a/src/webserver/mod.rs +++ b/src/webserver/mod.rs @@ -33,6 +33,7 @@ mod content_security_policy; pub mod database; pub mod error_with_status; pub mod http; +pub mod http_client; pub mod http_request_info; mod https; pub mod request_variables; diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index ea06ba79..d75792aa 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,15 +1,17 @@ use std::{future::Future, pin::Pin, str::FromStr, sync::Arc}; -use crate::app_config::AppConfig; +use crate::{app_config::AppConfig, AppState}; use actix_web::{ dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, middleware::Condition, - Error, + web, Error, }; use anyhow::anyhow; use awc::Client; use openidconnect::{AsyncHttpClient, IssuerUrl}; +use super::http_client::make_http_client; + #[derive(Clone, Debug)] pub struct OidcConfig { pub issuer_url: IssuerUrl, @@ -39,11 +41,12 @@ impl TryFrom<&AppConfig> for OidcConfig { pub struct OidcMiddleware { pub config: Option>, + app_state: web::Data, } impl OidcMiddleware { - pub fn new(config: &AppConfig) -> Condition { - let config = OidcConfig::try_from(config); + pub fn new(app_state: &web::Data) -> Condition { + let config = OidcConfig::try_from(&app_state.config); match &config { Ok(config) => { log::debug!("Setting up OIDC with issuer: {}", config.issuer_url); @@ -57,14 +60,21 @@ impl OidcMiddleware { } } let config = config.ok().map(Arc::new); - Condition::new(config.is_some(), Self { config }) + Condition::new( + config.is_some(), + Self { + config, + app_state: web::Data::clone(app_state), + }, + ) } } async fn discover_provider_metadata( + app_config: &AppConfig, issuer_url: IssuerUrl, ) -> anyhow::Result { - let http_client = AwcHttpClient::new(); + let http_client = AwcHttpClient::new(app_config)?; let provider_metadata = openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, &http_client).await?; Ok(provider_metadata) @@ -84,9 +94,10 @@ where fn new_transform(&self, service: S) -> Self::Future { let config = self.config.clone(); + let app_state = web::Data::clone(&self.app_state); Box::pin(async move { match config { - Some(config) => Ok(OidcService::new(service, Arc::clone(&config)) + Some(config) => Ok(OidcService::new(service, &app_state, Arc::clone(&config)) .await .map_err(|err| { log::error!( @@ -102,16 +113,23 @@ where pub struct OidcService { service: S, + app_state: web::Data, config: Arc, provider_metadata: openidconnect::core::CoreProviderMetadata, } impl OidcService { - pub async fn new(service: S, config: Arc) -> anyhow::Result { + pub async fn new( + service: S, + app_state: &web::Data, + config: Arc, + ) -> anyhow::Result { + let issuer_url = config.issuer_url.clone(); Ok(Self { service, - config: Arc::clone(&config), - provider_metadata: discover_provider_metadata(config.issuer_url.clone()).await?, + app_state: web::Data::clone(app_state), + config, + provider_metadata: discover_provider_metadata(&app_state.config, issuer_url).await?, }) } } @@ -146,10 +164,10 @@ pub struct AwcHttpClient { } impl AwcHttpClient { - pub fn new() -> Self { - Self { - client: Client::default(), - } + pub fn new(app_config: &AppConfig) -> anyhow::Result { + Ok(Self { + client: make_http_client(app_config)?, + }) } } @@ -175,7 +193,7 @@ async fn execute_oidc_request_with_awc( ) -> Result>, anyhow::Error> { let awc_method = awc::http::Method::from_bytes(request.method().as_str().as_bytes())?; let awc_uri = awc::http::Uri::from_str(&request.uri().to_string())?; - log::debug!("Executing OIDC request: {} {}", awc_method, awc_uri); + log::debug!("Executing OIDC request: {awc_method} {awc_uri}"); let mut req = client.request(awc_method, awc_uri); for (name, value) in request.headers() { req = req.insert_header((name.as_str(), value.to_str()?)); From d5fd55446261efbe38fb93bc26bf7a799f98777d Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sat, 26 Apr 2025 03:18:47 +0200 Subject: [PATCH 07/30] oidc: implement redirects - Add `host` configuration option for specifying the application's web address in configuration.md and app_config.rs. - Update docker-compose.yaml to include SQLPAGE_HOST and SQLPAGE_OIDC_ISSUER_URL environment variables. - Enhance OIDC middleware to utilize the new `host` setting for redirect URLs and improve cookie handling in oidc.rs. --- Cargo.lock | 2 +- configuration.md | 18 +++ examples/single sign on/docker-compose.yaml | 11 ++ src/app_config.rs | 4 + src/webserver/oidc.rs | 135 +++++++++++++++++--- 5 files changed, 149 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 06adec09..5426fd8a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2548,7 +2548,7 @@ checksum = "51e219e79014df21a225b1860a479e2dcd7cbd9130f4defd4bd0e191ea31d67d" dependencies = [ "base64 0.22.1", "chrono", - "getrandom 0.2.15", + "getrandom 0.2.16", "http 1.3.1", "rand 0.8.5", "serde", diff --git a/configuration.md b/configuration.md index bb5884a6..798a97d1 100644 --- a/configuration.md +++ b/configuration.md @@ -13,6 +13,7 @@ Here are the available configuration options and their default values: | `database_password` | | Database password. If set, this will override any password specified in the `database_url`. This allows you to keep the password separate from the connection string for better security. | | `port` | 8080 | Like listen_on, but specifies only the port. | | `unix_socket` | | Path to a UNIX socket to listen on instead of the TCP port. If specified, SQLPage will accept HTTP connections only on this socket and not on any TCP port. This option is mutually exclusive with `listen_on` and `port`. +| `host` | | The web address where your application is accessible (e.g., "myapp.example.com"). Used for login redirects with OIDC. | | `max_database_pool_connections` | PostgreSQL: 50
MySql: 75
SQLite: 16
MSSQL: 100 | How many simultaneous database connections to open at most | | `database_connection_idle_timeout_seconds` | SQLite: None
All other: 30 minutes | Automatically close database connections after this period of inactivity | | `database_connection_max_lifetime_seconds` | SQLite: None
All other: 60 minutes | Always close database connections after this amount of time | @@ -95,6 +96,23 @@ To set up OIDC, you'll need to: 1. Register your application with an OIDC provider 2. Configure the provider's details in SQLPage +#### Setting Your Application's Address + +When users log in through an OIDC provider, they need to be sent back to your application afterward. For this to work correctly, you need to tell SQLPage where your application is located online: + +- Use the `host` setting to specify your application's web address (for example, "myapp.example.com") +- If you already have the `https_domain` setting set (to fetch https certificates for your site), then you don't need to duplicate it into `host`. + +Example configuration: +```json +{ + "oidc_issuer_url": "https://accounts.google.com", + "oidc_client_id": "your-client-id", + "oidc_client_secret": "your-client-secret", + "host": "myapp.example.com" +} +``` + #### Cloud Identity Providers - **Google** diff --git a/examples/single sign on/docker-compose.yaml b/examples/single sign on/docker-compose.yaml index 9aa93a8c..3dae9715 100644 --- a/examples/single sign on/docker-compose.yaml +++ b/examples/single sign on/docker-compose.yaml @@ -14,6 +14,8 @@ services: - ./sqlpage:/etc/sqlpage environment: # OIDC configuration + - SQLPAGE_HOST=localhost:8080 + - SQLPAGE_OIDC_ISSUER_URL=http://localhost:8181/realms/sqlpage_demo - OIDC_AUTHORIZATION_ENDPOINT=http://localhost:8181/realms/sqlpage_demo/protocol/openid-connect/auth - OIDC_TOKEN_ENDPOINT=http://localhost:8181/realms/sqlpage_demo/protocol/openid-connect/token - OIDC_USERINFO_ENDPOINT=http://localhost:8181/realms/sqlpage_demo/protocol/openid-connect/userinfo @@ -28,6 +30,9 @@ services: # SQLPage configuration - RUST_LOG=sqlpage=debug network_mode: host + depends_on: + keycloak: + condition: service_healthy keycloak: build: @@ -39,3 +44,9 @@ services: volumes: - ./keycloak-configuration.json:/opt/keycloak/data/import/realm.json network_mode: host + healthcheck: + test: ["CMD-SHELL", "/opt/keycloak/bin/kcadm.sh get realms/sqlpage_demo --server http://localhost:8181 --realm master --user admin --password admin || exit 1"] + interval: 10s + timeout: 2s + retries: 5 + start_period: 5s diff --git a/src/app_config.rs b/src/app_config.rs index cf215520..5b7742ad 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -222,6 +222,10 @@ pub struct AppConfig { /// and will automatically request a certificate from Let's Encrypt /// using the ACME protocol (requesting a TLS-ALPN-01 challenge). pub https_domain: Option, + + /// The hostname where your application is publicly accessible (e.g., "myapp.example.com"). + /// This is used for OIDC redirect URLs. If not set, https_domain will be used instead. + pub host: Option, /// The email address to use when requesting a certificate from Let's Encrypt. /// Defaults to `contact@`. diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index d75792aa..9a9c8c3f 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -4,20 +4,28 @@ use crate::{app_config::AppConfig, AppState}; use actix_web::{ dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, middleware::Condition, - web, Error, + web, Error, HttpResponse, }; -use anyhow::anyhow; +use anyhow::{anyhow, Context}; use awc::Client; -use openidconnect::{AsyncHttpClient, IssuerUrl}; +use openidconnect::{ + core::{CoreAuthDisplay, CoreAuthenticationFlow}, + AsyncHttpClient, CsrfToken, EmptyAdditionalClaims, EndpointMaybeSet, EndpointNotSet, + EndpointSet, IssuerUrl, Nonce, RedirectUrl, Scope, +}; use super::http_client::make_http_client; +const SQLPAGE_AUTH_COOKIE_NAME: &str = "sqlpage_auth"; +const SQLPAGE_REDIRECT_URI: &str = "/sqlpage/oidc_callback"; + #[derive(Clone, Debug)] pub struct OidcConfig { pub issuer_url: IssuerUrl, pub client_id: String, pub client_secret: String, - pub scopes: String, + pub app_host: String, + pub scopes: Vec, } impl TryFrom<&AppConfig> for OidcConfig { @@ -25,16 +33,26 @@ impl TryFrom<&AppConfig> for OidcConfig { fn try_from(config: &AppConfig) -> Result { let issuer_url = config.oidc_issuer_url.as_ref().ok_or(None)?; - let client_secret = config - .oidc_client_secret + let client_secret = config.oidc_client_secret.as_ref().ok_or(Some( + "The \"oidc_client_secret\" setting is required to authenticate with the OIDC provider", + ))?; + + let app_host = config + .host .as_ref() - .ok_or(Some("Missing oidc_client_secret"))?; + .or_else(|| config.https_domain.as_ref()) + .ok_or(Some("The \"host\" or \"https_domain\" setting is required to build the OIDC redirect URL"))?; Ok(Self { issuer_url: issuer_url.clone(), client_id: config.oidc_client_id.clone(), client_secret: client_secret.clone(), - scopes: config.oidc_scopes.clone(), + scopes: config + .oidc_scopes + .split_whitespace() + .map(|s| Scope::new(s.to_string())) + .collect(), + app_host: app_host.clone(), }) } } @@ -80,13 +98,12 @@ async fn discover_provider_metadata( Ok(provider_metadata) } -impl Transform for OidcMiddleware +impl Transform for OidcMiddleware where - S: Service, Error = Error> + 'static, + S: Service, Error = Error> + 'static, S::Future: 'static, - B: 'static, { - type Response = ServiceResponse; + type Response = ServiceResponse; type Error = Error; type InitError = (); type Transform = OidcService; @@ -115,7 +132,7 @@ pub struct OidcService { service: S, app_state: web::Data, config: Arc, - provider_metadata: openidconnect::core::CoreProviderMetadata, + client: OidcClient, } impl OidcService { @@ -125,24 +142,40 @@ impl OidcService { config: Arc, ) -> anyhow::Result { let issuer_url = config.issuer_url.clone(); + let provider_metadata = discover_provider_metadata(&app_state.config, issuer_url).await?; + let client: OidcClient = make_oidc_client(&config, provider_metadata)?; Ok(Self { service, app_state: web::Data::clone(app_state), config, - provider_metadata: discover_provider_metadata(&app_state.config, issuer_url).await?, + client, }) } + + fn build_auth_url(&self, request: &ServiceRequest) -> String { + let (auth_url, csrf_token, nonce) = self + .client + .authorize_url( + CoreAuthenticationFlow::AuthorizationCode, + CsrfToken::new_random, + Nonce::new_random, + ) + // Set the desired scopes. + .add_scopes(self.config.scopes.iter().cloned()) + .url(); + auth_url.to_string() + } } type LocalBoxFuture = Pin + 'static>>; +use actix_web::body::BoxBody; -impl Service for OidcService +impl Service for OidcService where - S: Service, Error = Error>, + S: Service, Error = Error>, S::Future: 'static, - B: 'static, { - type Response = ServiceResponse; + type Response = ServiceResponse; type Error = Error; type Future = LocalBoxFuture>; @@ -150,8 +183,20 @@ where fn call(&self, request: ServiceRequest) -> Self::Future { log::debug!("Started OIDC middleware with config: {:?}", self.config); - let future = self.service.call(request); + match get_sqlpage_auth_cookie(&request) { + Some(cookie) => { + log::trace!("Found SQLPage auth cookie: {cookie}"); + } + None => { + log::trace!("No SQLPage auth cookie found, redirecting to login"); + let auth_url = self.build_auth_url(&request); + return Box::pin(async move { + Ok(request.into_response(build_redirect_response(auth_url))) + }); + } + } + let future = self.service.call(request); Box::pin(async move { let response = future.await?; Ok(response) @@ -159,6 +204,18 @@ where } } +fn build_redirect_response(auth_url: String) -> HttpResponse { + HttpResponse::TemporaryRedirect() + .append_header(("Location", auth_url)) + .body("Redirecting to the login page.") +} + +fn get_sqlpage_auth_cookie(request: &ServiceRequest) -> Option { + let cookie = request.cookie(SQLPAGE_AUTH_COOKIE_NAME)?; + log::error!("TODO: actually check the validity of the cookie"); + Some(cookie.value().to_string()) +} + pub struct AwcHttpClient { client: Client, } @@ -226,5 +283,43 @@ impl std::fmt::Display for StringError { std::fmt::Display::fmt(&self.0, f) } } - +type OidcClient = openidconnect::core::CoreClient< + EndpointSet, + EndpointNotSet, + EndpointNotSet, + EndpointNotSet, + EndpointMaybeSet, + EndpointMaybeSet, +>; impl std::error::Error for StringError {} + +fn make_oidc_client( + config: &Arc, + provider_metadata: openidconnect::core::CoreProviderMetadata, +) -> anyhow::Result { + let client_id = openidconnect::ClientId::new(config.client_id.clone()); + let client_secret = openidconnect::ClientSecret::new(config.client_secret.clone()); + + let local_hosts = ["localhost", "127.0.0.1", "::1"]; + let is_localhost = local_hosts.iter().any(|host| { + config.app_host.starts_with(host) + && config + .app_host + .get(host.len()..(host.len() + 1)) + .is_none_or(|c| c == ":") + }); + let redirect_url = RedirectUrl::new(format!( + "{}://{}{}", + if is_localhost { "http" } else { "https" }, + config.app_host, + SQLPAGE_REDIRECT_URI, + ))?; + let client = openidconnect::core::CoreClient::from_provider_metadata( + provider_metadata, + client_id, + Some(client_secret), + ) + .set_redirect_uri(redirect_url); + + Ok(client) +} From 32bda9791963cd4eda2821f2c2a996cbd4533576 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 00:00:44 +0200 Subject: [PATCH 08/30] improve local oidc configurability --- src/webserver/oidc.rs | 48 +++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 9a9c8c3f..8d80e1a7 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -41,7 +41,15 @@ impl TryFrom<&AppConfig> for OidcConfig { .host .as_ref() .or_else(|| config.https_domain.as_ref()) - .ok_or(Some("The \"host\" or \"https_domain\" setting is required to build the OIDC redirect URL"))?; + .cloned() + .unwrap_or_else(|| { + let host = config.listen_on().to_string(); + log::warn!( + "No host or https_domain provided in the configuration, using \"{}\" as the app host to build the redirect URL. This will only work locally.", + host + ); + host + }); Ok(Self { issuer_url: issuer_url.clone(), @@ -300,20 +308,30 @@ 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 local_hosts = ["localhost", "127.0.0.1", "::1"]; - let is_localhost = local_hosts.iter().any(|host| { - config.app_host.starts_with(host) - && config - .app_host - .get(host.len()..(host.len() + 1)) - .is_none_or(|c| c == ":") - }); - let redirect_url = RedirectUrl::new(format!( - "{}://{}{}", - if is_localhost { "http" } else { "https" }, - config.app_host, - SQLPAGE_REDIRECT_URI, - ))?; + let mut redirect_url = RedirectUrl::new(format!( + "https://{}{}", + config.app_host, SQLPAGE_REDIRECT_URI, + )) + .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(_)) => true, + Some(openidconnect::url::Host::Ipv6(_)) => true, + None => false, + }; + if needs_http { + log::debug!("Redirect URL is local, changing to HTTP"); + redirect_url = RedirectUrl::new(format!( + "http://{}{}", + config.app_host, SQLPAGE_REDIRECT_URI, + ))?; + } + log::debug!("Redirect URL: {redirect_url}"); let client = openidconnect::core::CoreClient::from_provider_metadata( provider_metadata, client_id, From e193fdfe27132291e2908e7741546372e196f039 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 00:01:55 +0200 Subject: [PATCH 09/30] log --- src/webserver/oidc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 8d80e1a7..acabf74c 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -325,7 +325,7 @@ fn make_oidc_client( None => false, }; if needs_http { - log::debug!("Redirect URL is local, changing to 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, From ef9dd3166bef97d3bb7650796d3b2acea1194eb8 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 00:05:34 +0200 Subject: [PATCH 10/30] Update warning message in OIDC configuration to clarify how to disable it by providing a host setting --- src/webserver/oidc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index acabf74c..bfc227ac 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -45,7 +45,7 @@ impl TryFrom<&AppConfig> for OidcConfig { .unwrap_or_else(|| { let host = config.listen_on().to_string(); log::warn!( - "No host or https_domain provided in the configuration, using \"{}\" as the app host to build the redirect URL. This will only work locally.", + "No host or https_domain provided in the configuration, using \"{}\" as the app host to build the redirect URL. This will only work locally. Disable this warning by providing a value for the \"host\" setting.", host ); host From 08c97f624975bd624b14fbf18bdece9c8c975314 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 00:11:23 +0200 Subject: [PATCH 11/30] Update OIDC redirect logging to use info level with client ID --- src/webserver/oidc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index bfc227ac..5651b910 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -331,7 +331,7 @@ fn make_oidc_client( config.app_host, SQLPAGE_REDIRECT_URI, ))?; } - log::debug!("Redirect URL: {redirect_url}"); + log::info!("OIDC redirect URL for {}: {redirect_url}", config.client_id); let client = openidconnect::core::CoreClient::from_provider_metadata( provider_metadata, client_id, From 5e8b12d048aaa54eb7c2123e495434e42e3ff5af Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 00:15:15 +0200 Subject: [PATCH 12/30] Refactor unauthenticated request handling in OIDC service - Extracted logic for handling unauthenticated requests into a separate method `handle_unauthenticated_request`. - Updated the main request handling flow to utilize the new method for improved readability and maintainability. --- src/webserver/oidc.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 5651b910..73500fe3 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -173,6 +173,13 @@ impl OidcService { .url(); auth_url.to_string() } + + fn handle_unauthenticated_request(&self, request: ServiceRequest) -> LocalBoxFuture, Error>> { + let auth_url = self.build_auth_url(&request); + Box::pin(async move { + Ok(request.into_response(build_redirect_response(auth_url))) + }) + } } type LocalBoxFuture = Pin + 'static>>; @@ -196,12 +203,8 @@ where log::trace!("Found SQLPage auth cookie: {cookie}"); } None => { - log::trace!("No SQLPage auth cookie found, redirecting to login"); - let auth_url = self.build_auth_url(&request); - - return Box::pin(async move { - Ok(request.into_response(build_redirect_response(auth_url))) - }); + log::trace!("No SQLPage auth cookie found"); + return self.handle_unauthenticated_request(request); } } let future = self.service.call(request); From d70fac3e99972f0e947a2a5700443c48eacf518c Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 09:58:19 +0200 Subject: [PATCH 13/30] Enhance OIDC service with callback handling and token processing - Introduced `handle_oidc_callback` method to manage OIDC callback requests. - Added `process_oidc_callback` and `exchange_code_for_token` methods for token exchange logic. - Updated `handle_unauthenticated_request` to check for callback URL and redirect accordingly. - Refactored `build_redirect_response` to improve clarity in response handling. --- src/webserver/oidc.rs | 83 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 73500fe3..401beb0e 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -136,11 +136,12 @@ where } } +#[derive(Clone)] pub struct OidcService { service: S, app_state: web::Data, config: Arc, - client: OidcClient, + client: Arc, } impl OidcService { @@ -156,7 +157,7 @@ impl OidcService { service, app_state: web::Data::clone(app_state), config, - client, + client: Arc::new(client), }) } @@ -168,18 +169,82 @@ impl OidcService { CsrfToken::new_random, Nonce::new_random, ) - // Set the desired scopes. .add_scopes(self.config.scopes.iter().cloned()) .url(); auth_url.to_string() } - - fn handle_unauthenticated_request(&self, request: ServiceRequest) -> LocalBoxFuture, Error>> { + + fn handle_unauthenticated_request( + &self, + request: ServiceRequest, + ) -> LocalBoxFuture, Error>> { + // Check if this is the OIDC callback URL + if request.path() == SQLPAGE_REDIRECT_URI { + return self.handle_oidc_callback(request); + } + + // If not the callback URL, redirect to auth as before let auth_url = self.build_auth_url(&request); + Box::pin(async move { Ok(request.into_response(build_redirect_response(auth_url))) }) + } + + fn handle_oidc_callback( + &self, + request: ServiceRequest, + ) -> LocalBoxFuture, Error>> { + let client = Arc::clone(&self.client); + let (http_req, _payload) = request.into_parts(); + let query_string = http_req.query_string().to_owned(); Box::pin(async move { - Ok(request.into_response(build_redirect_response(auth_url))) + let result = Self::process_oidc_callback(&client, &query_string).await; + match result { + Ok(response) => Ok(ServiceResponse::new(http_req, response).map_into_boxed_body()), + Err(e) => { + log::error!("Failed to process OIDC callback: {}", e); + Ok(ServiceResponse::new( + http_req, + HttpResponse::BadRequest().body("Authentication failed"), + ) + .map_into_boxed_body()) + } + } }) } + + async fn process_oidc_callback( + client: &Arc, + query_params: &str, + ) -> anyhow::Result { + let token_response = Self::exchange_code_for_token(client, query_params).await?; + let mut response = build_redirect_response(format!("/")); + Self::set_auth_cookie(&mut response, &token_response); + Ok(response) + } + + async fn exchange_code_for_token( + client: &Arc, + query_string: &str, + ) -> anyhow::Result { + todo!("Extract 'code' and 'state' from query_string"); + todo!("Verify the state matches the expected CSRF token"); + todo!("Use client.exchange_code() to get the token response"); + } + + + + fn set_auth_cookie( + response: &mut HttpResponse, + token_response: &openidconnect::core::CoreTokenResponse, + ) { + // Extract token information (access token, id token, etc.) + todo!("Extract access_token and id_token from token_response"); + + // Create a secure cookie with the token information + todo!("Create a cookie with token information"); + + // Add the cookie to the response + todo!("response.cookie() to add the cookie to the response"); + } } type LocalBoxFuture = Pin + 'static>>; @@ -215,10 +280,10 @@ where } } -fn build_redirect_response(auth_url: String) -> HttpResponse { +fn build_redirect_response(target_url: String) -> HttpResponse { HttpResponse::TemporaryRedirect() - .append_header(("Location", auth_url)) - .body("Redirecting to the login page.") + .append_header(("Location", target_url)) + .body("Redirecting...") } fn get_sqlpage_auth_cookie(request: &ServiceRequest) -> Option { From a5cb4799d5aebeca7f7a359c839193c56717e151 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 10:37:07 +0200 Subject: [PATCH 14/30] in handle_oidc_callback use service_request.into_response --- src/webserver/oidc.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 401beb0e..f413aefe 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -193,19 +193,16 @@ impl OidcService { request: ServiceRequest, ) -> LocalBoxFuture, Error>> { let client = Arc::clone(&self.client); - let (http_req, _payload) = request.into_parts(); - let query_string = http_req.query_string().to_owned(); + Box::pin(async move { + let query_string = request.query_string(); let result = Self::process_oidc_callback(&client, &query_string).await; match result { - Ok(response) => Ok(ServiceResponse::new(http_req, response).map_into_boxed_body()), + Ok(response) => Ok(request.into_response(response)), Err(e) => { log::error!("Failed to process OIDC callback: {}", e); - Ok(ServiceResponse::new( - http_req, - HttpResponse::BadRequest().body("Authentication failed"), - ) - .map_into_boxed_body()) + Ok(request + .into_response(HttpResponse::BadRequest().body("Authentication failed"))) } } }) @@ -230,8 +227,6 @@ impl OidcService { todo!("Use client.exchange_code() to get the token response"); } - - fn set_auth_cookie( response: &mut HttpResponse, token_response: &openidconnect::core::CoreTokenResponse, From 938f51cd66c23494ea4236b0989b551d3e50a89c Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 10:37:10 +0200 Subject: [PATCH 15/30] fmt --- src/app_config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app_config.rs b/src/app_config.rs index 5b7742ad..188f2e51 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -222,7 +222,7 @@ pub struct AppConfig { /// and will automatically request a certificate from Let's Encrypt /// using the ACME protocol (requesting a TLS-ALPN-01 challenge). pub https_domain: Option, - + /// The hostname where your application is publicly accessible (e.g., "myapp.example.com"). /// This is used for OIDC redirect URLs. If not set, https_domain will be used instead. pub host: Option, From e305b8938d64fda055bd30ddaae69a679175c269 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 14:19:03 +0200 Subject: [PATCH 16/30] Implement oidc code exchange and token storage --- src/webserver/oidc.rs | 120 +++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 48 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index f413aefe..b5c23f54 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -4,15 +4,17 @@ use crate::{app_config::AppConfig, AppState}; use actix_web::{ dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, middleware::Condition, - web, Error, HttpResponse, + web::{self, Query}, + Error, HttpResponse, }; use anyhow::{anyhow, Context}; use awc::Client; use openidconnect::{ core::{CoreAuthDisplay, CoreAuthenticationFlow}, AsyncHttpClient, CsrfToken, EmptyAdditionalClaims, EndpointMaybeSet, EndpointNotSet, - EndpointSet, IssuerUrl, Nonce, RedirectUrl, Scope, + EndpointSet, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, TokenResponse, }; +use serde::Deserialize; use super::http_client::make_http_client; @@ -97,12 +99,11 @@ impl OidcMiddleware { } async fn discover_provider_metadata( - app_config: &AppConfig, + http_client: &AwcHttpClient, issuer_url: IssuerUrl, ) -> anyhow::Result { - let http_client = AwcHttpClient::new(app_config)?; let provider_metadata = - openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, &http_client).await?; + openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, http_client).await?; Ok(provider_metadata) } @@ -141,7 +142,8 @@ pub struct OidcService { service: S, app_state: web::Data, config: Arc, - client: Arc, + oidc_client: Arc, + http_client: Arc, } impl OidcService { @@ -151,19 +153,21 @@ impl OidcService { config: Arc, ) -> anyhow::Result { let issuer_url = config.issuer_url.clone(); - let provider_metadata = discover_provider_metadata(&app_state.config, issuer_url).await?; + let http_client = AwcHttpClient::new(&app_state.config)?; + let provider_metadata = discover_provider_metadata(&http_client, issuer_url).await?; let client: OidcClient = make_oidc_client(&config, provider_metadata)?; Ok(Self { service, app_state: web::Data::clone(app_state), config, - client: Arc::new(client), + oidc_client: Arc::new(client), + http_client: Arc::new(http_client), }) } fn build_auth_url(&self, request: &ServiceRequest) -> String { let (auth_url, csrf_token, nonce) = self - .client + .oidc_client .authorize_url( CoreAuthenticationFlow::AuthorizationCode, CsrfToken::new_random, @@ -192,54 +196,20 @@ impl OidcService { &self, request: ServiceRequest, ) -> LocalBoxFuture, Error>> { - let client = Arc::clone(&self.client); + let oidc_client = Arc::clone(&self.oidc_client); + let http_client = Arc::clone(&self.http_client); Box::pin(async move { let query_string = request.query_string(); - let result = Self::process_oidc_callback(&client, &query_string).await; - match result { + match process_oidc_callback(&oidc_client, &http_client, query_string).await { Ok(response) => Ok(request.into_response(response)), Err(e) => { - log::error!("Failed to process OIDC callback: {}", e); - Ok(request - .into_response(HttpResponse::BadRequest().body("Authentication failed"))) + log::error!("Failed to process OIDC callback with params {query_string}: {e}"); + Ok(request.into_response(HttpResponse::BadRequest().body(e.to_string()))) } } }) } - - async fn process_oidc_callback( - client: &Arc, - query_params: &str, - ) -> anyhow::Result { - let token_response = Self::exchange_code_for_token(client, query_params).await?; - let mut response = build_redirect_response(format!("/")); - Self::set_auth_cookie(&mut response, &token_response); - Ok(response) - } - - async fn exchange_code_for_token( - client: &Arc, - query_string: &str, - ) -> anyhow::Result { - todo!("Extract 'code' and 'state' from query_string"); - todo!("Verify the state matches the expected CSRF token"); - todo!("Use client.exchange_code() to get the token response"); - } - - fn set_auth_cookie( - response: &mut HttpResponse, - token_response: &openidconnect::core::CoreTokenResponse, - ) { - // Extract token information (access token, id token, etc.) - todo!("Extract access_token and id_token from token_response"); - - // Create a secure cookie with the token information - todo!("Create a cookie with token information"); - - // Add the cookie to the response - todo!("response.cookie() to add the cookie to the response"); - } } type LocalBoxFuture = Pin + 'static>>; @@ -275,6 +245,54 @@ where } } +async fn process_oidc_callback( + oidc_client: &Arc, + http_client: &Arc, + query_string: &str, +) -> anyhow::Result { + let params = Query::::from_query(query_string)?.into_inner(); + let token_response = exchange_code_for_token(oidc_client, http_client, params).await?; + let mut response = build_redirect_response(format!("/")); + set_auth_cookie(&mut response, &token_response)?; + Ok(response) +} + +async fn exchange_code_for_token( + oidc_client: &OidcClient, + http_client: &AwcHttpClient, + oidc_callback_params: OidcCallbackParams, +) -> anyhow::Result { + // TODO: Verify the state matches the expected CSRF token + let token_response = oidc_client + .exchange_code(openidconnect::AuthorizationCode::new( + oidc_callback_params.code, + ))? + .request_async(http_client) + .await?; + Ok(token_response) +} + +fn set_auth_cookie( + response: &mut HttpResponse, + token_response: &openidconnect::core::CoreTokenResponse, +) -> anyhow::Result<()> { + let access_token = token_response.access_token(); + log::debug!("Received access token: {}", access_token.secret()); + let id_token = token_response + .id_token() + .context("No ID token found in the token response. You may have specified an oauth2 provider that does not support OIDC.")?; + + let cookie = actix_web::cookie::Cookie::build(SQLPAGE_AUTH_COOKIE_NAME, id_token.to_string()) + .secure(true) + .http_only(true) + .same_site(actix_web::cookie::SameSite::Lax) + .path("/") + .finish(); + + response.add_cookie(&cookie).unwrap(); + Ok(()) +} + fn build_redirect_response(target_url: String) -> HttpResponse { HttpResponse::TemporaryRedirect() .append_header(("Location", target_url)) @@ -404,3 +422,9 @@ fn make_oidc_client( Ok(client) } + +#[derive(Debug, Deserialize)] +struct OidcCallbackParams { + code: String, + state: String, +} From 9f22532d44ec400bfe3599a64307fd0778e39a22 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 23:09:44 +0200 Subject: [PATCH 17/30] validate oidc cookies - Updated `get_sqlpage_auth_cookie` to return a result for better error handling and validation of the SQLPage auth cookie. - Improved logging throughout the OIDC service for better traceability of requests and responses. - Adjusted the handling of OIDC callback parameters to include context in error messages. --- src/webserver/oidc.rs | 57 ++++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index b5c23f54..f02e7546 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -10,9 +10,10 @@ use actix_web::{ use anyhow::{anyhow, Context}; use awc::Client; use openidconnect::{ - core::{CoreAuthDisplay, CoreAuthenticationFlow}, + core::{CoreAuthDisplay, CoreAuthenticationFlow, CoreGenderClaim, CoreIdToken}, AsyncHttpClient, CsrfToken, EmptyAdditionalClaims, EndpointMaybeSet, EndpointNotSet, - EndpointSet, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, TokenResponse, + EndpointSet, IdToken, IdTokenClaims, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, + TokenResponse, }; use serde::Deserialize; @@ -182,12 +183,13 @@ impl OidcService { &self, request: ServiceRequest, ) -> LocalBoxFuture, Error>> { - // Check if this is the OIDC callback URL + log::debug!("Handling unauthenticated request to {}", request.path()); if request.path() == SQLPAGE_REDIRECT_URI { + log::debug!("The request is the OIDC callback"); return self.handle_oidc_callback(request); } - // If not the callback URL, redirect to auth as before + log::debug!("Redirecting to OIDC provider"); let auth_url = self.build_auth_url(&request); Box::pin(async move { Ok(request.into_response(build_redirect_response(auth_url))) }) } @@ -228,14 +230,19 @@ where fn call(&self, request: ServiceRequest) -> Self::Future { log::debug!("Started OIDC middleware with config: {:?}", self.config); - match get_sqlpage_auth_cookie(&request) { - Some(cookie) => { + let oidc_client = Arc::clone(&self.oidc_client); + match get_sqlpage_auth_cookie(&oidc_client, &request) { + Ok(Some(cookie)) => { log::trace!("Found SQLPage auth cookie: {cookie}"); } - None => { + Ok(None) => { log::trace!("No SQLPage auth cookie found"); return self.handle_unauthenticated_request(request); } + Err(e) => { + log::error!("Found an invalid SQLPage auth cookie: {e}"); + return self.handle_unauthenticated_request(request); + } } let future = self.service.call(request); Box::pin(async move { @@ -250,8 +257,13 @@ async fn process_oidc_callback( http_client: &Arc, query_string: &str, ) -> anyhow::Result { - let params = Query::::from_query(query_string)?.into_inner(); + let params = Query::::from_query(query_string) + .with_context(|| format!("{SQLPAGE_REDIRECT_URI}: failed to parse OIDC callback parameters from {query_string}"))? + .into_inner(); + log::debug!("Processing OIDC callback with params: {params:?}. Requesting token..."); let token_response = exchange_code_for_token(oidc_client, http_client, params).await?; + log::debug!("Received token response: {token_response:?}"); + // TODO: redirect to the original URL instead of / let mut response = build_redirect_response(format!("/")); set_auth_cookie(&mut response, &token_response)?; Ok(response) @@ -277,12 +289,14 @@ fn set_auth_cookie( token_response: &openidconnect::core::CoreTokenResponse, ) -> anyhow::Result<()> { let access_token = token_response.access_token(); - log::debug!("Received access token: {}", access_token.secret()); + log::trace!("Received access token: {}", access_token.secret()); let id_token = token_response .id_token() .context("No ID token found in the token response. You may have specified an oauth2 provider that does not support OIDC.")?; - let cookie = actix_web::cookie::Cookie::build(SQLPAGE_AUTH_COOKIE_NAME, id_token.to_string()) + let id_token_str = id_token.to_string(); + log::trace!("Setting auth cookie: {SQLPAGE_AUTH_COOKIE_NAME}=\"{id_token_str}\""); + let cookie = actix_web::cookie::Cookie::build(SQLPAGE_AUTH_COOKIE_NAME, id_token_str) .secure(true) .http_only(true) .same_site(actix_web::cookie::SameSite::Lax) @@ -299,10 +313,25 @@ fn build_redirect_response(target_url: String) -> HttpResponse { .body("Redirecting...") } -fn get_sqlpage_auth_cookie(request: &ServiceRequest) -> Option { - let cookie = request.cookie(SQLPAGE_AUTH_COOKIE_NAME)?; - log::error!("TODO: actually check the validity of the cookie"); - Some(cookie.value().to_string()) +fn get_sqlpage_auth_cookie( + oidc_client: &OidcClient, + request: &ServiceRequest, +) -> anyhow::Result> { + let Some(cookie) = request.cookie(SQLPAGE_AUTH_COOKIE_NAME) else { + return Ok(None); + }; + let cookie_value = cookie.value().to_string(); + + let verifier = oidc_client.id_token_verifier(); + let id_token = CoreIdToken::from_str(&cookie_value) + .with_context(|| anyhow!("Invalid SQLPage auth cookie"))?; + + let nonce_verifier = |_: Option<&Nonce>| Ok(()); + let claims: &IdTokenClaims = id_token + .claims(&verifier, nonce_verifier) + .with_context(|| anyhow!("Invalid SQLPage auth cookie"))?; + log::debug!("The current user is: {claims:?}"); + Ok(Some(cookie_value)) } pub struct AwcHttpClient { From 8e07bab93e60828d10e671a266e5cebd26482fe8 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 23:17:18 +0200 Subject: [PATCH 18/30] OIDC callback: redirect to the auth URL on failure. --- src/webserver/oidc.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index f02e7546..93f3977c 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -200,6 +200,7 @@ impl OidcService { ) -> LocalBoxFuture, Error>> { let oidc_client = Arc::clone(&self.oidc_client); let http_client = Arc::clone(&self.http_client); + let oidc_config = Arc::clone(&self.config); Box::pin(async move { let query_string = request.query_string(); @@ -207,7 +208,8 @@ impl OidcService { Ok(response) => Ok(request.into_response(response)), Err(e) => { log::error!("Failed to process OIDC callback with params {query_string}: {e}"); - Ok(request.into_response(HttpResponse::BadRequest().body(e.to_string()))) + let auth_url = build_auth_url(&oidc_client, &oidc_config.scopes); + Ok(request.into_response(build_redirect_response(auth_url))) } } }) @@ -457,3 +459,15 @@ struct OidcCallbackParams { code: String, state: String, } + +fn build_auth_url(oidc_client: &OidcClient, scopes: &[Scope]) -> String { + let (auth_url, csrf_token, nonce) = oidc_client + .authorize_url( + CoreAuthenticationFlow::AuthorizationCode, + CsrfToken::new_random, + Nonce::new_random, + ) + .add_scopes(scopes.iter().cloned()) + .url(); + auth_url.to_string() +} From 9f302d90e24313ad1d09723546586c09fabfe8e1 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 23:37:22 +0200 Subject: [PATCH 19/30] oidc use localhost for redirect config instead of 0.0.0.0 by default --- src/webserver/oidc.rs | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 93f3977c..139b7937 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -40,19 +40,7 @@ impl TryFrom<&AppConfig> for OidcConfig { "The \"oidc_client_secret\" setting is required to authenticate with the OIDC provider", ))?; - let app_host = config - .host - .as_ref() - .or_else(|| config.https_domain.as_ref()) - .cloned() - .unwrap_or_else(|| { - let host = config.listen_on().to_string(); - log::warn!( - "No host or https_domain provided in the configuration, using \"{}\" as the app host to build the redirect URL. This will only work locally. Disable this warning by providing a value for the \"host\" setting.", - host - ); - host - }); + let app_host = get_app_host(config); Ok(Self { issuer_url: issuer_url.clone(), @@ -68,6 +56,31 @@ impl TryFrom<&AppConfig> for OidcConfig { } } +fn get_app_host(config: &AppConfig) -> String { + if let Some(host) = &config.host { + return host.clone(); + } + if let Some(https_domain) = &config.https_domain { + return https_domain.clone(); + } + + let socket_addr = config.listen_on(); + let ip = socket_addr.ip(); + let host = if ip.is_unspecified() || ip.is_loopback() { + format!("localhost:{}", socket_addr.port()) + } else { + socket_addr.to_string() + }; + log::warn!( + "No host or https_domain provided in the configuration, \ + using \"{}\" as the app host to build the redirect URL. \ + This will only work locally. \ + Disable this warning by providing a value for the \"host\" setting.", + host + ); + host +} + pub struct OidcMiddleware { pub config: Option>, app_state: web::Data, @@ -79,7 +92,6 @@ impl OidcMiddleware { match &config { Ok(config) => { log::debug!("Setting up OIDC with issuer: {}", config.issuer_url); - // contains secrets } Err(Some(err)) => { log::error!("Invalid OIDC configuration: {err}"); From 591900885b3e116df085c52c69cb06c542c94f60 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 27 Apr 2025 23:39:45 +0200 Subject: [PATCH 20/30] Enhance OIDC provider metadata discovery with improved logging and error context --- src/webserver/oidc.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 139b7937..381c9dc7 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -115,8 +115,12 @@ async fn discover_provider_metadata( http_client: &AwcHttpClient, issuer_url: IssuerUrl, ) -> anyhow::Result { + log::debug!("Discovering provider metadata for {}", issuer_url); let provider_metadata = - openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, http_client).await?; + openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, http_client) + .await + .with_context(|| format!("Failed to discover OIDC provider metadata"))?; + log::debug!("Provider metadata discovered: {provider_metadata:?}"); Ok(provider_metadata) } From 55bf29672a628db879279eec57aa76e3bc5b5196 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Mon, 28 Apr 2025 01:06:08 +0200 Subject: [PATCH 21/30] maintain the initial URL during OIDC authentication - Added state cookie handling to maintain the initial URL during OIDC authentication. - Refactored `build_auth_url` to accept the initial URL as a parameter. - Enhanced `process_oidc_callback` to retrieve the state from the cookie and redirect accordingly. --- src/webserver/oidc.rs | 68 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 381c9dc7..4729b904 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -15,12 +15,13 @@ use openidconnect::{ EndpointSet, IdToken, IdTokenClaims, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, TokenResponse, }; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use super::http_client::make_http_client; const SQLPAGE_AUTH_COOKIE_NAME: &str = "sqlpage_auth"; const SQLPAGE_REDIRECT_URI: &str = "/sqlpage/oidc_callback"; +const SQLPAGE_STATE_COOKIE_NAME: &str = "sqlpage_oidc_state"; #[derive(Clone, Debug)] pub struct OidcConfig { @@ -206,8 +207,19 @@ impl OidcService { } log::debug!("Redirecting to OIDC provider"); - let auth_url = self.build_auth_url(&request); - Box::pin(async move { Ok(request.into_response(build_redirect_response(auth_url))) }) + + let auth_url = build_auth_url( + &self.oidc_client, + &self.config.scopes, + request.path().to_string(), + ); + Box::pin(async move { + let state_cookie = create_state_cookie(&request); + let mut response = build_redirect_response(auth_url); + + response.add_cookie(&state_cookie)?; + Ok(request.into_response(response)) + }) } fn handle_oidc_callback( @@ -220,11 +232,15 @@ impl OidcService { Box::pin(async move { let query_string = request.query_string(); - match process_oidc_callback(&oidc_client, &http_client, query_string).await { + match process_oidc_callback(&oidc_client, &http_client, query_string, &request).await { Ok(response) => Ok(request.into_response(response)), Err(e) => { log::error!("Failed to process OIDC callback with params {query_string}: {e}"); - let auth_url = build_auth_url(&oidc_client, &oidc_config.scopes); + let auth_url = build_auth_url( + &oidc_client, + &oidc_config.scopes, + request.path().to_string(), + ); Ok(request.into_response(build_redirect_response(auth_url))) } } @@ -274,15 +290,22 @@ async fn process_oidc_callback( oidc_client: &Arc, http_client: &Arc, query_string: &str, + request: &ServiceRequest, ) -> anyhow::Result { + let state = get_state_from_cookie(request)?; + let params = Query::::from_query(query_string) - .with_context(|| format!("{SQLPAGE_REDIRECT_URI}: failed to parse OIDC callback parameters from {query_string}"))? + .with_context(|| { + format!( + "{SQLPAGE_REDIRECT_URI}: failed to parse OIDC callback parameters from {query_string}" + ) + })? .into_inner(); log::debug!("Processing OIDC callback with params: {params:?}. Requesting token..."); let token_response = exchange_code_for_token(oidc_client, http_client, params).await?; log::debug!("Received token response: {token_response:?}"); - // TODO: redirect to the original URL instead of / - let mut response = build_redirect_response(format!("/")); + + let mut response = build_redirect_response(state.initial_url); set_auth_cookie(&mut response, &token_response)?; Ok(response) } @@ -476,7 +499,7 @@ struct OidcCallbackParams { state: String, } -fn build_auth_url(oidc_client: &OidcClient, scopes: &[Scope]) -> String { +fn build_auth_url(oidc_client: &OidcClient, scopes: &[Scope], initial_url: String) -> String { let (auth_url, csrf_token, nonce) = oidc_client .authorize_url( CoreAuthenticationFlow::AuthorizationCode, @@ -487,3 +510,30 @@ fn build_auth_url(oidc_client: &OidcClient, scopes: &[Scope]) -> String { .url(); auth_url.to_string() } + +#[derive(Debug, Serialize, Deserialize)] +struct OidcLoginState { + #[serde(rename = "u")] + initial_url: String, +} + +fn create_state_cookie(request: &ServiceRequest) -> actix_web::cookie::Cookie { + let state = OidcLoginState { + initial_url: request.path().to_string(), + }; + let state_json = serde_json::to_string(&state).unwrap(); + actix_web::cookie::Cookie::build(SQLPAGE_STATE_COOKIE_NAME, state_json) + .secure(true) + .http_only(true) + .same_site(actix_web::cookie::SameSite::Lax) + .path("/") + .finish() +} + +fn get_state_from_cookie(request: &ServiceRequest) -> anyhow::Result { + let state_cookie = request.cookie(SQLPAGE_STATE_COOKIE_NAME).with_context(|| { + format!("No {SQLPAGE_STATE_COOKIE_NAME} cookie found for {SQLPAGE_REDIRECT_URI}") + })?; + serde_json::from_str(state_cookie.value()) + .with_context(|| format!("Failed to parse OIDC state from cookie")) +} From 92ba450fe2693264e4e611c2455bbf12a6e089ef Mon Sep 17 00:00:00 2001 From: lovasoa Date: Mon, 28 Apr 2025 13:46:15 +0200 Subject: [PATCH 22/30] implement csrf token --- src/webserver/oidc.rs | 126 +++++++++++++++++++++++++++++++----------- 1 file changed, 94 insertions(+), 32 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 4729b904..6b763d3d 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,7 +1,14 @@ -use std::{future::Future, pin::Pin, str::FromStr, sync::Arc}; +use std::{ + future::Future, + hash::{DefaultHasher, Hash, Hasher}, + pin::Pin, + str::FromStr, + sync::Arc, +}; use crate::{app_config::AppConfig, AppState}; use actix_web::{ + cookie::Cookie, dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, middleware::Condition, web::{self, Query}, @@ -10,11 +17,13 @@ use actix_web::{ use anyhow::{anyhow, Context}; use awc::Client; use openidconnect::{ - core::{CoreAuthDisplay, CoreAuthenticationFlow, CoreGenderClaim, CoreIdToken}, + core::{CoreAuthenticationFlow, CoreGenderClaim, CoreIdToken}, + url::Url, AsyncHttpClient, CsrfToken, EmptyAdditionalClaims, EndpointMaybeSet, EndpointNotSet, - EndpointSet, IdToken, IdTokenClaims, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, + EndpointSet, IdTokenClaims, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, TokenResponse, }; +use password_hash::{rand_core::OsRng, SaltString}; use serde::{Deserialize, Serialize}; use super::http_client::make_http_client; @@ -208,18 +217,9 @@ impl OidcService { log::debug!("Redirecting to OIDC provider"); - let auth_url = build_auth_url( - &self.oidc_client, - &self.config.scopes, - request.path().to_string(), - ); - Box::pin(async move { - let state_cookie = create_state_cookie(&request); - let mut response = build_redirect_response(auth_url); - - response.add_cookie(&state_cookie)?; - Ok(request.into_response(response)) - }) + let response = + build_auth_provider_redirect_response(&self.oidc_client, &self.config, &request); + Box::pin(async move { Ok(request.into_response(response)) }) } fn handle_oidc_callback( @@ -236,12 +236,9 @@ impl OidcService { Ok(response) => Ok(request.into_response(response)), Err(e) => { log::error!("Failed to process OIDC callback with params {query_string}: {e}"); - let auth_url = build_auth_url( - &oidc_client, - &oidc_config.scopes, - request.path().to_string(), - ); - Ok(request.into_response(build_redirect_response(auth_url))) + let resp = + build_auth_provider_redirect_response(&oidc_client, &oidc_config, &request); + Ok(request.into_response(resp)) } } }) @@ -301,6 +298,12 @@ async fn process_oidc_callback( ) })? .into_inner(); + + if state.csrf_token.secret() != params.state.secret() { + log::debug!("CSRF token mismatch: expected {state:?}, got {params:?}"); + return Err(anyhow!("Invalid CSRF token: {}", params.state.secret())); + } + log::debug!("Processing OIDC callback with params: {params:?}. Requesting token..."); let token_response = exchange_code_for_token(oidc_client, http_client, params).await?; log::debug!("Received token response: {token_response:?}"); @@ -337,7 +340,7 @@ fn set_auth_cookie( let id_token_str = id_token.to_string(); log::trace!("Setting auth cookie: {SQLPAGE_AUTH_COOKIE_NAME}=\"{id_token_str}\""); - let cookie = actix_web::cookie::Cookie::build(SQLPAGE_AUTH_COOKIE_NAME, id_token_str) + let cookie = Cookie::build(SQLPAGE_AUTH_COOKIE_NAME, id_token_str) .secure(true) .http_only(true) .same_site(actix_web::cookie::SameSite::Lax) @@ -348,6 +351,19 @@ fn set_auth_cookie( Ok(()) } +fn build_auth_provider_redirect_response( + oidc_client: &OidcClient, + oidc_config: &Arc, + request: &ServiceRequest, +) -> HttpResponse { + let AuthUrl { url, params } = build_auth_url(oidc_client, &oidc_config.scopes); + let state_cookie = create_state_cookie(request, params); + HttpResponse::TemporaryRedirect() + .append_header(("Location", url.to_string())) + .cookie(state_cookie) + .body("Redirecting...") +} + fn build_redirect_response(target_url: String) -> HttpResponse { HttpResponse::TemporaryRedirect() .append_header(("Location", target_url)) @@ -496,33 +512,79 @@ fn make_oidc_client( #[derive(Debug, Deserialize)] struct OidcCallbackParams { code: String, - state: String, + state: CsrfToken, } -fn build_auth_url(oidc_client: &OidcClient, scopes: &[Scope], initial_url: String) -> String { - let (auth_url, csrf_token, nonce) = oidc_client +struct AuthUrl { + url: Url, + params: AuthUrlParams, +} + +struct AuthUrlParams { + csrf_token: CsrfToken, + nonce: Nonce, +} + +fn build_auth_url(oidc_client: &OidcClient, scopes: &[Scope]) -> AuthUrl { + let nonce_source = Nonce::new_random(); + let hashed_nonce = Nonce::new(hash_nonce(&nonce_source)); + let (url, csrf_token, _nonce) = oidc_client .authorize_url( CoreAuthenticationFlow::AuthorizationCode, CsrfToken::new_random, - Nonce::new_random, + || hashed_nonce, ) .add_scopes(scopes.iter().cloned()) .url(); - auth_url.to_string() + AuthUrl { + url, + params: AuthUrlParams { + csrf_token, + nonce: nonce_source, + }, + } } #[derive(Debug, Serialize, Deserialize)] struct OidcLoginState { + /// The URL to redirect to after the login process is complete. #[serde(rename = "u")] initial_url: String, + /// The CSRF token to use for the login process. + #[serde(rename = "c")] + csrf_token: CsrfToken, + /// The source nonce to use for the login process. It must be checked against the hash + /// stored in the ID token. + #[serde(rename = "n")] + nonce: Nonce, } -fn create_state_cookie(request: &ServiceRequest) -> actix_web::cookie::Cookie { - let state = OidcLoginState { - initial_url: request.path().to_string(), - }; +fn hash_nonce(nonce: &Nonce) -> String { + use argon2::password_hash::{rand_core::OsRng, PasswordHasher, SaltString}; + let salt = SaltString::generate(&mut OsRng); + // low-cost parameters + let params = argon2::Params::new(8, 1, 1, None).expect("bug: invalid Argon2 parameters"); + let argon2 = argon2::Argon2::new(argon2::Algorithm::Argon2id, argon2::Version::V0x13, params); + let hash = argon2 + .hash_password(nonce.secret().as_bytes(), &salt) + .expect("bug: failed to hash nonce"); + hash.to_string() +} + +impl OidcLoginState { + fn new(request: &ServiceRequest, auth_url: AuthUrlParams) -> Self { + Self { + initial_url: request.path().to_string(), + csrf_token: auth_url.csrf_token, + nonce: auth_url.nonce, + } + } +} + +fn create_state_cookie(request: &ServiceRequest, auth_url: AuthUrlParams) -> Cookie { + let state = OidcLoginState::new(request, auth_url); let state_json = serde_json::to_string(&state).unwrap(); - actix_web::cookie::Cookie::build(SQLPAGE_STATE_COOKIE_NAME, state_json) + Cookie::build(SQLPAGE_STATE_COOKIE_NAME, state_json) .secure(true) .http_only(true) .same_site(actix_web::cookie::SameSite::Lax) From 1535b4a3ec5619926c1a0fbe9f59bd01bac169fb Mon Sep 17 00:00:00 2001 From: lovasoa Date: Mon, 28 Apr 2025 13:46:34 +0200 Subject: [PATCH 23/30] update deps --- Cargo.lock | 2 +- Cargo.toml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5426fd8a..738c143d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3571,7 +3571,7 @@ dependencies = [ "async-stream", "async-trait", "awc", - "base64 0.22.1", + "base64 0.21.7", "chrono", "clap", "config", diff --git a/Cargo.toml b/Cargo.toml index 7336b27e..1eac8e3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,11 +50,11 @@ include_dir = "0.7.2" config = { version = "0.15.4", features = ["json"] } markdown = { version = "1.0.0-alpha.23", features = ["log"] } password-hash = "0.5.0" -argon2 = "0.5.0" +argon2 = "0.5.3" actix-web-httpauth = "0.8.0" rand = "0.9.0" actix-multipart = "0.7.2" -base64 = "0.22" +base64 = "0.21.7" rustls-acme = "0.9.2" dotenvy = "0.15.7" csv-async = { version = "1.2.6", features = ["tokio"] } From 3b500c75b669fdce0919d988f7cec57268379df2 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Mon, 28 Apr 2025 13:46:45 +0200 Subject: [PATCH 24/30] update sso examples --- examples/single sign on/docker-compose.yaml | 10 ---------- examples/single sign on/keycloak-configuration.json | 2 +- examples/single sign on/sqlpage/sqlpage.yaml | 3 +++ 3 files changed, 4 insertions(+), 11 deletions(-) create mode 100644 examples/single sign on/sqlpage/sqlpage.yaml diff --git a/examples/single sign on/docker-compose.yaml b/examples/single sign on/docker-compose.yaml index 3dae9715..5c372ff8 100644 --- a/examples/single sign on/docker-compose.yaml +++ b/examples/single sign on/docker-compose.yaml @@ -13,16 +13,6 @@ services: - .:/var/www - ./sqlpage:/etc/sqlpage environment: - # OIDC configuration - - SQLPAGE_HOST=localhost:8080 - - SQLPAGE_OIDC_ISSUER_URL=http://localhost:8181/realms/sqlpage_demo - - OIDC_AUTHORIZATION_ENDPOINT=http://localhost:8181/realms/sqlpage_demo/protocol/openid-connect/auth - - OIDC_TOKEN_ENDPOINT=http://localhost:8181/realms/sqlpage_demo/protocol/openid-connect/token - - OIDC_USERINFO_ENDPOINT=http://localhost:8181/realms/sqlpage_demo/protocol/openid-connect/userinfo - - OIDC_END_SESSION_ENDPOINT=http://localhost:8181/realms/sqlpage_demo/protocol/openid-connect/logout - - OIDC_CLIENT_ID=sqlpage - - OIDC_CLIENT_SECRET=qiawfnYrYzsmoaOZT28rRjPPRamfvrYr - # CAS (central authentication system) configuration # (you can ignore this if you're only using OpenID Connect) - CAS_ROOT_URL=http://localhost:8181/realms/sqlpage_demo/protocol/cas diff --git a/examples/single sign on/keycloak-configuration.json b/examples/single sign on/keycloak-configuration.json index 63188a69..34b56fcd 100644 --- a/examples/single sign on/keycloak-configuration.json +++ b/examples/single sign on/keycloak-configuration.json @@ -3597,7 +3597,7 @@ "alwaysDisplayInConsole": true, "clientAuthenticatorType": "client-secret", "secret": "qiawfnYrYzsmoaOZT28rRjPPRamfvrYr", - "redirectUris": ["http://localhost:8080/oidc_redirect_handler.sql"], + "redirectUris": ["http://localhost:8080/sqlpage/oidc_callback"], "webOrigins": ["http://localhost:8080"], "notBefore": 0, "bearerOnly": false, diff --git a/examples/single sign on/sqlpage/sqlpage.yaml b/examples/single sign on/sqlpage/sqlpage.yaml new file mode 100644 index 00000000..b2e42599 --- /dev/null +++ b/examples/single sign on/sqlpage/sqlpage.yaml @@ -0,0 +1,3 @@ +oidc_issuer_url: http://localhost:8181/realms/sqlpage_demo +oidc_client_id: sqlpage +oidc_client_secret: qiawfnYrYzsmoaOZT28rRjPPRamfvrYr # For a safer setup, use environment variables to store this From c040cc74ecb8114c93fca93e54a3e80317ec1170 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Mon, 28 Apr 2025 19:36:35 +0200 Subject: [PATCH 25/30] nonce verification - Improved error logging for invalid auth cookies and ID token verification. - Introduced nonce verification logic to ensure security during OIDC authentication. - Adjusted parameters for nonce hashing to optimize for short-lived tokens. --- src/webserver/oidc.rs | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 6b763d3d..39121cab 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -271,7 +271,7 @@ where return self.handle_unauthenticated_request(request); } Err(e) => { - log::error!("Found an invalid SQLPage auth cookie: {e}"); + log::error!("An auth cookie is present but could not be verified: {e:?}"); return self.handle_unauthenticated_request(request); } } @@ -379,14 +379,15 @@ fn get_sqlpage_auth_cookie( }; let cookie_value = cookie.value().to_string(); + let state = get_state_from_cookie(request)?; let verifier = oidc_client.id_token_verifier(); let id_token = CoreIdToken::from_str(&cookie_value) - .with_context(|| anyhow!("Invalid SQLPage auth cookie"))?; + .with_context(|| format!("Invalid SQLPage auth cookie: {cookie_value:?}"))?; - let nonce_verifier = |_: Option<&Nonce>| Ok(()); + let nonce_verifier = |nonce: Option<&Nonce>| check_nonce(nonce, &state.nonce); let claims: &IdTokenClaims = id_token .claims(&verifier, nonce_verifier) - .with_context(|| anyhow!("Invalid SQLPage auth cookie"))?; + .with_context(|| format!("Could not verify the ID token: {cookie_value:?}"))?; log::debug!("The current user is: {claims:?}"); Ok(Some(cookie_value)) } @@ -562,8 +563,8 @@ struct OidcLoginState { fn hash_nonce(nonce: &Nonce) -> String { use argon2::password_hash::{rand_core::OsRng, PasswordHasher, SaltString}; let salt = SaltString::generate(&mut OsRng); - // low-cost parameters - let params = argon2::Params::new(8, 1, 1, None).expect("bug: invalid Argon2 parameters"); + // low-cost parameters: oidc tokens are short-lived + let params = argon2::Params::new(8, 1, 1, Some(16)).expect("bug: invalid Argon2 parameters"); let argon2 = argon2::Argon2::new(argon2::Algorithm::Argon2id, argon2::Version::V0x13, params); let hash = argon2 .hash_password(nonce.secret().as_bytes(), &salt) @@ -571,6 +572,31 @@ fn hash_nonce(nonce: &Nonce) -> String { hash.to_string() } +fn check_nonce(id_token_nonce: Option<&Nonce>, state_nonce: &Nonce) -> Result<(), String> { + match id_token_nonce { + Some(id_token_nonce) => nonce_matches(id_token_nonce, state_nonce), + None => Err("No nonce found in the ID token".to_string()), + } +} + +fn nonce_matches(id_token_nonce: &Nonce, state_nonce: &Nonce) -> Result<(), String> { + log::debug!("Checking nonce: {} == {}", id_token_nonce.secret(), state_nonce.secret()); + let hash = argon2::password_hash::PasswordHash::new(&id_token_nonce.secret()).map_err(|e| { + format!( + "Failed to parse state nonce ({}): {e}", + id_token_nonce.secret() + ) + })?; + argon2::password_hash::PasswordVerifier::verify_password( + &argon2::Argon2::default(), + state_nonce.secret().as_bytes(), + &hash, + ) + .map_err(|e| format!("Failed to verify nonce ({}): {e}", state_nonce.secret()))?; + log::debug!("Nonce successfully verified"); + Ok(()) +} + impl OidcLoginState { fn new(request: &ServiceRequest, auth_url: AuthUrlParams) -> Self { Self { From 51c08d04350e2fd5927a99c25ef40c63c56cbb46 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 29 Apr 2025 23:36:09 +0200 Subject: [PATCH 26/30] Refactor OIDC logging and improve documentation - Updated logging statements for better clarity and context. - Refactored code for nonce verification and error handling. - Enhanced documentation in `app_config.rs` for clarity on `https_domain` usage. --- src/app_config.rs | 2 +- src/webserver/oidc.rs | 54 +++++++++++++++++-------------------------- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/src/app_config.rs b/src/app_config.rs index 188f2e51..462591aa 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -224,7 +224,7 @@ pub struct AppConfig { pub https_domain: Option, /// The hostname where your application is publicly accessible (e.g., "myapp.example.com"). - /// This is used for OIDC redirect URLs. If not set, https_domain will be used instead. + /// This is used for OIDC redirect URLs. If not set, `https_domain` will be used instead. pub host: Option, /// The email address to use when requesting a certificate from Let's Encrypt. diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 39121cab..fe06ca2d 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,10 +1,4 @@ -use std::{ - future::Future, - hash::{DefaultHasher, Hash, Hasher}, - pin::Pin, - str::FromStr, - sync::Arc, -}; +use std::{future::Future, pin::Pin, str::FromStr, sync::Arc}; use crate::{app_config::AppConfig, AppState}; use actix_web::{ @@ -23,7 +17,6 @@ use openidconnect::{ EndpointSet, IdTokenClaims, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, TokenResponse, }; -use password_hash::{rand_core::OsRng, SaltString}; use serde::{Deserialize, Serialize}; use super::http_client::make_http_client; @@ -83,10 +76,9 @@ fn get_app_host(config: &AppConfig) -> String { }; log::warn!( "No host or https_domain provided in the configuration, \ - using \"{}\" as the app host to build the redirect URL. \ + using \"{host}\" as the app host to build the redirect URL. \ This will only work locally. \ - Disable this warning by providing a value for the \"host\" setting.", - host + Disable this warning by providing a value for the \"host\" setting." ); host } @@ -125,11 +117,11 @@ async fn discover_provider_metadata( http_client: &AwcHttpClient, issuer_url: IssuerUrl, ) -> anyhow::Result { - log::debug!("Discovering provider metadata for {}", issuer_url); + log::debug!("Discovering provider metadata for {issuer_url}"); let provider_metadata = openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, http_client) .await - .with_context(|| format!("Failed to discover OIDC provider metadata"))?; + .with_context(|| "Failed to discover OIDC provider metadata".to_string())?; log::debug!("Provider metadata discovered: {provider_metadata:?}"); Ok(provider_metadata) } @@ -192,19 +184,6 @@ impl OidcService { }) } - fn build_auth_url(&self, request: &ServiceRequest) -> String { - let (auth_url, csrf_token, nonce) = self - .oidc_client - .authorize_url( - CoreAuthenticationFlow::AuthorizationCode, - CsrfToken::new_random, - Nonce::new_random, - ) - .add_scopes(self.config.scopes.iter().cloned()) - .url(); - auth_url.to_string() - } - fn handle_unauthenticated_request( &self, request: ServiceRequest, @@ -271,7 +250,13 @@ where return self.handle_unauthenticated_request(request); } Err(e) => { - log::error!("An auth cookie is present but could not be verified: {e:?}"); + log::debug!( + "{:?}", + e.context( + "An auth cookie is present but could not be verified. \ + Redirecting to OIDC provider to re-authenticate." + ) + ); return self.handle_unauthenticated_request(request); } } @@ -488,8 +473,7 @@ fn make_oidc_client( })?; let needs_http = match redirect_url.url().host() { Some(openidconnect::url::Host::Domain(domain)) => domain == "localhost", - Some(openidconnect::url::Host::Ipv4(_)) => true, - Some(openidconnect::url::Host::Ipv6(_)) => true, + Some(openidconnect::url::Host::Ipv4(_) | openidconnect::url::Host::Ipv6(_)) => true, None => false, }; if needs_http { @@ -563,7 +547,7 @@ struct OidcLoginState { fn hash_nonce(nonce: &Nonce) -> String { use argon2::password_hash::{rand_core::OsRng, PasswordHasher, SaltString}; let salt = SaltString::generate(&mut OsRng); - // low-cost parameters: oidc tokens are short-lived + // low-cost parameters: oidc tokens are short-lived and the source nonce is high-entropy let params = argon2::Params::new(8, 1, 1, Some(16)).expect("bug: invalid Argon2 parameters"); let argon2 = argon2::Argon2::new(argon2::Algorithm::Argon2id, argon2::Version::V0x13, params); let hash = argon2 @@ -580,8 +564,12 @@ fn check_nonce(id_token_nonce: Option<&Nonce>, state_nonce: &Nonce) -> Result<() } fn nonce_matches(id_token_nonce: &Nonce, state_nonce: &Nonce) -> Result<(), String> { - log::debug!("Checking nonce: {} == {}", id_token_nonce.secret(), state_nonce.secret()); - let hash = argon2::password_hash::PasswordHash::new(&id_token_nonce.secret()).map_err(|e| { + log::debug!( + "Checking nonce: {} == {}", + id_token_nonce.secret(), + state_nonce.secret() + ); + let hash = argon2::password_hash::PasswordHash::new(id_token_nonce.secret()).map_err(|e| { format!( "Failed to parse state nonce ({}): {e}", id_token_nonce.secret() @@ -623,5 +611,5 @@ fn get_state_from_cookie(request: &ServiceRequest) -> anyhow::Result Date: Tue, 29 Apr 2025 23:37:47 +0200 Subject: [PATCH 27/30] Remove unused app_state field from OidcService struct --- src/webserver/oidc.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index fe06ca2d..fa2d4aba 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -159,7 +159,6 @@ where #[derive(Clone)] pub struct OidcService { service: S, - app_state: web::Data, config: Arc, oidc_client: Arc, http_client: Arc, @@ -177,7 +176,6 @@ impl OidcService { let client: OidcClient = make_oidc_client(&config, provider_metadata)?; Ok(Self { service, - app_state: web::Data::clone(app_state), config, oidc_client: Arc::new(client), http_client: Arc::new(http_client), From 5ad010bd63b784dfe6860c521038f8a261c599a5 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Wed, 30 Apr 2025 00:20:19 +0200 Subject: [PATCH 28/30] Enhance OIDC client error handling and refactor HTTP request types - Added context to OIDC client creation error handling. - Updated HTTP request and response types for better integration with the openidconnect library. - Introduced AwcWrapperError for improved error management in HTTP calls. --- src/webserver/oidc.rs | 46 ++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index fa2d4aba..1bb95285 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -173,7 +173,8 @@ impl OidcService { let issuer_url = config.issuer_url.clone(); let http_client = AwcHttpClient::new(&app_state.config)?; let provider_metadata = discover_provider_metadata(&http_client, issuer_url).await?; - let client: OidcClient = make_oidc_client(&config, provider_metadata)?; + let client: OidcClient = make_oidc_client(&config, provider_metadata) + .with_context(|| format!("Unable to create OIDC client with config: {config:?}"))?; Ok(Self { service, config, @@ -388,24 +389,23 @@ impl AwcHttpClient { } impl<'c> AsyncHttpClient<'c> for AwcHttpClient { - type Error = StringError; - type Future = Pin< - Box>, Self::Error>> + 'c>, - >; + type Error = AwcWrapperError; + type Future = + Pin> + 'c>>; - fn call(&'c self, request: openidconnect::http::Request>) -> Self::Future { + fn call(&'c self, request: openidconnect::HttpRequest) -> Self::Future { let client = self.client.clone(); Box::pin(async move { execute_oidc_request_with_awc(client, request) .await - .map_err(|err| StringError(format!("Failed to execute OIDC request: {err:?}"))) + .map_err(|err| AwcWrapperError(err)) }) } } async fn execute_oidc_request_with_awc( client: Client, - request: openidconnect::http::Request>, + request: openidconnect::HttpRequest, ) -> Result>, anyhow::Error> { let awc_method = awc::http::Method::from_bytes(request.method().as_str().as_bytes())?; let awc_uri = awc::http::Uri::from_str(&request.uri().to_string())?; @@ -414,30 +414,36 @@ async fn execute_oidc_request_with_awc( for (name, value) in request.headers() { req = req.insert_header((name.as_str(), value.to_str()?)); } - let mut response = req - .send_body(request.into_body()) - .await - .map_err(|e| anyhow!("{:?}", e))?; + let (req_head, body) = request.into_parts(); + let mut response = req.send_body(body).await.map_err(|e| { + anyhow!(e.to_string()).context(format!( + "Failed to send request: {} {}", + &req_head.method, &req_head.uri + )) + })?; let head = response.headers(); let mut resp_builder = openidconnect::http::Response::builder().status(response.status().as_u16()); for (name, value) in head { resp_builder = resp_builder.header(name.as_str(), value.to_str()?); } - let body = response.body().await?.to_vec(); + let body = response + .body() + .await + .with_context(|| format!("Couldnt read from {}", &req_head.uri))?; log::debug!( "Received OIDC response with status {}: {}", response.status(), String::from_utf8_lossy(&body) ); - let resp = resp_builder.body(body)?; + let resp = resp_builder.body(body.to_vec())?; Ok(resp) } -#[derive(Debug, PartialEq, Eq)] -pub struct StringError(String); +#[derive(Debug)] +pub struct AwcWrapperError(anyhow::Error); -impl std::fmt::Display for StringError { +impl std::fmt::Display for AwcWrapperError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { std::fmt::Display::fmt(&self.0, f) } @@ -450,7 +456,11 @@ type OidcClient = openidconnect::core::CoreClient< EndpointMaybeSet, EndpointMaybeSet, >; -impl std::error::Error for StringError {} +impl std::error::Error for AwcWrapperError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.0.source() + } +} fn make_oidc_client( config: &Arc, From e18d03f5b2ef5b9965f6fa35207e1a8dadff8d14 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Wed, 30 Apr 2025 00:22:18 +0200 Subject: [PATCH 29/30] clippy fixes - Changed http_client from Arc to Rc in OidcService for improved memory efficiency. - Updated related code to reflect the new ownership model for the HTTP client. --- src/webserver/oidc.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 1bb95285..1c6c016a 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,4 +1,4 @@ -use std::{future::Future, pin::Pin, str::FromStr, sync::Arc}; +use std::{future::Future, pin::Pin, rc::Rc, str::FromStr, sync::Arc}; use crate::{app_config::AppConfig, AppState}; use actix_web::{ @@ -161,7 +161,7 @@ pub struct OidcService { service: S, config: Arc, oidc_client: Arc, - http_client: Arc, + http_client: Rc, } impl OidcService { @@ -179,7 +179,7 @@ impl OidcService { service, config, oidc_client: Arc::new(client), - http_client: Arc::new(http_client), + http_client: Rc::new(http_client), }) } @@ -205,7 +205,7 @@ impl OidcService { request: ServiceRequest, ) -> LocalBoxFuture, Error>> { let oidc_client = Arc::clone(&self.oidc_client); - let http_client = Arc::clone(&self.http_client); + let http_client = Rc::clone(&self.http_client); let oidc_config = Arc::clone(&self.config); Box::pin(async move { @@ -268,8 +268,8 @@ where } async fn process_oidc_callback( - oidc_client: &Arc, - http_client: &Arc, + oidc_client: &OidcClient, + http_client: &AwcHttpClient, query_string: &str, request: &ServiceRequest, ) -> anyhow::Result { @@ -398,7 +398,7 @@ impl<'c> AsyncHttpClient<'c> for AwcHttpClient { Box::pin(async move { execute_oidc_request_with_awc(client, request) .await - .map_err(|err| AwcWrapperError(err)) + .map_err(AwcWrapperError) }) } } From 7f20cd2a44b0de220f3184a4cfd11d8f9e478c56 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Wed, 30 Apr 2025 01:45:43 +0200 Subject: [PATCH 30/30] initialize the oidc and http clients only once - Added OidcState struct to encapsulate OIDC configuration and client. - Refactored OidcMiddleware to utilize OidcState for improved state management. - Updated HTTP client handling in OIDC service methods for better integration with app data. - Enhanced logging for OIDC middleware initialization and request processing. --- src/lib.rs | 7 ++ src/template_helpers.rs | 6 +- src/webserver/http.rs | 2 + src/webserver/http_client.rs | 15 +++- src/webserver/mod.rs | 2 +- src/webserver/oidc.rs | 161 +++++++++++++++++------------------ 6 files changed, 103 insertions(+), 90 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f3679f4b..3d0efe57 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,8 +83,10 @@ pub mod webserver; use crate::app_config::AppConfig; use crate::filesystem::FileSystem; use crate::webserver::database::ParsedSqlFile; +use crate::webserver::oidc::OidcState; use file_cache::FileCache; use std::path::{Path, PathBuf}; +use std::sync::Arc; use templates::AllTemplates; use webserver::Database; @@ -102,6 +104,7 @@ pub struct AppState { sql_file_cache: FileCache, file_system: FileSystem, config: AppConfig, + pub oidc_state: Option>, } impl AppState { @@ -117,12 +120,16 @@ impl AppState { PathBuf::from("index.sql"), ParsedSqlFile::new(&db, include_str!("../index.sql"), Path::new("index.sql")), ); + + let oidc_state = crate::webserver::oidc::initialize_oidc_state(config).await?; + Ok(AppState { db, all_templates, sql_file_cache, file_system, config: config.clone(), + oidc_state, }) } } diff --git a/src/template_helpers.rs b/src/template_helpers.rs index 2896afb4..76bb8f77 100644 --- a/src/template_helpers.rs +++ b/src/template_helpers.rs @@ -628,15 +628,15 @@ mod tests { const ESCAPED_UNSAFE_MARKUP: &str = "<table><tr><td>"; #[test] fn test_html_blocks_with_various_settings() { - let helper = MarkdownHelper::default(); - let content = contents(); - struct TestCase { name: &'static str, preset: Option, expected_output: Result<&'static str, String>, } + let helper = MarkdownHelper::default(); + let content = contents(); + let test_cases = [ TestCase { name: "default settings", diff --git a/src/webserver/http.rs b/src/webserver/http.rs index 7885eea6..19e5c7a5 100644 --- a/src/webserver/http.rs +++ b/src/webserver/http.rs @@ -19,6 +19,7 @@ use actix_web::{ }; use actix_web::{HttpResponseBuilder, ResponseError}; +use super::http_client::make_http_client; use super::https::make_auto_rustls_config; use super::oidc::OidcMiddleware; use super::response_writer::ResponseWriter; @@ -478,6 +479,7 @@ pub fn create_app( middleware::TrailingSlash::MergeOnly, )) .app_data(payload_config(&app_state)) + .app_data(make_http_client(&app_state.config)) .app_data(form_config(&app_state)) .app_data(app_state) } diff --git a/src/webserver/http_client.rs b/src/webserver/http_client.rs index e2cee885..c8a43701 100644 --- a/src/webserver/http_client.rs +++ b/src/webserver/http_client.rs @@ -1,3 +1,4 @@ +use actix_web::dev::ServiceRequest; use anyhow::{anyhow, Context}; use std::sync::OnceLock; @@ -10,7 +11,7 @@ pub fn make_http_client(config: &crate::app_config::AppConfig) -> anyhow::Result log::debug!("Loading native certificates because system_root_ca_certificates is enabled"); let certs = rustls_native_certs::load_native_certs() .with_context(|| "Initial native certificates load failed")?; - log::info!("Loaded {} native certificates", certs.len()); + log::debug!("Loaded {} native HTTPS client certificates", certs.len()); let mut roots = rustls::RootCertStore::empty(); for cert in certs { log::trace!("Adding native certificate to root store: {cert:?}"); @@ -43,3 +44,15 @@ pub fn make_http_client(config: &crate::app_config::AppConfig) -> anyhow::Result log::debug!("Created HTTP client"); Ok(client) } + +pub(crate) fn get_http_client_from_appdata( + request: &ServiceRequest, +) -> anyhow::Result<&awc::Client> { + if let Some(result) = request.app_data::>() { + result + .as_ref() + .map_err(|e| anyhow!("HTTP client initialization failed: {e}")) + } else { + Err(anyhow!("HTTP client not found in app data")) + } +} diff --git a/src/webserver/mod.rs b/src/webserver/mod.rs index 67cdf25f..4de28ead 100644 --- a/src/webserver/mod.rs +++ b/src/webserver/mod.rs @@ -43,7 +43,7 @@ pub use error_with_status::ErrorWithStatus; pub use database::make_placeholder; pub use database::migrations::apply; -mod oidc; +pub mod oidc; pub mod response_writer; pub mod routing; mod static_content; diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 1c6c016a..20cbff33 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -1,7 +1,10 @@ -use std::{future::Future, pin::Pin, rc::Rc, str::FromStr, sync::Arc}; +use std::future::ready; +use std::{future::Future, pin::Pin, str::FromStr, sync::Arc}; +use crate::webserver::http_client::get_http_client_from_appdata; use crate::{app_config::AppConfig, AppState}; use actix_web::{ + body::BoxBody, cookie::Cookie, dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, middleware::Condition, @@ -21,6 +24,8 @@ use serde::{Deserialize, Serialize}; use super::http_client::make_http_client; +type LocalBoxFuture = Pin + 'static>>; + const SQLPAGE_AUTH_COOKIE_NAME: &str = "sqlpage_auth"; const SQLPAGE_REDIRECT_URI: &str = "/sqlpage/oidc_callback"; const SQLPAGE_STATE_COOKIE_NAME: &str = "sqlpage_oidc_state"; @@ -83,45 +88,54 @@ fn get_app_host(config: &AppConfig) -> String { host } +pub struct OidcState { + pub config: Arc, + pub client: Arc, +} + +pub async fn initialize_oidc_state( + app_config: &AppConfig, +) -> anyhow::Result>> { + let oidc_cfg = match OidcConfig::try_from(app_config) { + Ok(c) => Arc::new(c), + Err(None) => return Ok(None), // OIDC not configured + Err(Some(e)) => return Err(anyhow::anyhow!(e)), + }; + + let http_client = make_http_client(app_config)?; + let provider_metadata = + discover_provider_metadata(&http_client, oidc_cfg.issuer_url.clone()).await?; + let client = make_oidc_client(&oidc_cfg, provider_metadata)?; + + Ok(Some(Arc::new(OidcState { + config: oidc_cfg, + client: Arc::new(client), + }))) +} + pub struct OidcMiddleware { - pub config: Option>, - app_state: web::Data, + oidc_state: Option>, } impl OidcMiddleware { + #[must_use] pub fn new(app_state: &web::Data) -> Condition { - let config = OidcConfig::try_from(&app_state.config); - match &config { - Ok(config) => { - log::debug!("Setting up OIDC with issuer: {}", config.issuer_url); - } - Err(Some(err)) => { - log::error!("Invalid OIDC configuration: {err}"); - } - Err(None) => { - log::debug!("No OIDC configuration provided, skipping middleware."); - } - } - let config = config.ok().map(Arc::new); - Condition::new( - config.is_some(), - Self { - config, - app_state: web::Data::clone(app_state), - }, - ) + let oidc_state = app_state.oidc_state.clone(); + Condition::new(oidc_state.is_some(), Self { oidc_state }) } } async fn discover_provider_metadata( - http_client: &AwcHttpClient, + http_client: &awc::Client, issuer_url: IssuerUrl, ) -> anyhow::Result { log::debug!("Discovering provider metadata for {issuer_url}"); - let provider_metadata = - openidconnect::core::CoreProviderMetadata::discover_async(issuer_url, http_client) - .await - .with_context(|| "Failed to discover OIDC provider metadata".to_string())?; + let provider_metadata = openidconnect::core::CoreProviderMetadata::discover_async( + issuer_url, + &AwcHttpClient::from_client(http_client), + ) + .await + .with_context(|| "Failed to discover OIDC provider metadata".to_string())?; log::debug!("Provider metadata discovered: {provider_metadata:?}"); Ok(provider_metadata) } @@ -135,52 +149,28 @@ where type Error = Error; type InitError = (); type Transform = OidcService; - type Future = Pin> + 'static>>; + type Future = std::future::Ready>; fn new_transform(&self, service: S) -> Self::Future { - let config = self.config.clone(); - let app_state = web::Data::clone(&self.app_state); - Box::pin(async move { - match config { - Some(config) => Ok(OidcService::new(service, &app_state, Arc::clone(&config)) - .await - .map_err(|err| { - log::error!( - "Error creating OIDC service with issuer: {}: {err:?}", - config.issuer_url - ); - })?), - None => Err(()), - } - }) + match &self.oidc_state { + Some(state) => ready(Ok(OidcService::new(service, Arc::clone(state)))), + None => ready(Err(())), + } } } #[derive(Clone)] pub struct OidcService { service: S, - config: Arc, - oidc_client: Arc, - http_client: Rc, + oidc_state: Arc, } impl OidcService { - pub async fn new( - service: S, - app_state: &web::Data, - config: Arc, - ) -> anyhow::Result { - let issuer_url = config.issuer_url.clone(); - let http_client = AwcHttpClient::new(&app_state.config)?; - let provider_metadata = discover_provider_metadata(&http_client, issuer_url).await?; - let client: OidcClient = make_oidc_client(&config, provider_metadata) - .with_context(|| format!("Unable to create OIDC client with config: {config:?}"))?; - Ok(Self { + pub fn new(service: S, oidc_state: Arc) -> Self { + Self { service, - config, - oidc_client: Arc::new(client), - http_client: Rc::new(http_client), - }) + oidc_state, + } } fn handle_unauthenticated_request( @@ -195,8 +185,11 @@ impl OidcService { log::debug!("Redirecting to OIDC provider"); - let response = - build_auth_provider_redirect_response(&self.oidc_client, &self.config, &request); + let response = build_auth_provider_redirect_response( + &self.oidc_state.client, + &self.oidc_state.config, + &request, + ); Box::pin(async move { Ok(request.into_response(response)) }) } @@ -204,13 +197,12 @@ impl OidcService { &self, request: ServiceRequest, ) -> LocalBoxFuture, Error>> { - let oidc_client = Arc::clone(&self.oidc_client); - let http_client = Rc::clone(&self.http_client); - let oidc_config = Arc::clone(&self.config); + let oidc_client = Arc::clone(&self.oidc_state.client); + let oidc_config = Arc::clone(&self.oidc_state.config); Box::pin(async move { let query_string = request.query_string(); - match process_oidc_callback(&oidc_client, &http_client, query_string, &request).await { + match process_oidc_callback(&oidc_client, query_string, &request).await { Ok(response) => Ok(request.into_response(response)), Err(e) => { log::error!("Failed to process OIDC callback with params {query_string}: {e}"); @@ -223,9 +215,6 @@ impl OidcService { } } -type LocalBoxFuture = Pin + 'static>>; -use actix_web::body::BoxBody; - impl Service for OidcService where S: Service, Error = Error>, @@ -238,8 +227,11 @@ where forward_ready!(service); fn call(&self, request: ServiceRequest) -> Self::Future { - log::debug!("Started OIDC middleware with config: {:?}", self.config); - let oidc_client = Arc::clone(&self.oidc_client); + log::debug!( + "Started OIDC middleware with config: {:?}", + self.oidc_state.config + ); + let oidc_client = Arc::clone(&self.oidc_state.client); match get_sqlpage_auth_cookie(&oidc_client, &request) { Ok(Some(cookie)) => { log::trace!("Found SQLPage auth cookie: {cookie}"); @@ -269,10 +261,11 @@ where async fn process_oidc_callback( oidc_client: &OidcClient, - http_client: &AwcHttpClient, query_string: &str, request: &ServiceRequest, ) -> anyhow::Result { + let http_client = get_http_client_from_appdata(request)?; + let state = get_state_from_cookie(request)?; let params = Query::::from_query(query_string) @@ -299,15 +292,14 @@ async fn process_oidc_callback( async fn exchange_code_for_token( oidc_client: &OidcClient, - http_client: &AwcHttpClient, + http_client: &awc::Client, oidc_callback_params: OidcCallbackParams, ) -> anyhow::Result { - // TODO: Verify the state matches the expected CSRF token let token_response = oidc_client .exchange_code(openidconnect::AuthorizationCode::new( oidc_callback_params.code, ))? - .request_async(http_client) + .request_async(&AwcHttpClient::from_client(http_client)) .await?; Ok(token_response) } @@ -376,19 +368,18 @@ fn get_sqlpage_auth_cookie( Ok(Some(cookie_value)) } -pub struct AwcHttpClient { - client: Client, +pub struct AwcHttpClient<'c> { + client: &'c awc::Client, } -impl AwcHttpClient { - pub fn new(app_config: &AppConfig) -> anyhow::Result { - Ok(Self { - client: make_http_client(app_config)?, - }) +impl<'c> AwcHttpClient<'c> { + #[must_use] + pub fn from_client(client: &'c awc::Client) -> Self { + Self { client } } } -impl<'c> AsyncHttpClient<'c> for AwcHttpClient { +impl<'c> AsyncHttpClient<'c> for AwcHttpClient<'c> { type Error = AwcWrapperError; type Future = Pin> + 'c>>;