diff --git a/crates/config/src/identity.rs b/crates/config/src/identity.rs index d82231d4d..aa742b932 100644 --- a/crates/config/src/identity.rs +++ b/crates/config/src/identity.rs @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::path::PathBuf; -use crate::common::default_sql_driver; +use crate::common::{default_sql_driver, default_true}; use crate::pagination::ListLimitConfig; /// Identity provider. @@ -40,6 +40,19 @@ pub struct IdentityProvider { #[serde(default = "default_domain_config_dir")] pub domain_config_dir: PathBuf, + /// Whether per-domain identity/LDAP driver configuration is loaded from + /// `domain_config_dir` files. Off by default, as in python-keystone; the + /// resolution layer overlays file-based configuration only when this is + /// set. + #[serde(default)] + pub domain_specific_drivers_enabled: bool, + + /// Whether per-domain configuration is taken from the database, at the + /// highest precedence, in addition to any file-based configuration. On by + /// default, matching python-keystone. + #[serde(default = "default_true")] + pub domain_configurations_from_database: bool, + /// Identity provider driver. #[serde(default = "default_sql_driver")] pub driver: String, @@ -71,6 +84,8 @@ impl Default for IdentityProvider { caching: false, default_domain_id: default_domain_id(), domain_config_dir: default_domain_config_dir(), + domain_specific_drivers_enabled: false, + domain_configurations_from_database: default_true(), driver: default_sql_driver(), max_password_length: default_max_password_length(), password_hashing_algorithm: PasswordHashingAlgo::Bcrypt, diff --git a/crates/core-types/src/domain_config/config.rs b/crates/core-types/src/domain_config/config.rs index 8f2000a8a..59d531549 100644 --- a/crates/core-types/src/domain_config/config.rs +++ b/crates/core-types/src/domain_config/config.rs @@ -526,6 +526,45 @@ impl DomainConfig { .collect() } + /// Overlay another configuration onto this one, option by option. + /// + /// Every option `over` sets replaces this configuration's; an option only + /// this configuration sets is kept; a group present on only one side is + /// taken whole. This is how the resolution layer stacks the file-based + /// configuration under the database one, which takes precedence (issue + /// #958). + /// + /// Both sides are expected to have come from [`Self::from_value`] or + /// [`Self::from_options`], so their options are already whitelist-filtered; + /// like [`Self::resolve`], this does not re-validate. + /// + /// # Parameters + /// - `over`: The configuration whose options win. + pub fn overlay(&mut self, over: &DomainConfig) { + for group in over.groups.values() { + let target = self + .groups + .entry(group.name) + .or_insert_with(|| DomainConfigGroup::new(group.name)); + for (option, value) in &group.options { + target.options.insert(option.clone(), value.clone()); + } + } + } + + /// [`Self::overlay`] as a chainable, consuming call. + /// + /// # Parameters + /// - `over`: The configuration whose options win. + /// + /// # Returns + /// - `Self` - This configuration with `over` applied. + #[must_use] + pub fn overlaid_with(mut self, over: &DomainConfig) -> Self { + self.overlay(over); + self + } + /// The domain's `[identity]` section: the global one with the domain's /// overrides applied. /// diff --git a/crates/core-types/src/domain_config/config/tests.rs b/crates/core-types/src/domain_config/config/tests.rs index 3c3c135dd..29348f2ff 100644 --- a/crates/core-types/src/domain_config/config/tests.rs +++ b/crates/core-types/src/domain_config/config/tests.rs @@ -909,3 +909,80 @@ mod substitution { ); } } + +mod overlay { + //! Option-by-option merge of one configuration onto another — the primitive + //! the resolution layer stacks the file configuration under the database + //! one with. + + use super::*; + + /// A group's option, or `None` when the group or option is unset. + fn option<'a>( + config: &'a DomainConfig, + group: DomainConfigGroupName, + name: &str, + ) -> Option<&'a Value> { + config.group(group).and_then(|group| group.get(name)) + } + + #[test] + fn the_overlay_wins_an_option_both_sides_set() { + let mut base = config_from(json!({"ldap": {"url": "ldap://file"}})); + base.overlay(&config_from(json!({"ldap": {"url": "ldap://db"}}))); + assert_eq!( + option(&base, DomainConfigGroupName::Ldap, "url"), + Some(&json!("ldap://db")) + ); + } + + #[test] + fn an_option_only_the_base_set_is_kept() { + let base = config_from(json!({"ldap": {"url": "ldap://file", "suffix": "dc=file"}})) + .overlaid_with(&config_from(json!({"ldap": {"url": "ldap://db"}}))); + assert_eq!( + option(&base, DomainConfigGroupName::Ldap, "url"), + Some(&json!("ldap://db")), + "the overlapping option took the overlay value" + ); + assert_eq!( + option(&base, DomainConfigGroupName::Ldap, "suffix"), + Some(&json!("dc=file")), + "the base-only option survived" + ); + } + + #[test] + fn a_group_present_on_one_side_only_is_taken_whole() { + let merged = config_from(json!({"identity": {"driver": "ldap"}})) + .overlaid_with(&config_from(json!({"ldap": {"url": "ldap://db"}}))); + assert_eq!( + option(&merged, DomainConfigGroupName::Identity, "driver"), + Some(&json!("ldap")) + ); + assert_eq!( + option(&merged, DomainConfigGroupName::Ldap, "url"), + Some(&json!("ldap://db")) + ); + } + + #[test] + fn overlays_a_sensitive_option() { + let base = config_from(json!({"ldap": {"password": "file-secret"}})) + .overlaid_with(&config_from(json!({"ldap": {"password": PASSWORD}}))); + assert_eq!( + option(&base, DomainConfigGroupName::Ldap, "password"), + Some(&json!(PASSWORD)) + ); + } + + #[test] + fn overlaying_an_empty_configuration_changes_nothing() { + let base = config_from(json!({"ldap": {"url": "ldap://file"}})) + .overlaid_with(&DomainConfig::new()); + assert_eq!( + option(&base, DomainConfigGroupName::Ldap, "url"), + Some(&json!("ldap://file")) + ); + } +} diff --git a/crates/core/src/domain_config/mod.rs b/crates/core/src/domain_config/mod.rs index fa11fe048..b1ca5f8b8 100644 --- a/crates/core/src/domain_config/mod.rs +++ b/crates/core/src/domain_config/mod.rs @@ -26,6 +26,8 @@ pub mod backend; pub mod error; +pub mod resolver; pub use backend::DomainConfigBackend; pub use error::DomainConfigProviderError; +pub use resolver::DomainConfigResolver; diff --git a/crates/core/src/domain_config/resolver.rs b/crates/core/src/domain_config/resolver.rs new file mode 100644 index 000000000..045bc23ad --- /dev/null +++ b/crates/core/src/domain_config/resolver.rs @@ -0,0 +1,140 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//! # Domain configuration resolution +//! +//! A domain's configuration can come from two places: per-domain files under +//! `[identity] domain_config_dir` (the `fs` driver) and the database (the `sql` +//! driver, written through the config API). python-keystone layers the file +//! configuration first and lets the database one override it; this module does +//! the same, gated by the two `[identity]` switches: +//! +//! - `domain_specific_drivers_enabled` — consult the file source at all; +//! - `domain_configurations_from_database` — consult the database source, which +//! wins where both set the same option. +//! +//! The result is the raw, still-serializable [`DomainConfig`] the config API +//! returns. A consumer that needs a configuration a driver can use — the +//! identity backend selection of issue #960 — runs +//! [`DomainConfig::substitute`] and then `resolve_identity` / `resolve_ldap` +//! on top of it; those steps inline secrets and so produce a value that must +//! not reach a response, which is why they are deliberately left to the +//! caller. + +use std::sync::Arc; + +use openstack_keystone_config::Config; +use openstack_keystone_core_types::domain_config::DomainConfig; + +use crate::domain_config::backend::DomainConfigBackend; +use crate::domain_config::error::DomainConfigProviderError; +use crate::keystone::ServiceState; +use crate::plugin_manager::PluginManagerApi; + +/// Merges a domain's file-based and database-stored configuration into one. +/// +/// Holds a handle to each source it is configured to use; a source that is +/// switched off in `[identity]` is simply `None`, and a resolver with neither +/// resolves every domain to the empty configuration. +pub struct DomainConfigResolver { + /// The `fs` driver, `Some` when + /// `[identity] domain_specific_drivers_enabled` is set. + file: Option>, + /// The `sql` driver, `Some` when + /// `[identity] domain_configurations_from_database` is set. Overrides the + /// file source option by option. + database: Option>, +} + +impl DomainConfigResolver { + /// Wire the resolver from the running configuration and the registered + /// domain-config backends. + /// + /// # Parameters + /// - `config`: The running service configuration; its `[identity]` + /// switches decide which sources are consulted. + /// - `plugin_manager`: Provides the `"fs"` / `"sql"` backends by name. + /// + /// # Returns + /// - `Result` - The resolver, or + /// [`DomainConfigProviderError::UnsupportedDriver`] when an enabled + /// source has no registered backend. + pub fn new( + config: &Config, + plugin_manager: &P, + ) -> Result { + let file = if config.identity.domain_specific_drivers_enabled { + Some(plugin_manager.get_domain_config_backend("fs")?.clone()) + } else { + None + }; + let database = if config.identity.domain_configurations_from_database { + Some(plugin_manager.get_domain_config_backend("sql")?.clone()) + } else { + None + }; + Ok(Self { file, database }) + } + + /// A resolver with no source: every domain resolves to the empty + /// configuration. Used by the mocked provider builder. + /// + /// # Returns + /// - `Self` - A resolver that consults nothing. + pub fn disabled() -> Self { + Self { + file: None, + database: None, + } + } + + /// The effective stored configuration for a domain: the file source + /// overlaid by the database source. + /// + /// `%(option)s` references are not expanded and the global `[identity]` / + /// `[ldap]` sections are not applied — the result is the raw overlay the + /// config API serves. See the module note for the steps a driver-facing + /// consumer adds. + /// + /// # Parameters + /// - `state`: The current service state, handed to each backend. + /// - `domain_id`: The ID of the domain to resolve. + /// + /// # Returns + /// - `Result` - The merged + /// configuration, empty when no source has one for the domain, or the + /// first error a source returns. + pub async fn effective_config( + &self, + state: &ServiceState, + domain_id: &str, + ) -> Result { + let mut resolved = DomainConfig::new(); + if let Some(file) = &self.file + && let Some(stored) = file.get_domain_config(state, domain_id).await? + { + resolved.overlay(&stored); + } + if let Some(database) = &self.database + && let Some(stored) = database.get_domain_config(state, domain_id).await? + { + resolved.overlay(&stored); + } + Ok(resolved) + } +} + +#[cfg(test)] +#[path = "resolver/tests.rs"] +mod tests; diff --git a/crates/core/src/domain_config/resolver/tests.rs b/crates/core/src/domain_config/resolver/tests.rs new file mode 100644 index 000000000..8f772dcb0 --- /dev/null +++ b/crates/core/src/domain_config/resolver/tests.rs @@ -0,0 +1,162 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use serde_json::{Value, json}; + +use openstack_keystone_core_types::domain_config::DomainConfigGroupName; + +use super::*; +use crate::domain_config::backend::MockDomainConfigBackend; +use crate::tests::get_mocked_state; + +/// A `DomainConfig` from a request-shaped body. +fn config(body: Value) -> DomainConfig { + DomainConfig::from_value(body).expect("a valid domain configuration") +} + +/// A backend whose `get_domain_config` always answers `answer`. +fn source( + answer: Result, DomainConfigProviderError>, +) -> Arc { + let mut mock = MockDomainConfigBackend::new(); + mock.expect_get_domain_config().returning(move |_, _| { + answer + .as_ref() + .map(|maybe| maybe.clone()) + .map_err(|err| DomainConfigProviderError::Driver(err.to_string())) + }); + Arc::new(mock) +} + +/// An `ldap` option of a resolved configuration. +fn ldap<'a>(resolved: &'a DomainConfig, option: &str) -> Option<&'a Value> { + resolved + .group(DomainConfigGroupName::Ldap) + .and_then(|group| group.get(option)) +} + +#[tokio::test] +async fn no_source_resolves_to_the_empty_configuration() { + let state = get_mocked_state(None, None).await; + let resolver = DomainConfigResolver { + file: None, + database: None, + }; + assert!( + resolver + .effective_config(&state, "d1") + .await + .expect("resolvable") + .is_empty() + ); +} + +#[tokio::test] +async fn disabled_resolves_to_the_empty_configuration() { + let state = get_mocked_state(None, None).await; + assert!( + DomainConfigResolver::disabled() + .effective_config(&state, "d1") + .await + .expect("resolvable") + .is_empty() + ); +} + +#[tokio::test] +async fn the_file_source_alone_is_returned_verbatim() { + let state = get_mocked_state(None, None).await; + let resolver = DomainConfigResolver { + file: Some(source(Ok(Some(config( + json!({"ldap": {"url": "ldap://file"}}), + ))))), + database: None, + }; + let resolved = resolver + .effective_config(&state, "d1") + .await + .expect("resolvable"); + assert_eq!(ldap(&resolved, "url"), Some(&json!("ldap://file"))); +} + +#[tokio::test] +async fn the_database_source_alone_is_returned_verbatim() { + let state = get_mocked_state(None, None).await; + let resolver = DomainConfigResolver { + file: None, + database: Some(source(Ok(Some(config( + json!({"ldap": {"url": "ldap://db"}}), + ))))), + }; + let resolved = resolver + .effective_config(&state, "d1") + .await + .expect("resolvable"); + assert_eq!(ldap(&resolved, "url"), Some(&json!("ldap://db"))); +} + +#[tokio::test] +async fn the_database_overrides_the_file_option_by_option() { + let state = get_mocked_state(None, None).await; + let resolver = DomainConfigResolver { + file: Some(source(Ok(Some(config( + json!({"ldap": {"url": "ldap://file", "suffix": "dc=file"}}), + ))))), + database: Some(source(Ok(Some(config( + json!({"ldap": {"url": "ldap://db"}}), + ))))), + }; + let resolved = resolver + .effective_config(&state, "d1") + .await + .expect("resolvable"); + assert_eq!( + ldap(&resolved, "url"), + Some(&json!("ldap://db")), + "the database wins the shared option" + ); + assert_eq!( + ldap(&resolved, "suffix"), + Some(&json!("dc=file")), + "the file-only option survives" + ); +} + +#[tokio::test] +async fn a_source_without_a_configuration_contributes_nothing() { + let state = get_mocked_state(None, None).await; + let resolver = DomainConfigResolver { + file: Some(source(Ok(None))), + database: Some(source(Ok(Some(config( + json!({"ldap": {"url": "ldap://db"}}), + ))))), + }; + let resolved = resolver + .effective_config(&state, "d1") + .await + .expect("resolvable"); + assert_eq!(ldap(&resolved, "url"), Some(&json!("ldap://db"))); +} + +#[tokio::test] +async fn an_error_from_a_source_propagates() { + let state = get_mocked_state(None, None).await; + let resolver = DomainConfigResolver { + file: Some(source(Err(DomainConfigProviderError::Driver( + "boom".to_string(), + )))), + database: None, + }; + assert!(resolver.effective_config(&state, "d1").await.is_err()); +} diff --git a/crates/core/src/plugin_manager.rs b/crates/core/src/plugin_manager.rs index 21db79e37..a8bb57ddc 100644 --- a/crates/core/src/plugin_manager.rs +++ b/crates/core/src/plugin_manager.rs @@ -41,6 +41,7 @@ use crate::catalog::error::CatalogProviderError; use crate::credential::CredentialProviderError; use crate::credential::backend::CredentialBackend; use crate::domain_config::backend::DomainConfigBackend; +use crate::domain_config::error::DomainConfigProviderError; use crate::federation::backend::FederationBackend; use crate::federation::error::FederationProviderError; use crate::identity::backend::IdentityBackend; @@ -229,6 +230,24 @@ pub trait PluginManagerApi { name: S, ) -> Result<&Arc, CredentialProviderError>; + /// Get registered domain config backend. + /// + /// The registry holds one entry per source — `"fs"` for the filesystem + /// driver, `"sql"` for the database one — and the resolution layer asks + /// for each by name (see + /// [`crate::domain_config::DomainConfigResolver`]). + /// + /// # Parameters + /// - `name`: The name of the backend to retrieve. + /// + /// # Returns + /// - `Ok(&Arc)` if found, otherwise + /// `Err(DomainConfigProviderError::UnsupportedDriver)`. + fn get_domain_config_backend>( + &self, + name: S, + ) -> Result<&Arc, DomainConfigProviderError>; + /// Get registered dynamic plugin identity-binding index backend. /// /// # Parameters diff --git a/crates/core/src/provider.rs b/crates/core/src/provider.rs index fca264c05..d9736882d 100644 --- a/crates/core/src/provider.rs +++ b/crates/core/src/provider.rs @@ -40,6 +40,7 @@ use crate::catalog::MockCatalogProvider; use crate::credential::CredentialApi; #[cfg(any(test, feature = "mock"))] use crate::credential::MockCredentialProvider; +use crate::domain_config::DomainConfigResolver; use crate::error::KeystoneError; use crate::federation::FederationApi; #[cfg(any(test, feature = "mock"))] @@ -112,6 +113,11 @@ pub struct Provider { catalog: Box, /// Credential provider. credential: Box, + /// Per-domain configuration resolution layer (issue #958). Merges the + /// file-based and database-stored domain configuration; not yet consumed + /// at runtime. + #[builder(default = "DomainConfigResolver::disabled()")] + domain_config_resolver: DomainConfigResolver, /// Dynamic plugin identity-binding index provider. auth_plugin_identity: Box, /// Federation provider. @@ -325,6 +331,8 @@ impl Provider { cfg, plugin_manager, )?); + let domain_config_resolver = + crate::domain_config::DomainConfigResolver::new(cfg, plugin_manager)?; let auth_plugin_identity = Box::new( crate::auth_plugin_identity::DynamicPluginIdentityService::new(cfg, plugin_manager)?, ); @@ -380,6 +388,7 @@ impl Provider { assignment, catalog, credential, + domain_config_resolver, auth_plugin_identity, federation, identity, @@ -457,6 +466,11 @@ impl Provider { &*self.credential } + /// Get the domain configuration resolution layer (issue #958). + pub fn get_domain_config_resolver(&self) -> &DomainConfigResolver { + &self.domain_config_resolver + } + /// Get the federation provider. pub fn get_federation_provider(&self) -> &dyn FederationApi { &*self.federation diff --git a/crates/keystone/src/plugin_manager.rs b/crates/keystone/src/plugin_manager.rs index f386d36f7..6972ad16c 100644 --- a/crates/keystone/src/plugin_manager.rs +++ b/crates/keystone/src/plugin_manager.rs @@ -37,6 +37,8 @@ use openstack_keystone_core::catalog::backend::CatalogBackend; use openstack_keystone_core::catalog::error::CatalogProviderError; use openstack_keystone_core::credential::CredentialProviderError; use openstack_keystone_core::credential::backend::CredentialBackend; +use openstack_keystone_core::domain_config::backend::DomainConfigBackend; +use openstack_keystone_core::domain_config::error::DomainConfigProviderError; use openstack_keystone_core::federation::backend::FederationBackend; use openstack_keystone_core::federation::error::FederationProviderError; use openstack_keystone_core::identity::backend::IdentityBackend; @@ -86,6 +88,8 @@ pub struct PluginManager { catalog_backends: HashMap>, /// Credential backend plugins. credential_backends: HashMap>, + /// Domain config backend plugins (one per source: `"fs"`, `"sql"`). + domain_config_backends: HashMap>, /// Dynamic plugin identity-binding index backend plugins. auth_plugin_identity_backends: HashMap>, /// Federation backend plugins. @@ -220,6 +224,24 @@ impl PluginManagerApi for PluginManager { ) } + /// Get registered domain config backend. + /// + /// # Parameters + /// * `name` - The name of the source to retrieve (`"fs"` or `"sql"`). + /// + /// # Returns + /// A `Result` containing a reference to the `DomainConfigBackend` if + /// found, or a `DomainConfigProviderError::UnsupportedDriver`. + #[allow(clippy::borrowed_box)] + fn get_domain_config_backend>( + &self, + name: S, + ) -> Result<&Arc, DomainConfigProviderError> { + self.domain_config_backends.get(name.as_ref()).ok_or( + DomainConfigProviderError::UnsupportedDriver(name.as_ref().to_string()), + ) + } + /// Get registered dynamic plugin identity-binding index backend. /// /// # Parameters @@ -888,6 +910,7 @@ impl PluginManager { assignment_backends: HashMap::new(), catalog_backends: HashMap::new(), credential_backends: HashMap::new(), + domain_config_backends: HashMap::new(), auth_plugin_identity_backends: HashMap::new(), federation_backends: HashMap::new(), identity_backends: HashMap::new(), @@ -912,6 +935,7 @@ impl PluginManager { register_backends(config, &mut slf.assignment_backends).await?; register_backends(config, &mut slf.catalog_backends).await?; register_backends(config, &mut slf.credential_backends).await?; + register_backends(config, &mut slf.domain_config_backends).await?; register_backends(config, &mut slf.auth_plugin_identity_backends).await?; register_backends(config, &mut slf.federation_backends).await?; register_backends(config, &mut slf.identity_backends).await?; @@ -933,3 +957,27 @@ impl PluginManager { Ok(slf) } } + +#[cfg(test)] +mod tests { + use openstack_keystone_config::Config; + use openstack_keystone_core::plugin_manager::register_backends; + + use super::*; + + /// Both domain-config sources register into a domain-config-typed map, so + /// `with_config` can hand them to the resolver by name. Exercised through + /// `register_backends` directly: a full `PluginManager` needs on-disk + /// fernet keys the default config has none of. + #[tokio::test] + async fn registers_both_domain_config_sources() { + let mut backends: HashMap> = HashMap::new(); + register_backends(&Config::default(), &mut backends) + .await + .expect("domain-config drivers register from the default config"); + + assert!(backends.contains_key("fs")); + assert!(backends.contains_key("sql")); + assert!(!backends.contains_key("nope")); + } +}