From bc341af9d870693d26641265b36a147d6db8e24d Mon Sep 17 00:00:00 2001 From: Firstyear Date: Thu, 17 Aug 2023 15:52:12 +1000 Subject: [PATCH] Resolve issues with dyngroup members (#1986) --- server/Dockerfile | 1 + server/core/src/lib.rs | 51 +++++++++++ server/daemon/src/main.rs | 42 ++++++++- server/daemon/src/opt.rs | 22 +++++ server/lib/src/be/idl_arc_sqlite.rs | 20 +++++ server/lib/src/be/idl_sqlite.rs | 127 +++++++++++++++++++++++++++ server/lib/src/be/mod.rs | 18 ++++ server/lib/src/constants/acp.rs | 5 ++ server/lib/src/constants/entries.rs | 2 +- server/lib/src/plugins/attrunique.rs | 2 +- server/lib/src/plugins/base.rs | 2 +- server/lib/src/plugins/dyngroup.rs | 1 + server/lib/src/plugins/memberof.rs | 2 +- server/lib/src/plugins/refint.rs | 3 +- server/lib/src/plugins/spn.rs | 2 +- server/lib/src/server/migrations.rs | 14 +++ 16 files changed, 306 insertions(+), 8 deletions(-) diff --git a/server/Dockerfile b/server/Dockerfile index ceb8a48b6..98a1d5b81 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -21,6 +21,7 @@ RUN \ sccache \ cargo \ clang \ + gawk \ make \ automake \ autoconf \ diff --git a/server/core/src/lib.rs b/server/core/src/lib.rs index 35c39758e..2b905a617 100644 --- a/server/core/src/lib.rs +++ b/server/core/src/lib.rs @@ -238,6 +238,57 @@ pub fn dbscan_get_id2entry_core(config: &Configuration, id: u64) { }; } +pub fn dbscan_quarantine_id2entry_core(config: &Configuration, id: u64) { + let be = dbscan_setup_be!(config); + let mut be_wrtxn = be.write(); + + match be_wrtxn + .quarantine_entry(id) + .and_then(|_| be_wrtxn.commit()) + { + Ok(()) => { + println!("quarantined - {:>8}", id) + } + Err(e) => { + error!("Failed to quarantine id2entry value: {:?}", e); + } + }; +} + +pub fn dbscan_list_quarantined_core(config: &Configuration) { + let be = dbscan_setup_be!(config); + let mut be_rotxn = be.read(); + + match be_rotxn.list_quarantined() { + Ok(mut id_list) => { + id_list.sort_unstable_by_key(|k| k.0); + id_list.iter().for_each(|(id, value)| { + println!("{:>8}: {}", id, value); + }) + } + Err(e) => { + error!("Failed to retrieve id2entry list: {:?}", e); + } + }; +} + +pub fn dbscan_restore_quarantined_core(config: &Configuration, id: u64) { + let be = dbscan_setup_be!(config); + let mut be_wrtxn = be.write(); + + match be_wrtxn + .restore_quarantined(id) + .and_then(|_| be_wrtxn.commit()) + { + Ok(()) => { + println!("restored - {:>8}", id) + } + Err(e) => { + error!("Failed to restore quarantined id2entry value: {:?}", e); + } + }; +} + pub fn backup_server_core(config: &Configuration, dst_path: &str) { let schema = match Schema::new() { Ok(s) => s, diff --git a/server/daemon/src/main.rs b/server/daemon/src/main.rs index 851701c75..f5c7e8876 100644 --- a/server/daemon/src/main.rs +++ b/server/daemon/src/main.rs @@ -33,7 +33,8 @@ use kanidmd_core::config::{Configuration, LogLevel, ServerConfig}; use kanidmd_core::{ backup_server_core, cert_generate_core, create_server_core, dbscan_get_id2entry_core, dbscan_list_id2entry_core, dbscan_list_index_analysis_core, dbscan_list_index_core, - dbscan_list_indexes_core, domain_rename_core, reindex_server_core, restore_server_core, + dbscan_list_indexes_core, dbscan_list_quarantined_core, dbscan_quarantine_id2entry_core, + dbscan_restore_quarantined_core, domain_rename_core, reindex_server_core, restore_server_core, vacuum_server_core, verify_server_core, }; use sketching::tracing_forest::traits::*; @@ -67,7 +68,16 @@ impl KanidmdOpt { KanidmdOpt::Database { commands: DbCommands::Restore(ropt), } => &ropt.commonopts, - KanidmdOpt::RecoverAccount { commonopts, .. } => commonopts, + KanidmdOpt::DbScan { + commands: DbScanOpt::QuarantineId2Entry { commonopts, .. }, + } + | KanidmdOpt::DbScan { + commands: DbScanOpt::ListQuarantined { commonopts }, + } + | KanidmdOpt::DbScan { + commands: DbScanOpt::RestoreQuarantined { commonopts, .. }, + } + | KanidmdOpt::RecoverAccount { commonopts, .. } => commonopts, KanidmdOpt::DbScan { commands: DbScanOpt::ListIndex(dopt), } => &dopt.commonopts, @@ -568,6 +578,34 @@ async fn main() -> ExitCode { info!("👀 db scan - get id2 entry - {}", dopt.id); dbscan_get_id2entry_core(&config, dopt.id); } + + KanidmdOpt::DbScan { + commands: DbScanOpt::QuarantineId2Entry { + id, commonopts: _ + } + } => { + info!("☣️ db scan - quarantine id2 entry - {}", id); + dbscan_quarantine_id2entry_core(&config, *id); + } + + KanidmdOpt::DbScan { + commands: DbScanOpt::ListQuarantined { + commonopts: _ + } + } => { + info!("☣️ db scan - list quarantined"); + dbscan_list_quarantined_core(&config); + } + + KanidmdOpt::DbScan { + commands: DbScanOpt::RestoreQuarantined { + id, commonopts: _ + } + } => { + info!("☣️ db scan - restore quarantined entry - {}", id); + dbscan_restore_quarantined_core(&config, *id); + } + KanidmdOpt::DomainSettings { commands: DomainSettingsCmds::DomainChange(_dopt), } => { diff --git a/server/daemon/src/opt.rs b/server/daemon/src/opt.rs index c722ef430..70f16ccab 100644 --- a/server/daemon/src/opt.rs +++ b/server/daemon/src/opt.rs @@ -101,6 +101,28 @@ enum DbScanOpt { #[clap(name = "list-index-analysis")] /// List all content of index analysis ListIndexAnalysis(CommonOpt), + #[clap(name = "quarantine-id2entry")] + /// Given an entry id, quarantine the entry in a hidden db partition + QuarantineId2Entry { + /// The id of the entry to display + id: u64, + #[clap(flatten)] + commonopts: CommonOpt, + }, + #[clap(name = "list-quarantined")] + /// List the entries in quarantine + ListQuarantined { + #[clap(flatten)] + commonopts: CommonOpt, + }, + #[clap(name = "restore-quarantined")] + /// Given an entry id, restore the entry from the hidden db partition + RestoreQuarantined { + /// The id of the entry to display + id: u64, + #[clap(flatten)] + commonopts: CommonOpt, + }, } #[derive(Debug, Parser)] diff --git a/server/lib/src/be/idl_arc_sqlite.rs b/server/lib/src/be/idl_arc_sqlite.rs index 90549b43a..e4429baeb 100644 --- a/server/lib/src/be/idl_arc_sqlite.rs +++ b/server/lib/src/be/idl_arc_sqlite.rs @@ -369,6 +369,8 @@ pub trait IdlArcSqliteTransaction { fn list_id2entry(&self) -> Result, OperationError>; + fn list_quarantined(&self) -> Result, OperationError>; + fn list_index_content( &self, index_name: &str, @@ -449,6 +451,11 @@ impl<'a> IdlArcSqliteTransaction for IdlArcSqliteReadTransaction<'a> { self.db.list_id2entry() } + fn list_quarantined(&self) -> Result, OperationError> { + // No cache of quarantined entries. + self.db.list_quarantined() + } + fn list_index_content( &self, index_name: &str, @@ -538,6 +545,11 @@ impl<'a> IdlArcSqliteTransaction for IdlArcSqliteWriteTransaction<'a> { self.db.list_id2entry() } + fn list_quarantined(&self) -> Result, OperationError> { + // No cache of quarantined entries. + self.db.list_quarantined() + } + fn list_index_content( &self, index_name: &str, @@ -980,6 +992,14 @@ impl<'a> IdlArcSqliteWriteTransaction<'a> { } } + pub fn quarantine_entry(&self, id: u64) -> Result<(), OperationError> { + self.db.quarantine_entry(id) + } + + pub fn restore_quarantined(&self, id: u64) -> Result<(), OperationError> { + self.db.restore_quarantined(id) + } + pub fn create_name2uuid(&self) -> Result<(), OperationError> { self.db.create_name2uuid() } diff --git a/server/lib/src/be/idl_sqlite.rs b/server/lib/src/be/idl_sqlite.rs index 975c7d743..e5f4d912b 100644 --- a/server/lib/src/be/idl_sqlite.rs +++ b/server/lib/src/be/idl_sqlite.rs @@ -170,6 +170,7 @@ pub trait IdlSqliteTransaction { // I have now is probably really bad :( let mut results = Vec::new(); + // Faster to iterate over the compressed form believe it or not. /* let decompressed: Result, _> = idli.into_iter() .map(|u| i64::try_from(u).map_err(|_| OperationError::InvalidEntryId)) @@ -494,6 +495,39 @@ pub trait IdlSqliteTransaction { .collect() } + fn list_quarantined(&self) -> Result, OperationError> { + // This is a more direct version of get_identry_raw adapted for the simpler + // quarantine setup. + let mut stmt = self + .get_conn()? + .prepare(&format!( + "SELECT id, data FROM {}.id2entry_quarantine", + self.get_db_name() + )) + .map_err(sqlite_error)?; + let id2entry_iter = stmt + .query_map([], |row| { + Ok(IdSqliteEntry { + id: row.get(0)?, + data: row.get(1)?, + }) + }) + .map_err(sqlite_error)?; + let allids = id2entry_iter + .map(|v| { + v.map_err(sqlite_error).and_then(|ise| { + // Convert the idsqlite to id raw + ise.try_into() + }) + }) + .collect::, _>>()?; + + allids + .into_iter() + .map(|data| data.into_dbentry().map(|(id, db_e)| (id, db_e.to_string()))) + .collect() + } + fn get_id2entry(&self, id: u64) -> Result<(u64, String), OperationError> { let idl = IdList::Indexed(IDLBitRange::from_u64(id)); let mut allids = self.get_identry_raw(&idl)?; @@ -1126,6 +1160,80 @@ impl IdlSqliteWriteTransaction { Ok(slope) } + pub fn quarantine_entry(&self, id: u64) -> Result<(), OperationError> { + let iid = i64::try_from(id).map_err(|_| OperationError::InvalidEntryId)?; + + let id_sqlite_entry = self + .get_conn()? + .query_row( + &format!( + "DELETE FROM {}.id2entry WHERE id = :idl RETURNING id, data", + self.get_db_name() + ), + [&iid], + |row| { + Ok(IdSqliteEntry { + id: row.get(0)?, + data: row.get(1)?, + }) + }, + ) + .map_err(sqlite_error)?; + + trace!(?id_sqlite_entry); + + self.get_conn()? + .execute( + &format!( + "INSERT OR REPLACE INTO {}.id2entry_quarantine VALUES(:id, :data)", + self.get_db_name() + ), + named_params! { + ":id": &id_sqlite_entry.id, + ":data": &id_sqlite_entry.data.as_slice() + }, + ) + .map_err(sqlite_error) + .map(|_| ()) + } + + pub fn restore_quarantined(&self, id: u64) -> Result<(), OperationError> { + let iid = i64::try_from(id).map_err(|_| OperationError::InvalidEntryId)?; + + let id_sqlite_entry = self + .get_conn()? + .query_row( + &format!( + "DELETE FROM {}.id2entry_quarantine WHERE id = :idl RETURNING id, data", + self.get_db_name() + ), + [&iid], + |row| { + Ok(IdSqliteEntry { + id: row.get(0)?, + data: row.get(1)?, + }) + }, + ) + .map_err(sqlite_error)?; + + trace!(?id_sqlite_entry); + + self.get_conn()? + .execute( + &format!( + "INSERT INTO {}.id2entry VALUES(:id, :data)", + self.get_db_name() + ), + named_params! { + ":id": &id_sqlite_entry.id, + ":data": &id_sqlite_entry.data.as_slice() + }, + ) + .map_err(sqlite_error) + .map(|_| ()) + } + /// ⚠️ - This function will destroy all entries in the database. /// /// It should only be called internally by the backend in limited and @@ -1399,6 +1507,25 @@ impl IdlSqliteWriteTransaction { info!(entry = %dbv_id2entry, "dbv_id2entry migrated (externalid2uuid)"); } // * if v6 -> complete. + if dbv_id2entry == 6 { + self.get_conn()? + .execute( + &format!( + "CREATE TABLE IF NOT EXISTS {}.id2entry_quarantine ( + id INTEGER PRIMARY KEY ASC, + data BLOB NOT NULL + ) + ", + self.get_db_name() + ), + [], + ) + .map_err(sqlite_error)?; + + dbv_id2entry = 7; + info!(entry = %dbv_id2entry, "dbv_id2entry migrated (quarantine)"); + } + // * if v7 -> complete. self.set_db_version_key(DBV_ID2ENTRY, dbv_id2entry)?; diff --git a/server/lib/src/be/mod.rs b/server/lib/src/be/mod.rs index 4338456fa..66467f415 100644 --- a/server/lib/src/be/mod.rs +++ b/server/lib/src/be/mod.rs @@ -946,6 +946,10 @@ impl<'a> BackendReadTransaction<'a> { pub fn get_id2entry(&mut self, id: u64) -> Result<(u64, String), OperationError> { self.get_idlayer().get_id2entry(id) } + + pub fn list_quarantined(&mut self) -> Result, OperationError> { + self.get_idlayer().list_quarantined() + } } impl<'a> BackendTransaction for BackendWriteTransaction<'a> { @@ -1761,6 +1765,20 @@ impl<'a> BackendWriteTransaction<'a> { Ok(()) } + pub fn quarantine_entry(&mut self, id: u64) -> Result<(), OperationError> { + self.get_idlayer().quarantine_entry(id)?; + // We have to set the index version to 0 so that on next start we force + // a reindex to automatically occur. + self.set_db_index_version(0) + } + + pub fn restore_quarantined(&mut self, id: u64) -> Result<(), OperationError> { + self.get_idlayer().restore_quarantined(id)?; + // We have to set the index version to 0 so that on next start we force + // a reindex to automatically occur. + self.set_db_index_version(0) + } + pub fn commit(self) -> Result<(), OperationError> { let BackendWriteTransaction { idlayer, diff --git a/server/lib/src/constants/acp.rs b/server/lib/src/constants/acp.rs index 1be5a21fe..82e9cdee6 100644 --- a/server/lib/src/constants/acp.rs +++ b/server/lib/src/constants/acp.rs @@ -223,6 +223,7 @@ lazy_static! { ("acp_search_attr", Value::new_iutf8("class")), ("acp_search_attr", Value::new_iutf8("memberof")), ("acp_search_attr", Value::new_iutf8("member")), + ("acp_search_attr", Value::new_iutf8("dynmember")), ("acp_search_attr", Value::new_iutf8("uuid")), ("acp_search_attr", Value::new_iutf8("gidnumber")), ("acp_search_attr", Value::new_iutf8("loginshell")), @@ -561,6 +562,7 @@ lazy_static! { ("acp_search_attr", Value::new_iutf8("uuid")), ("acp_search_attr", Value::new_iutf8("description")), ("acp_search_attr", Value::new_iutf8("member")), + ("acp_search_attr", Value::new_iutf8("dynmember")), ("acp_modify_removedattr", Value::new_iutf8("name")), ("acp_modify_removedattr", Value::new_iutf8("description")), ("acp_modify_removedattr", Value::new_iutf8("member")), @@ -902,6 +904,7 @@ lazy_static! { ("acp_search_attr", Value::new_iutf8("uuid")), ("acp_search_attr", Value::new_iutf8("description")), ("acp_search_attr", Value::new_iutf8("member")), + ("acp_search_attr", Value::new_iutf8("dynmember")), ("acp_modify_removedattr", Value::new_iutf8("name")), ("acp_modify_removedattr", Value::new_iutf8("description")), ("acp_modify_removedattr", Value::new_iutf8("member")), @@ -1368,6 +1371,7 @@ lazy_static! { ("acp_search_attr", Value::new_iutf8("spn")), ("acp_search_attr", Value::new_iutf8("description")), ("acp_search_attr", Value::new_iutf8("member")), + ("acp_search_attr", Value::new_iutf8("dynmember")), ("acp_search_attr", Value::new_iutf8("gidnumber")), ("acp_modify_removedattr", Value::new_iutf8("gidnumber")), ("acp_modify_presentattr", Value::new_iutf8("class")), @@ -1447,6 +1451,7 @@ lazy_static! { ("acp_search_attr", Value::new_iutf8("spn")), ("acp_search_attr", Value::new_iutf8("description")), ("acp_search_attr", Value::new_iutf8("member")), + ("acp_search_attr", Value::new_iutf8("dynmember")), ("acp_search_attr", Value::new_iutf8("gidnumber")), ("acp_modify_removedattr", Value::new_iutf8("gidnumber")), ("acp_modify_presentattr", Value::new_iutf8("class")), diff --git a/server/lib/src/constants/entries.rs b/server/lib/src/constants/entries.rs index 867a83aed..dece34daf 100644 --- a/server/lib/src/constants/entries.rs +++ b/server/lib/src/constants/entries.rs @@ -585,7 +585,7 @@ lazy_static! { "description", Value::new_utf8s("System (local) info and metadata object.") ), - ("version", Value::Uint32(13)) + ("version", Value::Uint32(14)) ); } diff --git a/server/lib/src/plugins/attrunique.rs b/server/lib/src/plugins/attrunique.rs index b2437b654..a8fafcdf8 100644 --- a/server/lib/src/plugins/attrunique.rs +++ b/server/lib/src/plugins/attrunique.rs @@ -252,7 +252,7 @@ impl Plugin for AttrUnique { r } - #[instrument(level = "debug", name = "attrunique_verify", skip(qs))] + #[instrument(level = "debug", name = "attrunique::verify", skip_all)] fn verify(qs: &mut QueryServerReadTransaction) -> Vec> { // Only check live entries, not recycled. let filt_in = filter!(f_pres("class")); diff --git a/server/lib/src/plugins/base.rs b/server/lib/src/plugins/base.rs index a911246fa..f751bb14c 100644 --- a/server/lib/src/plugins/base.rs +++ b/server/lib/src/plugins/base.rs @@ -203,7 +203,7 @@ impl Plugin for Base { }) } - #[instrument(level = "debug", name = "base_verify", skip(qs))] + #[instrument(level = "debug", name = "base::verify", skip_all)] fn verify(qs: &mut QueryServerReadTransaction) -> Vec> { // Search for class = * let entries = match qs.internal_search(filter!(f_pres("class"))) { diff --git a/server/lib/src/plugins/dyngroup.rs b/server/lib/src/plugins/dyngroup.rs index e42b5c951..e7261f016 100644 --- a/server/lib/src/plugins/dyngroup.rs +++ b/server/lib/src/plugins/dyngroup.rs @@ -541,6 +541,7 @@ mod tests { .expect("No members on dyn group"); assert!(members.to_refer_single() == Some(UUID_TEST_GROUP)); + assert!(d_group.get_ava_set("member").is_none()); } ); } diff --git a/server/lib/src/plugins/memberof.rs b/server/lib/src/plugins/memberof.rs index fcabce34f..9fb7c5c01 100644 --- a/server/lib/src/plugins/memberof.rs +++ b/server/lib/src/plugins/memberof.rs @@ -284,7 +284,7 @@ impl Plugin for MemberOf { apply_memberof(qs, group_affect) } - #[instrument(level = "debug", name = "memberof_verify", skip(qs))] + #[instrument(level = "debug", name = "memberof::verify", skip_all)] fn verify(qs: &mut QueryServerReadTransaction) -> Vec> { let mut r = Vec::new(); diff --git a/server/lib/src/plugins/refint.rs b/server/lib/src/plugins/refint.rs index e6543d7f4..78557bc7a 100644 --- a/server/lib/src/plugins/refint.rs +++ b/server/lib/src/plugins/refint.rs @@ -164,7 +164,7 @@ impl Plugin for ReferentialIntegrity { qs.internal_apply_writable(work_set) } - #[instrument(level = "debug", name = "verify", skip(qs))] + #[instrument(level = "debug", name = "refint::verify", skip_all)] fn verify(qs: &mut QueryServerReadTransaction) -> Vec> { // Get all entries as cand // build a cand-uuid set @@ -232,6 +232,7 @@ impl ReferentialIntegrity { if skip_mb || skip_mo { None } else { + trace!(rtype_name = ?rtype.name, "examining"); c.get_ava_set(&rtype.name) } }) diff --git a/server/lib/src/plugins/spn.rs b/server/lib/src/plugins/spn.rs index ebeac9d7e..db9d7d1a3 100644 --- a/server/lib/src/plugins/spn.rs +++ b/server/lib/src/plugins/spn.rs @@ -73,7 +73,7 @@ impl Plugin for Spn { Self::post_modify_inner(qs, pre_cand, cand) } - #[instrument(level = "debug", name = "spn_verify", skip(qs))] + #[instrument(level = "debug", name = "spn::verify", skip_all)] fn verify(qs: &mut QueryServerReadTransaction) -> Vec> { // Verify that all items with spn's have valid spns. // We need to consider the case that an item has a different origin domain too, diff --git a/server/lib/src/server/migrations.rs b/server/lib/src/server/migrations.rs index cc43d917b..88cf54299 100644 --- a/server/lib/src/server/migrations.rs +++ b/server/lib/src/server/migrations.rs @@ -103,6 +103,10 @@ impl QueryServer { if system_info_version < 13 { write_txn.migrate_12_to_13()?; } + + if system_info_version < 14 { + write_txn.migrate_13_to_14()?; + } } write_txn.reload()?; @@ -407,6 +411,16 @@ impl<'a> QueryServerWriteTransaction<'a> { // Complete } + #[instrument(level = "debug", skip_all)] + pub fn migrate_13_to_14(&mut self) -> Result<(), OperationError> { + admin_warn!("starting 13 to 14 migration."); + let filter = filter!(f_eq("class", PVCLASS_DYNGROUP.clone())); + // Delete the incorrectly added "member" attr. + let modlist = ModifyList::new_purge("member"); + self.internal_modify(&filter, &modlist) + // Complete + } + #[instrument(level = "debug", skip_all)] pub fn initialise_schema_core(&mut self) -> Result<(), OperationError> { admin_debug!("initialise_schema_core -> start ...");