diff --git a/proto/src/constants.rs b/proto/src/constants.rs index 9f3b86588..25f315406 100644 --- a/proto/src/constants.rs +++ b/proto/src/constants.rs @@ -149,6 +149,7 @@ pub const ATTR_OBJECTCLASS: &str = "objectclass"; pub const ATTR_OTHER_NO_INDEX: &str = "other-no-index"; pub const ATTR_PASSKEYS: &str = "passkeys"; pub const ATTR_PASSWORD_IMPORT: &str = "password_import"; +pub const ATTR_PATCH_LEVEL: &str = "patch_level"; pub const ATTR_PHANTOM: &str = "phantom"; pub const ATTR_PRIMARY_CREDENTIAL: &str = "primary_credential"; pub const ATTR_TOTP_IMPORT: &str = "totp_import"; diff --git a/server/lib/src/constants/entries.rs b/server/lib/src/constants/entries.rs index e5c9a1c14..6af2a6182 100644 --- a/server/lib/src/constants/entries.rs +++ b/server/lib/src/constants/entries.rs @@ -145,6 +145,7 @@ pub enum Attribute { OtherNoIndex, PassKeys, PasswordImport, + PatchLevel, Phantom, PrimaryCredential, PrivateCookieKey, @@ -341,6 +342,7 @@ impl TryFrom for Attribute { ATTR_OTHER_NO_INDEX => Attribute::OtherNoIndex, ATTR_PASSKEYS => Attribute::PassKeys, ATTR_PASSWORD_IMPORT => Attribute::PasswordImport, + ATTR_PATCH_LEVEL => Attribute::PatchLevel, ATTR_PHANTOM => Attribute::Phantom, ATTR_PRIMARY_CREDENTIAL => Attribute::PrimaryCredential, ATTR_PRIVATE_COOKIE_KEY => Attribute::PrivateCookieKey, @@ -512,6 +514,7 @@ impl From for &'static str { Attribute::OtherNoIndex => ATTR_OTHER_NO_INDEX, Attribute::PassKeys => ATTR_PASSKEYS, Attribute::PasswordImport => ATTR_PASSWORD_IMPORT, + Attribute::PatchLevel => ATTR_PATCH_LEVEL, Attribute::Phantom => ATTR_PHANTOM, Attribute::PrimaryCredential => ATTR_PRIMARY_CREDENTIAL, Attribute::PrivateCookieKey => ATTR_PRIVATE_COOKIE_KEY, @@ -788,7 +791,7 @@ lazy_static! { Attribute::Description, Value::new_utf8s("System (local) info and metadata object.") ), - (Attribute::Version, Value::Uint32(19)) + (Attribute::Version, Value::Uint32(20)) ); pub static ref E_DOMAIN_INFO_V1: EntryInitNew = entry_init!( diff --git a/server/lib/src/constants/mod.rs b/server/lib/src/constants/mod.rs index 32581b174..bed2e6352 100644 --- a/server/lib/src/constants/mod.rs +++ b/server/lib/src/constants/mod.rs @@ -67,6 +67,7 @@ pub const DOMAIN_LEVEL_5: DomainVersion = 5; /// Domain Level introduced with 1.2.0. /// Deprcated as of 1.4.0 pub const DOMAIN_LEVEL_6: DomainVersion = 6; +pub const PATCH_LEVEL_1: u32 = 1; /// Domain Level introduced with 1.3.0. /// Deprcated as of 1.5.0 @@ -84,6 +85,7 @@ pub const DOMAIN_MIN_LEVEL: DomainVersion = DOMAIN_TGT_LEVEL; pub const DOMAIN_PREVIOUS_TGT_LEVEL: DomainVersion = DOMAIN_LEVEL_6; // The target supported domain functional level pub const DOMAIN_TGT_LEVEL: DomainVersion = DOMAIN_LEVEL_7; +pub const DOMAIN_TGT_PATCH_LEVEL: u32 = PATCH_LEVEL_1; // The maximum supported domain functional level pub const DOMAIN_MAX_LEVEL: DomainVersion = DOMAIN_LEVEL_7; // The maximum supported domain functional level diff --git a/server/lib/src/constants/schema.rs b/server/lib/src/constants/schema.rs index 4d7b4875a..eb2c6082d 100644 --- a/server/lib/src/constants/schema.rs +++ b/server/lib/src/constants/schema.rs @@ -690,6 +690,14 @@ pub static ref SCHEMA_ATTR_KEY_ACTION_IMPORT_JWS_ES256_DL6: SchemaAttribute = Sc ..Default::default() }; +pub static ref SCHEMA_ATTR_PATCH_LEVEL_DL7: SchemaAttribute = SchemaAttribute { + uuid: UUID_SCHEMA_ATTR_PATCH_LEVEL, + name: Attribute::PatchLevel.into(), + description: "".to_string(), + syntax: SyntaxType::Uint32, + ..Default::default() +}; + // === classes === pub static ref SCHEMA_CLASS_PERSON: SchemaClass = SchemaClass { @@ -1074,6 +1082,7 @@ pub static ref SCHEMA_CLASS_DOMAIN_INFO_DL7: SchemaClass = SchemaClass { Attribute::DomainSsid.into(), Attribute::DomainLdapBasedn.into(), Attribute::LdapAllowUnixPwBind.into(), + Attribute::PatchLevel.into(), ], systemmust: vec![ Attribute::Name.into(), diff --git a/server/lib/src/constants/uuids.rs b/server/lib/src/constants/uuids.rs index 6ce78360d..260a92f88 100644 --- a/server/lib/src/constants/uuids.rs +++ b/server/lib/src/constants/uuids.rs @@ -301,6 +301,7 @@ pub const UUID_SCHEMA_ATTR_KEY_ACTION_IMPORT_JWS_ES256: Uuid = uuid!("00000000-0000-0000-0000-ffff00000173"); pub const UUID_SCHEMA_CLASS_KEY_OBJECT_JWE_A128GCM: Uuid = uuid!("00000000-0000-0000-0000-ffff00000174"); +pub const UUID_SCHEMA_ATTR_PATCH_LEVEL: Uuid = uuid!("00000000-0000-0000-0000-ffff00000175"); // System and domain infos // I'd like to strongly criticise william of the past for making poor choices about these allocations. diff --git a/server/lib/src/repl/consumer.rs b/server/lib/src/repl/consumer.rs index 678d7feed..50230121f 100644 --- a/server/lib/src/repl/consumer.rs +++ b/server/lib/src/repl/consumer.rs @@ -296,6 +296,7 @@ impl<'a> QueryServerWriteTransaction<'a> { } ReplIncrementalContext::V1 { domain_version, + domain_patch_level, domain_uuid, ranges, schema_entries, @@ -303,6 +304,7 @@ impl<'a> QueryServerWriteTransaction<'a> { entries, } => self.consumer_apply_changes_v1( *domain_version, + *domain_patch_level, *domain_uuid, ranges, schema_entries, @@ -316,6 +318,7 @@ impl<'a> QueryServerWriteTransaction<'a> { fn consumer_apply_changes_v1( &mut self, ctx_domain_version: DomainVersion, + ctx_domain_patch_level: u32, ctx_domain_uuid: Uuid, ctx_ranges: &BTreeMap, ctx_schema_entries: &[ReplIncrementalEntryV1], @@ -330,6 +333,13 @@ impl<'a> QueryServerWriteTransaction<'a> { return Err(OperationError::ReplDomainLevelUnsatisfiable); }; + let domain_patch_level = self.get_domain_patch_level(); + + if ctx_domain_patch_level != domain_patch_level { + error!("Unable to proceed with consumer incremental - incoming domain patch level is not equal to our patch level. {} != {}", ctx_domain_patch_level, domain_patch_level); + return Err(OperationError::ReplDomainLevelUnsatisfiable); + }; + // Assert that the d_uuid matches the repl domain uuid. let db_uuid = self.be_txn.get_db_d_uuid()?; diff --git a/server/lib/src/repl/proto.rs b/server/lib/src/repl/proto.rs index 105dc5f49..7136d4f8b 100644 --- a/server/lib/src/repl/proto.rs +++ b/server/lib/src/repl/proto.rs @@ -752,6 +752,8 @@ pub enum ReplIncrementalContext { UnwillingToSupply, V1 { domain_version: DomainVersion, + #[serde(default)] + domain_patch_level: u32, domain_uuid: Uuid, // We need to send the current state of the ranges to populate into // the ranges so that lookups and ranges work properly, and the diff --git a/server/lib/src/repl/supplier.rs b/server/lib/src/repl/supplier.rs index 8c59cd379..1e404cf39 100644 --- a/server/lib/src/repl/supplier.rs +++ b/server/lib/src/repl/supplier.rs @@ -225,6 +225,7 @@ impl<'a> QueryServerReadTransaction<'a> { let schema = self.get_schema(); let domain_version = self.d_info.d_vers; + let domain_patch_level = self.d_info.d_patch_level; let domain_uuid = self.d_info.d_uuid; let schema_entries: Vec<_> = schema_entries @@ -249,6 +250,7 @@ impl<'a> QueryServerReadTransaction<'a> { // Build the incremental context. Ok(ReplIncrementalContext::V1 { domain_version, + domain_patch_level, domain_uuid, ranges, schema_entries, diff --git a/server/lib/src/server/migrations.rs b/server/lib/src/server/migrations.rs index a8ec14740..e924b40e8 100644 --- a/server/lib/src/server/migrations.rs +++ b/server/lib/src/server/migrations.rs @@ -40,8 +40,6 @@ impl QueryServer { .initialise_schema_core() .and_then(|_| write_txn.reload())?; - write_txn.reload()?; - // This is what tells us if the domain entry existed before or not. This // is now the primary method of migrations and version detection. let db_domain_version = match write_txn.internal_search_uuid(UUID_DOMAIN_INFO) { @@ -84,8 +82,20 @@ impl QueryServer { // No domain info was present, so neither was the rest of the IDM. We need to bootstrap // the base entries here. if db_domain_version == 0 { + // In this path because we create the dyn groups they are immediately added to the + // dyngroup cache and begin to operate. write_txn.initialise_idm()?; - } + } else { + // #2756 - if we *aren't* creating the base IDM entries, then we + // need to force dyn groups to reload since we're now at schema + // ready. This is done indirectly by ... reloading the schema again. + // + // This is because dyngroups don't load until server phase >= schemaready + // and the reload path for these is either a change in the dyngroup entry + // itself or a change to schema reloading. Since we aren't changing the + // dyngroup here, we have to go via the schema reload path. + write_txn.force_schema_reload(); + }; // Reload as init idm affects access controls. write_txn.reload()?; @@ -95,11 +105,12 @@ impl QueryServer { // This is the start of domain info related migrations which we will need in future // to handle replication. Due to the access control rework, and the addition of "managed by" - // syntax, we need to ensure both node "fence" replication from each other. We do this + // syntax, we need to ensure both nodes "fence" replication from each other. We do this // by changing domain infos to be incompatible during this phase. // The reloads will have populated this structure now. let domain_info_version = write_txn.get_domain_version(); + let domain_patch_level = write_txn.get_domain_patch_level(); debug!(?db_domain_version, "After setting internal domain info"); if domain_info_version < domain_target_level { @@ -114,6 +125,10 @@ impl QueryServer { .map(|()| { warn!("Domain level has been raised to {}", domain_target_level); })?; + + // Reload if anything in migrations requires it - this triggers the domain migrations + // which in turn can trigger schema reloads etc. + write_txn.reload()?; } else { // This forces pre-release versions to re-migrate each start up. This solves // the domain-version-sprawl issue so that during a development cycle we can @@ -127,12 +142,26 @@ impl QueryServer { // We did not already need a version migration as above if option_env!("KANIDM_PRE_RELEASE").is_some() && !cfg!(test) { write_txn.domain_remigrate(DOMAIN_PREVIOUS_TGT_LEVEL)?; + write_txn.reload()?; } } - // Reload if anything in migrations requires it - this triggers the domain migrations - // which in turn can trigger schema reloads etc. - write_txn.reload()?; + if domain_target_level >= DOMAIN_LEVEL_7 && domain_patch_level < DOMAIN_TGT_PATCH_LEVEL { + write_txn + .internal_modify_uuid( + UUID_DOMAIN_INFO, + &ModifyList::new_purge_and_set( + Attribute::PatchLevel, + Value::new_uint32(DOMAIN_TGT_PATCH_LEVEL), + ), + ) + .map(|()| { + warn!("Domain level has been raised to {}", domain_target_level); + })?; + + // Run the patch migrations + write_txn.reload()?; + }; // We are ready to run write_txn.set_phase(ServerPhase::Running); @@ -622,8 +651,8 @@ impl<'a> QueryServerWriteTransaction<'a> { self.internal_modify(&filter, &modlist)?; // Now update schema - let idm_schema_classes = [ + SCHEMA_ATTR_PATCH_LEVEL_DL7.clone().into(), SCHEMA_CLASS_DOMAIN_INFO_DL7.clone().into(), SCHEMA_CLASS_SERVICE_ACCOUNT_DL7.clone().into(), SCHEMA_CLASS_SYNC_ACCOUNT_DL7.clone().into(), @@ -673,6 +702,22 @@ impl<'a> QueryServerWriteTransaction<'a> { Ok(()) } + /// Patch Application - This triggers a one-shot fixup task for issue #2756 + /// to correct the content of dyngroups after the dyngroups are now loaded. + #[instrument(level = "info", skip_all)] + pub(crate) fn migrate_domain_patch_level_1(&mut self) -> Result<(), OperationError> { + admin_warn!("applying domain patch 1."); + + debug_assert!(*self.phase >= ServerPhase::SchemaReady); + + let filter = filter!(f_eq(Attribute::Class, EntryClass::DynGroup.into())); + let modlist = modlist!([m_pres(Attribute::Class, &EntryClass::DynGroup.into())]); + + self.internal_modify(&filter, &modlist).map(|()| { + info!("forced dyngroups to re-calculate memberships"); + }) + } + /// Migration domain level 7 to 8 #[instrument(level = "info", skip_all)] pub(crate) fn migrate_domain_7_to_8(&mut self) -> Result<(), OperationError> { diff --git a/server/lib/src/server/mod.rs b/server/lib/src/server/mod.rs index 47b792093..5b349b9ee 100644 --- a/server/lib/src/server/mod.rs +++ b/server/lib/src/server/mod.rs @@ -72,6 +72,7 @@ pub struct DomainInfo { pub(crate) d_name: String, pub(crate) d_display: String, pub(crate) d_vers: DomainVersion, + pub(crate) d_patch_level: u32, pub(crate) d_ldap_allow_unix_pw_bind: bool, } @@ -192,6 +193,8 @@ pub trait QueryServerTransaction<'a> { fn get_domain_version(&self) -> DomainVersion; + fn get_domain_patch_level(&self) -> u32; + fn get_domain_uuid(&self) -> Uuid; fn get_domain_name(&self) -> &str; @@ -1067,6 +1070,10 @@ impl<'a> QueryServerTransaction<'a> for QueryServerReadTransaction<'a> { self.d_info.d_vers } + fn get_domain_patch_level(&self) -> u32 { + self.d_info.d_patch_level + } + fn get_domain_uuid(&self) -> Uuid { self.d_info.d_uuid } @@ -1213,6 +1220,10 @@ impl<'a> QueryServerTransaction<'a> for QueryServerWriteTransaction<'a> { self.d_info.d_vers } + fn get_domain_patch_level(&self) -> u32 { + self.d_info.d_patch_level + } + fn get_domain_uuid(&self) -> Uuid { self.d_info.d_uuid } @@ -1256,6 +1267,7 @@ impl QueryServer { // Start with our level as zero. // This will be reloaded from the DB shortly :) d_vers: DOMAIN_LEVEL_0, + d_patch_level: 0, d_name: domain_name.clone(), // we set the domain_display_name to the configuration file's domain_name // here because the database is not started, so we cannot pull it from there. @@ -1786,25 +1798,37 @@ impl<'a> QueryServerWriteTransaction<'a> { /// Pulls the domain name from the database and updates the DomainInfo data in memory #[instrument(level = "debug", skip_all)] pub(crate) fn reload_domain_info_version(&mut self) -> Result<(), OperationError> { - let domain_info_version = self - .internal_search_uuid(UUID_DOMAIN_INFO) - .and_then(|e| { - e.get_ava_single_uint32(Attribute::Version) - .ok_or(OperationError::InvalidEntryState) - }) - .map_err(|err| { - error!(?err, "Error getting domain version"); - err + let domain_info = self.internal_search_uuid(UUID_DOMAIN_INFO).map_err(|err| { + error!(?err, "Error getting domain info"); + err + })?; + + let domain_info_version = domain_info + .get_ava_single_uint32(Attribute::Version) + .ok_or_else(|| { + error!("domain info missing attribute version"); + OperationError::InvalidEntryState })?; + let domain_info_patch_level = domain_info + .get_ava_single_uint32(Attribute::PatchLevel) + .unwrap_or(0); + // We have to set the domain version here so that features which check for it // will now see it's been increased. This also prevents recursion during reloads // inside of a domain migration. let mut_d_info = self.d_info.get_mut(); let previous_version = mut_d_info.d_vers; + let previous_patch_level = mut_d_info.d_patch_level; mut_d_info.d_vers = domain_info_version; + mut_d_info.d_patch_level = domain_info_patch_level; - if previous_version == domain_info_version || *self.phase < ServerPhase::DomainInfoReady { + // We must both be at the correct domain version *and* the correct patch level. If we are + // not, then we only proceed to migrate *if* our server boot phase is correct. + if (previous_version == domain_info_version + && previous_patch_level == domain_info_patch_level) + || *self.phase < ServerPhase::DomainInfoReady + { return Ok(()); } @@ -1830,6 +1854,12 @@ impl<'a> QueryServerWriteTransaction<'a> { self.migrate_domain_6_to_7()?; } + // Similar to the older system info migration handler, these allow "one shot" fixes + // to be issued and run by bumping the patch level. + if previous_patch_level <= PATCH_LEVEL_1 && domain_info_patch_level >= PATCH_LEVEL_1 { + self.migrate_domain_patch_level_1()?; + } + if previous_version <= DOMAIN_LEVEL_7 && domain_info_version >= DOMAIN_LEVEL_8 { self.migrate_domain_7_to_8()?; }