From 177ce9dd45acf8aaf992c9c73edcf85432f424db Mon Sep 17 00:00:00 2001 From: mbecker20 Date: Tue, 4 Mar 2025 21:58:42 -0800 Subject: [PATCH] update openidconnect dependency, and use reqwest rustls-tls-native-roots --- Cargo.lock | 121 +++++++------------------------ Cargo.toml | 4 +- bin/core/src/auth/oidc/client.rs | 32 ++++++-- bin/core/src/auth/oidc/mod.rs | 55 +++++--------- 4 files changed, 73 insertions(+), 139 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b071287e5..9e994f1f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -540,7 +540,7 @@ dependencies = [ "serde_path_to_error", "serde_urlencoded", "sha1", - "sync_wrapper 1.0.2", + "sync_wrapper", "tokio", "tokio-tungstenite", "tower 0.5.2", @@ -563,7 +563,7 @@ dependencies = [ "mime", "pin-project-lite", "rustversion", - "sync_wrapper 1.0.2", + "sync_wrapper", "tower-layer", "tower-service", "tracing", @@ -2479,7 +2479,7 @@ dependencies = [ "futures", "mongo_indexed", "partial_derive2", - "reqwest 0.12.12", + "reqwest", "resolver_api", "serde", "serde_json", @@ -2533,7 +2533,7 @@ dependencies = [ "periphery_client", "rand 0.9.0", "regex", - "reqwest 0.12.12", + "reqwest", "resolver_api", "response", "rustls 0.23.23", @@ -3034,16 +3034,16 @@ dependencies = [ [[package]] name = "oauth2" -version = "4.4.2" +version = "5.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c38841cdd844847e3e7c8d29cef9dcfed8877f8f56f9071f77843ecf3baf937f" +checksum = "51e219e79014df21a225b1860a479e2dcd7cbd9130f4defd4bd0e191ea31d67d" dependencies = [ - "base64 0.13.1", + "base64 0.22.1", "chrono", "getrandom 0.2.15", - "http 0.2.12", + "http 1.1.0", "rand 0.8.5", - "reqwest 0.11.27", + "reqwest", "serde", "serde_json", "serde_path_to_error", @@ -3078,7 +3078,7 @@ dependencies = [ "parse_link_header", "pem", "percent-encoding", - "reqwest 0.12.12", + "reqwest", "reqwest-conditional-middleware", "reqwest-middleware", "reqwest-retry", @@ -3102,16 +3102,16 @@ checksum = "1261fe7e33c73b354eab43b1273a57c8f967d0391e80353e51f764ac02cf6775" [[package]] name = "openidconnect" -version = "3.5.0" +version = "4.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f47e80a9cfae4462dd29c41e987edd228971d6565553fbc14b8a11e666d91590" +checksum = "6dd50d4a5e7730e754f94d977efe61f611aadd3131f6a2b464f6e3a4167e8ef7" dependencies = [ - "base64 0.13.1", + "base64 0.21.7", "chrono", "dyn-clone", "ed25519-dalek", "hmac", - "http 0.2.12", + "http 1.1.0", "itertools 0.10.5", "log", "oauth2", @@ -3121,7 +3121,6 @@ dependencies = [ "rsa", "serde", "serde-value", - "serde_derive", "serde_json", "serde_path_to_error", "serde_plain", @@ -3162,7 +3161,7 @@ dependencies = [ "bytes", "http 1.1.0", "opentelemetry", - "reqwest 0.12.12", + "reqwest", "tracing", ] @@ -3180,7 +3179,7 @@ dependencies = [ "opentelemetry-proto", "opentelemetry_sdk", "prost", - "reqwest 0.12.12", + "reqwest", "thiserror 2.0.12", "tonic", "tracing", @@ -3404,7 +3403,7 @@ version = "1.17.0-dev-3" dependencies = [ "anyhow", "komodo_client", - "reqwest 0.12.12", + "reqwest", "resolver_api", "serde", "serde_json", @@ -3737,47 +3736,6 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" -[[package]] -name = "reqwest" -version = "0.11.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd67538700a17451e7cba03ac727fb961abb7607553461627b97de0b89cf4a62" -dependencies = [ - "base64 0.21.7", - "bytes", - "encoding_rs", - "futures-core", - "futures-util", - "h2 0.3.26", - "http 0.2.12", - "http-body 0.4.6", - "hyper 0.14.31", - "hyper-rustls 0.24.2", - "ipnet", - "js-sys", - "log", - "mime", - "once_cell", - "percent-encoding", - "pin-project-lite", - "rustls 0.21.12", - "rustls-pemfile 1.0.4", - "serde", - "serde_json", - "serde_urlencoded", - "sync_wrapper 0.1.2", - "system-configuration 0.5.1", - "tokio", - "tokio-rustls 0.24.1", - "tower-service", - "url", - "wasm-bindgen", - "wasm-bindgen-futures", - "web-sys", - "webpki-roots 0.25.4", - "winreg", -] - [[package]] name = "reqwest" version = "0.12.12" @@ -3813,8 +3771,8 @@ dependencies = [ "serde", "serde_json", "serde_urlencoded", - "sync_wrapper 1.0.2", - "system-configuration 0.6.1", + "sync_wrapper", + "system-configuration", "tokio", "tokio-rustls 0.26.0", "tower 0.5.2", @@ -3835,7 +3793,7 @@ checksum = "f67ad7fdf5c0a015763fcd164bee294b13fb7b6f89f1b55961d40f00c3e32d6b" dependencies = [ "async-trait", "http 1.1.0", - "reqwest 0.12.12", + "reqwest", "reqwest-middleware", ] @@ -3848,7 +3806,7 @@ dependencies = [ "anyhow", "async-trait", "http 1.1.0", - "reqwest 0.12.12", + "reqwest", "serde", "thiserror 1.0.69", "tower-service", @@ -3867,7 +3825,7 @@ dependencies = [ "http 1.1.0", "hyper 1.5.1", "parking_lot 0.11.2", - "reqwest 0.12.12", + "reqwest", "reqwest-middleware", "retry-policies", "thiserror 1.0.69", @@ -3887,7 +3845,7 @@ dependencies = [ "getrandom 0.2.15", "http 1.1.0", "matchit", - "reqwest 0.12.12", + "reqwest", "reqwest-middleware", "tracing", ] @@ -4547,7 +4505,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d7351d61890f030c76e0bebcc203befb73a84cf1888db96597d2f1f6144b693" dependencies = [ "anyhow", - "reqwest 0.12.12", + "reqwest", "serde", "serde_derive", "serde_json", @@ -4694,12 +4652,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "sync_wrapper" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" - [[package]] name = "sync_wrapper" version = "1.0.2" @@ -4734,17 +4686,6 @@ dependencies = [ "windows", ] -[[package]] -name = "system-configuration" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba3a3adc5c275d719af8cb4272ea1c4a6d668a777f37e115f6d11ddbc1c8e0e7" -dependencies = [ - "bitflags 1.3.2", - "core-foundation 0.9.4", - "system-configuration-sys 0.5.0", -] - [[package]] name = "system-configuration" version = "0.6.1" @@ -4753,17 +4694,7 @@ checksum = "3c879d448e9d986b661742763247d3693ed13609438cf3d006f51f5368a5ba6b" dependencies = [ "bitflags 2.6.0", "core-foundation 0.9.4", - "system-configuration-sys 0.6.0", -] - -[[package]] -name = "system-configuration-sys" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a75fb188eb626b924683e3b95e3a48e63551fcfb51949de2f06a9d91dbee93c9" -dependencies = [ - "core-foundation-sys", - "libc", + "system-configuration-sys", ] [[package]] @@ -5094,7 +5025,7 @@ dependencies = [ "futures-core", "futures-util", "pin-project-lite", - "sync_wrapper 1.0.2", + "sync_wrapper", "tokio", "tower-layer", "tower-service", diff --git a/Cargo.toml b/Cargo.toml index 00f3971d6..511d7d3a6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,7 +44,7 @@ mungos = "3.2.0" svi = "1.0.1" # ASYNC -reqwest = { version = "0.12.12", default-features = false, features = ["json", "rustls-tls"] } +reqwest = { version = "0.12.12", default-features = false, features = ["json", "rustls-tls-native-roots"] } tokio = { version = "1.43.0", features = ["full"] } tokio-util = "0.7.13" futures = "0.3.31" @@ -85,7 +85,7 @@ envy = "0.4.2" # CRYPTO / AUTH uuid = { version = "1.15.1", features = ["v4", "fast-rng", "serde"] } -openidconnect = "3.5.0" +openidconnect = "4.0.0" urlencoding = "2.1.3" nom_pem = "4.0.0" bcrypt = "0.17.0" diff --git a/bin/core/src/auth/oidc/client.rs b/bin/core/src/auth/oidc/client.rs index 140036b42..88a1ff23d 100644 --- a/bin/core/src/auth/oidc/client.rs +++ b/bin/core/src/auth/oidc/client.rs @@ -2,17 +2,37 @@ use std::sync::OnceLock; use anyhow::Context; use openidconnect::{ - core::{CoreClient, CoreProviderMetadata}, - reqwest::async_http_client, - ClientId, ClientSecret, IssuerUrl, RedirectUrl, + core::*, Client, ClientId, ClientSecret, EmptyAdditionalClaims, + EndpointMaybeSet, EndpointNotSet, EndpointSet, IssuerUrl, + RedirectUrl, StandardErrorResponse, }; use crate::config::core_config; -static DEFAULT_OIDC_CLIENT: OnceLock> = +type OidcClient = Client< + EmptyAdditionalClaims, + CoreAuthDisplay, + CoreGenderClaim, + CoreJweContentEncryptionAlgorithm, + CoreJsonWebKey, + CoreAuthPrompt, + StandardErrorResponse, + CoreTokenResponse, + CoreTokenIntrospectionResponse, + CoreRevocableToken, + CoreRevocationErrorResponse, + EndpointSet, + EndpointNotSet, + EndpointNotSet, + EndpointNotSet, + EndpointMaybeSet, + EndpointMaybeSet, +>; + +static DEFAULT_OIDC_CLIENT: OnceLock> = OnceLock::new(); -pub fn default_oidc_client() -> Option<&'static CoreClient> { +pub fn default_oidc_client() -> Option<&'static OidcClient> { DEFAULT_OIDC_CLIENT .get() .expect("OIDC client get before init") @@ -35,7 +55,7 @@ pub async fn init_default_oidc_client() { // Use OpenID Connect Discovery to fetch the provider metadata. let provider_metadata = CoreProviderMetadata::discover_async( IssuerUrl::new(config.oidc_provider.clone())?, - async_http_client, + super::reqwest_client(), ) .await .context( diff --git a/bin/core/src/auth/oidc/mod.rs b/bin/core/src/auth/oidc/mod.rs index 2381e27c2..344b2407f 100644 --- a/bin/core/src/auth/oidc/mod.rs +++ b/bin/core/src/auth/oidc/mod.rs @@ -12,9 +12,8 @@ use komodo_client::entities::{ }; use mungos::mongodb::bson::{doc, Document}; use openidconnect::{ - core::CoreAuthenticationFlow, AccessTokenHash, AuthorizationCode, - CsrfToken, Nonce, OAuth2TokenResponse, PkceCodeChallenge, - PkceCodeVerifier, Scope, TokenResponse, + core::CoreAuthenticationFlow, AuthorizationCode, CsrfToken, Nonce, + Scope, TokenResponse, }; use reqwest::StatusCode; use serde::Deserialize; @@ -29,13 +28,22 @@ use super::RedirectQuery; pub mod client; +fn reqwest_client() -> &'static reqwest::Client { + static REQWEST: OnceLock = OnceLock::new(); + REQWEST.get_or_init(|| { + reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .expect("Invalid OIDC reqwest client") + }) +} + /// CSRF tokens can only be used once from the callback, /// and must be used within this timeframe const CSRF_VALID_FOR_MS: i64 = 120_000; // 2 minutes for user to log in. type RedirectUrl = Option; -type CsrfMap = - DashMap; +type CsrfMap = DashMap; fn csrf_verifier_tokens() -> &'static CsrfMap { static CSRF: OnceLock = OnceLock::new(); CSRF.get_or_init(Default::default) @@ -64,10 +72,6 @@ async fn login( let client = default_oidc_client().context("OIDC Client not configured")?; - // Generate a PKCE challenge. - let (pkce_challenge, pkce_verifier) = - PkceCodeChallenge::new_random_sha256(); - // Generate the authorization URL. let (auth_url, csrf_token, nonce) = client .authorize_url( @@ -77,18 +81,12 @@ async fn login( ) .add_scope(Scope::new("openid".to_string())) .add_scope(Scope::new("email".to_string())) - .set_pkce_challenge(pkce_challenge) .url(); // Data inserted here will be matched on callback side for csrf protection. csrf_verifier_tokens().insert( csrf_token.secret().clone(), - ( - pkce_verifier, - nonce, - redirect, - komodo_timestamp() + CSRF_VALID_FOR_MS, - ), + (nonce, redirect, komodo_timestamp() + CSRF_VALID_FOR_MS), ); let config = core_config(); @@ -135,10 +133,9 @@ async fn callback( query.state.context("Provider did not return state")?, ); - let (_, (pkce_verifier, nonce, redirect, valid_until)) = - csrf_verifier_tokens() - .remove(state.secret()) - .context("CSRF Token invalid")?; + let (_, (nonce, redirect, valid_until)) = csrf_verifier_tokens() + .remove(state.secret()) + .context("CSRF Token invalid")?; if komodo_timestamp() > valid_until { return Err(anyhow!( @@ -148,9 +145,8 @@ async fn callback( let token_response = client .exchange_code(AuthorizationCode::new(code)) - // Set the PKCE code verifier. - .set_pkce_verifier(pkce_verifier) - .request_async(openidconnect::reqwest::async_http_client) + .context("Failed to get Oauth token at exchange code")? + .request_async(reqwest_client()) .await .context("Failed to get Oauth token")?; @@ -175,19 +171,6 @@ async fn callback( .claims(&verifier, &nonce) .context("Failed to verify token claims")?; - // Verify the access token hash to ensure that the access token hasn't been substituted for - // another user's. - if let Some(expected_access_token_hash) = claims.access_token_hash() - { - let actual_access_token_hash = AccessTokenHash::from_token( - token_response.access_token(), - &id_token.signing_alg()?, - )?; - if actual_access_token_hash != *expected_access_token_hash { - return Err(anyhow!("Invalid access token")); - } - } - let user_id = claims.subject().as_str(); let db_client = db_client();