From bdd23da034721fc0d82161c06edf2d9d044a17f1 Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Thu, 8 Oct 2026 16:53:17 +0800 Subject: [PATCH] refactor(iam): separate federated provider views (#8415) --- crates/iam/src/federation/mod.rs | 2 +- crates/iam/src/federation/oidc/config.rs | 39 ++- crates/iam/src/federation/oidc/discovery.rs | 50 ++- crates/iam/src/federation/oidc/flow.rs | 26 +- crates/iam/src/federation/oidc/mod.rs | 1 + crates/iam/src/federation/provider.rs | 52 ++- crates/iam/src/federation/transaction.rs | 49 +-- crates/iam/src/lib.rs | 2 +- crates/iam/src/oidc.rs | 195 +++++++----- rustfs/src/admin/console.rs | 20 +- rustfs/src/admin/handlers/idp_compat.rs | 6 +- rustfs/src/admin/handlers/oidc.rs | 99 +++--- rustfs/src/admin/handlers/site_replication.rs | 299 +++++++++++++++--- rustfs/src/admin/runtime_sources.rs | 11 +- rustfs/src/app/context.rs | 130 ++++++-- rustfs/src/app/context/global.rs | 15 +- rustfs/src/app/context/handles.rs | 152 +++++++-- rustfs/src/app/context/interfaces.rs | 22 +- rustfs/src/runtime_sources.rs | 46 ++- rustfs/src/startup_auth.rs | 23 +- 20 files changed, 930 insertions(+), 309 deletions(-) diff --git a/crates/iam/src/federation/mod.rs b/crates/iam/src/federation/mod.rs index 59e85d0f4..a80956312 100644 --- a/crates/iam/src/federation/mod.rs +++ b/crates/iam/src/federation/mod.rs @@ -26,6 +26,6 @@ pub use model::{ FederatedAuthorization, FederatedClaims, FederatedCodeExchange, FederatedLoginSession, FederatedProviderRef, FederatedSession, FederatedSessionTransaction, OIDC_VIRTUAL_PARENT_CLAIM, VerifiedFederatedIdentity, }; -pub use provider::FederatedIdentityProvider; +pub use provider::{FederatedIdentityProvider, FederatedProviderView, FederatedRedirectPolicy}; pub use registry::FederatedIdentityRegistry; pub use transaction::FederatedIdentityService; diff --git a/crates/iam/src/federation/oidc/config.rs b/crates/iam/src/federation/oidc/config.rs index 26287b521..25b80f9d9 100644 --- a/crates/iam/src/federation/oidc/config.rs +++ b/crates/iam/src/federation/oidc/config.rs @@ -12,14 +12,28 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::oidc::{OidcProviderConfig, OidcSys}; +use crate::{ + federation::FederatedRedirectPolicy, + oidc::{OidcConfigSnapshot, OidcSys}, +}; -pub(super) fn provider_config<'a>(oidc: &'a OidcSys, id: &str) -> Option<&'a OidcProviderConfig> { - oidc.get_provider_config(id) +/// Read-only access to the active standard OIDC configuration. +/// +/// This interface stays separate from authentication and generic provider +/// views because its snapshots include OIDC-specific fields and secrets. +pub trait OidcConfigQuery: Send + Sync { + fn config_snapshot(&self) -> OidcConfigSnapshot; +} + +pub(super) fn redirect_policy(oidc: &OidcSys, provider_id: &str) -> Option { + oidc.get_provider_config(provider_id).map(|config| FederatedRedirectPolicy { + redirect_uri: config.redirect_uri.clone(), + allow_request_origin: config.redirect_uri_dynamic, + }) } pub(super) fn roles_claim_key(oidc: &OidcSys, provider_id: &str) -> Option { - provider_config(oidc, provider_id) + oidc.get_provider_config(provider_id) .map(|config| config.roles_claim.trim().to_string()) .filter(|claim| !claim.is_empty()) } @@ -33,4 +47,21 @@ mod tests { let oidc = OidcSys::empty().expect("empty OIDC configuration should be valid"); assert_eq!(roles_claim_key(&oidc, "default"), None); } + + #[test] + fn redirect_policy_keeps_configured_uri_and_dynamic_setting() { + let mut config = crate::oidc::test_config("default"); + config.redirect_uri = Some("https://console.example.test/callback".to_string()); + config.redirect_uri_dynamic = false; + let oidc = crate::oidc::make_test_sys(vec![config]); + + assert_eq!( + redirect_policy(&oidc, "default"), + Some(FederatedRedirectPolicy { + redirect_uri: Some("https://console.example.test/callback".to_string()), + allow_request_origin: false, + }) + ); + assert_eq!(redirect_policy(&oidc, "missing"), None); + } } diff --git a/crates/iam/src/federation/oidc/discovery.rs b/crates/iam/src/federation/oidc/discovery.rs index 3e883d67b..dd50fe3ac 100644 --- a/crates/iam/src/federation/oidc/discovery.rs +++ b/crates/iam/src/federation/oidc/discovery.rs @@ -12,12 +12,52 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::oidc::{OidcProviderSummary, OidcSys}; +use crate::{federation::FederatedProviderView, oidc::OidcSys}; -pub(super) fn list_providers(oidc: &OidcSys) -> Vec { - oidc.list_providers() +fn provider_view(provider: &crate::oidc::OidcProviderConfig) -> FederatedProviderView { + FederatedProviderView { + provider_id: provider.id.clone(), + display_name: provider.display_name.clone(), + } } -pub(super) fn list_visible_providers(oidc: &OidcSys) -> Vec { - oidc.list_visible_providers() +pub(super) fn list_providers(oidc: &OidcSys) -> Vec { + oidc.provider_configs().map(provider_view).collect() +} + +pub(super) fn list_visible_providers(oidc: &OidcSys) -> Vec { + oidc.provider_configs() + .filter(|provider| !provider.hide_from_ui) + .map(provider_view) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn provider_views_preserve_names_and_visibility() { + let mut visible = crate::oidc::test_config("default"); + visible.display_name = "Company SSO".to_string(); + let mut hidden = crate::oidc::test_config("workload"); + hidden.hide_from_ui = true; + let oidc = crate::oidc::make_test_sys(vec![visible, hidden]); + + let all = list_providers(&oidc); + assert_eq!(all.len(), 2); + assert!( + all.iter() + .any(|provider| { provider.provider_id == "default" && provider.display_name == "Company SSO" }) + ); + assert!(all.iter().any(|provider| provider.provider_id == "workload")); + + assert_eq!( + list_visible_providers(&oidc), + vec![FederatedProviderView { + provider_id: "default".to_string(), + display_name: "Company SSO".to_string(), + }] + ); + } } diff --git a/crates/iam/src/federation/oidc/flow.rs b/crates/iam/src/federation/oidc/flow.rs index 04e9badd1..e2d50d432 100644 --- a/crates/iam/src/federation/oidc/flow.rs +++ b/crates/iam/src/federation/oidc/flow.rs @@ -12,10 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::config::OidcConfigQuery; use super::{claims, config, discovery, http}; use crate::{ - federation::{FederatedAuthorization, FederatedCodeExchange, FederatedIdentityProvider, FederationError, Result}, - oidc::{OidcProviderConfig, OidcProviderSummary, OidcSys}, + federation::{ + FederatedAuthorization, FederatedCodeExchange, FederatedIdentityProvider, FederatedProviderView, FederatedRedirectPolicy, + FederationError, Result, + }, + oidc::{OidcConfigSnapshot, OidcSys}, }; use std::sync::Arc; @@ -29,22 +33,32 @@ impl StandardOidcAdapter { } } +impl OidcConfigQuery for StandardOidcAdapter { + fn config_snapshot(&self) -> OidcConfigSnapshot { + self.oidc.config_snapshot() + } +} + #[async_trait::async_trait] impl FederatedIdentityProvider for StandardOidcAdapter { fn has_providers(&self) -> bool { self.oidc.has_providers() } - fn list_providers(&self) -> Vec { + fn list_providers(&self) -> Vec { discovery::list_providers(&self.oidc) } - fn list_visible_providers(&self) -> Vec { + fn list_visible_providers(&self) -> Vec { discovery::list_visible_providers(&self.oidc) } - fn provider_config(&self, id: &str) -> Option<&OidcProviderConfig> { - config::provider_config(&self.oidc, id) + fn provider_config(&self, provider_id: &str) -> Option<&crate::oidc::OidcProviderConfig> { + self.oidc.get_provider_config(provider_id) + } + + fn redirect_policy(&self, provider_id: &str) -> Option { + config::redirect_policy(&self.oidc, provider_id) } async fn authorize_url(&self, provider_id: &str, redirect_uri: &str, redirect_after: Option) -> Result { diff --git a/crates/iam/src/federation/oidc/mod.rs b/crates/iam/src/federation/oidc/mod.rs index 360c38831..52e6b6432 100644 --- a/crates/iam/src/federation/oidc/mod.rs +++ b/crates/iam/src/federation/oidc/mod.rs @@ -18,4 +18,5 @@ mod discovery; mod flow; mod http; +pub use config::OidcConfigQuery; pub use flow::StandardOidcAdapter; diff --git a/crates/iam/src/federation/provider.rs b/crates/iam/src/federation/provider.rs index 3ee79598a..f463032f5 100644 --- a/crates/iam/src/federation/provider.rs +++ b/crates/iam/src/federation/provider.rs @@ -13,17 +13,43 @@ // limitations under the License. use super::{FederatedAuthorization, FederatedCodeExchange, Result}; -use crate::oidc::{OidcProviderConfig, OidcProviderSummary}; +use crate::oidc::OidcProviderConfig; +use serde::{Deserialize, Serialize}; + +/// Provider data exposed to login discovery consumers. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct FederatedProviderView { + pub provider_id: String, + pub display_name: String, +} + +/// Redirect inputs needed by the browser authorization flow. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FederatedRedirectPolicy { + pub redirect_uri: Option, + pub allow_request_origin: bool, +} #[async_trait::async_trait] pub trait FederatedIdentityProvider: Send + Sync { fn has_providers(&self) -> bool; - fn list_providers(&self) -> Vec; + fn list_providers(&self) -> Vec; - fn list_visible_providers(&self) -> Vec; + fn list_visible_providers(&self) -> Vec; - fn provider_config(&self, id: &str) -> Option<&OidcProviderConfig>; + /// Compatibility accessor for existing provider implementations. + /// Login redirect consumers should use [`Self::redirect_policy`]. + fn provider_config(&self, _provider_id: &str) -> Option<&OidcProviderConfig> { + None + } + + fn redirect_policy(&self, provider_id: &str) -> Option { + self.provider_config(provider_id).map(|config| FederatedRedirectPolicy { + redirect_uri: config.redirect_uri.clone(), + allow_request_origin: config.redirect_uri_dynamic, + }) + } async fn authorize_url(&self, provider_id: &str, redirect_uri: &str, redirect_after: Option) -> Result; @@ -35,3 +61,21 @@ pub trait FederatedIdentityProvider: Send + Sync { async fn build_logout_url(&self, logout_token: &str, post_logout_redirect_uri: &str) -> Result>; } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn provider_view_preserves_login_json_shape() { + let view = FederatedProviderView { + provider_id: "default".to_string(), + display_name: "Company SSO".to_string(), + }; + + assert_eq!( + serde_json::to_string(&[view]).expect("provider view should serialize"), + r#"[{"provider_id":"default","display_name":"Company SSO"}]"# + ); + } +} diff --git a/crates/iam/src/federation/transaction.rs b/crates/iam/src/federation/transaction.rs index 43f1709ed..9b3047571 100644 --- a/crates/iam/src/federation/transaction.rs +++ b/crates/iam/src/federation/transaction.rs @@ -13,14 +13,14 @@ // limitations under the License. use super::{ - FederatedIdentityRegistry, FederatedLoginSession, FederatedSession, FederatedSessionBinding, FederatedSessionTransaction, - FederationError, Result, + FederatedIdentityRegistry, FederatedLoginSession, FederatedProviderView, FederatedRedirectPolicy, FederatedSession, + FederatedSessionBinding, FederatedSessionTransaction, FederationError, Result, }; -use crate::oidc::{OidcProviderConfig, OidcProviderSummary}; +use crate::oidc::OidcProviderConfig; const DEFAULT_OIDC_PROVIDER_ID: &str = "default"; -fn sorted_provider_summaries(mut providers: Vec) -> Vec { +fn sorted_provider_views(mut providers: Vec) -> Vec { providers.sort_by(|left, right| { (left.provider_id != DEFAULT_OIDC_PROVIDER_ID) .cmp(&(right.provider_id != DEFAULT_OIDC_PROVIDER_ID)) @@ -42,16 +42,23 @@ impl FederatedIdentityService { self.registry.standard_oidc().has_providers() } - pub fn list_providers(&self) -> Vec { - sorted_provider_summaries(self.registry.standard_oidc().list_providers()) + pub fn list_providers(&self) -> Vec { + sorted_provider_views(self.registry.standard_oidc().list_providers()) } - pub fn list_visible_providers(&self) -> Vec { - sorted_provider_summaries(self.registry.standard_oidc().list_visible_providers()) + pub fn list_visible_providers(&self) -> Vec { + sorted_provider_views(self.registry.standard_oidc().list_visible_providers()) } - pub fn get_provider_config(&self, id: &str) -> Option<&OidcProviderConfig> { - self.registry.standard_oidc().provider_config(id) + pub fn redirect_policy(&self, provider_id: &str) -> Option { + self.registry.standard_oidc().redirect_policy(provider_id) + } + + /// Compatibility accessor for existing federation consumers. + /// Login redirects use [`Self::redirect_policy`], while OIDC-specific + /// configuration consumers use the OIDC configuration query. + pub fn get_provider_config(&self, provider_id: &str) -> Option<&OidcProviderConfig> { + self.registry.standard_oidc().provider_config(provider_id) } pub async fn authorize_url(&self, provider_id: &str, redirect_uri: &str, redirect_after: Option) -> Result { @@ -150,7 +157,6 @@ mod tests { use crate::federation::{ FederatedAuthorization, FederatedClaims, FederatedCodeExchange, FederatedIdentityProvider, FederatedSessionBindingError, }; - use crate::oidc::{OidcProviderConfig, OidcProviderSummary}; use rustfs_credentials::Credentials; use std::sync::{Arc, Mutex}; @@ -220,10 +226,10 @@ mod tests { } } - fn provider_summaries(provider_ids: &[&str]) -> Vec { + fn provider_views(provider_ids: &[&str]) -> Vec { provider_ids .iter() - .map(|provider_id| OidcProviderSummary { + .map(|provider_id| FederatedProviderView { provider_id: (*provider_id).to_string(), display_name: (*provider_id).to_string(), }) @@ -236,16 +242,19 @@ mod tests { true } - fn list_providers(&self) -> Vec { - provider_summaries(&self.listed_provider_ids) + fn list_providers(&self) -> Vec { + provider_views(&self.listed_provider_ids) } - fn list_visible_providers(&self) -> Vec { - provider_summaries(&self.visible_provider_ids) + fn list_visible_providers(&self) -> Vec { + provider_views(&self.visible_provider_ids) } - fn provider_config(&self, _id: &str) -> Option<&OidcProviderConfig> { - None + fn redirect_policy(&self, provider_id: &str) -> Option { + (provider_id == DEFAULT_OIDC_PROVIDER_ID).then_some(FederatedRedirectPolicy { + redirect_uri: None, + allow_request_origin: true, + }) } async fn authorize_url( @@ -354,7 +363,7 @@ mod tests { [DEFAULT_OIDC_PROVIDER_ID, "alpha", "zeta"] ); assert_eq!( - sorted_provider_summaries(provider_summaries(&["zeta", "alpha"])) + sorted_provider_views(provider_views(&["zeta", "alpha"])) .into_iter() .map(|provider| provider.provider_id) .collect::>(), diff --git a/crates/iam/src/lib.rs b/crates/iam/src/lib.rs index 0525bff7a..8671155d3 100644 --- a/crates/iam/src/lib.rs +++ b/crates/iam/src/lib.rs @@ -332,7 +332,7 @@ async fn init_oidc_sys_with_extra_root_ca_provider_inner( event = EVENT_OIDC_STATE, component = LOG_COMPONENT_IAM, subsystem = LOG_SUBSYSTEM_OIDC, - provider_count = sys.list_providers().len(), + provider_count = sys.provider_configs().count(), state = "ready", "OIDC runtime ready" ); diff --git a/crates/iam/src/oidc.rs b/crates/iam/src/oidc.rs index 94dc1f5a2..842a3caa5 100644 --- a/crates/iam/src/oidc.rs +++ b/crates/iam/src/oidc.rs @@ -682,7 +682,7 @@ impl<'c> AsyncHttpClient<'c> for ReqwestHttpClient { } } -// ---- Public types (unchanged API) ---- +// ---- Public types ---- const REDACTED_SECRET: &str = "***redacted***"; @@ -753,6 +753,31 @@ pub struct SourcedOidcProviderConfig { pub source: OidcProviderConfigSource, } +/// Immutable OIDC configuration entries returned by configuration queries. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OidcConfigSnapshot { + providers: Vec, +} + +impl OidcConfigSnapshot { + pub fn new(mut providers: Vec) -> Self { + providers.sort_by(|left, right| { + (left.config.id != "default") + .cmp(&(right.config.id != "default")) + .then_with(|| left.config.id.cmp(&right.config.id)) + }); + Self { providers } + } + + pub fn providers(&self) -> &[SourcedOidcProviderConfig] { + &self.providers + } + + pub fn into_providers(self) -> Vec { + self.providers + } +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct OidcProviderValidationResult { pub issuer: String, @@ -760,12 +785,8 @@ pub struct OidcProviderValidationResult { pub token_endpoint: Option, } -/// Summary info about a provider, returned to the console. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct OidcProviderSummary { - pub provider_id: String, - pub display_name: String, -} +/// Compatibility name for the provider summary returned by OIDC list APIs. +pub type OidcProviderSummary = crate::federation::FederatedProviderView; /// Claims extracted from an OIDC ID token. #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -932,7 +953,7 @@ impl ProviderState { /// Global OIDC manager for all configured providers. pub struct OidcSys { - configs: HashMap, + configs: HashMap, provider_states: RwLock>, state_store: OidcStateStore, http_client: ReqwestHttpClient, @@ -967,22 +988,22 @@ impl OidcSys { async fn new_with_http_client(http_client: ReqwestHttpClient) -> Result { let server_config = crate::server_config::current_server_config(); - let parsed_configs = load_effective_oidc_provider_configs(server_config.as_ref()); + let parsed_configs = load_oidc_config_snapshot(server_config.as_ref()); let mut configs = HashMap::new(); let mut provider_states = HashMap::new(); - for sourced_config in parsed_configs { - let config = sourced_config.config; + for sourced_config in parsed_configs.into_providers() { + let config = &sourced_config.config; if !config.enabled { debug!(provider = %config.id, "OIDC provider disabled"); continue; } - match Self::discover_provider(&config, &http_client).await { + match Self::discover_provider(config, &http_client).await { Ok(state) => { debug!(provider = %config.id, "OIDC provider discovered"); provider_states.insert(config.id.clone(), state); - configs.insert(config.id.clone(), config); + configs.insert(config.id.clone(), sourced_config); } Err(e) => { error!( @@ -1026,29 +1047,35 @@ impl OidcSys { !self.configs.is_empty() } - /// List all providers (including hidden ones). Used by site-replication and admin config. + pub(crate) fn provider_configs(&self) -> impl Iterator { + self.configs.values().map(|provider| &provider.config) + } + + /// List all providers, including providers hidden from the login UI. pub fn list_providers(&self) -> Vec { - self.configs - .values() - .map(|c| OidcProviderSummary { - provider_id: c.id.clone(), - display_name: c.display_name.clone(), + self.provider_configs() + .map(|config| OidcProviderSummary { + provider_id: config.id.clone(), + display_name: config.display_name.clone(), }) .collect() } - /// List only visible providers (excludes those with `hide_from_ui = true`). + /// List providers visible in the login UI. pub fn list_visible_providers(&self) -> Vec { - self.configs - .values() - .filter(|c| !c.hide_from_ui) - .map(|c| OidcProviderSummary { - provider_id: c.id.clone(), - display_name: c.display_name.clone(), + self.provider_configs() + .filter(|config| !config.hide_from_ui) + .map(|config| OidcProviderSummary { + provider_id: config.id.clone(), + display_name: config.display_name.clone(), }) .collect() } + pub fn config_snapshot(&self) -> OidcConfigSnapshot { + OidcConfigSnapshot::new(self.configs.values().cloned().collect()) + } + /// Build the PKCE authorization URL for a provider, store state in the state store. pub async fn authorize_url( &self, @@ -1057,8 +1084,7 @@ impl OidcSys { redirect_after: Option, ) -> Result { let config = self - .configs - .get(provider_id) + .get_provider_config(provider_id) .ok_or_else(|| format!("unknown OIDC provider: {provider_id}"))?; let state = self.ensure_provider_state(provider_id, config).await?; @@ -1116,8 +1142,7 @@ impl OidcSys { .ok_or_else(|| "invalid or expired OIDC state".to_string())?; let config = self - .configs - .get(&session.provider_id) + .get_provider_config(&session.provider_id) .ok_or_else(|| format!("unknown provider: {}", session.provider_id))?; let provider_state = self.get_provider_state(&session.provider_id)?; let issuer = provider_state.metadata.issuer().to_string(); @@ -1451,8 +1476,7 @@ impl OidcSys { .ok_or_else(|| "invalid or expired OIDC logout token".to_string())?; let config = self - .configs - .get(&session.provider_id) + .get_provider_config(&session.provider_id) .ok_or_else(|| format!("unknown OIDC provider: {}", session.provider_id))?; let state = self.ensure_provider_state(&session.provider_id, config).await?; let Some(end_session_endpoint) = state.metadata.console()?.additional_metadata().end_session_endpoint.clone() else { @@ -1478,7 +1502,7 @@ impl OidcSys { /// Map OIDC claims to rustfs policy names. pub fn map_claims_to_policies(&self, provider_id: &str, claims: &OidcClaims) -> (Vec, Vec) { - let config = match self.configs.get(provider_id) { + let config = match self.get_provider_config(provider_id) { Some(c) => c, None => return (vec![], vec![]), }; @@ -1571,7 +1595,7 @@ impl OidcSys { /// dedicated policy claim are never included, so those configurations keep requiring every /// policy to resolve. pub fn group_claim_policy_names(&self, provider_id: &str, claims: &OidcClaims) -> Vec { - let Some(config) = self.configs.get(provider_id) else { + let Some(config) = self.get_provider_config(provider_id) else { return Vec::new(); }; if !config.role_policy.trim().is_empty() { @@ -1678,7 +1702,7 @@ impl OidcSys { && issuer_host == provider_host && issuer_port == provider_port && issuer_path == provider_path - && let Some(config) = self.configs.get(id) + && let Some(config) = self.get_provider_config(id) { return Some((id.clone(), config.clone(), state.clone())); } @@ -1747,7 +1771,7 @@ impl OidcSys { /// Get a provider config by ID. pub fn get_provider_config(&self, id: &str) -> Option<&OidcProviderConfig> { - self.configs.get(id) + self.configs.get(id).map(|provider| &provider.config) } /// Parse all OIDC provider configs from environment variables. @@ -2225,6 +2249,10 @@ pub fn load_effective_oidc_provider_configs(server_config: Option<&ServerConfig> merge_oidc_provider_configs(env_configs, persisted_configs) } +pub fn load_oidc_config_snapshot(server_config: Option<&ServerConfig>) -> OidcConfigSnapshot { + OidcConfigSnapshot::new(load_effective_oidc_provider_configs(server_config)) +} + pub async fn validate_oidc_provider_config(config: &OidcProviderConfig) -> Result { validate_oidc_provider_config_with_extra_root_ca(config, None).await } @@ -2440,7 +2468,11 @@ fn claim_value_type_for_log(value: Option<&serde_json::Value>) -> &'static str { #[cfg(test)] pub(crate) fn make_test_sys(configs: Vec) -> OidcSys { - let configs = configs.into_iter().map(|config| (config.id.clone(), config)).collect(); + let configs = configs + .into_iter() + .map(test_sourced_config) + .map(|provider| (provider.config.id.clone(), provider)) + .collect(); OidcSys { configs, provider_states: RwLock::new(HashMap::new()), @@ -2449,6 +2481,14 @@ pub(crate) fn make_test_sys(configs: Vec) -> OidcSys { } } +#[cfg(test)] +fn test_sourced_config(config: OidcProviderConfig) -> SourcedOidcProviderConfig { + SourcedOidcProviderConfig { + config, + source: OidcProviderConfigSource::Persisted, + } +} + #[cfg(test)] pub(crate) fn test_config(id: &str) -> OidcProviderConfig { OidcProviderConfig { @@ -3041,7 +3081,7 @@ mod tests { .await .expect("initial OIDC discovery should succeed"); let sys = OidcSys { - configs: HashMap::from([(config.id.clone(), config.clone())]), + configs: HashMap::from([(config.id.clone(), test_sourced_config(config.clone()))]), provider_states: RwLock::new(HashMap::from([(config.id.clone(), state)])), state_store: OidcStateStore::new(), http_client, @@ -3175,7 +3215,7 @@ mod tests { let http_client = ReqwestHttpClient::with_policy(OutboundPolicy::from_allowed_origins(&base).unwrap()); let discovered = OidcSys::discover_provider(&config, &http_client).await.unwrap(); let sys = OidcSys { - configs: HashMap::from([(config.id.clone(), config)]), + configs: HashMap::from([(config.id.clone(), test_sourced_config(config))]), provider_states: RwLock::new(HashMap::from([("console".into(), discovered)])), state_store: OidcStateStore::new(), http_client, @@ -3403,13 +3443,13 @@ mod tests { .expect("workload discovery must succeed"); assert!(state.metadata.authorization_endpoint().is_none()); let sys = OidcSys { - configs: HashMap::from([(config.id.clone(), config.clone())]), + configs: HashMap::from([(config.id.clone(), test_sourced_config(config.clone()))]), provider_states: RwLock::new(HashMap::from([(config.id.clone(), state)])), state_store: OidcStateStore::new(), http_client, }; assert!(sys.has_providers()); - assert!(sys.list_visible_providers().is_empty()); + assert!(sys.provider_configs().all(|provider| provider.hide_from_ui)); let error = sys .authorize_url(&config.id, "https://console.example.com/callback", None) .await @@ -4033,7 +4073,7 @@ mod tests { fn test_oidc_sys_empty() { let sys = OidcSys::empty().expect("failed to initialize empty OIDC system"); assert!(!sys.has_providers()); - assert!(sys.list_providers().is_empty()); + assert!(sys.config_snapshot().providers().is_empty()); } #[test] @@ -4183,7 +4223,7 @@ mod tests { ) .await; let sys = OidcSys { - configs: HashMap::from([(provider_id.to_string(), config)]), + configs: HashMap::from([(provider_id.to_string(), test_sourced_config(config))]), provider_states: RwLock::new(HashMap::from([( provider_id.to_string(), ProviderState { @@ -4432,8 +4472,8 @@ mod tests { let listed = sys.list_visible_providers(); assert_eq!(listed.len(), 1); - assert!(listed.iter().any(|p| p.provider_id == "dex")); - assert!(!listed.iter().any(|p| p.provider_id == "kubernetes")); + assert!(listed.iter().any(|provider| provider.provider_id == "dex")); + assert!(!listed.iter().any(|provider| provider.provider_id == "kubernetes")); } #[test] @@ -4449,44 +4489,28 @@ mod tests { } #[test] - fn test_list_providers_includes_hidden_for_replication() { + fn test_config_snapshot_includes_hidden_for_replication() { + let visible = test_config("dex"); + let mut hidden = test_config("kubernetes"); + hidden.hide_from_ui = true; + + let sys = make_test_sys(vec![visible, hidden]); + + assert_eq!(sys.config_snapshot().providers().len(), 2); + } + + #[test] + fn test_list_providers_includes_hidden_for_compatibility() { let visible = test_config("dex"); let mut hidden = test_config("kubernetes"); hidden.hide_from_ui = true; let sys = make_test_sys(vec![visible, hidden]); - // Unfiltered list returns all (used by site-replication) assert_eq!(sys.list_providers().len(), 2); - // UI-filtered list hides the hidden one assert_eq!(sys.list_visible_providers().len(), 1); } - #[test] - fn test_list_providers_all_visible_by_default() { - let a = test_config("okta"); - let b = test_config("dex"); - - let sys = make_test_sys(vec![a, b]); - let listed = sys.list_visible_providers(); - - assert_eq!(listed.len(), 2); - } - - #[test] - fn test_list_visible_providers_all_hidden() { - let mut a = test_config("k8s-a"); - a.hide_from_ui = true; - let mut b = test_config("k8s-b"); - b.hide_from_ui = true; - - let sys = make_test_sys(vec![a, b]); - let listed = sys.list_visible_providers(); - - assert!(listed.is_empty()); - assert!(sys.has_providers()); - } - #[test] fn test_hide_from_ui_default_is_false() { let config = test_config("default"); @@ -4723,17 +4747,32 @@ mod tests { } #[test] - fn test_list_providers() { + fn test_config_snapshot_lists_providers() { let mut config = test_config("keycloak"); config.display_name = "Keycloak SSO".to_string(); let sys = make_test_sys(vec![config]); assert!(sys.has_providers()); - let summaries = sys.list_providers(); - assert_eq!(summaries.len(), 1); - assert_eq!(summaries[0].provider_id, "keycloak"); - assert_eq!(summaries[0].display_name, "Keycloak SSO"); + let snapshot = sys.config_snapshot(); + assert_eq!(snapshot.providers().len(), 1); + assert_eq!(snapshot.providers()[0].config.id, "keycloak"); + assert_eq!(snapshot.providers()[0].config.display_name, "Keycloak SSO"); + assert_eq!(snapshot.providers()[0].source, OidcProviderConfigSource::Persisted); + } + + #[test] + fn test_active_config_snapshot_puts_default_first() { + let sys = make_test_sys(vec![test_config("zeta"), test_config("default"), test_config("alpha")]); + + assert_eq!( + sys.config_snapshot() + .providers() + .iter() + .map(|provider| provider.config.id.as_str()) + .collect::>(), + ["default", "alpha", "zeta"] + ); } #[test] diff --git a/rustfs/src/admin/console.rs b/rustfs/src/admin/console.rs index a46a3e795..4019c9876 100644 --- a/rustfs/src/admin/console.rs +++ b/rustfs/src/admin/console.rs @@ -39,6 +39,7 @@ use http::{HeaderMap, HeaderName, HeaderValue, Method, StatusCode, Uri}; use mime_guess::from_path; use opentelemetry::global; use rust_embed::RustEmbed; +use rustfs_iam::federation::FederatedProviderView; use serde::Serialize; use std::{ net::{IpAddr, SocketAddr}, @@ -185,13 +186,7 @@ pub(crate) struct Config { license: License, doc: String, #[serde(skip_serializing_if = "Vec::is_empty")] - oidc: Vec, -} - -#[derive(Debug, Serialize, Clone)] -struct OidcProviderInfo { - provider_id: String, - display_name: String, + oidc: Vec, } impl Config { @@ -200,16 +195,7 @@ impl Config { // Collect OIDC provider info if available let oidc = current_federated_identity_service() - .map(|federation| { - federation - .list_visible_providers() - .into_iter() - .map(|p| OidcProviderInfo { - provider_id: p.provider_id, - display_name: p.display_name, - }) - .collect() - }) + .map(|federation| federation.list_visible_providers()) .unwrap_or_default(); Config { diff --git a/rustfs/src/admin/handlers/idp_compat.rs b/rustfs/src/admin/handlers/idp_compat.rs index fef9fd8cb..bc6417bc1 100644 --- a/rustfs/src/admin/handlers/idp_compat.rs +++ b/rustfs/src/admin/handlers/idp_compat.rs @@ -565,7 +565,8 @@ fn is_supported_idp_type(idp_type: &str) -> bool { fn idp_config_names(idp_type: &str) -> S3Result> { let config = current_active_server_config(); match idp_type { - IDP_TYPE_OPENID => Ok(rustfs_iam::oidc::load_effective_oidc_provider_configs(config.as_ref()) + IDP_TYPE_OPENID => Ok(rustfs_iam::oidc::load_oidc_config_snapshot(config.as_ref()) + .into_providers() .into_iter() .map(|provider| (provider.config.id, provider.config.enabled)) .collect()), @@ -583,7 +584,8 @@ fn idp_config_info(idp_type: &str, name: &str) -> S3Result<(bool, Vec { - let provider = rustfs_iam::oidc::load_effective_oidc_provider_configs(config.as_ref()) + let provider = rustfs_iam::oidc::load_oidc_config_snapshot(config.as_ref()) + .into_providers() .into_iter() .find(|provider| provider.config.id == name || (name == DEFAULT_DELIMITER && provider.config.id == "default")) .ok_or_else(|| s3_error!(NoSuchResource, "openid provider '{}' not found", name))?; diff --git a/rustfs/src/admin/handlers/oidc.rs b/rustfs/src/admin/handlers/oidc.rs index b148af73e..b5183ce05 100644 --- a/rustfs/src/admin/handlers/oidc.rs +++ b/rustfs/src/admin/handlers/oidc.rs @@ -325,7 +325,8 @@ impl Operation for GetOidcConfigHandler { let config = load_server_config_from_store().await?; let restart_required = oidc_restart_required(&config); - let providers = rustfs_iam::oidc::load_effective_oidc_provider_configs(Some(&config)) + let providers = rustfs_iam::oidc::load_oidc_config_snapshot(Some(&config)) + .into_providers() .into_iter() .map(|provider| OidcConfigView { provider_id: provider.config.id.clone(), @@ -697,45 +698,37 @@ impl Operation for OidcLogoutHandler { /// from request headers. For production deployments behind a reverse proxy, configuring /// an explicit redirect_uri is recommended to prevent header manipulation. fn derive_callback_uri(req: &S3Request, provider_id: &str) -> S3Result { - if let Some(federation) = current_federated_identity_service() - && let Some(config) = federation.get_provider_config(provider_id) - { - return derive_callback_uri_with_provider_config(req, provider_id, Some(config)); - } + let policy = current_federated_identity_service().and_then(|federation| federation.redirect_policy(provider_id)); - derive_callback_uri_with_provider_config(req, provider_id, None) + derive_callback_uri_with_redirect_policy(req, provider_id, policy.as_ref()) } -fn derive_callback_uri_with_provider_config( +fn derive_callback_uri_with_redirect_policy( req: &S3Request, provider_id: &str, - config: Option<&rustfs_iam::oidc::OidcProviderConfig>, + policy: Option<&rustfs_iam::federation::FederatedRedirectPolicy>, ) -> S3Result { - if let Some(config) = config { - if let Some(ref uri) = config.redirect_uri { - let parsed = Url::parse(uri).map_err(|_| s3_error!(InvalidRequest, "invalid configured redirect_uri"))?; - if !is_valid_scheme(parsed.scheme()) || parsed.host_str().is_none() { - return Err(s3_error!(InvalidRequest, "configured redirect_uri must be absolute http/https URL")); - } - return Ok(uri.clone()); - } - - if let Some(url) = browser_redirect_url(&oidc_callback_path(provider_id))? { - return Ok(url); - } - - if !config.redirect_uri_dynamic { - return Err(s3_error!( - InvalidRequest, - "provider requires explicit redirect_uri because redirect_uri_dynamic is disabled" - )); + if let Some(policy) = policy + && let Some(ref uri) = policy.redirect_uri + { + let parsed = Url::parse(uri).map_err(|_| s3_error!(InvalidRequest, "invalid configured redirect_uri"))?; + if !is_valid_scheme(parsed.scheme()) || parsed.host_str().is_none() { + return Err(s3_error!(InvalidRequest, "configured redirect_uri must be absolute http/https URL")); } + return Ok(uri.clone()); } if let Some(url) = browser_redirect_url(&oidc_callback_path(provider_id))? { return Ok(url); } + if policy.is_some_and(|policy| !policy.allow_request_origin) { + return Err(s3_error!( + InvalidRequest, + "provider requires explicit redirect_uri because redirect_uri_dynamic is disabled" + )); + } + let scheme = extract_request_scheme(req)?; let host = extract_request_host(req)?; @@ -938,8 +931,7 @@ fn oidc_restart_required(config: &ServerConfig) -> bool { } fn oidc_restart_required_from_active_config(config: &ServerConfig, active_config: Option<&ServerConfig>) -> bool { - rustfs_iam::oidc::load_effective_oidc_provider_configs(Some(config)) - != rustfs_iam::oidc::load_effective_oidc_provider_configs(active_config) + rustfs_iam::oidc::load_oidc_config_snapshot(Some(config)) != rustfs_iam::oidc::load_oidc_config_snapshot(active_config) } fn default_oidc_kvs() -> s3s::S3Result { @@ -1419,27 +1411,13 @@ mod tests { } } - fn test_provider_config(redirect_uri: Option<&str>, redirect_uri_dynamic: bool) -> rustfs_iam::oidc::OidcProviderConfig { - rustfs_iam::oidc::OidcProviderConfig { - id: "default".to_string(), - enabled: true, - config_url: "https://idp.example.com/.well-known/openid-configuration".to_string(), - issuer: None, - client_id: "rustfs-console".to_string(), - client_secret: None, - scopes: vec!["openid".to_string()], - other_audiences: Vec::new(), + fn test_redirect_policy( + redirect_uri: Option<&str>, + allow_request_origin: bool, + ) -> rustfs_iam::federation::FederatedRedirectPolicy { + rustfs_iam::federation::FederatedRedirectPolicy { redirect_uri: redirect_uri.map(ToString::to_string), - redirect_uri_dynamic, - claim_name: OIDC_DEFAULT_CLAIM_NAME.to_string(), - claim_prefix: String::new(), - role_policy: String::new(), - display_name: "default".to_string(), - groups_claim: OIDC_DEFAULT_GROUPS_CLAIM.to_string(), - roles_claim: OIDC_DEFAULT_ROLES_CLAIM.to_string(), - email_claim: OIDC_DEFAULT_EMAIL_CLAIM.to_string(), - username_claim: OIDC_DEFAULT_USERNAME_CLAIM.to_string(), - hide_from_ui: false, + allow_request_origin, } } @@ -1604,7 +1582,7 @@ mod tests { } #[test] - fn test_derive_callback_uri_falls_back_to_request_headers() { + fn test_derive_callback_uri_missing_provider_falls_back_to_request_headers() { let req = build_oidc_request( "http://internal/rustfs/admin/v3/oidc/authorize/default", Some("internal:9000"), @@ -1612,19 +1590,20 @@ mod tests { ); let callback = with_var(ENV_RUSTFS_BROWSER_REDIRECT_URL, None::<&str>, || { - derive_callback_uri(&req, "default").expect("callback URI should fall back to request headers") + derive_callback_uri_with_redirect_policy(&req, "missing", None) + .expect("a missing provider should keep the request-header fallback") }); - assert_eq!(callback, "https://internal:9000/rustfs/admin/v3/oidc/callback/default"); + assert_eq!(callback, "https://internal:9000/rustfs/admin/v3/oidc/callback/missing"); } #[test] fn test_derive_callback_uri_configured_redirect_uri_wins() { let req = build_oidc_request("http://internal/rustfs/admin/v3/oidc/authorize/default", Some("internal:9000"), None); - let config = test_provider_config(Some("https://configured.example.com/rustfs/admin/v3/oidc/callback/default"), false); + let policy = test_redirect_policy(Some("https://configured.example.com/rustfs/admin/v3/oidc/callback/default"), false); let callback = with_var(ENV_RUSTFS_BROWSER_REDIRECT_URL, Some("https://console.example.com"), || { - derive_callback_uri_with_provider_config(&req, "default", Some(&config)) + derive_callback_uri_with_redirect_policy(&req, "default", Some(&policy)) .expect("configured redirect_uri should be preferred") }); @@ -1634,10 +1613,10 @@ mod tests { #[test] fn test_derive_callback_uri_browser_redirect_url_satisfies_static_provider() { let req = build_oidc_request("http://internal/rustfs/admin/v3/oidc/authorize/default", Some("internal:9000"), None); - let config = test_provider_config(None, false); + let policy = test_redirect_policy(None, false); let callback = with_var(ENV_RUSTFS_BROWSER_REDIRECT_URL, Some("https://console.example.com"), || { - derive_callback_uri_with_provider_config(&req, "default", Some(&config)) + derive_callback_uri_with_redirect_policy(&req, "default", Some(&policy)) .expect("browser redirect URL should satisfy a non-dynamic provider") }); @@ -1647,10 +1626,10 @@ mod tests { #[test] fn test_derive_callback_uri_static_provider_requires_redirect_source() { let req = build_oidc_request("http://internal/rustfs/admin/v3/oidc/authorize/default", Some("internal:9000"), None); - let config = test_provider_config(None, false); + let policy = test_redirect_policy(None, false); let err = with_var(ENV_RUSTFS_BROWSER_REDIRECT_URL, None::<&str>, || { - derive_callback_uri_with_provider_config(&req, "default", Some(&config)) + derive_callback_uri_with_redirect_policy(&req, "default", Some(&policy)) .expect_err("non-dynamic provider without redirect source should fail") }); @@ -1664,10 +1643,10 @@ mod tests { Some("internal:9000"), Some("https"), ); - let config = test_provider_config(None, true); + let policy = test_redirect_policy(None, true); let callback = with_var(ENV_RUSTFS_BROWSER_REDIRECT_URL, None::<&str>, || { - derive_callback_uri_with_provider_config(&req, "default", Some(&config)) + derive_callback_uri_with_redirect_policy(&req, "default", Some(&policy)) .expect("dynamic provider should fall back to request headers") }); diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 73e4882c4..0ab9442c7 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -15,7 +15,7 @@ use crate::admin::auth::authorize_admin_request; use crate::admin::router::{AdminOperation, Operation, S3Router}; use crate::admin::runtime_sources::{ - current_deployment_id, current_federated_identity_service, current_iam_handle, current_object_store_handle, current_region, + current_deployment_id, current_federated_identity_runtime, current_iam_handle, current_object_store_handle, current_region, current_replication_pool_handle, current_replication_stats_handle, current_server_config, current_token_signing_key, object_store_from_req, }; @@ -2032,8 +2032,8 @@ async fn bootstrap_existing_metadata_after_add( } /// The OpenID half of the IDP settings a site reports to its site-replication -/// peers, built from the providers this site has configured (in -/// `list_providers` order) and the site's own region. +/// peers, built from the active OIDC configuration snapshot and the site's own +/// region. fn open_id_settings(providers: Vec<(String, OpenIDProviderSettings)>, region: String) -> OpenIDSettings { // `region` qualifies the provider identities, not the site: a site // without OpenID must report the empty settings in every region, or the @@ -2043,53 +2043,84 @@ fn open_id_settings(providers: Vec<(String, OpenIDProviderSettings)>, region: St return OpenIDSettings::default(); } + let mut providers = providers; + let claim_provider_index = providers + .iter() + .position(|(provider_id, _)| provider_id == "default") + .unwrap_or_default(); + let (_, claim_provider) = providers.remove(claim_provider_index); let mut settings = OpenIDSettings { enabled: true, region, + claim_provider, ..Default::default() }; for (provider_id, provider_settings) in providers { - let claim_provider_unset = settings.claim_provider.client_id.is_empty() - && settings.claim_provider.claim_name.is_empty() - && settings.claim_provider.role_policy.is_empty() - && settings.claim_provider.hashed_client_secret.is_empty(); - - if provider_id == "default" || claim_provider_unset { - settings.claim_provider = provider_settings; - } else { - settings.roles.insert(provider_id, provider_settings); - } + settings.roles.insert(provider_id, provider_settings); } settings } +fn open_id_settings_from_snapshot(snapshot: &rustfs_iam::oidc::OidcConfigSnapshot, region: String) -> OpenIDSettings { + let providers = snapshot + .providers() + .iter() + .map(|provider| { + let config = &provider.config; + (config.id.clone(), open_id_provider_settings(config)) + }) + .collect(); + + open_id_settings(providers, region) +} + +fn open_id_provider_settings(config: &rustfs_iam::oidc::OidcProviderConfig) -> OpenIDProviderSettings { + OpenIDProviderSettings { + claim_name: config.claim_name.clone(), + claim_userinfo_enabled: false, + role_policy: config.role_policy.clone(), + client_id: config.client_id.clone(), + hashed_client_secret: hash_client_secret(config.client_secret.as_deref()), + } +} + +fn open_id_settings_from_service(service: &rustfs_iam::federation::FederatedIdentityService, region: String) -> OpenIDSettings { + let providers = service + .list_providers() + .into_iter() + .filter_map(|provider| { + let config = service.get_provider_config(&provider.provider_id)?; + Some((provider.provider_id, open_id_provider_settings(config))) + }) + .collect(); + + open_id_settings(providers, region) +} + +fn open_id_settings_from_runtime( + oidc_config_query: Option<&dyn rustfs_iam::federation::oidc::OidcConfigQuery>, + service: Option<&rustfs_iam::federation::FederatedIdentityService>, + region: String, +) -> OpenIDSettings { + if let Some(oidc_config_query) = oidc_config_query { + return open_id_settings_from_snapshot(&oidc_config_query.config_snapshot(), region); + } + + service + .map(|service| open_id_settings_from_service(service, region)) + .unwrap_or_default() +} + fn local_idp_settings() -> IDPSettings { let mut settings = IDPSettings::default(); - if let Some(federation) = current_federated_identity_service() { - // A listed provider whose config cannot be resolved contributes - // nothing to the peer comparison, so it is dropped here rather than - // inside the settings builder. - let providers = federation - .list_providers() - .into_iter() - .filter_map(|provider| { - let config = federation.get_provider_config(&provider.provider_id)?; - Some(( - provider.provider_id.clone(), - OpenIDProviderSettings { - claim_name: config.claim_name.clone(), - claim_userinfo_enabled: false, - role_policy: config.role_policy.clone(), - client_id: config.client_id.clone(), - hashed_client_secret: hash_client_secret(config.client_secret.as_deref()), - }, - )) - }) - .collect(); - settings.open_id = open_id_settings(providers, current_region().map(|region| region.to_string()).unwrap_or_default()); - } + let region = current_region().map(|region| region.to_string()).unwrap_or_default(); + settings.open_id = current_federated_identity_runtime() + .map(|(service, oidc_config_query)| { + open_id_settings_from_runtime(oidc_config_query.as_deref(), Some(service.as_ref()), region) + }) + .unwrap_or_default(); let (ldap, ldap_configs) = load_ldap_idp_settings(); settings.ldap = ldap; @@ -11386,6 +11417,109 @@ mod tests { } } + fn oidc_snapshot_provider(id: &str, client_id: &str) -> rustfs_iam::oidc::SourcedOidcProviderConfig { + rustfs_iam::oidc::SourcedOidcProviderConfig { + config: rustfs_iam::oidc::OidcProviderConfig { + id: id.to_string(), + enabled: true, + config_url: format!("https://idp.example.com/{id}/.well-known/openid-configuration"), + issuer: None, + client_id: client_id.to_string(), + client_secret: Some("secret".to_string()), + scopes: vec!["openid".to_string()], + other_audiences: Vec::new(), + redirect_uri: None, + redirect_uri_dynamic: true, + claim_name: "groups".to_string(), + claim_prefix: String::new(), + role_policy: "readwrite".to_string(), + display_name: id.to_string(), + groups_claim: "groups".to_string(), + roles_claim: String::new(), + email_claim: "email".to_string(), + username_claim: "username".to_string(), + hide_from_ui: false, + }, + source: rustfs_iam::oidc::OidcProviderConfigSource::Persisted, + } + } + + struct LegacyOidcProvider { + configs: Vec, + } + + struct SnapshotOidcConfigQuery { + snapshot: rustfs_iam::oidc::OidcConfigSnapshot, + } + + impl rustfs_iam::federation::oidc::OidcConfigQuery for SnapshotOidcConfigQuery { + fn config_snapshot(&self) -> rustfs_iam::oidc::OidcConfigSnapshot { + self.snapshot.clone() + } + } + + #[async_trait::async_trait] + impl rustfs_iam::federation::FederatedIdentityProvider for LegacyOidcProvider { + fn has_providers(&self) -> bool { + !self.configs.is_empty() + } + + fn list_providers(&self) -> Vec { + self.configs + .iter() + .map(|config| rustfs_iam::federation::FederatedProviderView { + provider_id: config.id.clone(), + display_name: config.display_name.clone(), + }) + .collect() + } + + fn list_visible_providers(&self) -> Vec { + self.list_providers() + } + + fn provider_config(&self, provider_id: &str) -> Option<&rustfs_iam::oidc::OidcProviderConfig> { + self.configs.iter().find(|config| config.id == provider_id) + } + + async fn authorize_url( + &self, + _provider_id: &str, + _redirect_uri: &str, + _redirect_after: Option, + ) -> rustfs_iam::federation::Result { + unreachable!("site replication compatibility test does not authorize") + } + + async fn exchange_code( + &self, + _state: &str, + _code: &str, + _redirect_uri: &str, + ) -> rustfs_iam::federation::Result { + unreachable!("site replication compatibility test does not exchange codes") + } + + async fn verify_web_identity_token( + &self, + _jwt: &str, + ) -> rustfs_iam::federation::Result { + unreachable!("site replication compatibility test does not verify tokens") + } + + async fn create_logout_token(&self, _provider_id: &str, _id_token: &str) -> rustfs_iam::federation::Result { + unreachable!("site replication compatibility test does not create logout tokens") + } + + async fn build_logout_url( + &self, + _logout_token: &str, + _post_logout_redirect_uri: &str, + ) -> rustfs_iam::federation::Result> { + unreachable!("site replication compatibility test does not build logout URLs") + } + } + fn ldap_settings_kvs() -> rustfs_config::server_config::KVS { rustfs_config::server_config::KVS(vec![ rustfs_config::server_config::KV { @@ -11442,6 +11576,20 @@ mod tests { assert_eq!(settings.claim_provider.client_id, "client-a"); } + #[test] + fn test_open_id_settings_uses_first_named_provider_without_default() { + let settings = open_id_settings( + vec![ + ("alpha".to_string(), openid_provider("client-a")), + ("corp".to_string(), openid_provider("client-b")), + ], + "eu-site-1".to_string(), + ); + + assert_eq!(settings.claim_provider.client_id, "client-a"); + assert_eq!(settings.roles.get("corp").map(|provider| provider.client_id.as_str()), Some("client-b")); + } + #[test] fn idp_settings_preserve_site_replication_wire_shape() { let (ldap, ldap_configs) = ldap_settings_from_kvs(&ldap_settings_kvs()); @@ -11502,6 +11650,85 @@ mod tests { ); } + #[test] + fn oidc_config_snapshot_preserves_site_replication_settings() { + let snapshot = rustfs_iam::oidc::OidcConfigSnapshot::new(vec![ + oidc_snapshot_provider("corp", "client-b"), + oidc_snapshot_provider("default", "client-a"), + ]); + + assert_eq!( + snapshot + .providers() + .iter() + .map(|provider| provider.config.id.as_str()) + .collect::>(), + ["default", "corp"] + ); + + assert_eq!( + serde_json::to_value(open_id_settings_from_snapshot(&snapshot, "eu-site-1".to_string())) + .expect("serialize OpenID settings"), + serde_json::json!({ + "Enabled": true, + "Region": "eu-site-1", + "Roles": { + "corp": { + "ClaimName": "groups", + "ClaimUserinfoEnabled": false, + "RolePolicy": "readwrite", + "ClientID": "client-b", + "HashedClientSecret": "K7gNU3sdo-OL0wNhqoVWhr3g6s1xYv72ol_pe_Unols", + } + }, + "ClaimProvider": { + "ClaimName": "groups", + "ClaimUserinfoEnabled": false, + "RolePolicy": "readwrite", + "ClientID": "client-a", + "HashedClientSecret": "K7gNU3sdo-OL0wNhqoVWhr3g6s1xYv72ol_pe_Unols", + } + }) + ); + } + + #[test] + fn service_only_oidc_runtime_preserves_site_replication_settings() { + let default = oidc_snapshot_provider("default", "client-a"); + let corp = oidc_snapshot_provider("corp", "client-b"); + let expected = open_id_settings_from_snapshot( + &rustfs_iam::oidc::OidcConfigSnapshot::new(vec![corp.clone(), default.clone()]), + "eu-site-1".to_string(), + ); + let service = rustfs_iam::federation::FederatedIdentityService::new( + rustfs_iam::federation::FederatedIdentityRegistry::new(Arc::new(LegacyOidcProvider { + configs: vec![corp.config, default.config], + })), + ); + + assert_eq!( + serde_json::to_value(open_id_settings_from_runtime(None, Some(&service), "eu-site-1".to_string(),)) + .expect("serialize legacy OpenID settings"), + serde_json::to_value(expected).expect("serialize snapshot OpenID settings"), + ); + } + + #[test] + fn oidc_query_takes_precedence_over_service_only_compatibility() { + let query = SnapshotOidcConfigQuery { + snapshot: rustfs_iam::oidc::OidcConfigSnapshot::new(vec![oidc_snapshot_provider("default", "query-client")]), + }; + let service = rustfs_iam::federation::FederatedIdentityService::new( + rustfs_iam::federation::FederatedIdentityRegistry::new(Arc::new(LegacyOidcProvider { + configs: vec![oidc_snapshot_provider("default", "service-client").config], + })), + ); + + let settings = open_id_settings_from_runtime(Some(&query), Some(&service), "eu-site-1".to_string()); + + assert_eq!(settings.claim_provider.client_id, "query-client"); + } + // A whole provider object present on one side only must not spill its // credential-derived leaves into the rejection message. #[test] diff --git a/rustfs/src/admin/runtime_sources.rs b/rustfs/src/admin/runtime_sources.rs index 938b0ea86..513dfbbbf 100644 --- a/rustfs/src/admin/runtime_sources.rs +++ b/rustfs/src/admin/runtime_sources.rs @@ -23,11 +23,12 @@ use crate::app::object_usecase::DefaultObjectUsecase; use crate::runtime_sources as root_runtime_sources; pub(crate) use crate::runtime_sources::{ AppContext, ServerContextSlot, current_action_credentials, current_boot_time, current_bucket_metadata_handle, - current_bucket_monitor_handle, current_deployment_id, current_endpoints_handle, current_federated_identity_service, - current_iam_handle, current_kms_runtime_service_manager, current_notification_system_for_context, - current_object_data_cache_handle_for_context, current_object_store_handle_for_context, current_ready_iam_handle, - current_region, current_replication_pool_handle, current_replication_stats_handle, - current_replication_stats_handle_for_context, current_server_config_for_context, current_token_signing_key, + current_bucket_monitor_handle, current_deployment_id, current_endpoints_handle, current_federated_identity_runtime, + current_federated_identity_service, current_iam_handle, current_kms_runtime_service_manager, + current_notification_system_for_context, current_object_data_cache_handle_for_context, + current_object_store_handle_for_context, current_ready_iam_handle, current_region, current_replication_pool_handle, + current_replication_stats_handle, current_replication_stats_handle_for_context, current_server_config_for_context, + current_token_signing_key, }; #[cfg(test)] pub(crate) use crate::runtime_sources::{ diff --git a/rustfs/src/app/context.rs b/rustfs/src/app/context.rs index 9e6da5770..ad69727d4 100644 --- a/rustfs/src/app/context.rs +++ b/rustfs/src/app/context.rs @@ -38,7 +38,12 @@ use crate::app::object_data_cache::ObjectDataCacheAdapter; use crate::config::RustFSBufferConfig; use rustfs_config::server_config::Config; use rustfs_credentials::Credentials; -use rustfs_iam::{error::Error as IamError, federation::FederatedIdentityService, store::object::ObjectStore, sys::IamSys}; +use rustfs_iam::{ + error::Error as IamError, + federation::{FederatedIdentityService, oidc::OidcConfigQuery}, + store::object::ObjectStore, + sys::IamSys, +}; use rustfs_io_metrics::{PerformanceMetrics, internode_metrics::InternodeMetrics}; use rustfs_kms::{KmsServiceManager, ObjectEncryptionService}; use rustfs_lock::LockClient; @@ -93,7 +98,25 @@ pub fn resolve_federated_identity_service() -> Option Option> { + resolve_oidc_config_query_with(get_global_app_context()) +} + +/// Resolve the paired federation service and OIDC configuration query from one runtime observation. +pub(crate) fn resolve_federated_identity_runtime() -> Option { + resolve_federated_identity_runtime_with(get_global_app_context()) +} + +/// Publish the initialized federation service and OIDC configuration query. +pub fn publish_federated_identity_runtime( + service: Arc, + oidc_config_query: Arc, +) -> bool { + handles::publish_default_federated_identity_runtime(service, oidc_config_query) +} + +/// Publish a service-only federation runtime for compatibility integrations. pub fn publish_federated_identity_service(service: Arc) -> bool { handles::publish_default_federated_identity_service(service) } @@ -333,9 +356,21 @@ fn resolve_federated_identity_service_with(context: Option>) -> context.and_then(|context| context.federated_identity().handle()) } +fn resolve_oidc_config_query_with(context: Option>) -> Option> { + context.and_then(|context| context.federated_identity().oidc_config_query()) +} + +fn resolve_federated_identity_runtime_with(context: Option>) -> Option { + context.and_then(|context| context.federated_identity().runtime_snapshot()) +} + #[cfg(test)] -fn publish_federated_identity_service_with(context: Option>, service: Arc) -> bool { - context.is_some_and(|context| context.publish_federated_identity_service(service)) +fn publish_federated_identity_runtime_with( + context: Option>, + service: Arc, + oidc_config_query: Arc, +) -> bool { + context.is_some_and(|context| context.publish_federated_identity_runtime(service, oidc_config_query)) } fn resolve_ready_iam_handle_with(context: Arc) -> rustfs_iam::error::Result>> { @@ -518,7 +553,10 @@ mod tests { use crate::config::{RustFSBufferConfig, WorkloadProfile}; use async_trait::async_trait; use rustfs_iam::{ - federation::{FederatedIdentityRegistry, FederatedIdentityService, oidc::StandardOidcAdapter}, + federation::{ + FederatedIdentityRegistry, FederatedIdentityService, + oidc::{OidcConfigQuery, StandardOidcAdapter}, + }, oidc::OidcSys, store::object::ObjectStore, sys::IamSys, @@ -559,34 +597,65 @@ mod tests { } struct TestFederatedIdentityInterface { - service: StdRwLock>>, + runtime: StdRwLock>, + } + + struct TestFederatedIdentityRuntime { + service: Arc, + oidc_config_query: Arc, } impl TestFederatedIdentityInterface { - fn new(service: Option>) -> Self { + fn new(runtime: Option<(Arc, Arc)>) -> Self { Self { - service: StdRwLock::new(service), + runtime: StdRwLock::new(runtime.map(|(service, oidc_config_query)| TestFederatedIdentityRuntime { + service, + oidc_config_query, + })), } } } impl FederatedIdentityInterface for TestFederatedIdentityInterface { fn handle(&self) -> Option> { - self.service.read().ok().and_then(|service| service.as_ref().cloned()) + self.runtime + .read() + .ok() + .and_then(|runtime| runtime.as_ref().map(|runtime| Arc::clone(&runtime.service))) } - fn publish_handle(&self, service: Arc) -> bool { - let Ok(mut published_service) = self.service.write() else { + fn oidc_config_query(&self) -> Option> { + self.runtime + .read() + .ok() + .and_then(|runtime| runtime.as_ref().map(|runtime| Arc::clone(&runtime.oidc_config_query))) + } + + fn runtime_snapshot(&self) -> Option { + self.runtime.read().ok().and_then(|runtime| { + runtime + .as_ref() + .map(|runtime| (Arc::clone(&runtime.service), Some(Arc::clone(&runtime.oidc_config_query)))) + }) + } + + fn publish_runtime(&self, service: Arc, oidc_config_query: Arc) -> bool { + let Ok(mut published_runtime) = self.runtime.write() else { return false; }; - *published_service = Some(service); + *published_runtime = Some(TestFederatedIdentityRuntime { + service, + oidc_config_query, + }); true } } - fn test_federated_identity_service(oidc: OidcSys) -> Arc { + fn test_federated_identity_runtime(oidc: OidcSys) -> (Arc, Arc) { let adapter = Arc::new(StandardOidcAdapter::new(Arc::new(oidc))); - Arc::new(FederatedIdentityService::new(FederatedIdentityRegistry::new(adapter))) + let oidc_config_query = adapter.clone(); + let service = Arc::new(FederatedIdentityService::new(FederatedIdentityRegistry::new(adapter))); + (service, oidc_config_query) } struct TestKmsInterface { @@ -1012,8 +1081,8 @@ mod tests { Ok(sys) => sys, Err(err) => unreachable!("test OIDC fallback sys should initialize: {err}"), }; - let context_oidc = test_federated_identity_service(context_oidc_sys); - let fallback_oidc = test_federated_identity_service(fallback_oidc_sys); + let (context_oidc, context_oidc_query) = test_federated_identity_runtime(context_oidc_sys); + let (fallback_oidc, fallback_oidc_query) = test_federated_identity_runtime(fallback_oidc_sys); let context_token_signing_key = "context-token-signing-key".to_string(); let context = Arc::new(AppContext::with_test_interfaces( @@ -1124,12 +1193,33 @@ mod tests { ); assert!(resolve_iam_ready_with(Some(context.clone())).expect("context IAM ready")); assert!(resolve_federated_identity_service_with(Some(context.clone())).is_none()); - assert!(publish_federated_identity_service_with(Some(context.clone()), context_oidc.clone())); + assert!(resolve_oidc_config_query_with(Some(context.clone())).is_none()); + assert!(publish_federated_identity_runtime_with( + Some(context.clone()), + context_oidc.clone(), + context_oidc_query.clone(), + )); let resolved_oidc = resolve_federated_identity_service_with(Some(context.clone())); assert!(resolved_oidc.as_ref().is_some_and(|oidc| Arc::ptr_eq(oidc, &context_oidc))); - assert!(publish_federated_identity_service_with(Some(context.clone()), fallback_oidc.clone())); + let resolved_oidc_query = resolve_oidc_config_query_with(Some(context.clone())).expect("context OIDC config query"); + assert!(Arc::ptr_eq(&resolved_oidc_query, &context_oidc_query)); + let (resolved_oidc, resolved_oidc_query) = + resolve_federated_identity_runtime_with(Some(context.clone())).expect("context federated identity runtime"); + assert!(Arc::ptr_eq(&resolved_oidc, &context_oidc)); + assert!( + resolved_oidc_query + .as_ref() + .is_some_and(|query| Arc::ptr_eq(query, &context_oidc_query)) + ); + assert!(publish_federated_identity_runtime_with( + Some(context.clone()), + fallback_oidc.clone(), + fallback_oidc_query.clone(), + )); let resolved_oidc = resolve_federated_identity_service_with(Some(context.clone())); assert!(resolved_oidc.as_ref().is_some_and(|oidc| Arc::ptr_eq(oidc, &fallback_oidc))); + let resolved_oidc_query = resolve_oidc_config_query_with(Some(context.clone())).expect("fallback OIDC config query"); + assert!(Arc::ptr_eq(&resolved_oidc_query, &fallback_oidc_query)); assert_eq!( resolve_token_signing_key_with(Some(context.clone())).as_deref(), Some(context_token_signing_key.as_str()) @@ -1250,9 +1340,11 @@ mod tests { assert!(resolve_iam_ready_with(None).is_none()); assert!(resolve_iam_handle_with(None).is_none()); assert!(resolve_federated_identity_service_with(None).is_none()); + assert!(resolve_oidc_config_query_with(None).is_none()); + assert!(resolve_federated_identity_runtime_with(None).is_none()); assert!(resolve_token_signing_key_with(None).is_none()); assert!(resolve_notify_interface_for_context(None).is_none()); - assert!(!publish_federated_identity_service_with(None, context_oidc)); + assert!(!publish_federated_identity_runtime_with(None, context_oidc, context_oidc_query)); assert!(resolve_bucket_metadata_handle_with(None).is_none()); assert!(resolve_bucket_monitor_handle_with(None).is_none()); assert!(resolve_replication_pool_handle_with(None).is_none()); diff --git a/rustfs/src/app/context/global.rs b/rustfs/src/app/context/global.rs index b26151223..b5a8047cc 100644 --- a/rustfs/src/app/context/global.rs +++ b/rustfs/src/app/context/global.rs @@ -35,7 +35,11 @@ use super::interfaces::{ }; use crate::app::object_data_cache::ObjectDataCacheAdapter; use crate::app::object_traffic_health::ObjectTrafficHealth; -use rustfs_iam::{federation::FederatedIdentityService, store::object::ObjectStore, sys::IamSys}; +use rustfs_iam::{ + federation::{FederatedIdentityService, oidc::OidcConfigQuery}, + store::object::ObjectStore, + sys::IamSys, +}; use rustfs_kms::KmsServiceManager; use std::sync::{Arc, OnceLock}; @@ -157,6 +161,15 @@ impl AppContext { self.federated_identity.clone() } + pub fn publish_federated_identity_runtime( + &self, + service: Arc, + oidc_config_query: Arc, + ) -> bool { + self.federated_identity.publish_runtime(service, oidc_config_query) + } + + /// Publish a service-only federation runtime for compatibility integrations. pub fn publish_federated_identity_service(&self, service: Arc) -> bool { self.federated_identity.publish_handle(service) } diff --git a/rustfs/src/app/context/handles.rs b/rustfs/src/app/context/handles.rs index fdc9f99cc..f2c9d0944 100644 --- a/rustfs/src/app/context/handles.rs +++ b/rustfs/src/app/context/handles.rs @@ -20,18 +20,23 @@ use super::super::storage_api::context::runtime::{ }; use super::interfaces::{ ActionCredentialInterface, BootTimeInterface, BucketMetadataInterface, BucketMonitorInterface, BufferConfigInterface, - DeploymentIdInterface, EndpointsInterface, ExpiryStateInterface, FederatedIdentityInterface, IamInterface, - InternodeMetricsInterface, KmsInterface, KmsRuntimeInterface, LocalNodeNameInterface, LockClientInterface, - LockClientsInterface, NotificationSystemInterface, NotifyInterface, OutboundTlsRuntimeInterface, PerformanceMetricsInterface, - RegionInterface, ReplicationPoolInterface, ReplicationStatsInterface, RuntimePortInterface, S3SelectDbInterface, - ScannerMetricsInterface, ServerConfigInterface, StorageClassInterface, TierConfigInterface, TransitionStateInterface, + DeploymentIdInterface, EndpointsInterface, ExpiryStateInterface, FederatedIdentityInterface, + FederatedIdentityRuntimeSnapshot, IamInterface, InternodeMetricsInterface, KmsInterface, KmsRuntimeInterface, + LocalNodeNameInterface, LockClientInterface, LockClientsInterface, NotificationSystemInterface, NotifyInterface, + OutboundTlsRuntimeInterface, PerformanceMetricsInterface, RegionInterface, ReplicationPoolInterface, + ReplicationStatsInterface, RuntimePortInterface, S3SelectDbInterface, ScannerMetricsInterface, ServerConfigInterface, + StorageClassInterface, TierConfigInterface, TransitionStateInterface, }; use super::runtime_sources; use crate::config::RustFSBufferConfig; use async_trait::async_trait; use rustfs_config::server_config::Config; use rustfs_credentials::Credentials; -use rustfs_iam::{federation::FederatedIdentityService, store::object::ObjectStore, sys::IamSys}; +use rustfs_iam::{ + federation::{FederatedIdentityService, oidc::OidcConfigQuery}, + store::object::ObjectStore, + sys::IamSys, +}; use rustfs_io_metrics::{PerformanceMetrics, internode_metrics::InternodeMetrics}; use rustfs_kms::KmsServiceManager; use rustfs_lock::LockClient; @@ -75,15 +80,32 @@ impl IamInterface for IamHandle { } } -/// Default federated identity service interface adapter. +/// Default federation runtime interface adapter. pub struct FederatedIdentityHandle { - service: StdRwLock>>, + runtime: StdRwLock>, +} + +struct FederatedIdentityRuntime { + service: Arc, + oidc_config_query: Option>, } impl FederatedIdentityHandle { pub fn new(service: Option>) -> Self { Self { - service: StdRwLock::new(service), + runtime: StdRwLock::new(service.map(|service| FederatedIdentityRuntime { + service, + oidc_config_query: None, + })), + } + } + + pub fn with_runtime(runtime: Option<(Arc, Arc)>) -> Self { + Self { + runtime: StdRwLock::new(runtime.map(|(service, oidc_config_query)| FederatedIdentityRuntime { + service, + oidc_config_query: Some(oidc_config_query), + })), } } } @@ -96,14 +118,40 @@ impl Default for FederatedIdentityHandle { impl FederatedIdentityInterface for FederatedIdentityHandle { fn handle(&self) -> Option> { - self.service.read().ok().and_then(|service| service.as_ref().cloned()) + self.runtime_snapshot().map(|(service, _)| service) + } + + fn oidc_config_query(&self) -> Option> { + self.runtime_snapshot().and_then(|(_, oidc_config_query)| oidc_config_query) + } + + fn runtime_snapshot(&self) -> Option { + self.runtime.read().ok().and_then(|runtime| { + runtime + .as_ref() + .map(|runtime| (Arc::clone(&runtime.service), runtime.oidc_config_query.as_ref().map(Arc::clone))) + }) } fn publish_handle(&self, service: Arc) -> bool { - let Ok(mut published_service) = self.service.write() else { + let Ok(mut published_runtime) = self.runtime.write() else { return false; }; - *published_service = Some(service); + *published_runtime = Some(FederatedIdentityRuntime { + service, + oidc_config_query: None, + }); + true + } + + fn publish_runtime(&self, service: Arc, oidc_config_query: Arc) -> bool { + let Ok(mut published_runtime) = self.runtime.write() else { + return false; + }; + *published_runtime = Some(FederatedIdentityRuntime { + service, + oidc_config_query: Some(oidc_config_query), + }); true } } @@ -584,6 +632,14 @@ pub fn default_federated_identity_interface() -> Arc, + oidc_config_query: Arc, +) -> bool { + default_federated_identity_handle().publish_runtime(service, oidc_config_query) +} + +/// Publish a service-only federation runtime for compatibility integrations. pub fn publish_default_federated_identity_service(service: Arc) -> bool { default_federated_identity_handle().publish_handle(service) } @@ -592,6 +648,12 @@ pub fn federated_identity_interface(service: Option, Arc)>, +) -> Arc { + Arc::new(FederatedIdentityHandle::with_runtime(runtime)) +} + pub fn default_region_interface() -> Arc { Arc::new(RegionHandle) } @@ -624,28 +686,34 @@ pub fn default_buffer_config_interface() -> Arc { mod tests { use super::{ KmsRuntimeHandle, KmsServiceManager, ServerConfigHandle, default_federated_identity_interface, - federated_identity_interface, publish_default_federated_identity_service, runtime_sources, + federated_identity_interface, federated_identity_runtime_interface, publish_default_federated_identity_runtime, + runtime_sources, }; use crate::app::context::interfaces::{KmsRuntimeInterface, ServerConfigInterface}; use rustfs_config::server_config::Config; use rustfs_iam::{ - federation::{FederatedIdentityRegistry, FederatedIdentityService, oidc::StandardOidcAdapter}, + federation::{ + FederatedIdentityRegistry, FederatedIdentityService, + oidc::{OidcConfigQuery, StandardOidcAdapter}, + }, oidc::OidcSys, }; use std::collections::HashMap; use std::sync::Arc; - fn test_federated_identity_service() -> Arc { + fn test_federated_identity_runtime() -> (Arc, Arc) { let oidc = OidcSys::empty().expect("empty OIDC configuration should be valid"); let adapter = Arc::new(StandardOidcAdapter::new(Arc::new(oidc))); - Arc::new(FederatedIdentityService::new(FederatedIdentityRegistry::new(adapter))) + let oidc_config_query = adapter.clone(); + let service = Arc::new(FederatedIdentityService::new(FederatedIdentityRegistry::new(adapter))); + (service, oidc_config_query) } #[test] fn federated_identity_handle_preserves_early_publish_and_allows_replacement() { - let first = test_federated_identity_service(); - let second = test_federated_identity_service(); - let interface = federated_identity_interface(Some(first.clone())); + let (first, first_query) = test_federated_identity_runtime(); + let (second, second_query) = test_federated_identity_runtime(); + let interface = federated_identity_runtime_interface(Some((first.clone(), first_query.clone()))); assert!( interface @@ -654,21 +722,57 @@ mod tests { .is_some_and(|resolved| Arc::ptr_eq(resolved, &first)) ); - assert!(interface.publish_handle(second.clone())); + let resolved_query = interface.oidc_config_query().expect("initial OIDC config query"); + assert!(Arc::ptr_eq(&resolved_query, &first_query)); + let (resolved_service, resolved_query) = interface.runtime_snapshot().expect("initial federated identity runtime"); + assert!(Arc::ptr_eq(&resolved_service, &first)); + assert!(resolved_query.as_ref().is_some_and(|query| Arc::ptr_eq(query, &first_query))); + + assert!(interface.publish_runtime(second.clone(), second_query.clone())); assert!( interface .handle() .as_ref() .is_some_and(|resolved| Arc::ptr_eq(resolved, &second)) ); + assert!(interface.oidc_config_query().is_some()); + let (resolved_service, resolved_query) = interface.runtime_snapshot().expect("replacement federated identity runtime"); + assert!(Arc::ptr_eq(&resolved_service, &second)); + assert!(resolved_query.as_ref().is_some_and(|query| Arc::ptr_eq(query, &second_query))); + } + + #[test] + fn federated_identity_handle_preserves_service_only_compatibility() { + let (service, _) = test_federated_identity_runtime(); + let (replacement, _) = test_federated_identity_runtime(); + let interface = federated_identity_interface(Some(service.clone())); + + assert!( + interface + .handle() + .as_ref() + .is_some_and(|resolved| Arc::ptr_eq(resolved, &service)) + ); + assert!(interface.oidc_config_query().is_none()); + assert!(interface.publish_handle(replacement.clone())); + assert!( + interface + .handle() + .as_ref() + .is_some_and(|resolved| Arc::ptr_eq(resolved, &replacement)) + ); + assert!(interface.oidc_config_query().is_none()); + let (resolved_service, resolved_query) = interface.runtime_snapshot().expect("service-only federated identity runtime"); + assert!(Arc::ptr_eq(&resolved_service, &replacement)); + assert!(resolved_query.is_none()); } #[test] fn default_federated_identity_handle_shares_early_publish_and_replacement() { - let first = test_federated_identity_service(); - let second = test_federated_identity_service(); + let (first, first_query) = test_federated_identity_runtime(); + let (second, second_query) = test_federated_identity_runtime(); - assert!(publish_default_federated_identity_service(first.clone())); + assert!(publish_default_federated_identity_runtime(first.clone(), first_query)); let interface = default_federated_identity_interface(); assert!( interface @@ -677,7 +781,7 @@ mod tests { .is_some_and(|resolved| Arc::ptr_eq(resolved, &first)) ); - assert!(publish_default_federated_identity_service(second.clone())); + assert!(publish_default_federated_identity_runtime(second.clone(), second_query)); assert!( interface .handle() diff --git a/rustfs/src/app/context/interfaces.rs b/rustfs/src/app/context/interfaces.rs index eeea0da4e..ac136ee11 100644 --- a/rustfs/src/app/context/interfaces.rs +++ b/rustfs/src/app/context/interfaces.rs @@ -22,7 +22,11 @@ use crate::config::RustFSBufferConfig; use async_trait::async_trait; use rustfs_config::server_config::Config; use rustfs_credentials::Credentials; -use rustfs_iam::{federation::FederatedIdentityService, store::object::ObjectStore, sys::IamSys}; +use rustfs_iam::{ + federation::{FederatedIdentityService, oidc::OidcConfigQuery}, + store::object::ObjectStore, + sys::IamSys, +}; use rustfs_io_metrics::{PerformanceMetrics, internode_metrics::InternodeMetrics}; use rustfs_kms::KmsServiceManager; use rustfs_lock::LockClient; @@ -43,12 +47,26 @@ pub trait IamInterface: Send + Sync { } } -/// Federated identity service interface for admin and runtime consumers. +/// Published federation runtime handles for authentication and OIDC configuration consumers. +pub type FederatedIdentityRuntimeSnapshot = (Arc, Option>); + pub trait FederatedIdentityInterface: Send + Sync { fn handle(&self) -> Option>; + fn oidc_config_query(&self) -> Option> { + None + } + fn runtime_snapshot(&self) -> Option { + self.handle().map(|service| (service, self.oidc_config_query())) + } + /// Compatibility publication hook for service-only implementations. + /// Standard OIDC runtimes publish the paired query through + /// [`Self::publish_runtime`]. fn publish_handle(&self, _service: Arc) -> bool { false } + fn publish_runtime(&self, service: Arc, _oidc_config_query: Arc) -> bool { + self.publish_handle(service) + } } /// KMS interface for application-layer use-cases. diff --git a/rustfs/src/runtime_sources.rs b/rustfs/src/runtime_sources.rs index 830a38e4e..e685878a3 100644 --- a/rustfs/src/runtime_sources.rs +++ b/rustfs/src/runtime_sources.rs @@ -14,7 +14,7 @@ use crate::app::context; use rustfs_config::ENV_RUSTFS_BROWSER_REDIRECT_URL; -use rustfs_iam::federation::FederatedIdentityService; +use rustfs_iam::federation::{FederatedIdentityService, oidc::OidcConfigQuery}; use std::sync::Arc; use tracing::warn; @@ -35,6 +35,7 @@ pub(crate) use context::{ resolve_bucket_monitor_handle as current_bucket_monitor_handle, resolve_buffer_config as current_buffer_config, resolve_daily_tier_stats as current_daily_tier_stats, resolve_deployment_id as current_deployment_id, resolve_encryption_service as current_encryption_service, resolve_endpoints_handle as current_endpoints_handle, + resolve_federated_identity_runtime as current_federated_identity_runtime, resolve_federated_identity_service as current_federated_identity_service, resolve_iam_handle as current_iam_handle, resolve_iam_ready as current_iam_ready, resolve_internode_metrics as current_internode_metrics, resolve_kms_runtime_service_manager as current_kms_runtime_service_manager, @@ -58,7 +59,15 @@ pub(crate) use context::{ resolve_tier_config_handle as current_tier_config_handle, resolve_token_signing_key as current_token_signing_key, }; -pub(crate) fn publish_federated_identity_service(service: Arc) -> bool { +pub(crate) fn publish_federated_identity_runtime( + service: Arc, + oidc_config_query: Arc, +) -> bool { + warn_on_browser_redirect_fallback(&service); + context::publish_federated_identity_runtime(service, oidc_config_query) +} + +fn warn_on_browser_redirect_fallback(service: &FederatedIdentityService) { let browser_redirect_url = rustfs_utils::get_env_opt_str(ENV_RUSTFS_BROWSER_REDIRECT_URL); if service.has_providers() && browser_redirect_url.as_deref().is_none_or(|value| value.trim().is_empty()) { warn!( @@ -72,8 +81,6 @@ pub(crate) fn publish_federated_identity_service(service: Arc Option> { #[cfg(test)] mod tests { use super::*; - use rustfs_iam::{ - federation::{ - FederatedAuthorization, FederatedCodeExchange, FederatedIdentityProvider, FederatedIdentityRegistry, - Result as FederationResult, - }, - oidc::{OidcProviderConfig, OidcProviderSummary}, + use rustfs_iam::federation::oidc::OidcConfigQuery; + use rustfs_iam::federation::{ + FederatedAuthorization, FederatedCodeExchange, FederatedIdentityProvider, FederatedIdentityRegistry, + FederatedProviderView, FederatedRedirectPolicy, Result as FederationResult, }; + use rustfs_iam::oidc::OidcConfigSnapshot; use std::{ io::{self, Write}, sync::Mutex, @@ -119,15 +125,15 @@ mod tests { self.has_providers } - fn list_providers(&self) -> Vec { + fn list_providers(&self) -> Vec { Vec::new() } - fn list_visible_providers(&self) -> Vec { + fn list_visible_providers(&self) -> Vec { Vec::new() } - fn provider_config(&self, _id: &str) -> Option<&OidcProviderConfig> { + fn redirect_policy(&self, _provider_id: &str) -> Option { None } @@ -161,6 +167,12 @@ mod tests { } } + impl OidcConfigQuery for TestProvider { + fn config_snapshot(&self) -> OidcConfigSnapshot { + OidcConfigSnapshot::new(Vec::new()) + } + } + #[derive(Clone, Default)] struct CapturedLogs(Arc>>); @@ -185,9 +197,10 @@ mod tests { } } - fn test_service(has_providers: bool) -> Arc { + fn test_runtime(has_providers: bool) -> (Arc, Arc) { let provider = Arc::new(TestProvider { has_providers }); - Arc::new(FederatedIdentityService::new(FederatedIdentityRegistry::new(provider))) + let service = Arc::new(FederatedIdentityService::new(FederatedIdentityRegistry::new(provider.clone()))); + (service, provider) } fn capture_startup_publication(has_providers: bool, browser_redirect_url: Option<&str>) -> String { @@ -204,7 +217,8 @@ mod tests { ); tracing::subscriber::with_default(subscriber, || { - assert!(publish_federated_identity_service(test_service(has_providers))); + let (service, oidc_config_query) = test_runtime(has_providers); + assert!(publish_federated_identity_runtime(service, oidc_config_query)); }); String::from_utf8(captured.lock().expect("captured log lock").clone()).expect("captured logs must be UTF-8") diff --git a/rustfs/src/startup_auth.rs b/rustfs/src/startup_auth.rs index 9120b1fde..3674105f2 100644 --- a/rustfs/src/startup_auth.rs +++ b/rustfs/src/startup_auth.rs @@ -13,7 +13,10 @@ // limitations under the License. use rustfs_iam::{ - federation::{FederatedIdentityRegistry, FederatedIdentityService, oidc::StandardOidcAdapter}, + federation::{ + FederatedIdentityRegistry, FederatedIdentityService, + oidc::{OidcConfigQuery, StandardOidcAdapter}, + }, get_oidc, init_oidc_sys_with_extra_root_ca_provider, oidc::{OidcExtraRootCaMaterial, OidcExtraRootCaProvider, OidcSys}, }; @@ -56,7 +59,8 @@ pub(crate) async fn init_auth_integrations() -> Result<()> { match init_oidc_sys_with_extra_root_ca_provider(oidc_extra_root_ca_provider()).await { Ok(()) => { if let Some(oidc) = get_oidc() { - crate::runtime_sources::publish_federated_identity_service(standard_oidc_service(oidc)); + let (service, oidc_config_query) = standard_oidc_runtime(oidc); + crate::runtime_sources::publish_federated_identity_runtime(service, oidc_config_query); } } Err(e) => { @@ -73,10 +77,11 @@ pub(crate) async fn init_auth_integrations() -> Result<()> { Ok(()) } -fn standard_oidc_service(oidc: Arc) -> Arc { +fn standard_oidc_runtime(oidc: Arc) -> (Arc, Arc) { let adapter = Arc::new(StandardOidcAdapter::new(oidc)); + let oidc_config_query = adapter.clone(); let registry = FederatedIdentityRegistry::new(adapter); - Arc::new(FederatedIdentityService::new(registry)) + (Arc::new(FederatedIdentityService::new(registry)), oidc_config_query) } pub(crate) fn oidc_extra_root_ca_provider() -> OidcExtraRootCaProvider { @@ -121,14 +126,16 @@ mod tests { use super::*; #[test] - fn standard_oidc_service_retains_one_oidc_arc() { + fn standard_oidc_runtime_retains_one_oidc_arc() { let oidc = Arc::new(OidcSys::empty().expect("empty OIDC configuration should be valid")); let retained = Arc::clone(&oidc); - let service = standard_oidc_service(oidc); + let (service, oidc_config_query) = standard_oidc_runtime(oidc); - assert_eq!(Arc::strong_count(&retained), 2, "the service should retain one OIDC Arc"); + assert_eq!(Arc::strong_count(&retained), 2, "the runtime should retain one OIDC Arc"); drop(service); - assert_eq!(Arc::strong_count(&retained), 1, "dropping the service should release its OIDC Arc"); + assert_eq!(Arc::strong_count(&retained), 2, "the query should retain the shared adapter"); + drop(oidc_config_query); + assert_eq!(Arc::strong_count(&retained), 1, "dropping the runtime should release its OIDC Arc"); } }