From 7a52ffca5f5b70bc2d41269bd399ce2686d8b6b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Wed, 16 Oct 2024 18:36:49 +0200 Subject: [PATCH 01/11] fix: Pre-empt accidentally leaking PII in logs This allows using the debug impls of our User structs without worrying too much about accidentally exposing PII. Note that this means `external_user_id` should *never* contain PII, and as such we'll have to change the CSV source. An issue about this will be opened separately. --- src/user.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/user.rs b/src/user.rs index 9a255d8..d9730ff 100644 --- a/src/user.rs +++ b/src/user.rs @@ -7,7 +7,7 @@ use zitadel_rust_client::v1::{Email, Gender, Idp, ImportHumanUserRequest, Phone, use crate::{config::FeatureFlags, FeatureFlag}; /// Source-agnostic representation of a user -#[derive(Clone, Debug)] +#[derive(Clone)] pub(crate) struct User { /// The user's first name pub(crate) first_name: StringOrBytes, @@ -37,6 +37,20 @@ impl User { } } +impl std::fmt::Debug for User { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("User") + .field("first_name", &"***") + .field("last_name", &"***") + .field("email", &"***") + .field("phone", &"***") + .field("preferred_username", &"***") + .field("external_user_id", &self.external_user_id) + .field("enabled", &self.enabled) + .finish() + } +} + /// Crate-internal representation of a Zitadel user #[derive(Clone, Debug)] pub struct ZitadelUser { @@ -59,7 +73,7 @@ impl ZitadelUser { /// Return the name to be used in logs to identify this user pub(crate) fn log_name(&self) -> String { - format!("email={}", &self.user_data.email) + format!("external_id={}", &self.user_data.external_user_id) } /// Get idp link as required by Zitadel @@ -108,7 +122,7 @@ impl From for ImportHumanUserRequest { impl Display for ZitadelUser { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "email={}", &self.user_data.email) + write!(f, "external_id={}", &self.user_data.external_user_id) } } From 2ad0b2d55b8632fd487e3c064b05d5fffb177135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Wed, 30 Oct 2024 00:50:09 +0100 Subject: [PATCH 02/11] feat!: Stop relying on a local cache to track changes --- Cargo.lock | 12 +- Cargo.toml | 3 + config.sample.yaml | 5 - src/config.rs | 72 +---- src/lib.rs | 243 +++++++++++++++++ src/main.rs | 6 +- src/sources.rs | 19 +- src/sources/csv.rs | 14 +- src/sources/ldap.rs | 127 ++------- src/sources/ukt.rs | 17 -- src/user.rs | 229 +++++++++------- src/zitadel.rs | 634 +++++++++++++++----------------------------- tests/e2e.rs | 63 ++--- 13 files changed, 698 insertions(+), 746 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3b49e96..f913e55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -725,11 +725,12 @@ dependencies = [ "chrono", "config", "csv", + "futures", "http 1.1.0", "indoc", - "itertools 0.13.0", "ldap-poller", "ldap3", + "native-tls", "reqwest 0.11.27", "serde", "serde_json", @@ -1404,15 +1405,6 @@ dependencies = [ "either", ] -[[package]] -name = "itertools" -version = "0.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" -dependencies = [ - "either", -] - [[package]] name = "itoa" version = "1.0.11" diff --git a/Cargo.toml b/Cargo.toml index 46c8328..9071b9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,9 @@ zitadel-rust-client = { git = "https://github.com/famedly/zitadel-rust-client", wiremock = "0.6.2" csv = "1.3.0" tempfile = "3.12.0" +futures = "0.3.31" +ldap3 = { version = "0.11.1", default-features = false, features = ["tls-native"] } +native-tls = "0.2.12" [dependencies.tonic] version = "*" diff --git a/config.sample.yaml b/config.sample.yaml index 5ee56bb..c478477 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -87,11 +87,6 @@ sources: # needed with the `ldaps` scheme, as the server will already be # hosting TLS. danger_use_start_tls: false - - # Path to the file that keeps track of previously synced LDAP entries. - # This file should be persisted, otherwise users may become out of sync. - cache_path: /opt/famedly-sync-agent/famedly-sync.cache - # Configuration for the UKT source - a custom endpoint provided by UKT, # which gives a list of emails of users that should be deleted from Zitadel. ukt: diff --git a/src/config.rs b/src/config.rs index 6b31566..cb3f1b6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,17 +6,11 @@ use std::{ use anyhow::{bail, Result}; use serde::Deserialize; -use tracing::{error, warn}; use url::Url; use crate::{ - sources::{ - csv::{CsvSource, CsvSourceConfig}, - ldap::{LdapSource, LdapSourceConfig}, - ukt::{UktSource, UktSourceConfig}, - Source, - }, - zitadel::{Zitadel, ZitadelConfig}, + sources::{csv::CsvSourceConfig, ldap::LdapSourceConfig, ukt::UktSourceConfig}, + zitadel::ZitadelConfig, }; /// App prefix for env var configuration @@ -76,62 +70,6 @@ impl Config { Ok(self) } - - /// Perform a sync operation - pub async fn perform_sync(&self) -> Result<()> { - if !self.feature_flags.is_enabled(FeatureFlag::SsoLogin) { - anyhow::bail!("Non-SSO configuration is currently not supported"); - } - - let mut sources: Vec> = Vec::new(); - - if let Some(ldap_config) = &self.sources.ldap { - let ldap = LdapSource::new( - ldap_config.clone(), - self.feature_flags.is_enabled(FeatureFlag::DryRun), - ); - sources.push(Box::new(ldap)); - } - - if let Some(ukt_config) = &self.sources.ukt { - let ukt = UktSource::new(ukt_config.clone()); - sources.push(Box::new(ukt)); - } - - if let Some(csv_config) = &self.sources.csv { - let csv = CsvSource::new(csv_config.clone()); - sources.push(Box::new(csv)); - } - - // Setup Zitadel client - let zitadel = Zitadel::new(self).await?; - - // Sync from each available source - for source in sources.iter() { - let diff = match source.get_diff().await { - Ok(diff) => diff, - Err(e) => { - error!("Failed to get diff from {}: {:?}", source.get_name(), e); - continue; - } - }; - - if !self.feature_flags.is_enabled(FeatureFlag::DeactivateOnly) { - if let Err(e) = zitadel.import_new_users(diff.new_users).await { - warn!("Failed to import new users from {}: {:?}", source.get_name(), e); - } - if let Err(e) = zitadel.delete_users_by_id(diff.deleted_user_ids).await { - warn!("Failed to delete users from {}: {:?}", source.get_name(), e); - } - } - - if let Err(e) = zitadel.update_users(diff.changed_users).await { - warn!("Failed to update users from {}: {:?}", source.get_name(), e); - } - } - - Ok(()) - } } /// Opt-in features @@ -306,7 +244,7 @@ mod tests { #[test] fn test_config_from_file() { - let tempdir = TempDir::new().expect("failed to initialize cache dir"); + let tempdir = TempDir::new().expect("failed to initialize tempdir"); let file_path = create_config_file(tempdir.path()); let config = Config::new(file_path.as_path()).expect("Failed to create config object"); @@ -315,7 +253,7 @@ mod tests { #[test] fn test_config_env_var_override() { - let tempdir = TempDir::new().expect("failed to initialize cache dir"); + let tempdir = TempDir::new().expect("failed to initialize tempdir"); let file_path = create_config_file(tempdir.path()); let env_var_name = format!("{ENV_VAR_CONFIG_PREFIX}__FEATURE_FLAGS"); @@ -352,7 +290,7 @@ mod tests { #[test] fn test_config_env_var_feature_flag() { - let tempdir = TempDir::new().expect("failed to initialize cache dir"); + let tempdir = TempDir::new().expect("failed to initialize tempdir"); let file_path = create_config_file(tempdir.path()); let env_var_name = format!("{ENV_VAR_CONFIG_PREFIX}__FEATURE_FLAGS"); diff --git a/src/lib.rs b/src/lib.rs index 9ded581..efac5f0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,12 +1,255 @@ //! Sync tool between other sources and our infrastructure based on Zitadel. +use anyhow::{Context, Result}; +use futures::{Stream, StreamExt}; +use user::{StringOrBytes, User}; +use zitadel::Zitadel; mod config; mod sources; mod user; mod zitadel; +use std::collections::VecDeque; + pub use config::{Config, FeatureFlag}; pub use sources::{ csv::test_helpers as csv_test_helpers, ldap::AttributeMapping, ukt::test_helpers as ukt_test_helpers, }; +use sources::{csv::CsvSource, ldap::LdapSource, ukt::UktSource, Source}; + +/// Helper function to add metadata to streamed zitadel users +// TODO: If async closures become a reality, this should be factored +// into the `zitadel::search_result_to_user` function +async fn get_next_zitadel_user( + stream: &mut (impl Stream> + Send + Unpin), + zitadel: &mut Zitadel, +) -> Result> { + match stream.next().await.transpose()? { + Some(mut zitadel_user) => { + let preferred_username: Option = zitadel + .zitadel_client + .get_user_metadata(&zitadel_user.1, "preferred_username") + .await + .ok() + .and_then(|metadata| metadata.metadata().value()) + .map(Into::into); + + zitadel_user.0.preferred_username = preferred_username; + + Ok(Some(zitadel_user)) + } + None => Ok(None), + } +} + +/// Perform a sync operation +pub async fn perform_sync(config: &Config) -> Result<()> { + /// Get users from a source + async fn get_users_from_source(source: impl Source + Send) -> Result> { + source + .get_sorted_users() + .await + .map(VecDeque::from) + .context(format!("Failed to query users from {}", source.get_name())) + } + + let csv = config.sources.csv.clone().map(CsvSource::new); + let ldap = config.sources.ldap.clone().map(LdapSource::new); + let ukt = config.sources.ukt.clone().map(UktSource::new); + + // The ukt source is handled specially, since it doesn't behave as + // the others + if let Some(ukt) = ukt { + match ukt.get_removed_user_emails().await { + Ok(users) => delete_users_by_email(config, users).await?, + Err(err) => { + anyhow::bail!("Failed to query users from ukt: {:?}", err); + } + } + + return Ok(()); + } + + let mut users = match (csv, ldap, ukt) { + (Some(csv), None, None) => get_users_from_source(csv).await?, + (None, Some(ldap), None) => get_users_from_source(ldap).await?, + (None, None, Some(_)) => VecDeque::new(), + _ => { + anyhow::bail!("Exactly one source must be defined"); + } + }; + + if config.feature_flags.is_enabled(FeatureFlag::DeactivateOnly) { + disable_users(config, &mut users).await?; + } else { + sync_users(config, &mut users).await?; + } + + Ok(()) +} + +/// Delete a list of users given their email addresses +async fn delete_users_by_email(config: &Config, emails: Vec) -> Result<()> { + let mut zitadel = Zitadel::new(config).await?; + let mut stream = zitadel.get_users_by_email(emails)?; + + while let Some(zitadel_user) = get_next_zitadel_user(&mut stream, &mut zitadel).await? { + zitadel.delete_user(&zitadel_user.1).await?; + } + + Ok(()) +} + +/// Only disable users +async fn disable_users(config: &Config, users: &mut VecDeque) -> Result<()> { + // We only care about disabled users for this flow + users.retain(|user| !user.enabled); + + let mut zitadel = Zitadel::new(config).await?; + let mut stream = zitadel.list_users()?; + + while let Some(zitadel_user) = get_next_zitadel_user(&mut stream, &mut zitadel).await? { + if users.front().map(|user| user.external_user_id.clone()) + == Some(zitadel_user.0.external_user_id) + { + zitadel.delete_user(&zitadel_user.1).await?; + users.pop_front(); + } + } + + Ok(()) +} + +/// Fully sync users +async fn sync_users(config: &Config, sync_users: &mut VecDeque) -> Result<()> { + // Treat any disabled users as deleted, so we simply pretend they + // are not in the list + sync_users.retain(|user| user.enabled); + + let mut zitadel = Zitadel::new(config).await?; + let mut stream = zitadel.list_users()?; + + let mut source_user = sync_users.pop_front(); + let mut zitadel_user = get_next_zitadel_user(&mut stream, &mut zitadel).await?; + + loop { + tracing::debug!("Comparing users {:?} and {:?}", source_user, zitadel_user); + + match (source_user.clone(), zitadel_user.clone()) { + (None, None) => { + tracing::info!("Sync completed successfully"); + break; + } + + // Excess Zitadel users are not present in the sync + // source, so we delete them + (None, Some((_, zitadel_id))) => { + let res = zitadel.delete_user(&zitadel_id).await; + if let Err(error) = res { + tracing::error!( + "Failed to delete user with Zitadel ID `{}`: {}", + zitadel_id, + error + ); + } + + zitadel_user = get_next_zitadel_user(&mut stream, &mut zitadel).await?; + } + + // Excess sync source users are not yet in Zitadel, so + // we import them + (Some(new_user), None) => { + let res = zitadel.import_user(&new_user).await; + if let Err(error) = res { + tracing::error!( + "Failed to import user `{}`: {}", + new_user.external_user_id, + error + ); + } + + source_user = sync_users.pop_front(); + } + + // If the sync source user matches the Zitadel user, the + // user is already synced and we can move on + (Some(new_user), Some((existing_user, _))) if new_user == existing_user => { + zitadel_user = get_next_zitadel_user(&mut stream, &mut zitadel).await?; + source_user = sync_users.pop_front(); + } + + // If the user ID of the user to be synced to Zitadel is < + // the user ID of the current Zitadel user, we found a new + // user which we should be importing + (Some(new_user), Some((existing_user, _))) + if new_user.external_user_id < existing_user.external_user_id => + { + let res = zitadel.import_user(&new_user).await; + if let Err(error) = res { + tracing::error!( + "Failed to import user `{}`: {}", + new_user.external_user_id, + error + ); + } + + source_user = sync_users.pop_front(); + // Don't fetch the next zitadel user yet + } + + // If the user ID of the user to be synced to Zitadel is > + // the user ID of the current Zitadel user, the Zitadel + // user needs to be deleted + (Some(new_user), Some((existing_user, zitadel_id))) + if new_user.external_user_id > existing_user.external_user_id => + { + let res = zitadel.delete_user(&zitadel_id).await; + if let Err(error) = res { + tracing::error!( + "Failed to delete user with Zitadel ID `{}`: {}", + zitadel_id, + error + ); + } + + zitadel_user = get_next_zitadel_user(&mut stream, &mut zitadel).await?; + // Don't move to the next source user yet + } + + // If the users don't match (since we've failed the former + // checks), but the user IDs are the same, the user has + // been updated + (Some(new_user), Some((existing_user, zitadel_id))) + if new_user.external_user_id == existing_user.external_user_id => + { + let res = zitadel.update_user(&zitadel_id, &existing_user, &new_user).await; + if let Err(error) = res { + tracing::error!( + "Failed to update user `{}`: {}", + new_user.external_user_id, + error + ); + } + + zitadel_user = get_next_zitadel_user(&mut stream, &mut zitadel).await?; + source_user = sync_users.pop_front(); + } + + // Since the user IDs form a partial order, they must be + // either equal, less than, or greater than, one another. + // + // Since all other possible conditions are checked in the + // first case, this particular case is unreachable. + (Some(new_user), Some((existing_user, _))) => { + tracing::error!( + "Unreachable condition met for users `{}` and `{}`", + new_user.external_user_id, + existing_user.external_user_id + ); + } + } + } + + Ok(()) +} diff --git a/src/main.rs b/src/main.rs index f748f10..111fc91 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,7 +2,7 @@ use std::{path::Path, process::ExitCode, str::FromStr}; use anyhow::{Context, Result}; -use famedly_sync::Config; +use famedly_sync::{perform_sync, Config}; use tracing::level_filters::LevelFilter; #[tokio::main] @@ -20,7 +20,7 @@ async fn main() -> ExitCode { #[allow(clippy::print_stderr)] async fn run_sync() -> Result<()> { let config = { - let config_path = std::env::var("FAMEDLY_SYNC_CONFIG").unwrap_or("config.yaml".into()); + let config_path = std::env::var("FAMEDLY_LDAP_SYNC_CONFIG").unwrap_or("config.yaml".into()); let config_path = Path::new(&config_path); match Config::new(config_path) { Ok(config) => config, @@ -44,5 +44,5 @@ async fn run_sync() -> Result<()> { tracing::subscriber::set_global_default(subscriber) .context("Setting default tracing subscriber failed")?; - config.perform_sync().await + perform_sync(&config).await } diff --git a/src/sources.rs b/src/sources.rs index 048825e..32c5b87 100644 --- a/src/sources.rs +++ b/src/sources.rs @@ -3,18 +3,29 @@ use anyhow::Result; use async_trait::async_trait; -use crate::zitadel::SourceDiff; - pub mod csv; pub mod ldap; pub mod ukt; +use crate::user::User; + /// A source of data we want to sync from. #[async_trait] pub trait Source { /// Get source name for debugging. fn get_name(&self) -> &'static str; - /// Get changes from the source. - async fn get_diff(&self) -> Result; + /// Get a stream of the sources' users, sorted by external user ID + // Ideally we would return a `Stream` here, as this would allow us + // to cut down significantly on memory use, however none of our + // sources currently support returning results sorted, so we would + // need to buffer the results to sort them anyway. + // + // In addition, `async_trait` does not currently support returning + // `impl` traits, making that technically infeasible with Rust. + // + // TODO: If we do get sources which *do* support sorting, and Rust + // gains this feature, we should probably switch to a stream here, + // though (and update existing sources to return sorted streams). + async fn get_sorted_users(&self) -> Result>; } diff --git a/src/sources/csv.rs b/src/sources/csv.rs index ae8a694..697bf67 100644 --- a/src/sources/csv.rs +++ b/src/sources/csv.rs @@ -8,7 +8,7 @@ use csv::Reader; use serde::Deserialize; use super::Source; -use crate::{user::User, zitadel::SourceDiff}; +use crate::user::User; /// CSV Source pub struct CsvSource { @@ -22,12 +22,10 @@ impl Source for CsvSource { "CSV" } - async fn get_diff(&self) -> Result { - let new_users = self.read_csv()?; - // TODO: Implement changed and deleted users - // Holding off on this until we get rid of the cache concept - // https://github.com/famedly/famedly-sync/issues/53 - return Ok(SourceDiff { new_users, changed_users: vec![], deleted_user_ids: vec![] }); + async fn get_sorted_users(&self) -> Result> { + let mut new_users = self.read_csv()?; + new_users.sort_by(|a, b| a.external_user_id.cmp(&b.external_user_id)); + return Ok(new_users); } } @@ -80,7 +78,7 @@ impl CsvData { first_name: csv_data.first_name.into(), last_name: csv_data.last_name.into(), phone: if csv_data.phone.is_empty() { None } else { Some(csv_data.phone.into()) }, - preferred_username: csv_data.email.clone().into(), + preferred_username: Some(csv_data.email.clone().into()), external_user_id: csv_data.email.into(), enabled: true, } diff --git a/src/sources/ldap.rs b/src/sources/ldap.rs index bd0561d..1b3ca7d 100644 --- a/src/sources/ldap.rs +++ b/src/sources/ldap.rs @@ -1,14 +1,11 @@ //! LDAP source for syncing with Famedly's Zitadel. -use std::{ - fmt::Display, - path::{Path, PathBuf}, -}; +use std::{fmt::Display, path::PathBuf}; use anyhow::{anyhow, bail, Context, Result}; use async_trait::async_trait; use ldap_poller::{ - config::TLSConfig, ldap::EntryStatus, ldap3::SearchEntry, AttributeConfig, Cache, CacheMethod, + config::TLSConfig, ldap::EntryStatus, ldap3::SearchEntry, AttributeConfig, CacheMethod, ConnectionConfig, Ldap, SearchEntryExt, Searches, }; use serde::Deserialize; @@ -17,17 +14,12 @@ use tokio_stream::{wrappers::ReceiverStream, StreamExt}; use url::Url; use super::Source; -use crate::{ - user::{StringOrBytes, User}, - zitadel::{ChangedUser, SourceDiff, UserId}, -}; +use crate::user::{StringOrBytes, User}; /// LDAP sync source pub struct LdapSource { /// LDAP configuration ldap_config: LdapSourceConfig, - /// Dry run flag (prevents writing cache) - is_dry_run: bool, } #[async_trait] @@ -36,74 +28,44 @@ impl Source for LdapSource { "LDAP" } - async fn get_diff(&self) -> Result { - let cache = read_cache(&self.ldap_config.cache_path).await?; - let (mut ldap_client, ldap_receiver) = Ldap::new(self.ldap_config.clone().into(), cache); - - let is_dry_run = self.is_dry_run; - let cache_path = self.ldap_config.cache_path.clone(); + async fn get_sorted_users(&self) -> Result> { + let (mut ldap_client, ldap_receiver) = Ldap::new(self.ldap_config.clone().into(), None); let sync_handle: tokio::task::JoinHandle> = tokio::spawn(async move { ldap_client.sync_once(None).await.context("failed to sync/fetch data from LDAP")?; - - if is_dry_run { - tracing::warn!("Not writing ldap cache during a dry run"); - } else { - let cache = ldap_client.persist_cache().await; - tokio::fs::write( - &cache_path, - bincode::serialize(&cache).context("failed to serialize cache")?, - ) - .await - .context("failed to write cache")?; - } - tracing::info!("Finished syncing LDAP data"); - Ok(()) }); - let (added, changed, removed) = self.get_user_changes(ldap_receiver).await?; - + let mut added = self.get_user_changes(ldap_receiver).await?; sync_handle.await??; - Ok(SourceDiff { - new_users: added, - changed_users: changed.into_iter().map(|(old, new)| ChangedUser { old, new }).collect(), - deleted_user_ids: removed, - }) + // TODO: Find out if we can use the AD extension for receiving sorted data + added.sort_by(|a, b| a.external_user_id.cmp(&b.external_user_id)); + + Ok(added) } } impl LdapSource { /// Create a new LDAP source - pub fn new(ldap_config: LdapSourceConfig, is_dry_run: bool) -> Self { - Self { ldap_config, is_dry_run } + pub fn new(ldap_config: LdapSourceConfig) -> Self { + Self { ldap_config } } /// Get user changes from an ldap receiver pub async fn get_user_changes( &self, ldap_receiver: Receiver, - ) -> Result<(Vec, Vec<(User, User)>, Vec)> { + ) -> Result> { ReceiverStream::new(ldap_receiver) - .fold(Ok((vec![], vec![], vec![])), |acc, entry_status| { - let (mut added, mut changed, mut removed) = acc?; - match entry_status { - EntryStatus::New(entry) => { - tracing::debug!("New entry: {:?}", entry); - added.push(self.parse_user(entry)?); - } - EntryStatus::Changed { old, new } => { - tracing::debug!("Changes found for {:?} -> {:?}", old, new); - changed.push((self.parse_user(old)?, self.parse_user(new)?)); - } - EntryStatus::Removed(entry) => { - tracing::debug!("Deleted user {}", String::from_utf8_lossy(&entry)); - removed.push(UserId::Nick(String::from_utf8(entry.clone())?)); - } + .fold(Ok(vec![]), |acc, entry_status| { + let mut added = acc?; + if let EntryStatus::New(entry) = entry_status { + tracing::debug!("New entry: {:?}", entry); + added.push(self.parse_user(entry)?); }; - Ok((added, changed, removed)) + Ok(added) }) .await } @@ -137,7 +99,7 @@ impl LdapSource { Ok(User { first_name, last_name, - preferred_username, + preferred_username: Some(preferred_username), email, external_user_id: ldap_user_id, phone, @@ -171,21 +133,6 @@ fn read_search_entry(entry: &SearchEntry, attribute: &AttributeMapping) -> Resul bail!("missing `{}` values for `{}`", attribute, entry.dn) } -/// Read the ldap sync cache -pub async fn read_cache(path: &Path) -> Result> { - Ok(match tokio::fs::read(path).await { - Ok(data) => Some(bincode::deserialize(&data).context("cache deserialization failed")?), - Err(err) => { - if err.kind() == std::io::ErrorKind::NotFound { - tracing::info!("LDAP sync cache missing"); - None - } else { - bail!(err) - } - } - }) -} - /// LDAP-specific configuration #[derive(Debug, Clone, Deserialize, PartialEq)] pub struct LdapSourceConfig { @@ -213,8 +160,6 @@ pub struct LdapSourceConfig { pub use_attribute_filter: bool, /// TLS-related configuration pub tls: Option, - /// Where to cache the last known LDAP state - pub cache_path: PathBuf, } impl From for ldap_poller::Config { @@ -264,7 +209,7 @@ impl From for ldap_poller::Config { attributes.phone.get_name(), ], }, - cache_method: CacheMethod::ModificationTime, + cache_method: CacheMethod::Disabled, check_for_deleted_entries: cfg.check_for_deleted_entries, } } @@ -409,7 +354,6 @@ mod tests { server_certificate: ./tests/environment/certs/server.crt danger_disable_tls_verify: false danger_use_start_tls: false - cache_path: ./test feature_flags: [] "#}; @@ -460,8 +404,7 @@ mod tests { async fn test_get_user_changes_new_and_changed() { let (tx, rx) = mpsc::channel(32); let config = load_config(); - let ldap_source = - LdapSource { ldap_config: config.sources.ldap.unwrap(), is_dry_run: false }; + let ldap_source = LdapSource { ldap_config: config.sources.ldap.unwrap() }; let mut user = new_user(); @@ -500,26 +443,15 @@ mod tests { let result = ldap_source.get_user_changes(rx).await; assert!(result.is_ok(), "Failed to get user changes: {:?}", result); - let (added, changed, removed) = result.unwrap(); + let added = result.unwrap(); assert_eq!(added.len(), 1, "Unexpected number of added users"); - assert_eq!(changed.len(), 1, "Unexpected number of changed users"); - assert_eq!(removed.len(), 0, "Unexpected number of removed users"); - - // Verify the changes - let changed_user_entry = &changed[0].1; - assert_eq!( - changed_user_entry.email, - StringOrBytes::String("newemail@example.com".to_owned()) - ); - assert_eq!(changed_user_entry.phone, Some(StringOrBytes::String("987654321".to_owned()))); } #[tokio::test] async fn test_get_user_changes_removed() { let (tx, rx) = mpsc::channel(32); let config = load_config(); - let ldap_source = - LdapSource { ldap_config: config.sources.ldap.unwrap(), is_dry_run: false }; + let ldap_source = LdapSource { ldap_config: config.sources.ldap.unwrap() }; let user = new_user(); @@ -541,17 +473,14 @@ mod tests { let result = ldap_source.get_user_changes(rx).await; assert!(result.is_ok(), "Failed to get user changes: {:?}", result); - let (added, changed, removed) = result.unwrap(); + let added = result.unwrap(); assert_eq!(added.len(), 1, "Unexpected number of added users"); - assert_eq!(changed.len(), 0, "Unexpected number of changed users"); - assert_eq!(removed.len(), 1, "Unexpected number of removed users"); } #[tokio::test] async fn test_parse_user() { let config = load_config(); - let ldap_source = - LdapSource { ldap_config: config.sources.ldap.unwrap(), is_dry_run: false }; + let ldap_source = LdapSource { ldap_config: config.sources.ldap.unwrap() }; let entry = SearchEntry { dn: "uid=testuser,ou=testorg,dc=example,dc=org".to_owned(), @@ -564,10 +493,10 @@ mod tests { let user = result.unwrap(); assert_eq!(user.first_name, StringOrBytes::String("Test".to_owned())); assert_eq!(user.last_name, StringOrBytes::String("User".to_owned())); - assert_eq!(user.preferred_username, StringOrBytes::String("testuser".to_owned())); + assert_eq!(user.preferred_username, Some(StringOrBytes::String("testuser".to_owned()))); assert_eq!(user.email, StringOrBytes::String("testuser@example.com".to_owned())); assert_eq!(user.phone, Some(StringOrBytes::String("123456789".to_owned()))); - assert_eq!(user.preferred_username, StringOrBytes::String("testuser".to_owned())); + assert_eq!(user.preferred_username, Some(StringOrBytes::String("testuser".to_owned()))); assert_eq!(user.external_user_id, StringOrBytes::String("testuser".to_owned())); assert!(user.enabled); } diff --git a/src/sources/ukt.rs b/src/sources/ukt.rs index 79b4c01..31acd25 100644 --- a/src/sources/ukt.rs +++ b/src/sources/ukt.rs @@ -3,15 +3,11 @@ use std::collections::HashMap; use anyhow::{Context, Result}; -use async_trait::async_trait; use chrono::Utc; use reqwest::Client; use serde::Deserialize; use url::Url; -use super::Source; -use crate::zitadel::{SourceDiff, UserId}; - /// UKT Source pub struct UktSource { /// UKT Source configuration @@ -20,19 +16,6 @@ pub struct UktSource { client: Client, } -#[async_trait] -impl Source for UktSource { - fn get_name(&self) -> &'static str { - "UKT" - } - - async fn get_diff(&self) -> Result { - let deleted_user_emails = self.get_removed_user_emails().await?; - let deleted_user_ids = deleted_user_emails.into_iter().map(UserId::Login).collect(); - return Ok(SourceDiff { new_users: vec![], changed_users: vec![], deleted_user_ids }); - } -} - impl UktSource { /// Create a new UKT source pub fn new(ukt_config: UktSourceConfig) -> Self { diff --git a/src/user.rs b/src/user.rs index d9730ff..c1adc5a 100644 --- a/src/user.rs +++ b/src/user.rs @@ -1,10 +1,9 @@ //! User data helpers use std::fmt::Display; +use anyhow::{anyhow, Result}; use base64::prelude::{Engine, BASE64_STANDARD}; -use zitadel_rust_client::v1::{Email, Gender, Idp, ImportHumanUserRequest, Phone, Profile}; - -use crate::{config::FeatureFlags, FeatureFlag}; +use zitadel_rust_client::v2::users::HumanUser; /// Source-agnostic representation of a user #[derive(Clone)] @@ -20,20 +19,65 @@ pub(crate) struct User { /// Whether the user is enabled pub(crate) enabled: bool, /// The user's preferred username - pub(crate) preferred_username: StringOrBytes, - /// The user's LDAP ID + pub(crate) preferred_username: Option, + /// The user's external (non-Zitadel) ID pub(crate) external_user_id: StringOrBytes, } impl User { - /// Convert the agnostic user to a Zitadel user - pub fn to_zitadel_user(&self, feature_flags: &FeatureFlags, idp_id: &str) -> ZitadelUser { - ZitadelUser { - user_data: self.clone(), - needs_email_verification: feature_flags.is_enabled(FeatureFlag::VerifyEmail), - needs_phone_verification: feature_flags.is_enabled(FeatureFlag::VerifyPhone), - idp_id: feature_flags.contains(&FeatureFlag::SsoLogin).then(|| idp_id.to_owned()), - } + /// Convert a Zitadel user to our internal representation + pub fn try_from_zitadel_user(user: HumanUser, external_id: String) -> Result { + let first_name = user + .profile() + .and_then(|profile| profile.given_name()) + .ok_or(anyhow!("Missing first name for {}", external_id))? + .clone(); + + let last_name = user + .profile() + .and_then(|profile| profile.family_name()) + .ok_or(anyhow!("Missing last name for {}", external_id))? + .clone(); + + let email = user + .email() + .and_then(|human_email| human_email.email()) + .ok_or(anyhow!("Missing email address for {}", external_id))? + .clone(); + + let phone = user.phone().and_then(|human_phone| human_phone.phone()); + + let external_user_id = match BASE64_STANDARD.decode(external_id.clone()) { + Ok(bytes) => bytes.into(), + Err(_) => external_id.into(), + }; + + Ok(Self { + first_name: first_name.into(), + last_name: last_name.into(), + email: email.into(), + phone: phone.map(|phone| phone.clone().into()), + preferred_username: None, + external_user_id, + enabled: true, + }) + } + + /// Get a display name for this user + pub fn get_display_name(&self) -> String { + format!("{}, {}", self.last_name, self.first_name) + } +} + +impl PartialEq for User { + fn eq(&self, other: &Self) -> bool { + self.first_name == other.first_name + && self.last_name == other.last_name + && self.email == other.email + && self.phone == other.phone + && self.enabled == other.enabled + && self.preferred_username == other.preferred_username + && self.external_user_id == other.external_user_id } } @@ -51,83 +95,8 @@ impl std::fmt::Debug for User { } } -/// Crate-internal representation of a Zitadel user -#[derive(Clone, Debug)] -pub struct ZitadelUser { - /// Details about the user - pub(crate) user_data: User, - - /// Whether the user should be prompted to verify their email - pub(crate) needs_email_verification: bool, - /// Whether the user should be prompted to verify their phone number - pub(crate) needs_phone_verification: bool, - /// The ID of the identity provider to link with, if any - pub(crate) idp_id: Option, -} - -impl ZitadelUser { - /// Get a display name for the user - pub(crate) fn get_display_name(&self) -> String { - format!("{}, {}", self.user_data.last_name, self.user_data.first_name) - } - - /// Return the name to be used in logs to identify this user - pub(crate) fn log_name(&self) -> String { - format!("external_id={}", &self.user_data.external_user_id) - } - - /// Get idp link as required by Zitadel - fn get_idps(&self) -> Vec { - if let Some(idp_id) = self.idp_id.clone() { - vec![Idp { - config_id: idp_id, - external_user_id: self.user_data.external_user_id.clone().to_string(), - display_name: self.get_display_name(), - }] - } else { - vec![] - } - } -} - -impl From for ImportHumanUserRequest { - fn from(user: ZitadelUser) -> Self { - Self { - user_name: user.user_data.email.clone().to_string(), - profile: Some(Profile { - first_name: user.user_data.first_name.clone().to_string(), - last_name: user.user_data.last_name.clone().to_string(), - display_name: user.get_display_name(), - gender: Gender::Unspecified.into(), // 0 means "unspecified", - nick_name: user.user_data.external_user_id.clone().to_string(), - preferred_language: String::default(), - }), - email: Some(Email { - email: user.user_data.email.clone().to_string(), - is_email_verified: !user.needs_email_verification, - }), - phone: user.user_data.phone.as_ref().map(|phone| Phone { - phone: phone.to_owned().to_string(), - is_phone_verified: !user.needs_phone_verification, - }), - password: String::default(), - hashed_password: None, - password_change_required: false, - request_passwordless_registration: false, - otp_code: String::default(), - idps: user.get_idps(), - } - } -} - -impl Display for ZitadelUser { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "external_id={}", &self.user_data.external_user_id) - } -} - /// A structure that can either be a string or bytes -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq)] pub(crate) enum StringOrBytes { /// A string String(String), @@ -135,6 +104,17 @@ pub(crate) enum StringOrBytes { Bytes(Vec), } +impl StringOrBytes { + /// Represent the object as raw bytes, regardless of whether it + /// can be represented as a string + pub fn as_bytes(&self) -> &[u8] { + match self { + Self::String(string) => string.as_bytes(), + Self::Bytes(bytes) => bytes, + } + } +} + impl PartialEq for StringOrBytes { fn eq(&self, other: &Self) -> bool { match (self, other) { @@ -146,6 +126,23 @@ impl PartialEq for StringOrBytes { } } +impl Ord for StringOrBytes { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + match (self, other) { + (Self::String(s), Self::String(o)) => s.cmp(o), + (Self::String(s), Self::Bytes(o)) => s.as_bytes().cmp(o.as_slice()), + (Self::Bytes(s), Self::String(o)) => s.as_slice().cmp(o.as_bytes()), + (Self::Bytes(s), Self::Bytes(o)) => s.cmp(o), + } + } +} + +impl PartialOrd for StringOrBytes { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl Display for StringOrBytes { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -160,3 +157,57 @@ impl From for StringOrBytes { Self::String(value) } } + +impl From> for StringOrBytes { + fn from(value: Vec) -> Self { + Self::Bytes(value) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn strb_from_string(string: &str) -> StringOrBytes { + StringOrBytes::from(string.to_owned()) + } + + fn strb_from_bytes(bytes: &[u8]) -> StringOrBytes { + StringOrBytes::Bytes(bytes.to_owned()) + } + + #[test] + fn test_strb_equality() { + assert_eq!(strb_from_string("a"), strb_from_string("a")); + assert_ne!(strb_from_string("a"), strb_from_string("b")); + + assert_eq!(strb_from_string("a"), strb_from_bytes(b"a")); + assert_ne!(strb_from_string("a"), strb_from_bytes(b"b")); + + assert_eq!(strb_from_bytes(b"a"), strb_from_bytes(b"a")); + assert_ne!(strb_from_bytes(b"a"), strb_from_bytes(b"b")); + + assert_eq!(strb_from_bytes(b"\xc3\x28"), strb_from_bytes(b"\xc3\x28")); + assert_ne!(strb_from_bytes(b"a"), strb_from_bytes(b"\xc3\x28")); + } + + #[test] + fn test_strb_order() { + assert!(strb_from_string("a") < strb_from_string("b")); + assert!(strb_from_string("b") > strb_from_string("a")); + assert!(strb_from_string("b") < strb_from_string("c")); + assert!(strb_from_string("a") < strb_from_string("c")); + + assert!(strb_from_bytes(b"a") < strb_from_bytes(b"b")); + assert!(strb_from_bytes(b"b") > strb_from_bytes(b"a")); + assert!(strb_from_bytes(b"b") < strb_from_bytes(b"c")); + assert!(strb_from_bytes(b"a") < strb_from_bytes(b"c")); + + assert!(strb_from_string("a") < strb_from_bytes(b"b")); + assert!(strb_from_string("b") > strb_from_bytes(b"a")); + assert!(strb_from_string("b") < strb_from_bytes(b"c")); + assert!(strb_from_string("a") < strb_from_bytes(b"c")); + + assert!(strb_from_bytes(b"\xc3\x28") < strb_from_bytes(b"\xc3\x29")); + } +} diff --git a/src/zitadel.rs b/src/zitadel.rs index 5761a06..54b02b9 100644 --- a/src/zitadel.rs +++ b/src/zitadel.rs @@ -1,18 +1,26 @@ //! Helper functions for submitting data to Zitadel use std::path::PathBuf; -use anyhow::{bail, Context, Result}; +use anyhow::{anyhow, Context, Result}; +use futures::{Stream, StreamExt}; use serde::Deserialize; use url::Url; use uuid::{uuid, Uuid}; -use zitadel_rust_client::v1::{ - error::{Error as ZitadelError, TonicErrorCode}, - Zitadel as ZitadelClient, +use zitadel_rust_client::{ + v1::Zitadel as ZitadelClientV1, + v2::{ + users::{ + AddHumanUserRequest, IdpLink, InUserEmailsQuery, ListUsersRequest, Organization, + SearchQuery, SetHumanEmail, SetHumanPhone, SetHumanProfile, SetMetadataEntry, + TypeQuery, UpdateHumanUserRequest, User as ZitadelUser, UserFieldName, Userv2Type, + }, + Zitadel as ZitadelClient, + }, }; use crate::{ config::{Config, FeatureFlags}, - user::{StringOrBytes, User, ZitadelUser}, + user::User, FeatureFlag, }; @@ -30,7 +38,10 @@ pub(crate) struct Zitadel { /// Optional set of features feature_flags: FeatureFlags, /// The backing Zitadel zitadel_client - zitadel_client: ZitadelClient, + pub zitadel_client: ZitadelClient, + /// The backing Ztiadel client, but for v1 API requests - some are + /// still required since the v2 API doesn't cover everything + zitadel_client_v1: ZitadelClientV1, } impl Zitadel { @@ -41,434 +52,263 @@ impl Zitadel { .await .context("failed to configure zitadel_client")?; + let zitadel_client_v1 = + ZitadelClientV1::new(config.zitadel.url.clone(), config.zitadel.key_file.clone()) + .await + .context("failed to configure zitadel_client_v1")?; + Ok(Self { zitadel_config: config.zitadel.clone(), feature_flags: config.feature_flags.clone(), zitadel_client, + zitadel_client_v1, }) } - /// Import a list of new users into Zitadel - pub(crate) async fn import_new_users(&self, users: Vec) -> Result<()> { - for user in users { - let zitadel_user = - user.to_zitadel_user(&self.feature_flags, &self.zitadel_config.idp_id); - let status = self.import_user(&zitadel_user).await; - - if let Err(error) = status { - tracing::error!( - "Failed to sync-import user `{}`: {:?}", - zitadel_user.log_name(), - error - ); - - if Self::is_invalid_phone_error(error) { - let zitadel_user = ZitadelUser { - user_data: User { phone: None, ..zitadel_user.user_data }, - ..zitadel_user - }; - - let retry_status = self.import_user(&zitadel_user).await; - - match retry_status { - Ok(_) => { - tracing::info!( - "Retry sync-import succeeded for user `{}`", - zitadel_user.log_name() - ); - } - Err(retry_error) => { - tracing::error!( - "Retry sync-import failed for user `{}`: {:?}", - zitadel_user.log_name(), - retry_error - ); - } - } - } - } - } - - Ok(()) - } - - /// Delete a list of Zitadel users given their IDs - pub(crate) async fn delete_users_by_id(&self, users: Vec) -> Result<()> { - for user_id in users { - match user_id { - UserId::Login(login) => { - let status = self.delete_user_by_email(&login).await; - if let Err(error) = status { - tracing::error!("Failed to delete user by email `{}`: {:?}", login, error); - } - } - UserId::Nick(nick) => { - let status = self.delete_user_by_nick(&nick).await; - if let Err(error) = status { - tracing::error!("Failed to delete user by nick `{}`: {:?}", nick, error); - } - } - UserId::ZitadelId(id) => { - let status = self.delete_user_by_id(&id).await; - if let Err(error) = status { - tracing::error!("Failed to delete user by id `{}`: {:?}", id, error); - } - } - } - } - - Ok(()) - } - - /// Update a list of old/new user maps - pub(crate) async fn update_users(&self, users: Vec) -> Result<()> { - let disabled: Vec = users - .iter() - .filter(|user| user.old.enabled && !user.new.enabled) - .map(|user| { - user.new.to_zitadel_user(&self.feature_flags, &self.zitadel_config.idp_id).clone() + /// Get a list of users by their email addresses + pub fn get_users_by_email( + &mut self, + emails: Vec, + ) -> Result> + Send> { + self.zitadel_client + .list_users( + ListUsersRequest::new(vec![ + SearchQuery::new().with_type_query(TypeQuery::new(Userv2Type::Human)), + SearchQuery::new().with_in_user_emails_query( + InUserEmailsQuery::new().with_user_emails(emails), + ), + ]) + .with_asc(true) + .with_sorting_column(UserFieldName::NickName), + ) + .map(|stream| { + stream.map(|user| { + let id = user.user_id().ok_or(anyhow!("Missing Zitadel user ID"))?.clone(); + let user = search_result_to_user(user)?; + Ok((user, id)) + }) }) - .collect(); + } - let enabled: Vec = users - .iter() - .filter(|user| !user.old.enabled && user.new.enabled) - .map(|user| { - user.new.to_zitadel_user(&self.feature_flags, &self.zitadel_config.idp_id).clone() - }) - .collect(); - - let changed: Vec<(ZitadelUser, ZitadelUser)> = users - .into_iter() - .filter(|user| user.new.enabled && user.old.enabled == user.new.enabled) - .map(|user| { - ( - user.old - .to_zitadel_user(&self.feature_flags, &self.zitadel_config.idp_id) - .clone(), - user.new - .to_zitadel_user(&self.feature_flags, &self.zitadel_config.idp_id) - .clone(), - ) + /// Return a stream of Zitadel users + pub fn list_users(&mut self) -> Result> + Send> { + self.zitadel_client + .list_users( + ListUsersRequest::new(vec![ + SearchQuery::new().with_type_query(TypeQuery::new(Userv2Type::Human)) + ]) + .with_asc(true) + .with_sorting_column(UserFieldName::NickName), + ) + .map(|stream| { + stream.map(|user| { + let id = user.user_id().ok_or(anyhow!("Missing Zitadel user ID"))?.clone(); + let user = search_result_to_user(user)?; + Ok((user, id)) + }) }) - .collect(); - - for user in disabled { - let status = self.delete_user(&user).await; - - if let Err(error) = status { - tracing::error!("Failed to delete user `{}`: {:?}`", user.log_name(), error); - } - } - - if !self.feature_flags.is_enabled(FeatureFlag::DeactivateOnly) { - for user in enabled { - let status = self.import_user(&user).await; + } - if let Err(error) = status { - tracing::error!("Failed to re-create user `{}`: {:?}", user.log_name(), error); - } - } + /// Delete a Zitadel user + pub async fn delete_user(&mut self, zitadel_id: &str) -> Result<()> { + tracing::info!("Deleting user with Zitadel ID: {}", zitadel_id); - for (old, new) in changed { - let status = self.update_user(&old, &new).await; - - if let Err(error) = status { - tracing::error!("Failed to sync-update user `{}`: {:?}", new.log_name(), error); - - if Self::is_invalid_phone_error(error) { - let new = - ZitadelUser { user_data: User { phone: None, ..new.user_data }, ..new }; - - let retry_status = self.update_user(&old, &new).await; - - match retry_status { - Ok(_) => { - tracing::info!( - "Retry sync-update succeeded for user `{}`", - new.log_name() - ); - } - Err(retry_error) => { - tracing::error!( - "Retry sync-update failed for user `{}`: {:?}", - new.log_name(), - retry_error - ); - } - } - } - } - } + if self.feature_flags.is_enabled(FeatureFlag::DryRun) { + tracing::warn!("Skipping deletion due to dry run"); + return Ok(()); } - Ok(()) + self.zitadel_client.delete_user(zitadel_id).await.map(|_o| ()) } - /// Update a Zitadel user - async fn update_user(&self, old: &ZitadelUser, new: &ZitadelUser) -> Result<()> { + /// Import a user into Zitadel + pub async fn import_user(&mut self, imported_user: &User) -> Result<()> { + tracing::info!("Importing user with external ID: {}", imported_user.external_user_id); + if self.feature_flags.is_enabled(FeatureFlag::DryRun) { - tracing::info!("Not updating user due to dry run: {:?} -> {:?}", old, new); + tracing::warn!("Skipping import due to dry run"); return Ok(()); } - let Some(user_id) = self.get_user_id(old).await? else { - bail!("could not find user `{}` to update", old.user_data.email); - }; + let mut metadata = vec![SetMetadataEntry::new( + "localpart".to_owned(), + Uuid::new_v5(&FAMEDLY_NAMESPACE, imported_user.external_user_id.as_bytes()).to_string(), + )]; - if old.user_data.email != new.user_data.email { - self.zitadel_client - .update_human_user_name( - &self.zitadel_config.organization_id, - user_id.clone(), - new.user_data.email.clone().to_string(), - ) - .await?; + if let Some(preferred_username) = imported_user.preferred_username.clone() { + metadata.push(SetMetadataEntry::new( + "preferred_username".to_owned(), + preferred_username.to_string(), + )); + } - tracing::warn!( - "User email/login changed for {} -> {}", - old.user_data.email, - new.user_data.email + let mut user = AddHumanUserRequest::new( + SetHumanProfile::new( + imported_user.first_name.to_string(), + imported_user.last_name.to_string(), + ) + .with_nick_name(imported_user.external_user_id.to_string()) + .with_display_name(imported_user.get_display_name()), + SetHumanEmail::new(imported_user.email.to_string()) + .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyEmail)), + ) + .with_organization( + Organization::new().with_org_id(self.zitadel_config.organization_id.clone()), + ) + .with_metadata(metadata); + + if let Some(phone) = imported_user.phone.clone() { + user.set_phone( + SetHumanPhone::new() + .with_phone(phone.to_string()) + .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyPhone)), ); }; - if old.user_data.first_name != new.user_data.first_name - || old.user_data.last_name != new.user_data.last_name - { - self.zitadel_client - .update_human_user_profile( - &self.zitadel_config.organization_id, - user_id.clone(), - new.user_data.first_name.clone().to_string(), - new.user_data.last_name.clone().to_string(), - None, - Some(new.get_display_name()), - None, - None, - ) - .await?; - }; + if self.feature_flags.is_enabled(FeatureFlag::SsoLogin) { + user.set_idp_links(vec![IdpLink::new() + .with_user_id(imported_user.external_user_id.to_string()) + .with_idp_id(self.zitadel_config.idp_id.clone()) + // TODO: Figure out if this is the correct value; empty is not permitted + .with_user_name(imported_user.email.to_string())]); + } - match (&old.user_data.phone, &new.user_data.phone) { - (Some(_), None) => { - self.zitadel_client - .remove_human_user_phone(&self.zitadel_config.organization_id, user_id.clone()) - .await?; - } - (_, Some(new_phone)) => { - self.zitadel_client - .update_human_user_phone( - &self.zitadel_config.organization_id, - user_id.clone(), - new_phone.clone().to_string(), - !self.feature_flags.is_enabled(FeatureFlag::VerifyPhone), + match self.zitadel_client.create_human_user(user.clone()).await { + Ok(res) => { + let id = res + .user_id() + .ok_or(anyhow!( + "Failed to create user ID for external user `{}`", + imported_user.external_user_id + ))? + .clone(); + + self.zitadel_client_v1 + .add_user_grant( + Some(self.zitadel_config.organization_id.clone()), + id, + self.zitadel_config.project_id.clone(), + None, + vec![FAMEDLY_USER_ROLE.to_owned()], ) .await?; } - (None, None) => {} - }; - - if old.user_data.email != new.user_data.email { - self.zitadel_client - .update_human_user_email( - &self.zitadel_config.organization_id, - user_id.clone(), - new.user_data.email.clone().to_string(), - !self.feature_flags.is_enabled(FeatureFlag::VerifyEmail), - ) - .await?; - }; - - if old.user_data.preferred_username != new.user_data.preferred_username { - self.zitadel_client - .set_user_metadata( - Some(&self.zitadel_config.organization_id), - user_id, - "preferred_username".to_owned(), - &new.user_data.preferred_username.clone().to_string(), - ) - .await?; - }; - - tracing::info!("Successfully updated user {}", old.user_data.email); - - Ok(()) - } - - /// Delete a Zitadel user given only their LDAP id - async fn delete_user_by_id(&self, user_id: &str) -> Result<()> { - if self.feature_flags.is_enabled(FeatureFlag::DryRun) { - tracing::info!("Not deleting user `{}` due to dry run", user_id); - return Ok(()); - } - let user = self - .zitadel_client - .get_user_by_nick_name( - Some(self.zitadel_config.organization_id.clone()), - user_id.to_owned(), - ) - .await?; - match user { - Some(user) => self.zitadel_client.remove_user(user.id).await?, - None => bail!("Could not find user with ldap uid '{user_id}' for deletion"), + Err(error) => { + // If the phone number is invalid + if error.to_string().contains("PHONE-so0wa") { + user.reset_phone(); + self.zitadel_client.create_human_user(user).await?; + } else { + anyhow::bail!(error) + } + } } - tracing::info!("Successfully deleted user {}", user_id); - Ok(()) } - /// Delete a Zitadel user given only their email address used as login name - async fn delete_user_by_email(&self, email: &str) -> Result<()> { - if self.feature_flags.is_enabled(FeatureFlag::DryRun) { - tracing::info!("Not deleting user `{}` due to dry run", email); - return Ok(()); - } - - let user = self.zitadel_client.get_user_by_login_name(email).await?; - match user { - Some(user) => self.zitadel_client.remove_user(user.id).await?, - None => tracing::info!("Could not find user with email '{email}' for deletion"), - } + /// Update a user + pub async fn update_user( + &mut self, + zitadel_id: &str, + old_user: &User, + updated_user: &User, + ) -> Result<()> { + tracing::info!( + "Updating user `{}` to `{}`", + old_user.external_user_id, + updated_user.external_user_id + ); - tracing::info!("Successfully deleted user {}", email); - - Ok(()) - } - - /// Delete a Zitadel user given only their nick name - async fn delete_user_by_nick(&self, nick: &str) -> Result<()> { if self.feature_flags.is_enabled(FeatureFlag::DryRun) { - tracing::info!("Not deleting user `{}` due to dry run", nick); + tracing::warn!("Skipping update due to dry run"); return Ok(()); } - let user = self - .zitadel_client - .get_user_by_nick_name( - Some(self.zitadel_config.organization_id.clone()), - nick.to_owned(), - ) - .await?; - match user { - Some(user) => self.zitadel_client.remove_user(user.id).await?, - None => tracing::info!("Could not find user with nick '{nick}' for deletion"), - } - - tracing::info!("Successfully deleted user {}", nick); - - Ok(()) - } - - /// Retrieve the Zitadel user ID of a user, or None if the user - /// cannot be found - async fn get_user_id(&self, user: &ZitadelUser) -> Result> { - let status = self - .zitadel_client - .get_user_by_login_name(&user.user_data.email.clone().to_string()) - .await; - - if let Err(ZitadelError::TonicResponseError(ref error)) = status { - if error.code() == TonicErrorCode::NotFound { - return Ok(None); - } - } - - Ok(status.map(|user| user.map(|u| u.id))?) - } + let mut request = UpdateHumanUserRequest::new(); - /// Delete a Zitadel user - async fn delete_user(&self, user: &ZitadelUser) -> Result<()> { - if self.feature_flags.is_enabled(FeatureFlag::DryRun) { - tracing::info!("Not deleting user due to dry run: {:?}", user); - return Ok(()); - } - - if let Some(user_id) = self.get_user_id(user).await? { - self.zitadel_client.remove_user(user_id).await?; - } else { - bail!("could not find user `{}` for deletion", user.user_data.email); + if old_user.email != updated_user.email { + request.set_username(updated_user.email.to_string()); + request.set_email( + SetHumanEmail::new(updated_user.email.to_string()) + .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyEmail)), + ); } - tracing::info!("Successfully deleted user {}", user.user_data.email); - - Ok(()) - } - - /// Import a user into Zitadel - async fn import_user(&self, user: &ZitadelUser) -> Result<()> { - if self.feature_flags.is_enabled(FeatureFlag::DryRun) { - tracing::info!("Not importing user due to dry run: {:?}", user); - return Ok(()); + if old_user.first_name != updated_user.first_name + || old_user.last_name != updated_user.last_name + { + request.set_profile( + SetHumanProfile::new( + updated_user.first_name.to_string(), + updated_user.last_name.to_string(), + ) + .with_display_name(updated_user.get_display_name()), + ); } - if !user.user_data.enabled { - tracing::info!("Not importing disabled user: {:?}", user); - return Ok(()); + if old_user.phone != updated_user.phone { + if let Some(phone) = updated_user.phone.clone() { + request.set_phone( + SetHumanPhone::new() + .with_phone(phone.to_string()) + .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyPhone)), + ); + } else { + self.zitadel_client.remove_phone(zitadel_id).await?; + } } - let new_user_id = self - .zitadel_client - .create_human_user(&self.zitadel_config.organization_id, user.clone().into()) - .await?; - - self.zitadel_client - .set_user_metadata( - Some(&self.zitadel_config.organization_id), - new_user_id.clone(), - "preferred_username".to_owned(), - &user.user_data.preferred_username.clone().to_string(), - ) - .await?; - - let id = match &user.user_data.external_user_id { - StringOrBytes::String(value) => value.as_bytes(), - StringOrBytes::Bytes(value) => value, + if let Err(error) = self.zitadel_client.update_human_user(zitadel_id, request.clone()).await + { + // If the new phone number is invalid + if error.to_string().contains("PHONE-so0wa") { + request.reset_phone(); + self.zitadel_client.update_human_user(zitadel_id, request).await?; + + if let Err(error) = self.zitadel_client.remove_phone(zitadel_id).await { + // If the user didn't start out with a phone + if !error.to_string().contains("COMMAND-ieJ2e") { + anyhow::bail!(error); + } + }; + } else { + anyhow::bail!(error); + } }; - self.zitadel_client - .set_user_metadata( - Some(&self.zitadel_config.organization_id), - new_user_id.clone(), - "localpart".to_owned(), - &Uuid::new_v5(&FAMEDLY_NAMESPACE, id).to_string(), - ) - .await?; - - self.zitadel_client - .add_user_grant( - Some(self.zitadel_config.organization_id.clone()), - new_user_id, - self.zitadel_config.project_id.clone(), - None, - vec![FAMEDLY_USER_ROLE.to_owned()], - ) - .await?; - - tracing::info!("Successfully imported user {:?}", user); + if old_user.preferred_username != updated_user.preferred_username { + if let Some(preferred_username) = updated_user.preferred_username.clone() { + self.zitadel_client + .set_user_metadata( + zitadel_id, + "preferred_username", + &preferred_username.to_string(), + ) + .await?; + } else { + self.zitadel_client.delete_user_metadata(zitadel_id, "preferred_username").await?; + } + } Ok(()) } +} - /// Check if an error is an invalid phone error - fn is_invalid_phone_error(error: anyhow::Error) -> bool { - /// Part of the error message returned by Zitadel - /// when a phone number is invalid for a new user - const INVALID_PHONE_IMPORT_ERROR: &str = "invalid ImportHumanUserRequest_Phone"; - - /// Part of the error message returned by Zitadel - /// when a phone number is invalid for an existing user being updated - const INVALID_PHONE_UPDATE_ERROR: &str = "invalid UpdateHumanPhoneRequest"; - - if let Ok(ZitadelError::TonicResponseError(ref error)) = error.downcast::() { - return error.code() == TonicErrorCode::InvalidArgument - && (error.message().contains(INVALID_PHONE_IMPORT_ERROR) - || error.message().contains(INVALID_PHONE_UPDATE_ERROR)); - } - - false - } +/// Convert a Zitadel search result to a user +fn search_result_to_user(user: ZitadelUser) -> Result { + let human_user = user.human().ok_or(anyhow!("Machine user found in human user search"))?; + let nick_name = human_user + .profile() + .and_then(|p| p.nick_name()) + .ok_or(anyhow!("Missing external ID found for user"))?; + + // TODO: If async closures become a reality, we + // should capture the correct preferred_username + // here. + let user = User::try_from_zitadel_user(human_user.clone(), nick_name.clone())?; + Ok(user) } /// Configuration related to Famedly Zitadel @@ -485,35 +325,3 @@ pub struct ZitadelConfig { /// IDP ID provided by Famedly Zitadel pub idp_id: String, } - -/// The different ways to identify a user in Zitadel -#[derive(Debug)] -pub enum UserId { - /// The login name is actually the email address - Login(String), - /// The nick name is actually the LDAP ID - Nick(String), - /// The Zitadel ID - #[allow(dead_code)] - ZitadelId(String), -} - -/// The difference between the source and Zitadel -#[derive(Debug)] -pub struct SourceDiff { - /// New users - pub new_users: Vec, - /// Changed users - pub changed_users: Vec, - /// Deleted user IDs - pub deleted_user_ids: Vec, -} - -/// A user that has changed returned from the LDAP poller -#[derive(Debug)] -pub struct ChangedUser { - /// The old state - pub old: User, - /// The new state - pub new: User, -} diff --git a/tests/e2e.rs b/tests/e2e.rs index c281b0b..fe2ee1a 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -5,6 +5,7 @@ use std::{collections::HashSet, path::Path, time::Duration}; use base64::prelude::{Engine, BASE64_STANDARD}; use famedly_sync::{ csv_test_helpers::temp_csv_file, + perform_sync, ukt_test_helpers::{ get_mock_server_url, prepare_endpoint_mock, prepare_oauth2_mock, ENDPOINT_PATH, OAUTH2_PATH, }, @@ -47,7 +48,7 @@ async fn test_e2e_simple_sync() { .await; let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -120,7 +121,7 @@ async fn test_e2e_sync_disabled_user() { .await; let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel.get_user_by_login_name("disabled_user@famedly.de").await; @@ -159,7 +160,7 @@ async fn test_e2e_sso() { ) .await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -189,11 +190,11 @@ async fn test_e2e_sync_change() { .await; let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); ldap.change_user("change", vec![("telephoneNumber", HashSet::from(["+12015550123"]))]).await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -228,18 +229,18 @@ async fn test_e2e_sync_disable_and_reenable() { let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel.get_user_by_login_name("disable@famedly.de").await; assert!(user.is_ok_and(|u| u.is_some())); ldap.change_user("disable", vec![("shadowFlag", HashSet::from(["514"]))]).await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let user = zitadel.get_user_by_login_name("disable@famedly.de").await; assert!(user.is_err_and(|error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound))); ldap.change_user("disable", vec![("shadowFlag", HashSet::from(["512"]))]).await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel.get_user_by_login_name("disable@famedly.de").await; assert!(user.is_ok_and(|u| u.is_some())); @@ -261,12 +262,12 @@ async fn test_e2e_sync_email_change() { .await; let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); ldap.change_user("email_change", vec![("mail", HashSet::from(["email_changed@famedly.de"]))]) .await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel.get_user_by_login_name("email_changed@famedly.de").await; @@ -290,7 +291,7 @@ async fn test_e2e_sync_deletion() { .await; let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = @@ -299,7 +300,7 @@ async fn test_e2e_sync_deletion() { ldap.delete_user("deleted").await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let user = zitadel.get_user_by_login_name("deleted@famedly.de").await; assert!(user.is_err_and(|error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound))); @@ -330,7 +331,7 @@ async fn test_e2e_ldaps() { ) .await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -367,7 +368,7 @@ async fn test_e2e_ldaps_starttls() { ) .await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -386,7 +387,7 @@ async fn test_e2e_no_phone() { .await; let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -433,7 +434,7 @@ async fn test_e2e_sync_invalid_phone() { .await; let config = config().await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -468,7 +469,7 @@ async fn test_e2e_sync_invalid_phone() { ldap.change_user("good_gone_bad_phone", vec![("telephoneNumber", HashSet::from(["abc"]))]) .await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); let user = zitadel .get_user_by_login_name("good_gone_bad_phone@famedly.de") @@ -525,7 +526,7 @@ async fn test_e2e_binary_attr() { let org_id = config.zitadel.organization_id.clone(); - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -586,7 +587,7 @@ async fn test_e2e_binary_attr_valid_utf8() { let org_id = config.zitadel.organization_id.clone(); - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -632,17 +633,17 @@ async fn test_e2e_dry_run() { let zitadel = open_zitadel_connection().await; // Assert the user does not sync, because this is a dry run - dry_run_config.perform_sync().await.expect("syncing failed"); + perform_sync(&dry_run_config).await.expect("syncing failed"); assert!(zitadel.get_user_by_login_name("dry_run@famedly.de").await.is_err_and( |error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound), )); // Actually sync the user so we can test other changes= - config.perform_sync().await.expect("syncing failed"); + perform_sync(config).await.expect("syncing failed"); // Assert that a change in phone number does not sync ldap.change_user("dry_run", vec![("telephoneNumber", HashSet::from(["+12015550124"]))]).await; - dry_run_config.perform_sync().await.expect("syncing failed"); + perform_sync(&dry_run_config).await.expect("syncing failed"); let user = zitadel .get_user_by_login_name("dry_run@famedly.de") .await @@ -655,7 +656,7 @@ async fn test_e2e_dry_run() { // Assert that disabling a user does not sync ldap.change_user("dry_run", vec![("shadowFlag", HashSet::from(["514"]))]).await; - dry_run_config.perform_sync().await.expect("syncing failed"); + perform_sync(&dry_run_config).await.expect("syncing failed"); assert!(zitadel .get_user_by_login_name("dry_run@famedly.de") .await @@ -663,7 +664,7 @@ async fn test_e2e_dry_run() { // Assert that a user deletion does not sync ldap.delete_user("dry_run").await; - dry_run_config.perform_sync().await.expect("syncing failed"); + perform_sync(&dry_run_config).await.expect("syncing failed"); assert!(zitadel .get_user_by_login_name("dry_run@famedly.de") .await @@ -721,7 +722,7 @@ async fn test_e2e_sync_deactivated_only() { ldap.change_user("reenabled_disable_only", vec![("shadowFlag", HashSet::from(["514"]))]).await; let mut config = config().await.clone(); - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel.get_user_by_login_name("disable_disable_only@famedly.de").await; @@ -754,7 +755,7 @@ async fn test_e2e_sync_deactivated_only() { .await; ldap.delete_user("deleted_disable_only").await; ldap.change_user("reenabled_disable_only", vec![("shadowFlag", HashSet::from(["512"]))]).await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let user = zitadel.get_user_by_login_name("disable_disable_only@famedly.de").await; assert!(user.is_err_and(|error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound))); @@ -836,7 +837,7 @@ async fn test_e2e_ukt_sync() { let user = user.expect("could not find user"); assert_eq!(user.user_name, "delete_me@famedly.de"); - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let user = zitadel.get_user_by_login_name("delete_me@famedly.de").await; assert!(user.is_err_and(|error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound))); @@ -847,7 +848,7 @@ async fn test_e2e_ukt_sync() { async fn test_e2e_csv_sync() { let mut config = config().await.clone(); - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; let user = zitadel @@ -909,7 +910,7 @@ async fn test_e2e_csv_sync() { john.doe@example.com,Changed_Name,Changed_Surname,+2222222222 "#}; let _file = temp_csv_file(&mut config, csv_content); - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let user = zitadel .get_user_by_login_name("john.doe@example.com") @@ -1009,7 +1010,7 @@ async fn test_e2e_full_sync() { // and the ./tests/environment/files/test-users.csv was already imported let _file = temp_csv_file(&mut config, csv_content); - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -1068,7 +1069,7 @@ async fn test_e2e_full_sync() { .await; ldap.delete_user("not_to_be_there_later").await; - config.perform_sync().await.expect("syncing failed"); + perform_sync(&config).await.expect("syncing failed"); let user = zitadel .get_user_by_login_name("to_be_changed@famedly.de") From a21fa1f500aacd435301170f93049e27f72d70ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Thu, 31 Oct 2024 14:38:38 +0100 Subject: [PATCH 03/11] WIP: test: Split apart multi-source oriented test setup --- src/config.rs | 6 +- src/lib.rs | 2 +- tests/e2e.rs | 102 +++++++++++++------- tests/environment/config.template.yaml | 40 -------- tests/environment/csv-config.template.yaml | 3 + tests/environment/ldap-config.template.yaml | 27 ++++++ tests/environment/ukt-config.template.yaml | 8 ++ 7 files changed, 108 insertions(+), 80 deletions(-) create mode 100644 tests/environment/csv-config.template.yaml create mode 100644 tests/environment/ldap-config.template.yaml create mode 100644 tests/environment/ukt-config.template.yaml diff --git a/src/config.rs b/src/config.rs index cb3f1b6..90ff538 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,10 +8,8 @@ use anyhow::{bail, Result}; use serde::Deserialize; use url::Url; -use crate::{ - sources::{csv::CsvSourceConfig, ldap::LdapSourceConfig, ukt::UktSourceConfig}, - zitadel::ZitadelConfig, -}; +pub use crate::sources::{csv::CsvSourceConfig, ldap::LdapSourceConfig, ukt::UktSourceConfig}; +use crate::zitadel::ZitadelConfig; /// App prefix for env var configuration const ENV_VAR_CONFIG_PREFIX: &str = "FAMEDLY_SYNC"; diff --git a/src/lib.rs b/src/lib.rs index efac5f0..1c48917 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,7 @@ mod zitadel; use std::collections::VecDeque; -pub use config::{Config, FeatureFlag}; +pub use config::{Config, FeatureFlag, LdapSourceConfig}; pub use sources::{ csv::test_helpers as csv_test_helpers, ldap::AttributeMapping, ukt::test_helpers as ukt_test_helpers, diff --git a/tests/e2e.rs b/tests/e2e.rs index fe2ee1a..2ad676e 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -9,9 +9,10 @@ use famedly_sync::{ ukt_test_helpers::{ get_mock_server_url, prepare_endpoint_mock, prepare_oauth2_mock, ENDPOINT_PATH, OAUTH2_PATH, }, - AttributeMapping, Config, FeatureFlag, + AttributeMapping, Config, FeatureFlag, LdapSourceConfig, }; use ldap3::{Ldap as LdapClient, LdapConnAsync, LdapConnSettings, Mod}; +use serde::{de::IntoDeserializer, Deserialize}; use tempfile::TempDir; use test_log::test; use tokio::sync::OnceCell; @@ -47,7 +48,7 @@ async fn test_e2e_simple_sync() { ) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -120,7 +121,7 @@ async fn test_e2e_sync_disabled_user() { ) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -145,7 +146,7 @@ async fn test_e2e_sync_disabled_user() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_sso() { - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); config.feature_flags.push(FeatureFlag::SsoLogin); let mut ldap = Ldap::new().await; @@ -189,7 +190,7 @@ async fn test_e2e_sync_change() { ) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); ldap.change_user("change", vec![("telephoneNumber", HashSet::from(["+12015550123"]))]).await; @@ -227,7 +228,7 @@ async fn test_e2e_sync_disable_and_reenable() { ) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -261,7 +262,7 @@ async fn test_e2e_sync_email_change() { ) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); ldap.change_user("email_change", vec![("mail", HashSet::from(["email_changed@famedly.de"]))]) @@ -290,7 +291,7 @@ async fn test_e2e_sync_deletion() { ) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -309,7 +310,7 @@ async fn test_e2e_sync_deletion() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_ldaps() { - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); config .sources .ldap @@ -345,7 +346,7 @@ async fn test_e2e_ldaps() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_ldaps_starttls() { - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); config .sources .ldap @@ -363,7 +364,7 @@ async fn test_e2e_ldaps_starttls() { "Bobby", "starttls@famedly.de", Some("+12015550123"), - "starttls", + "starttls2", false, ) .await; @@ -386,7 +387,7 @@ async fn test_e2e_no_phone() { ldap.create_user("Bob", "Tables", "Bobby", "no_phone@famedly.de", None, "no_phone", false) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -433,7 +434,7 @@ async fn test_e2e_sync_invalid_phone() { ) .await; - let config = config().await; + let config = ldap_config().await; perform_sync(config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -488,7 +489,7 @@ async fn test_e2e_sync_invalid_phone() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_binary_attr() { - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); // OpenLDAP checks if types match, so we need to use an attribute // that can actually be binary. @@ -553,7 +554,7 @@ async fn test_e2e_binary_attr() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_binary_attr_valid_utf8() { - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); // OpenLDAP checks if types match, so we need to use an attribute // that can actually be binary. @@ -614,8 +615,8 @@ async fn test_e2e_binary_attr_valid_utf8() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_dry_run() { - let mut dry_run_config = config().await.clone(); - let config = config().await; + let mut dry_run_config = ldap_config().await.clone(); + let config = ldap_config().await; dry_run_config.feature_flags.push(FeatureFlag::DryRun); let mut ldap = Ldap::new().await; @@ -721,7 +722,7 @@ async fn test_e2e_sync_deactivated_only() { ldap.change_user("reenabled_disable_only", vec![("shadowFlag", HashSet::from(["514"]))]).await; - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); perform_sync(&config).await.expect("syncing failed"); let zitadel = open_zitadel_connection().await; @@ -789,7 +790,7 @@ async fn test_e2e_ukt_sync() { prepare_oauth2_mock(&mock_server).await; prepare_endpoint_mock(&mock_server, "delete_me@famedly.de").await; - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); config .sources @@ -846,7 +847,7 @@ async fn test_e2e_ukt_sync() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_csv_sync() { - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); perform_sync(&config).await.expect("syncing failed"); @@ -943,7 +944,7 @@ async fn test_e2e_full_sync() { prepare_oauth2_mock(&mock_server).await; prepare_endpoint_mock(&mock_server, "not_to_be_there@famedly.de").await; - let mut config = config().await.clone(); + let mut config = ldap_config().await.clone(); config .sources .ukt @@ -1094,7 +1095,7 @@ struct Ldap { impl Ldap { async fn new() -> Self { - let config = config().await.clone(); + let config = ldap_config().await.clone(); let mut settings = LdapConnSettings::new(); if let Some(ref ldap_config) = config.sources.ldap { @@ -1149,7 +1150,7 @@ impl Ldap { attrs.push(("telephoneNumber", HashSet::from([phone]))); } - let base_dn = config() + let base_dn = ldap_config() .await .sources .ldap @@ -1178,7 +1179,7 @@ impl Ldap { .map(|(attribute, changes)| Mod::Replace(attribute, changes)) .collect(); - let base_dn = config() + let base_dn = ldap_config() .await .sources .ldap @@ -1196,7 +1197,7 @@ impl Ldap { } async fn delete_user(&mut self, uid: &str) { - let base_dn = config() + let base_dn = ldap_config() .await .sources .ldap @@ -1216,29 +1217,60 @@ impl Ldap { /// Open a connection to the configured Zitadel backend async fn open_zitadel_connection() -> Zitadel { - let zitadel_config = config().await.zitadel.clone(); + let zitadel_config = ldap_config().await.zitadel.clone(); Zitadel::new(zitadel_config.url, zitadel_config.key_file) .await .expect("failed to set up Zitadel client") } /// Get the module's test environment config -async fn config() -> &'static Config { +async fn ldap_config() -> &'static Config { CONFIG .get_or_init(|| async { let mut config = Config::new(Path::new("tests/environment/config.yaml")) .expect("failed to parse test env file"); - let tempdir = TEMPDIR - .get_or_init(|| async { TempDir::new().expect("failed to initialize cache dir") }) - .await; + config.sources.ldap = serde_json::from_slice( + &std::fs::read(Path::new("tests/environment/ldap-config.template.yaml")) + .expect("failed to read ldap config file"), + ) + .expect("failed to parse ldap config"); config - .sources - .ldap - .as_mut() - .expect("ldap must be configured for this test") - .cache_path = tempdir.path().join("cache.bin"); + }) + .await +} + +/// Get the module's test environment config +async fn ukt_config() -> &'static Config { + CONFIG + .get_or_init(|| async { + let mut config = Config::new(Path::new("tests/environment/config.yaml")) + .expect("failed to parse test env file"); + + config.sources.ldap = serde_json::from_slice( + &std::fs::read(Path::new("tests/environment/ukt-config.template.yaml")) + .expect("failed to read ukt config file"), + ) + .expect("failed to parse ukt config"); + + config + }) + .await +} + +/// Get the module's test environment config +async fn csv_config() -> &'static Config { + CONFIG + .get_or_init(|| async { + let mut config = Config::new(Path::new("tests/environment/config.yaml")) + .expect("failed to parse test env file"); + + config.sources.ldap = serde_json::from_slice( + &std::fs::read(Path::new("tests/environment/csv-config.template.yaml")) + .expect("failed to read csv config file"), + ) + .expect("failed to parse csv config"); config }) diff --git a/tests/environment/config.template.yaml b/tests/environment/config.template.yaml index e3fe7b0..7e4c1aa 100644 --- a/tests/environment/config.template.yaml +++ b/tests/environment/config.template.yaml @@ -5,45 +5,5 @@ zitadel: project_id: @PROJECT_ID@ idp_id: @IDP_ID@ -sources: - ldap: - url: ldap://localhost:1389 - base_dn: ou=testorg,dc=example,dc=org - bind_dn: cn=admin,dc=example,dc=org - bind_password: adminpassword - user_filter: "(objectClass=shadowAccount)" - timeout: 5 - check_for_deleted_entries: true - use_attribute_filter: true - attributes: - first_name: "cn" # objectClass: person - last_name: "sn" # objectClass: person - preferred_username: "displayName" # objectClass: inetOrgPerson - email: "mail" # objectClass: inetOrgPerson - phone: "telephoneNumber" # objectClass: person - user_id: "uid" - status: - name: "shadowFlag" # objectClass: shadowAccount - is_binary: false - disable_bitmasks: [0x2, 0x10] - tls: - client_key: ./tests/environment/certs/client.key - client_certificate: ./tests/environment/certs/client.crt - server_certificate: ./tests/environment/certs/server.crt - danger_disable_tls_verify: false - danger_use_start_tls: false - cache_path: ./test - - ukt: - endpoint_url: https://list.example.invalid/usersync4chat/maillist - oauth2_url: https://list.example.invalid/token - client_id: mock_client_id - client_secret: mock_client_secret - scope: "openid read-maillist" - grant_type: client_credentials - - csv: - file_path: ./tests/environment/files/test-users.csv - feature_flags: - sso_login diff --git a/tests/environment/csv-config.template.yaml b/tests/environment/csv-config.template.yaml new file mode 100644 index 0000000..087363e --- /dev/null +++ b/tests/environment/csv-config.template.yaml @@ -0,0 +1,3 @@ +sources: + csv: + file_path: ./tests/environment/files/test-users.csv diff --git a/tests/environment/ldap-config.template.yaml b/tests/environment/ldap-config.template.yaml new file mode 100644 index 0000000..07942fa --- /dev/null +++ b/tests/environment/ldap-config.template.yaml @@ -0,0 +1,27 @@ +sources: + ldap: + url: ldap://localhost:1389 + base_dn: ou=testorg,dc=example,dc=org + bind_dn: cn=admin,dc=example,dc=org + bind_password: adminpassword + user_filter: "(objectClass=shadowAccount)" + timeout: 5 + check_for_deleted_entries: true + use_attribute_filter: true + attributes: + first_name: "cn" # objectClass: person + last_name: "sn" # objectClass: person + preferred_username: "displayName" # objectClass: inetOrgPerson + email: "mail" # objectClass: inetOrgPerson + phone: "telephoneNumber" # objectClass: person + user_id: "uid" + status: + name: "shadowFlag" # objectClass: shadowAccount + is_binary: false + disable_bitmasks: [0x2, 0x10] + tls: + client_key: ./tests/environment/certs/client.key + client_certificate: ./tests/environment/certs/client.crt + server_certificate: ./tests/environment/certs/server.crt + danger_disable_tls_verify: false + danger_use_start_tls: false diff --git a/tests/environment/ukt-config.template.yaml b/tests/environment/ukt-config.template.yaml new file mode 100644 index 0000000..5b90c93 --- /dev/null +++ b/tests/environment/ukt-config.template.yaml @@ -0,0 +1,8 @@ +sources: + ukt: + endpoint_url: https://list.example.invalid/usersync4chat/maillist + oauth2_url: https://list.example.invalid/token + client_id: mock_client_id + client_secret: mock_client_secret + scope: "openid read-maillist" + grant_type: client_credentials From 1a6cb1ae086f672ef75c62a3e304fec9abb57649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Thu, 31 Oct 2024 14:38:48 +0100 Subject: [PATCH 04/11] WIP: test: Update Zitadel version for test env --- tests/environment/docker-compose.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/environment/docker-compose.yaml b/tests/environment/docker-compose.yaml index e2f1dd5..cf6b463 100644 --- a/tests/environment/docker-compose.yaml +++ b/tests/environment/docker-compose.yaml @@ -42,7 +42,7 @@ services: condition: 'service_healthy' zitadel: - image: ghcr.io/zitadel/zitadel:v2.58.3 + image: ghcr.io/zitadel/zitadel:v2.64.1 command: start-from-init --masterkey "MasterkeyNeedsToHave32Characters" --tlsMode disabled --config /zitadel-config/zitadel-config.yaml --steps /zitadel-config/zitadel-init.yaml ports: - 8080:8080 From 49684cc3298ac722e4fe4b76635514c39834b312 Mon Sep 17 00:00:00 2001 From: Jan Cibulka Date: Mon, 4 Nov 2024 12:37:30 +0200 Subject: [PATCH 05/11] test: Support single-source sync --- config.sample.yaml | 10 +- tests/e2e.rs | 111 ++++++++------------ tests/environment/config.template.yaml | 3 + tests/environment/csv-config.template.yaml | 9 +- tests/environment/ldap-config.template.yaml | 52 +++++---- tests/environment/ukt-config.template.yaml | 14 ++- 6 files changed, 92 insertions(+), 107 deletions(-) diff --git a/config.sample.yaml b/config.sample.yaml index c478477..a4a6a9d 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -19,6 +19,7 @@ feature_flags: # - deactivate_only # Only deactivate users, do not create or update them. Keep in mind LDAP is cached and all the changes made on LDAP will be written to the cache as if they where applied. Therefore, only the deactivation changes will be applied to Zitadel but **all the other changes will be lost**. # Configuration for the sources to sync from. +# IMPORTANT: Only one source can be present at a time. sources: # Configuration for the LDAP source. Using caching, LDAP source checks for new, updated, and deleted users in the LDAP server. ldap: @@ -103,10 +104,11 @@ sources: # Grant type grant_type: client_credentials - # Configuration for the CSV source - reads a CSV file - # and creates **new** users in Famedly's Zitadel. - # Expected structure of the CSV file is as follows: - # email,first_name,last_name,phone + # Configuration for the CSV sources + # Updates Zitadel to match the CSV file. + #! DANGER: This will delete all users that are not in the CSV file! csv: # Path to the CSV file to read from. + # Expected structure of the CSV file is as follows: + # email,first_name,last_name,phone file_path: ./tests/environment/files/test-users.csv diff --git a/tests/e2e.rs b/tests/e2e.rs index 2ad676e..7dcd576 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -9,11 +9,9 @@ use famedly_sync::{ ukt_test_helpers::{ get_mock_server_url, prepare_endpoint_mock, prepare_oauth2_mock, ENDPOINT_PATH, OAUTH2_PATH, }, - AttributeMapping, Config, FeatureFlag, LdapSourceConfig, + AttributeMapping, Config, FeatureFlag, }; use ldap3::{Ldap as LdapClient, LdapConnAsync, LdapConnSettings, Mod}; -use serde::{de::IntoDeserializer, Deserialize}; -use tempfile::TempDir; use test_log::test; use tokio::sync::OnceCell; use url::Url; @@ -24,8 +22,9 @@ use zitadel_rust_client::v1::{ Email, Gender, ImportHumanUserRequest, Phone, Profile, UserType, Zitadel, }; -static CONFIG: OnceCell = OnceCell::const_new(); -static TEMPDIR: OnceCell = OnceCell::const_new(); +static CONFIG_WITH_LDAP: OnceCell = OnceCell::const_new(); +static CONFIG_WITH_CSV: OnceCell = OnceCell::const_new(); +static CONFIG_WITH_UKT: OnceCell = OnceCell::const_new(); /// The Famedly UUID namespace to use to generate v5 UUIDs. const FAMEDLY_NAMESPACE: Uuid = uuid!("d9979cff-abee-4666-bc88-1ec45a843fb8"); @@ -790,7 +789,7 @@ async fn test_e2e_ukt_sync() { prepare_oauth2_mock(&mock_server).await; prepare_endpoint_mock(&mock_server, "delete_me@famedly.de").await; - let mut config = ldap_config().await.clone(); + let mut config = ukt_config().await.clone(); config .sources @@ -847,7 +846,7 @@ async fn test_e2e_ukt_sync() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_csv_sync() { - let mut config = ldap_config().await.clone(); + let mut config = csv_config().await.clone(); perform_sync(&config).await.expect("syncing failed"); @@ -905,7 +904,7 @@ async fn test_e2e_csv_sync() { let grant = grants.result.first().expect("no user grants found"); assert!(grant.role_keys.clone().into_iter().any(|key| key == FAMEDLY_USER_ROLE)); - // Not possible to re-import an existing user (as checked by unique email) + // Re-import an existing user to update (as checked by unique email) let csv_content = indoc::indoc! {r#" email,first_name,last_name,phone john.doe@example.com,Changed_Name,Changed_Surname,+2222222222 @@ -925,10 +924,10 @@ async fn test_e2e_csv_sync() { let phone = user.phone.expect("user lacks a phone number"); let email = user.email.expect("user lacks an email address"); - assert_eq!(profile.first_name, "John"); - assert_eq!(profile.last_name, "Doe"); - assert_eq!(profile.display_name, "Doe, John"); - assert_eq!(phone.phone, "+1111111111"); + assert_eq!(profile.first_name, "Changed_Name"); + assert_eq!(profile.last_name, "Changed_Surname"); + assert_eq!(profile.display_name, "Changed_Surname, Changed_Name"); + assert_eq!(phone.phone, "+2222222222"); assert!(phone.is_phone_verified); assert_eq!(email.email, "john.doe@example.com"); assert!(email.is_email_verified); @@ -939,23 +938,12 @@ async fn test_e2e_csv_sync() { #[test(tokio::test)] #[test_log(default_log_filter = "debug")] -async fn test_e2e_full_sync() { +async fn test_e2e_ldap_with_ukt_sync() { let mock_server = MockServer::start().await; prepare_oauth2_mock(&mock_server).await; prepare_endpoint_mock(&mock_server, "not_to_be_there@famedly.de").await; - let mut config = ldap_config().await.clone(); - config - .sources - .ukt - .as_mut() - .map(|ukt| { - ukt.oauth2_url = get_mock_server_url(&mock_server, OAUTH2_PATH) - .expect("Failed to get mock server URL"); - ukt.endpoint_url = get_mock_server_url(&mock_server, ENDPOINT_PATH) - .expect("Failed to get mock server URL"); - }) - .expect("UKT configuration is missing"); + // LDAP SYNC let mut ldap = Ldap::new().await; ldap.create_user( @@ -1002,44 +990,34 @@ async fn test_e2e_full_sync() { ) .await; - let csv_content = indoc::indoc! {r#" - email,first_name,last_name,phone - csv_sync@example.com,John,Doe,+1111111111 - "#}; - // Have to create a new temp file - // because we can't re-use users between tests - // and the ./tests/environment/files/test-users.csv was already imported - let _file = temp_csv_file(&mut config, csv_content); + let ldap_config = ldap_config().await.clone(); + perform_sync(&ldap_config).await.expect("syncing failed"); - perform_sync(&config).await.expect("syncing failed"); + // UKT SYNC - let zitadel = open_zitadel_connection().await; + let mut ukt_config = ukt_config().await.clone(); + ukt_config + .sources + .ukt + .as_mut() + .map(|ukt| { + ukt.oauth2_url = get_mock_server_url(&mock_server, OAUTH2_PATH) + .expect("Failed to get mock server URL"); + ukt.endpoint_url = get_mock_server_url(&mock_server, ENDPOINT_PATH) + .expect("Failed to get mock server URL"); + }) + .expect("UKT configuration is missing"); - let user = zitadel - .get_user_by_login_name("csv_sync@example.com") - .await - .expect("could not query Zitadel users"); - assert!(user.is_some()); - let user = user.expect("could not find user"); - assert_eq!(user.user_name, "csv_sync@example.com"); - if let Some(UserType::Human(user)) = user.r#type { - let profile = user.profile.expect("user lacks a profile"); - let phone = user.phone.expect("user lacks a phone number"); - let email = user.email.expect("user lacks an email address"); + perform_sync(&ukt_config).await.expect("syncing failed"); - assert_eq!(profile.first_name, "John"); - assert_eq!(profile.last_name, "Doe"); - assert_eq!(profile.display_name, "Doe, John"); - assert_eq!(phone.phone, "+1111111111"); - assert!(phone.is_phone_verified); - assert_eq!(email.email, "csv_sync@example.com"); - assert!(email.is_email_verified); - } else { - panic!("user lacks details"); - } + // VERIFY RESULTS OF SYNC + + let zitadel = open_zitadel_connection().await; let user = zitadel.get_user_by_login_name("not_to_be_there@famedly.de").await; - assert!(user.is_err_and(|error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound))); + assert!(user.is_err_and(|error| matches!(error, + ZitadelError::TonicResponseError(status) if status.code() == + TonicErrorCode::NotFound))); let user = zitadel .get_user_by_login_name("to_be_there@famedly.de") @@ -1066,11 +1044,15 @@ async fn test_e2e_full_sync() { _ => panic!("human user became a machine user?"), } + // UPDATES IN LDAP + ldap.change_user("to_be_changed", vec![("telephoneNumber", HashSet::from(["+12015550123"]))]) .await; ldap.delete_user("not_to_be_there_later").await; - perform_sync(&config).await.expect("syncing failed"); + perform_sync(&ldap_config).await.expect("syncing failed"); + + // VERIFY SECOND LDAP SYNC let user = zitadel .get_user_by_login_name("to_be_changed@famedly.de") @@ -1084,7 +1066,6 @@ async fn test_e2e_full_sync() { } _ => panic!("human user became a machine user?"), } - let user = zitadel.get_user_by_login_name("not_to_be_there_later@famedly.de").await; assert!(user.is_err_and(|error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound))); } @@ -1225,12 +1206,12 @@ async fn open_zitadel_connection() -> Zitadel { /// Get the module's test environment config async fn ldap_config() -> &'static Config { - CONFIG + CONFIG_WITH_LDAP .get_or_init(|| async { let mut config = Config::new(Path::new("tests/environment/config.yaml")) .expect("failed to parse test env file"); - config.sources.ldap = serde_json::from_slice( + config.sources.ldap = serde_yaml::from_slice( &std::fs::read(Path::new("tests/environment/ldap-config.template.yaml")) .expect("failed to read ldap config file"), ) @@ -1243,12 +1224,12 @@ async fn ldap_config() -> &'static Config { /// Get the module's test environment config async fn ukt_config() -> &'static Config { - CONFIG + CONFIG_WITH_UKT .get_or_init(|| async { let mut config = Config::new(Path::new("tests/environment/config.yaml")) .expect("failed to parse test env file"); - config.sources.ldap = serde_json::from_slice( + config.sources.ukt = serde_yaml::from_slice( &std::fs::read(Path::new("tests/environment/ukt-config.template.yaml")) .expect("failed to read ukt config file"), ) @@ -1261,12 +1242,12 @@ async fn ukt_config() -> &'static Config { /// Get the module's test environment config async fn csv_config() -> &'static Config { - CONFIG + CONFIG_WITH_CSV .get_or_init(|| async { let mut config = Config::new(Path::new("tests/environment/config.yaml")) .expect("failed to parse test env file"); - config.sources.ldap = serde_json::from_slice( + config.sources.csv = serde_yaml::from_slice( &std::fs::read(Path::new("tests/environment/csv-config.template.yaml")) .expect("failed to read csv config file"), ) diff --git a/tests/environment/config.template.yaml b/tests/environment/config.template.yaml index 7e4c1aa..5f92a36 100644 --- a/tests/environment/config.template.yaml +++ b/tests/environment/config.template.yaml @@ -7,3 +7,6 @@ zitadel: feature_flags: - sso_login + +sources: + test: 1 diff --git a/tests/environment/csv-config.template.yaml b/tests/environment/csv-config.template.yaml index 087363e..5766673 100644 --- a/tests/environment/csv-config.template.yaml +++ b/tests/environment/csv-config.template.yaml @@ -1,3 +1,6 @@ -sources: - csv: - file_path: ./tests/environment/files/test-users.csv +# Updates Zitadel to match the CSV file. +#! DANGER: This will delete all users that are not in the CSV file! + +# Expected structure of the CSV file is as follows: +# email,first_name,last_name,phone +file_path: ./tests/environment/files/test-users.csv diff --git a/tests/environment/ldap-config.template.yaml b/tests/environment/ldap-config.template.yaml index 07942fa..950d747 100644 --- a/tests/environment/ldap-config.template.yaml +++ b/tests/environment/ldap-config.template.yaml @@ -1,27 +1,25 @@ -sources: - ldap: - url: ldap://localhost:1389 - base_dn: ou=testorg,dc=example,dc=org - bind_dn: cn=admin,dc=example,dc=org - bind_password: adminpassword - user_filter: "(objectClass=shadowAccount)" - timeout: 5 - check_for_deleted_entries: true - use_attribute_filter: true - attributes: - first_name: "cn" # objectClass: person - last_name: "sn" # objectClass: person - preferred_username: "displayName" # objectClass: inetOrgPerson - email: "mail" # objectClass: inetOrgPerson - phone: "telephoneNumber" # objectClass: person - user_id: "uid" - status: - name: "shadowFlag" # objectClass: shadowAccount - is_binary: false - disable_bitmasks: [0x2, 0x10] - tls: - client_key: ./tests/environment/certs/client.key - client_certificate: ./tests/environment/certs/client.crt - server_certificate: ./tests/environment/certs/server.crt - danger_disable_tls_verify: false - danger_use_start_tls: false +url: ldap://localhost:1389 +base_dn: ou=testorg,dc=example,dc=org +bind_dn: cn=admin,dc=example,dc=org +bind_password: adminpassword +user_filter: "(objectClass=shadowAccount)" +timeout: 5 +check_for_deleted_entries: true +use_attribute_filter: true +attributes: + first_name: "cn" # objectClass: person + last_name: "sn" # objectClass: person + preferred_username: "displayName" # objectClass: inetOrgPerson + email: "mail" # objectClass: inetOrgPerson + phone: "telephoneNumber" # objectClass: person + user_id: "uid" + status: + name: "shadowFlag" # objectClass: shadowAccount + is_binary: false + disable_bitmasks: [0x2, 0x10] +tls: + client_key: ./tests/environment/certs/client.key + client_certificate: ./tests/environment/certs/client.crt + server_certificate: ./tests/environment/certs/server.crt + danger_disable_tls_verify: false + danger_use_start_tls: false diff --git a/tests/environment/ukt-config.template.yaml b/tests/environment/ukt-config.template.yaml index 5b90c93..7157db4 100644 --- a/tests/environment/ukt-config.template.yaml +++ b/tests/environment/ukt-config.template.yaml @@ -1,8 +1,6 @@ -sources: - ukt: - endpoint_url: https://list.example.invalid/usersync4chat/maillist - oauth2_url: https://list.example.invalid/token - client_id: mock_client_id - client_secret: mock_client_secret - scope: "openid read-maillist" - grant_type: client_credentials +endpoint_url: https://list.example.invalid/usersync4chat/maillist +oauth2_url: https://list.example.invalid/token +client_id: mock_client_id +client_secret: mock_client_secret +scope: "openid read-maillist" +grant_type: client_credentials From 93abd63cd4bebdf93f8814a1511c557c1a9f5047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Mon, 4 Nov 2024 16:11:05 +0100 Subject: [PATCH 06/11] doc: Split apart sample configurations --- sample-configs/csv-config.sample.yaml | 30 +++++++++++++++ .../ldap-config.sample.yaml | 27 +------------- sample-configs/ukt-config.sample.yaml | 37 +++++++++++++++++++ src/config.rs | 7 +++- 4 files changed, 73 insertions(+), 28 deletions(-) create mode 100644 sample-configs/csv-config.sample.yaml rename config.sample.yaml => sample-configs/ldap-config.sample.yaml (77%) create mode 100644 sample-configs/ukt-config.sample.yaml diff --git a/sample-configs/csv-config.sample.yaml b/sample-configs/csv-config.sample.yaml new file mode 100644 index 0000000..bbba672 --- /dev/null +++ b/sample-configs/csv-config.sample.yaml @@ -0,0 +1,30 @@ +# Configuration for Famedly's Zitadel - has to be provided by Famedly +zitadel: + # The Famedly user endpoint to sync to. + url: https://auth.famedly.de + # The Famedly-provided service user credentials. + key_file: /opt/famedly-sync-agent/service-user.json + # The organization whose users to sync. + organization_id: 278274756195721220 + # The project to grant users access to. + project_id: 278274945274880004 + # The identity provider ID to enable SSO login for + idp_id: 281430143275106308 + +feature_flags: + - verify_email # Whether to ask users to verify their email addresses post sync + - verify_phone # Whether to ask users to verify their phone numbers post sync + # - sso_login # Whether to enable SSO login - Please note that his has some drawbacks and limitations, see the help center article for more information + # - dry_run # Disable syncing users to Zitadel - Intended to ensure syncs are working before productive deployment + # - deactivate_only # Only deactivate users, do not create or update them. + +# Configuration for the sources to sync from. +sources: + # Configuration for the CSV sources + # Updates Zitadel to match the CSV file. + #! DANGER: This will delete all users that are not in the CSV file! + csv: + # Path to the CSV file to read from. + # Expected structure of the CSV file is as follows: + # email,first_name,last_name,phone + file_path: ./tests/environment/files/test-users.csv diff --git a/config.sample.yaml b/sample-configs/ldap-config.sample.yaml similarity index 77% rename from config.sample.yaml rename to sample-configs/ldap-config.sample.yaml index a4a6a9d..679ccfb 100644 --- a/config.sample.yaml +++ b/sample-configs/ldap-config.sample.yaml @@ -16,10 +16,9 @@ feature_flags: - verify_phone # Whether to ask users to verify their phone numbers post sync # - sso_login # Whether to enable SSO login - Please note that his has some drawbacks and limitations, see the help center article for more information # - dry_run # Disable syncing users to Zitadel - Intended to ensure syncs are working before productive deployment - # - deactivate_only # Only deactivate users, do not create or update them. Keep in mind LDAP is cached and all the changes made on LDAP will be written to the cache as if they where applied. Therefore, only the deactivation changes will be applied to Zitadel but **all the other changes will be lost**. + # - deactivate_only # Only deactivate users, do not create or update them. # Configuration for the sources to sync from. -# IMPORTANT: Only one source can be present at a time. sources: # Configuration for the LDAP source. Using caching, LDAP source checks for new, updated, and deleted users in the LDAP server. ldap: @@ -88,27 +87,3 @@ sources: # needed with the `ldaps` scheme, as the server will already be # hosting TLS. danger_use_start_tls: false - # Configuration for the UKT source - a custom endpoint provided by UKT, - # which gives a list of emails of users that should be deleted from Zitadel. - ukt: - # Endpoint URL to fetch the list of users from. - endpoint_url: https://list.example.invalid/usersync4chat/maillist - # OAuth2 URL to fetch the token from. - oauth2_url: https://list.example.invalid/token - # Client ID - client_id: mock_client_id - # Client Secret - client_secret: mock_client_secret - # Scope of what to fetch - scope: "openid read-maillist" - # Grant type - grant_type: client_credentials - - # Configuration for the CSV sources - # Updates Zitadel to match the CSV file. - #! DANGER: This will delete all users that are not in the CSV file! - csv: - # Path to the CSV file to read from. - # Expected structure of the CSV file is as follows: - # email,first_name,last_name,phone - file_path: ./tests/environment/files/test-users.csv diff --git a/sample-configs/ukt-config.sample.yaml b/sample-configs/ukt-config.sample.yaml new file mode 100644 index 0000000..64458ca --- /dev/null +++ b/sample-configs/ukt-config.sample.yaml @@ -0,0 +1,37 @@ +# Configuration for Famedly's Zitadel - has to be provided by Famedly +zitadel: + # The Famedly user endpoint to sync to. + url: https://auth.famedly.de + # The Famedly-provided service user credentials. + key_file: /opt/famedly-sync-agent/service-user.json + # The organization whose users to sync. + organization_id: 278274756195721220 + # The project to grant users access to. + project_id: 278274945274880004 + # The identity provider ID to enable SSO login for + idp_id: 281430143275106308 + +feature_flags: + - verify_email # Whether to ask users to verify their email addresses post sync + - verify_phone # Whether to ask users to verify their phone numbers post sync + # - sso_login # Whether to enable SSO login - Please note that his has some drawbacks and limitations, see the help center article for more information + # - dry_run # Disable syncing users to Zitadel - Intended to ensure syncs are working before productive deployment + # - deactivate_only # Only deactivate users, do not create or update them. + +# Configuration for the sources to sync from. +sources: + # Configuration for the UKT source - a custom endpoint provided by UKT, + # which gives a list of emails of users that should be deleted from Zitadel. + ukt: + # Endpoint URL to fetch the list of users from. + endpoint_url: https://list.example.invalid/usersync4chat/maillist + # OAuth2 URL to fetch the token from. + oauth2_url: https://list.example.invalid/token + # Client ID + client_id: mock_client_id + # Client Secret + client_secret: mock_client_secret + # Scope of what to fetch + scope: "openid read-maillist" + # Grant type + grant_type: client_credentials diff --git a/src/config.rs b/src/config.rs index 90ff538..7791779 100644 --- a/src/config.rs +++ b/src/config.rs @@ -235,8 +235,11 @@ mod tests { #[tokio::test] async fn test_sample_config() { - let config = Config::new(Path::new("./config.sample.yaml")); - + let config = Config::new(Path::new("./sample-configs/csv-config.sample.yaml")); + assert!(config.is_ok(), "Invalid config: {:?}", config); + let config = Config::new(Path::new("./sample-configs/ldap-config.sample.yaml")); + assert!(config.is_ok(), "Invalid config: {:?}", config); + let config = Config::new(Path::new("./sample-configs/ukt-config.sample.yaml")); assert!(config.is_ok(), "Invalid config: {:?}", config); } From 9da33c8670ec8529972bba075083a18fd40a3b79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Mon, 4 Nov 2024 16:28:51 +0100 Subject: [PATCH 07/11] chore: Remove bincode dependency --- Cargo.lock | 110 ++++++++++++++++++++++++++++++++++++----------------- Cargo.toml | 1 - 2 files changed, 76 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f913e55..edc8955 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -244,15 +244,6 @@ version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8c3c1a368f70d6cf7302d78f8f7093da241fb8e8807c05cc9e51a125895a6d5b" -[[package]] -name = "bincode" -version = "1.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" -dependencies = [ - "serde", -] - [[package]] name = "bitflags" version = "1.3.2" @@ -310,6 +301,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "cfg_aliases" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" + [[package]] name = "chrono" version = "0.4.38" @@ -721,7 +718,6 @@ dependencies = [ "anyhow", "async-trait", "base64 0.22.1", - "bincode", "chrono", "config", "csv", @@ -731,7 +727,7 @@ dependencies = [ "ldap-poller", "ldap3", "native-tls", - "reqwest 0.11.27", + "reqwest 0.12.8", "serde", "serde_json", "serde_yaml", @@ -1233,6 +1229,7 @@ dependencies = [ "hyper 1.4.1", "hyper-util", "rustls 0.23.14", + "rustls-native-certs 0.8.0", "rustls-pki-types", "tokio", "tokio-rustls 0.26.0", @@ -1251,19 +1248,6 @@ dependencies = [ "tokio-io-timeout", ] -[[package]] -name = "hyper-tls" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6183ddfa99b85da61a140bea0efc93fdf56ceaa041b37d553518030827f9905" -dependencies = [ - "bytes", - "hyper 0.14.30", - "native-tls", - "tokio", - "tokio-native-tls", -] - [[package]] name = "hyper-tls" version = "0.6.0" @@ -2195,6 +2179,55 @@ dependencies = [ "ipnet", ] +[[package]] +name = "quinn" +version = "0.11.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8c7c5fdde3cdae7203427dc4f0a68fe0ed09833edc525a03456b153b79828684" +dependencies = [ + "bytes", + "pin-project-lite", + "quinn-proto", + "quinn-udp", + "rustc-hash", + "rustls 0.23.14", + "socket2", + "thiserror", + "tokio", + "tracing", +] + +[[package]] +name = "quinn-proto" +version = "0.11.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fadfaed2cd7f389d0161bb73eeb07b7b78f8691047a6f3e73caaeae55310a4a6" +dependencies = [ + "bytes", + "rand", + "ring 0.17.8", + "rustc-hash", + "rustls 0.23.14", + "slab", + "thiserror", + "tinyvec", + "tracing", +] + +[[package]] +name = "quinn-udp" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e346e016eacfff12233c243718197ca12f148c84e1e84268a896699b41c71780" +dependencies = [ + "cfg_aliases", + "libc", + "once_cell", + "socket2", + "tracing", + "windows-sys 0.59.0", +] + [[package]] name = "quote" version = "1.0.37" @@ -2303,17 +2336,14 @@ dependencies = [ "http-body 0.4.6", "hyper 0.14.30", "hyper-rustls 0.24.2", - "hyper-tls 0.5.0", "ipnet", "js-sys", "log", "mime", - "native-tls", "once_cell", "percent-encoding", "pin-project-lite", "rustls 0.21.12", - "rustls-native-certs 0.6.3", "rustls-pemfile 1.0.4", "serde", "serde_json", @@ -2321,7 +2351,6 @@ dependencies = [ "sync_wrapper 0.1.2", "system-configuration 0.5.1", "tokio", - "tokio-native-tls", "tokio-rustls 0.24.1", "tower-service", "url", @@ -2349,7 +2378,7 @@ dependencies = [ "http-body-util", "hyper 1.4.1", "hyper-rustls 0.27.3", - "hyper-tls 0.6.0", + "hyper-tls", "hyper-util", "ipnet", "js-sys", @@ -2359,7 +2388,11 @@ dependencies = [ "once_cell", "percent-encoding", "pin-project-lite", + "quinn", + "rustls 0.23.14", + "rustls-native-certs 0.8.0", "rustls-pemfile 2.2.0", + "rustls-pki-types", "serde", "serde_json", "serde_urlencoded", @@ -2367,6 +2400,7 @@ dependencies = [ "system-configuration 0.6.1", "tokio", "tokio-native-tls", + "tokio-rustls 0.26.0", "tower-service", "url", "wasm-bindgen", @@ -2463,6 +2497,12 @@ version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" +[[package]] +name = "rustc-hash" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "583034fd73374156e66797ed8e5b0d5690409c9226b22d87cb7f19821c05d152" + [[package]] name = "rustc_version" version = "0.4.1" @@ -2531,6 +2571,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "415d9944693cb90382053259f89fbb077ea730ad7273047ec63b19bc9b160ba8" dependencies = [ "once_cell", + "ring 0.17.8", "rustls-pki-types", "rustls-webpki 0.102.8", "subtle", @@ -2551,21 +2592,22 @@ dependencies = [ [[package]] name = "rustls-native-certs" -version = "0.6.3" +version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9aace74cb666635c918e9c12bc0d348266037aa8eb599b5cba565709a8dff00" +checksum = "e5bfb394eeed242e909609f56089eecfe5fda225042e8b171791b9c95f5931e5" dependencies = [ "openssl-probe", - "rustls-pemfile 1.0.4", + "rustls-pemfile 2.2.0", + "rustls-pki-types", "schannel", "security-framework", ] [[package]] name = "rustls-native-certs" -version = "0.7.3" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5bfb394eeed242e909609f56089eecfe5fda225042e8b171791b9c95f5931e5" +checksum = "fcaf18a4f2be7326cd874a5fa579fae794320a0f388d365dca7e480e55f83f8a" dependencies = [ "openssl-probe", "rustls-pemfile 2.2.0", diff --git a/Cargo.toml b/Cargo.toml index 9071b9d..7b0bcd9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,6 @@ publish = false anyhow = { version = "1.0.81", features = ["backtrace"] } async-trait = "0.1.82" base64 = "0.22.1" -bincode = "1.3.3" chrono = "0.4.19" config = { version = "0.14.0" } http = "1.1.0" From da8973180a90428e27e94ebea97ca8d50e1dc877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Tue, 5 Nov 2024 15:20:58 +0100 Subject: [PATCH 08/11] refactor: Move uuid method to user struct impl --- src/user.rs | 12 ++++++++++++ src/zitadel.rs | 10 ++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/user.rs b/src/user.rs index c1adc5a..5adca23 100644 --- a/src/user.rs +++ b/src/user.rs @@ -3,8 +3,12 @@ use std::fmt::Display; use anyhow::{anyhow, Result}; use base64::prelude::{Engine, BASE64_STANDARD}; +use uuid::{uuid, Uuid}; use zitadel_rust_client::v2::users::HumanUser; +/// The Famedly UUID namespace to use to generate v5 UUIDs. +const FAMEDLY_NAMESPACE: Uuid = uuid!("d9979cff-abee-4666-bc88-1ec45a843fb8"); + /// Source-agnostic representation of a user #[derive(Clone)] pub(crate) struct User { @@ -67,6 +71,14 @@ impl User { pub fn get_display_name(&self) -> String { format!("{}, {}", self.last_name, self.first_name) } + + /// Return the user's UUID according to the Famedly UUID spec. + /// + /// See + /// https://www.notion.so/famedly/Famedly-UUID-Specification-adc576f0f2d449bba2f6f13b2611738f + pub fn famedly_uuid(&self) -> String { + Uuid::new_v5(&FAMEDLY_NAMESPACE, self.external_user_id.as_bytes()).to_string() + } } impl PartialEq for User { diff --git a/src/zitadel.rs b/src/zitadel.rs index 54b02b9..eaccd28 100644 --- a/src/zitadel.rs +++ b/src/zitadel.rs @@ -5,7 +5,6 @@ use anyhow::{anyhow, Context, Result}; use futures::{Stream, StreamExt}; use serde::Deserialize; use url::Url; -use uuid::{uuid, Uuid}; use zitadel_rust_client::{ v1::Zitadel as ZitadelClientV1, v2::{ @@ -24,9 +23,6 @@ use crate::{ FeatureFlag, }; -/// The Famedly UUID namespace to use to generate v5 UUIDs. -const FAMEDLY_NAMESPACE: Uuid = uuid!("d9979cff-abee-4666-bc88-1ec45a843fb8"); - /// The Zitadel project role to assign to users. const FAMEDLY_USER_ROLE: &str = "User"; @@ -130,10 +126,8 @@ impl Zitadel { return Ok(()); } - let mut metadata = vec![SetMetadataEntry::new( - "localpart".to_owned(), - Uuid::new_v5(&FAMEDLY_NAMESPACE, imported_user.external_user_id.as_bytes()).to_string(), - )]; + let mut metadata = + vec![SetMetadataEntry::new("localpart".to_owned(), imported_user.famedly_uuid())]; if let Some(preferred_username) = imported_user.preferred_username.clone() { metadata.push(SetMetadataEntry::new( From 25fb716772aba3d9883c24c996072751d0c5f67d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Thu, 7 Nov 2024 12:07:13 +0100 Subject: [PATCH 09/11] WIP: fix!: Rework handling of binary fields from LDAP --- src/lib.rs | 7 +- src/sources/csv.rs | 42 +++----- src/sources/ldap.rs | 98 +++++++++++------ src/user.rs | 147 ++------------------------ src/zitadel.rs | 35 +++--- tests/e2e.rs | 252 ++++++++++++++++++++++---------------------- 6 files changed, 233 insertions(+), 348 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1c48917..939b603 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,7 @@ //! Sync tool between other sources and our infrastructure based on Zitadel. use anyhow::{Context, Result}; use futures::{Stream, StreamExt}; -use user::{StringOrBytes, User}; +use user::User; use zitadel::Zitadel; mod config; @@ -27,13 +27,12 @@ async fn get_next_zitadel_user( ) -> Result> { match stream.next().await.transpose()? { Some(mut zitadel_user) => { - let preferred_username: Option = zitadel + let preferred_username = zitadel .zitadel_client .get_user_metadata(&zitadel_user.1, "preferred_username") .await .ok() - .and_then(|metadata| metadata.metadata().value()) - .map(Into::into); + .and_then(|metadata| metadata.metadata().value()); zitadel_user.0.preferred_username = preferred_username; diff --git a/src/sources/csv.rs b/src/sources/csv.rs index 697bf67..61d85de 100644 --- a/src/sources/csv.rs +++ b/src/sources/csv.rs @@ -74,12 +74,12 @@ impl CsvData { /// Convert CsvData to User data fn to_user(csv_data: CsvData) -> User { User { - email: csv_data.email.clone().into(), - first_name: csv_data.first_name.into(), - last_name: csv_data.last_name.into(), - phone: if csv_data.phone.is_empty() { None } else { Some(csv_data.phone.into()) }, - preferred_username: Some(csv_data.email.clone().into()), - external_user_id: csv_data.email.into(), + email: csv_data.email.clone(), + first_name: csv_data.first_name, + last_name: csv_data.last_name, + phone: if csv_data.phone.is_empty() { None } else { Some(csv_data.phone) }, + preferred_username: Some(csv_data.email.clone()), + external_user_id: csv_data.email, enabled: true, } } @@ -114,7 +114,7 @@ mod tests { use indoc::indoc; use super::*; - use crate::{user::StringOrBytes, Config}; + use crate::Config; const EXAMPLE_CONFIG: &str = indoc! {r#" zitadel: @@ -155,27 +155,11 @@ mod tests { let users = result.expect("Failed to get users"); assert_eq!(users.len(), 4, "Unexpected number of users"); - assert_eq!( - users[0].first_name, - StringOrBytes::String("John".to_owned()), - "Unexpected first name at index 0" - ); - assert_eq!( - users[0].email, - StringOrBytes::String("john.doe@example.com".to_owned()), - "Unexpected email at index 0" - ); - assert_eq!( - users[3].last_name, - StringOrBytes::String("Williams".to_owned()), - "Unexpected last name at index 3" - ); + assert_eq!(users[0].first_name, "John", "Unexpected first name at index 0"); + assert_eq!(users[0].email, "john.doe@example.com", "Unexpected email at index 0"); + assert_eq!(users[3].last_name, "Williams", "Unexpected last name at index 3"); assert_eq!(users[2].phone, None, "Unexpected phone at index 2"); - assert_eq!( - users[3].phone, - Some(StringOrBytes::String("+4444444444".to_owned())), - "Unexpected phone at index 3" - ); + assert_eq!(users[3].phone, Some("+4444444444".to_owned()), "Unexpected phone at index 3"); } #[test] @@ -252,12 +236,12 @@ mod tests { assert_eq!(users.len(), 1, "Unexpected number of users"); assert_eq!( users[0].email, - StringOrBytes::String("jane.smith@example.com".to_owned()), + "jane.smith@example.com", "Unexpected email at index 0" ); assert_eq!( users[0].last_name, - StringOrBytes::String("Smith".to_owned()), + "Smith", "Unexpected last name at index 0" ); } diff --git a/src/sources/ldap.rs b/src/sources/ldap.rs index 1b3ca7d..3de0505 100644 --- a/src/sources/ldap.rs +++ b/src/sources/ldap.rs @@ -2,8 +2,9 @@ use std::{fmt::Display, path::PathBuf}; -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; +use base64::prelude::{Engine, BASE64_STANDARD}; use ldap_poller::{ config::TLSConfig, ldap::EntryStatus, ldap3::SearchEntry, AttributeConfig, CacheMethod, ConnectionConfig, Ldap, SearchEntryExt, Searches, @@ -14,7 +15,7 @@ use tokio_stream::{wrappers::ReceiverStream, StreamExt}; use url::Url; use super::Source; -use crate::user::{StringOrBytes, User}; +use crate::user::User; /// LDAP sync source pub struct LdapSource { @@ -88,13 +89,24 @@ impl LdapSource { .iter() .any(|flag| status_as_int & flag != 0); - let first_name = read_search_entry(&entry, &self.ldap_config.attributes.first_name)?; - let last_name = read_search_entry(&entry, &self.ldap_config.attributes.last_name)?; - let preferred_username = - read_search_entry(&entry, &self.ldap_config.attributes.preferred_username)?; - let email = read_search_entry(&entry, &self.ldap_config.attributes.email)?; - let ldap_user_id = read_search_entry(&entry, &self.ldap_config.attributes.user_id)?; - let phone = read_search_entry(&entry, &self.ldap_config.attributes.phone).ok(); + let ldap_user_id = match read_search_entry(&entry, &self.ldap_config.attributes.user_id)? { + // TODO(tlater): Use an encoding that preserves alphabetic order + StringOrBytes::Bytes(byte_id) => BASE64_STANDARD.encode(byte_id), + StringOrBytes::String(string_id) => BASE64_STANDARD.encode(string_id), + }; + + let first_name = + read_string_entry(&entry, &self.ldap_config.attributes.first_name, &ldap_user_id)?; + let last_name = + read_string_entry(&entry, &self.ldap_config.attributes.last_name, &ldap_user_id)?; + let preferred_username = read_string_entry( + &entry, + &self.ldap_config.attributes.preferred_username, + &ldap_user_id, + )?; + let email = read_string_entry(&entry, &self.ldap_config.attributes.email, &ldap_user_id)?; + let phone = + read_string_entry(&entry, &self.ldap_config.attributes.phone, &ldap_user_id).ok(); Ok(User { first_name, @@ -108,29 +120,39 @@ impl LdapSource { } } +/// Read an an attribute, but assert that it is a string +fn read_string_entry( + entry: &SearchEntry, + attribute: &AttributeMapping, + id: &str, +) -> Result { + match read_search_entry(entry, attribute)? { + StringOrBytes::String(entry) => Ok(entry), + StringOrBytes::Bytes(_) => { + Err(anyhow!("Unacceptable binary value for {} of user `{}`", attribute, id)) + } + } +} + /// Read an attribute from the entry fn read_search_entry(entry: &SearchEntry, attribute: &AttributeMapping) -> Result { match attribute { AttributeMapping::OptionalBinary { name, is_binary: false } | AttributeMapping::NoBinaryOption(name) => { - if let Some(attr) = entry.attr_first(name) { - return Ok(StringOrBytes::String(attr.to_owned())); - }; - } - AttributeMapping::OptionalBinary { name, is_binary: true } => { - if let Some(binary_attr) = entry.bin_attr_first(name) { - return Ok(StringOrBytes::Bytes(binary_attr.to_vec())); - }; - - // If attributes encode as valid UTF-8, they will - // not be in the bin_attr list - if let Some(attr) = entry.attr_first(name) { - return Ok(StringOrBytes::Bytes(attr.as_bytes().to_vec())); - }; + entry.attr_first(name).map(|entry| StringOrBytes::String(entry.to_owned())) } + AttributeMapping::OptionalBinary { name, is_binary: true } => entry + .bin_attr_first(name) + // If an entry encodes as UTF-8, it will still only be + // available from the `.attr_first` function, even if ldap + // presents it with the `::` delimiter. + // + // Hence the configuration, we just treat it as binary + // data if this is requested. + .or_else(|| entry.attr_first(name).map(str::as_bytes)) + .map(|entry| StringOrBytes::Bytes(entry.to_vec())), } - - bail!("missing `{}` values for `{}`", attribute, entry.dn) + .ok_or(anyhow!("missing `{}` values for `{}`", attribute, entry.dn)) } /// LDAP-specific configuration @@ -308,16 +330,26 @@ pub struct LdapTlsConfig { pub danger_use_start_tls: bool, } +/// A structure that can either be a string or bytes +#[derive(Clone, Debug)] +enum StringOrBytes { + /// A string + String(String), + /// A byte string + Bytes(Vec), +} + #[cfg(test)] mod tests { use std::collections::HashMap; + use base64::prelude::{Engine, BASE64_STANDARD}; use indoc::indoc; use ldap3::SearchEntry; use ldap_poller::ldap::EntryStatus; use tokio::sync::mpsc; - use crate::{sources::ldap::LdapSource, user::StringOrBytes, Config}; + use crate::{sources::ldap::LdapSource, Config}; const EXAMPLE_CONFIG: &str = indoc! {r#" zitadel: @@ -491,13 +523,13 @@ mod tests { let result = ldap_source.parse_user(entry); assert!(result.is_ok(), "Failed to parse user: {:?}", result); let user = result.unwrap(); - assert_eq!(user.first_name, StringOrBytes::String("Test".to_owned())); - assert_eq!(user.last_name, StringOrBytes::String("User".to_owned())); - assert_eq!(user.preferred_username, Some(StringOrBytes::String("testuser".to_owned()))); - assert_eq!(user.email, StringOrBytes::String("testuser@example.com".to_owned())); - assert_eq!(user.phone, Some(StringOrBytes::String("123456789".to_owned()))); - assert_eq!(user.preferred_username, Some(StringOrBytes::String("testuser".to_owned()))); - assert_eq!(user.external_user_id, StringOrBytes::String("testuser".to_owned())); + assert_eq!(user.first_name, "Test"); + assert_eq!(user.last_name, "User"); + assert_eq!(user.preferred_username, Some("testuser".to_owned())); + assert_eq!(user.email, "testuser@example.com"); + assert_eq!(user.phone, Some("123456789".to_owned())); + assert_eq!(user.preferred_username, Some("testuser".to_owned())); + assert_eq!(user.external_user_id, BASE64_STANDARD.encode("testuser")); assert!(user.enabled); } } diff --git a/src/user.rs b/src/user.rs index 5adca23..c0db447 100644 --- a/src/user.rs +++ b/src/user.rs @@ -1,8 +1,5 @@ //! User data helpers -use std::fmt::Display; - use anyhow::{anyhow, Result}; -use base64::prelude::{Engine, BASE64_STANDARD}; use uuid::{uuid, Uuid}; use zitadel_rust_client::v2::users::HumanUser; @@ -13,19 +10,19 @@ const FAMEDLY_NAMESPACE: Uuid = uuid!("d9979cff-abee-4666-bc88-1ec45a843fb8"); #[derive(Clone)] pub(crate) struct User { /// The user's first name - pub(crate) first_name: StringOrBytes, + pub(crate) first_name: String, /// The user's last name - pub(crate) last_name: StringOrBytes, + pub(crate) last_name: String, /// The user's email address - pub(crate) email: StringOrBytes, + pub(crate) email: String, /// The user's phone number - pub(crate) phone: Option, + pub(crate) phone: Option, /// Whether the user is enabled pub(crate) enabled: bool, /// The user's preferred username - pub(crate) preferred_username: Option, + pub(crate) preferred_username: Option, /// The user's external (non-Zitadel) ID - pub(crate) external_user_id: StringOrBytes, + pub(crate) external_user_id: String, } impl User { @@ -51,18 +48,13 @@ impl User { let phone = user.phone().and_then(|human_phone| human_phone.phone()); - let external_user_id = match BASE64_STANDARD.decode(external_id.clone()) { - Ok(bytes) => bytes.into(), - Err(_) => external_id.into(), - }; - Ok(Self { - first_name: first_name.into(), - last_name: last_name.into(), - email: email.into(), - phone: phone.map(|phone| phone.clone().into()), + first_name, + last_name, + email, + phone: phone.cloned(), preferred_username: None, - external_user_id, + external_user_id: external_id, enabled: true, }) } @@ -106,120 +98,3 @@ impl std::fmt::Debug for User { .finish() } } - -/// A structure that can either be a string or bytes -#[derive(Clone, Debug, Eq)] -pub(crate) enum StringOrBytes { - /// A string - String(String), - /// A byte string - Bytes(Vec), -} - -impl StringOrBytes { - /// Represent the object as raw bytes, regardless of whether it - /// can be represented as a string - pub fn as_bytes(&self) -> &[u8] { - match self { - Self::String(string) => string.as_bytes(), - Self::Bytes(bytes) => bytes, - } - } -} - -impl PartialEq for StringOrBytes { - fn eq(&self, other: &Self) -> bool { - match (self, other) { - (Self::String(s), Self::String(o)) => s == o, - (Self::String(s), Self::Bytes(o)) => s.as_bytes() == o, - (Self::Bytes(s), Self::String(o)) => s == o.as_bytes(), - (Self::Bytes(s), Self::Bytes(o)) => s == o, - } - } -} - -impl Ord for StringOrBytes { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - match (self, other) { - (Self::String(s), Self::String(o)) => s.cmp(o), - (Self::String(s), Self::Bytes(o)) => s.as_bytes().cmp(o.as_slice()), - (Self::Bytes(s), Self::String(o)) => s.as_slice().cmp(o.as_bytes()), - (Self::Bytes(s), Self::Bytes(o)) => s.cmp(o), - } - } -} - -impl PartialOrd for StringOrBytes { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Display for StringOrBytes { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - StringOrBytes::String(value) => write!(f, "{}", value), - StringOrBytes::Bytes(value) => write!(f, "{}", BASE64_STANDARD.encode(value)), - } - } -} - -impl From for StringOrBytes { - fn from(value: String) -> Self { - Self::String(value) - } -} - -impl From> for StringOrBytes { - fn from(value: Vec) -> Self { - Self::Bytes(value) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn strb_from_string(string: &str) -> StringOrBytes { - StringOrBytes::from(string.to_owned()) - } - - fn strb_from_bytes(bytes: &[u8]) -> StringOrBytes { - StringOrBytes::Bytes(bytes.to_owned()) - } - - #[test] - fn test_strb_equality() { - assert_eq!(strb_from_string("a"), strb_from_string("a")); - assert_ne!(strb_from_string("a"), strb_from_string("b")); - - assert_eq!(strb_from_string("a"), strb_from_bytes(b"a")); - assert_ne!(strb_from_string("a"), strb_from_bytes(b"b")); - - assert_eq!(strb_from_bytes(b"a"), strb_from_bytes(b"a")); - assert_ne!(strb_from_bytes(b"a"), strb_from_bytes(b"b")); - - assert_eq!(strb_from_bytes(b"\xc3\x28"), strb_from_bytes(b"\xc3\x28")); - assert_ne!(strb_from_bytes(b"a"), strb_from_bytes(b"\xc3\x28")); - } - - #[test] - fn test_strb_order() { - assert!(strb_from_string("a") < strb_from_string("b")); - assert!(strb_from_string("b") > strb_from_string("a")); - assert!(strb_from_string("b") < strb_from_string("c")); - assert!(strb_from_string("a") < strb_from_string("c")); - - assert!(strb_from_bytes(b"a") < strb_from_bytes(b"b")); - assert!(strb_from_bytes(b"b") > strb_from_bytes(b"a")); - assert!(strb_from_bytes(b"b") < strb_from_bytes(b"c")); - assert!(strb_from_bytes(b"a") < strb_from_bytes(b"c")); - - assert!(strb_from_string("a") < strb_from_bytes(b"b")); - assert!(strb_from_string("b") > strb_from_bytes(b"a")); - assert!(strb_from_string("b") < strb_from_bytes(b"c")); - assert!(strb_from_string("a") < strb_from_bytes(b"c")); - - assert!(strb_from_bytes(b"\xc3\x28") < strb_from_bytes(b"\xc3\x29")); - } -} diff --git a/src/zitadel.rs b/src/zitadel.rs index eaccd28..cefdc06 100644 --- a/src/zitadel.rs +++ b/src/zitadel.rs @@ -130,20 +130,15 @@ impl Zitadel { vec![SetMetadataEntry::new("localpart".to_owned(), imported_user.famedly_uuid())]; if let Some(preferred_username) = imported_user.preferred_username.clone() { - metadata.push(SetMetadataEntry::new( - "preferred_username".to_owned(), - preferred_username.to_string(), - )); + metadata + .push(SetMetadataEntry::new("preferred_username".to_owned(), preferred_username)); } let mut user = AddHumanUserRequest::new( - SetHumanProfile::new( - imported_user.first_name.to_string(), - imported_user.last_name.to_string(), - ) - .with_nick_name(imported_user.external_user_id.to_string()) - .with_display_name(imported_user.get_display_name()), - SetHumanEmail::new(imported_user.email.to_string()) + SetHumanProfile::new(imported_user.first_name.clone(), imported_user.last_name.clone()) + .with_nick_name(imported_user.external_user_id.clone()) + .with_display_name(imported_user.get_display_name()), + SetHumanEmail::new(imported_user.email.clone()) .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyEmail)), ) .with_organization( @@ -154,17 +149,17 @@ impl Zitadel { if let Some(phone) = imported_user.phone.clone() { user.set_phone( SetHumanPhone::new() - .with_phone(phone.to_string()) + .with_phone(phone.clone()) .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyPhone)), ); }; if self.feature_flags.is_enabled(FeatureFlag::SsoLogin) { user.set_idp_links(vec![IdpLink::new() - .with_user_id(imported_user.external_user_id.to_string()) + .with_user_id(imported_user.external_user_id.clone()) .with_idp_id(self.zitadel_config.idp_id.clone()) // TODO: Figure out if this is the correct value; empty is not permitted - .with_user_name(imported_user.email.to_string())]); + .with_user_name(imported_user.email.clone())]); } match self.zitadel_client.create_human_user(user.clone()).await { @@ -223,9 +218,9 @@ impl Zitadel { let mut request = UpdateHumanUserRequest::new(); if old_user.email != updated_user.email { - request.set_username(updated_user.email.to_string()); + request.set_username(updated_user.email.clone()); request.set_email( - SetHumanEmail::new(updated_user.email.to_string()) + SetHumanEmail::new(updated_user.email.clone()) .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyEmail)), ); } @@ -235,8 +230,8 @@ impl Zitadel { { request.set_profile( SetHumanProfile::new( - updated_user.first_name.to_string(), - updated_user.last_name.to_string(), + updated_user.first_name.clone(), + updated_user.last_name.clone(), ) .with_display_name(updated_user.get_display_name()), ); @@ -246,7 +241,7 @@ impl Zitadel { if let Some(phone) = updated_user.phone.clone() { request.set_phone( SetHumanPhone::new() - .with_phone(phone.to_string()) + .with_phone(phone.clone()) .with_is_verified(!self.feature_flags.is_enabled(FeatureFlag::VerifyPhone)), ); } else { @@ -278,7 +273,7 @@ impl Zitadel { .set_user_metadata( zitadel_id, "preferred_username", - &preferred_username.to_string(), + &preferred_username.clone(), ) .await?; } else { diff --git a/tests/e2e.rs b/tests/e2e.rs index 7dcd576..36b263f 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -9,7 +9,7 @@ use famedly_sync::{ ukt_test_helpers::{ get_mock_server_url, prepare_endpoint_mock, prepare_oauth2_mock, ENDPOINT_PATH, OAUTH2_PATH, }, - AttributeMapping, Config, FeatureFlag, + Config, FeatureFlag, }; use ldap3::{Ldap as LdapClient, LdapConnAsync, LdapConnSettings, Mod}; use test_log::test; @@ -485,131 +485,131 @@ async fn test_e2e_sync_invalid_phone() { } } -#[test(tokio::test)] -#[test_log(default_log_filter = "debug")] -async fn test_e2e_binary_attr() { - let mut config = ldap_config().await.clone(); - - // OpenLDAP checks if types match, so we need to use an attribute - // that can actually be binary. - config - .sources - .ldap - .as_mut() - .expect("ldap must be configured for this test") - .attributes - .preferred_username = AttributeMapping::OptionalBinary { - name: "userSMIMECertificate".to_owned(), - is_binary: true, - }; - - let mut ldap = Ldap::new().await; - ldap.create_user( - "Bob", - "Tables", - "Bobby", - "binary@famedly.de", - Some("+12015550123"), - "binary", - false, - ) - .await; - ldap.change_user( - "binary", - vec![( - "userSMIMECertificate".as_bytes(), - // It's important that this is invalid UTF-8 - HashSet::from([[0xA0, 0xA1].as_slice()]), - )], - ) - .await; - - let org_id = config.zitadel.organization_id.clone(); - - perform_sync(&config).await.expect("syncing failed"); - - let zitadel = open_zitadel_connection().await; - let user = zitadel - .get_user_by_login_name("binary@famedly.de") - .await - .expect("could not query Zitadel users"); - - assert!(user.is_some()); - - if let Some(user) = user { - let preferred_username = zitadel - .get_user_metadata(Some(org_id), &user.id, "preferred_username") - .await - .expect("could not get user metadata"); - - assert_eq!( - preferred_username - .map(|u| BASE64_STANDARD.decode(u).expect("failed to decode binary attr")), - Some([0xA0, 0xA1].to_vec()) - ); - } -} - -#[test(tokio::test)] -#[test_log(default_log_filter = "debug")] -async fn test_e2e_binary_attr_valid_utf8() { - let mut config = ldap_config().await.clone(); - - // OpenLDAP checks if types match, so we need to use an attribute - // that can actually be binary. - config - .sources - .ldap - .as_mut() - .expect("ldap must be configured for this test") - .attributes - .preferred_username = AttributeMapping::OptionalBinary { - name: "userSMIMECertificate".to_owned(), - is_binary: true, - }; - - let mut ldap = Ldap::new().await; - ldap.create_user( - "Bob", - "Tables", - "Bobby", - "binaryutf8@famedly.de", - Some("+12015550123"), - "binaryutf8", - false, - ) - .await; - ldap.change_user( - "binaryutf8", - vec![("userSMIMECertificate".as_bytes(), HashSet::from(["validutf8".as_bytes()]))], - ) - .await; - - let org_id = config.zitadel.organization_id.clone(); - - perform_sync(&config).await.expect("syncing failed"); - - let zitadel = open_zitadel_connection().await; - let user = zitadel - .get_user_by_login_name("binaryutf8@famedly.de") - .await - .expect("could not query Zitadel users"); - - assert!(user.is_some()); - - if let Some(user) = user { - let preferred_username = zitadel - .get_user_metadata(Some(org_id), &user.id, "preferred_username") - .await - .expect("could not get user metadata"); - - assert_eq!( - preferred_username - .map(|u| BASE64_STANDARD.decode(u).expect("failed to decode binary attr")), - Some("validutf8".as_bytes().to_vec()) - ); - } -} +// #[test(tokio::test)] +// #[test_log(default_log_filter = "debug")] +// async fn test_e2e_binary_attr() { +// let mut config = ldap_config().await.clone(); + +// // OpenLDAP checks if types match, so we need to use an attribute +// // that can actually be binary. +// config +// .sources +// .ldap +// .as_mut() +// .expect("ldap must be configured for this test") +// .attributes +// .preferred_username = AttributeMapping::OptionalBinary { +// name: "userSMIMECertificate".to_owned(), +// is_binary: true, +// }; + +// let mut ldap = Ldap::new().await; +// ldap.create_user( +// "Bob", +// "Tables", +// "Bobby", +// "binary@famedly.de", +// Some("+12015550123"), +// "binary", +// false, +// ) +// .await; +// ldap.change_user( +// "binary", +// vec![( +// "userSMIMECertificate".as_bytes(), +// // It's important that this is invalid UTF-8 +// HashSet::from([[0xA0, 0xA1].as_slice()]), +// )], +// ) +// .await; + +// let org_id = config.zitadel.organization_id.clone(); + +// perform_sync(&config).await.expect("syncing failed"); + +// let zitadel = open_zitadel_connection().await; +// let user = zitadel +// .get_user_by_login_name("binary@famedly.de") +// .await +// .expect("could not query Zitadel users"); + +// assert!(user.is_some()); + +// if let Some(user) = user { +// let preferred_username = zitadel +// .get_user_metadata(Some(org_id), &user.id, "preferred_username") +// .await +// .expect("could not get user metadata"); + +// assert_eq!( +// preferred_username +// .map(|u| BASE64_STANDARD.decode(u).expect("failed to decode binary attr")), +// Some([0xA0, 0xA1].to_vec()) +// ); +// } +// } + +// #[test(tokio::test)] +// #[test_log(default_log_filter = "debug")] +// async fn test_e2e_binary_attr_valid_utf8() { +// let mut config = ldap_config().await.clone(); + +// // OpenLDAP checks if types match, so we need to use an attribute +// // that can actually be binary. +// config +// .sources +// .ldap +// .as_mut() +// .expect("ldap must be configured for this test") +// .attributes +// .preferred_username = AttributeMapping::OptionalBinary { +// name: "userSMIMECertificate".to_owned(), +// is_binary: true, +// }; + +// let mut ldap = Ldap::new().await; +// ldap.create_user( +// "Bob", +// "Tables", +// "Bobby", +// "binaryutf8@famedly.de", +// Some("+12015550123"), +// "binaryutf8", +// false, +// ) +// .await; +// ldap.change_user( +// "binaryutf8", +// vec![("userSMIMECertificate".as_bytes(), HashSet::from(["validutf8".as_bytes()]))], +// ) +// .await; + +// let org_id = config.zitadel.organization_id.clone(); + +// perform_sync(&config).await.expect("syncing failed"); + +// let zitadel = open_zitadel_connection().await; +// let user = zitadel +// .get_user_by_login_name("binaryutf8@famedly.de") +// .await +// .expect("could not query Zitadel users"); + +// assert!(user.is_some()); + +// if let Some(user) = user { +// let preferred_username = zitadel +// .get_user_metadata(Some(org_id), &user.id, "preferred_username") +// .await +// .expect("could not get user metadata"); + +// assert_eq!( +// preferred_username +// .map(|u| BASE64_STANDARD.decode(u).expect("failed to decode binary attr")), +// Some("validutf8".as_bytes().to_vec()) +// ); +// } +// } #[test(tokio::test)] #[test_log(default_log_filter = "debug")] From 482cd7d25dc9c1698988a342cc76b3a5671a835e Mon Sep 17 00:00:00 2001 From: Jan Cibulka Date: Mon, 11 Nov 2024 21:52:26 +0200 Subject: [PATCH 10/11] refactor: Use external ID encoding supporting lexicographical order --- Cargo.lock | 1 + Cargo.toml | 1 + src/sources/csv.rs | 12 +- src/sources/ldap.rs | 18 +- tests/e2e.rs | 534 +++++++++++++++++++++++++++++++++----------- 5 files changed, 419 insertions(+), 147 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index edc8955..48f48bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -722,6 +722,7 @@ dependencies = [ "config", "csv", "futures", + "hex", "http 1.1.0", "indoc", "ldap-poller", diff --git a/Cargo.toml b/Cargo.toml index 7b0bcd9..0975b98 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ tempfile = "3.12.0" futures = "0.3.31" ldap3 = { version = "0.11.1", default-features = false, features = ["tls-native"] } native-tls = "0.2.12" +hex = "0.4.3" [dependencies.tonic] version = "*" diff --git a/src/sources/csv.rs b/src/sources/csv.rs index 61d85de..5dd43a0 100644 --- a/src/sources/csv.rs +++ b/src/sources/csv.rs @@ -234,15 +234,7 @@ mod tests { let users = result.expect("Failed to get users"); assert_eq!(users.len(), 1, "Unexpected number of users"); - assert_eq!( - users[0].email, - "jane.smith@example.com", - "Unexpected email at index 0" - ); - assert_eq!( - users[0].last_name, - "Smith", - "Unexpected last name at index 0" - ); + assert_eq!(users[0].email, "jane.smith@example.com", "Unexpected email at index 0"); + assert_eq!(users[0].last_name, "Smith", "Unexpected last name at index 0"); } } diff --git a/src/sources/ldap.rs b/src/sources/ldap.rs index 3de0505..fc4f699 100644 --- a/src/sources/ldap.rs +++ b/src/sources/ldap.rs @@ -4,7 +4,6 @@ use std::{fmt::Display, path::PathBuf}; use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; -use base64::prelude::{Engine, BASE64_STANDARD}; use ldap_poller::{ config::TLSConfig, ldap::EntryStatus, ldap3::SearchEntry, AttributeConfig, CacheMethod, ConnectionConfig, Ldap, SearchEntryExt, Searches, @@ -90,9 +89,9 @@ impl LdapSource { .any(|flag| status_as_int & flag != 0); let ldap_user_id = match read_search_entry(&entry, &self.ldap_config.attributes.user_id)? { - // TODO(tlater): Use an encoding that preserves alphabetic order - StringOrBytes::Bytes(byte_id) => BASE64_STANDARD.encode(byte_id), - StringOrBytes::String(string_id) => BASE64_STANDARD.encode(string_id), + // Use hex encoding instead of base64 for consistent alphabetical order + StringOrBytes::Bytes(byte_id) => hex::encode(byte_id), + StringOrBytes::String(string_id) => hex::encode(string_id.as_bytes()), }; let first_name = @@ -128,9 +127,11 @@ fn read_string_entry( ) -> Result { match read_search_entry(entry, attribute)? { StringOrBytes::String(entry) => Ok(entry), - StringOrBytes::Bytes(_) => { - Err(anyhow!("Unacceptable binary value for {} of user `{}`", attribute, id)) - } + StringOrBytes::Bytes(_) => Err(anyhow!( + "Binary values are not accepted: attribute `{}` of user `{}`", + attribute, + id + )), } } @@ -343,7 +344,6 @@ enum StringOrBytes { mod tests { use std::collections::HashMap; - use base64::prelude::{Engine, BASE64_STANDARD}; use indoc::indoc; use ldap3::SearchEntry; use ldap_poller::ldap::EntryStatus; @@ -529,7 +529,7 @@ mod tests { assert_eq!(user.email, "testuser@example.com"); assert_eq!(user.phone, Some("123456789".to_owned())); assert_eq!(user.preferred_username, Some("testuser".to_owned())); - assert_eq!(user.external_user_id, BASE64_STANDARD.encode("testuser")); + assert_eq!(user.external_user_id, hex::encode("testuser")); assert!(user.enabled); } } diff --git a/tests/e2e.rs b/tests/e2e.rs index 36b263f..0928a62 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -1,15 +1,15 @@ #![cfg(test)] +#![allow(clippy::expect_fun_call)] use std::{collections::HashSet, path::Path, time::Duration}; -use base64::prelude::{Engine, BASE64_STANDARD}; use famedly_sync::{ csv_test_helpers::temp_csv_file, perform_sync, ukt_test_helpers::{ get_mock_server_url, prepare_endpoint_mock, prepare_oauth2_mock, ENDPOINT_PATH, OAUTH2_PATH, }, - Config, FeatureFlag, + AttributeMapping, Config, FeatureFlag, }; use ldap3::{Ldap as LdapClient, LdapConnAsync, LdapConnSettings, Mod}; use test_log::test; @@ -32,6 +32,198 @@ const FAMEDLY_NAMESPACE: Uuid = uuid!("d9979cff-abee-4666-bc88-1ec45a843fb8"); /// The Zitadel project role to assign to users. const FAMEDLY_USER_ROLE: &str = "User"; +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_user_id_encoding() { + async fn verify_user_encoding( + ldap: &mut Ldap, + zitadel: &Zitadel, + config: &Config, + uid: &str, + email: &str, + ) -> Result<(), String> { + let login_name = email; + let expected_hex_id = hex::encode(uid.as_bytes()); + + ldap.create_user("Test", "User", "TU", login_name, None, uid, false).await; + + perform_sync(config).await.map_err(|e| format!("Sync failed: {}", e))?; + + let user = zitadel + .get_user_by_login_name(login_name) + .await + .map_err(|e| format!("Failed to get user: {}", e))? + .ok_or_else(|| "User not found".to_owned())?; + + match user.r#type { + Some(UserType::Human(user)) => { + let profile = user.profile.ok_or_else(|| "User lacks profile".to_owned())?; + + if profile.nick_name != expected_hex_id { + return Err(format!( + "ID mismatch for '{}': expected '{}', got '{}'", + uid, expected_hex_id, profile.nick_name + )); + } + Ok(()) + } + _ => Err("User lacks human details".to_owned()), + } + } + + /// Test cases for verifying correct user ID encoding + /// (uid, email) + const TEST_CASES: &[(&str, &str)] = &[ + // Basic cases + ("simple123", "simple123@example.com"), + ("MiXed123Case", "mixed123case@example.com"), + // Special characters + ("u.s-e_r", "user@example.com"), + ("123", "123@example.com"), + // Unicode + ("üsernamÉ", "username@example.com"), + ("ὈΔΥΣΣΕΎΣ", "odysseus@example.com"), + ("Потребител", "potrebitel@example.com"), + // Long string + ("ThisIsAVeryLongUsernameThatShouldStillWork123456789", "long@example.com"), + ]; + + // Run all test cases + let config = ldap_config().await; + let mut ldap = Ldap::new().await; + let zitadel = open_zitadel_connection().await; + + for (uid, email) in TEST_CASES { + if let Err(error) = verify_user_encoding(&mut ldap, &zitadel, config, uid, email).await { + panic!("Test failed for ID '{}': {}", uid, error); + } + } +} + +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_user_id_sync_ordering() { + struct TestUser<'a> { + uid: &'a str, + email: &'a str, + phone: &'a str, + } + + const TEST_USERS: &[TestUser] = &[ + TestUser { uid: "üser", email: "youser@example.com", phone: "+6666666666" }, + TestUser { uid: "aaa", email: "aaa@example.com", phone: "+1111111111" }, + TestUser { uid: "777", email: "777@example.com", phone: "+5555555555" }, + TestUser { uid: "bbb", email: "bbb@example.com", phone: "+3333333333" }, + TestUser { uid: "🦀", email: "crab@example.com", phone: "+1000000001" }, + TestUser { uid: "한글", email: "korean@example.com", phone: "+1000000002" }, + TestUser { uid: "عربي", email: "arabic@example.com", phone: "+1000000005" }, + ]; + + // Setup + let config = ldap_config().await; + let mut ldap = Ldap::new().await; + let zitadel = open_zitadel_connection().await; + + // Create all users in LDAP + for user in TEST_USERS { + ldap.create_user("Test", "User", "TU", user.email, Some(user.phone), user.uid, false).await; + } + + // Initial sync + perform_sync(config).await.expect("Initial sync failed"); + + // Verify all users exist with correct data + for user in TEST_USERS { + let expected_hex_id = hex::encode(user.uid.as_bytes()); + + let zitadel_user = zitadel + .get_user_by_login_name(user.email) + .await + .expect(&format!("Failed to get user {}", user.email)) + .expect(&format!("User {} not found", user.email)); + + match zitadel_user.r#type { + Some(UserType::Human(human)) => { + // Verify ID encoding + let profile = human.profile.expect(&format!("User {} lacks profile", user.email)); + assert_eq!( + profile.nick_name, + expected_hex_id, + "Wrong ID encoding for user {}, got '{:?}', expected '{:?}'", + user.email, + String::from_utf8_lossy(&hex::decode(profile.nick_name.clone()).unwrap()), + String::from_utf8_lossy(&hex::decode(expected_hex_id.clone()).unwrap()) + ); + + // Verify phone number to ensure complete sync + let phone = human.phone.expect(&format!("User {} lacks phone", user.email)); + assert_eq!(phone.phone, user.phone, "Wrong phone for user {}", user.email); + } + _ => panic!("User {} lacks human details", user.email), + } + } + + // Now update all users with new data + for user in TEST_USERS { + ldap.change_user( + user.uid, + // Just change the last_name (sn) attribute to the user's uid with SN prefix + vec![("sn", HashSet::from([format!("SN{}", user.uid).as_str()]))], + ) + .await; + } + + // Sync again + perform_sync(config).await.expect("Update sync failed"); + + // Verify updates were applied in correct order + for user in TEST_USERS { + let zitadel_user = zitadel + .get_user_by_login_name(user.email) + .await + .expect(&format!("Failed to get updated user {}", user.email)) + .expect(&format!("Updated user {} not found", user.email)); + + match zitadel_user.r#type { + Some(UserType::Human(human)) => { + let profile = + human.profile.expect(&format!("Updated user {} lacks profile", user.email)); + let last_name = profile.last_name; + assert_eq!( + last_name, + format!("SN{}", user.uid), + "Wrong updated last_name for user {}", + user.email + ); + } + _ => panic!("Updated user {} lacks human details", user.email), + } + } + + // Finally delete users in reverse order + for user in TEST_USERS.iter().rev() { + ldap.delete_user(user.uid).await; + } + + // Final sync + perform_sync(config).await.expect("Deletion sync failed"); + + // Verify all users were deleted in correct order + for user in TEST_USERS { + let result = zitadel.get_user_by_login_name(user.email).await; + + assert!( + matches!( + result, + Err(ZitadelError::TonicResponseError(status)) + if status.code() == TonicErrorCode::NotFound + ), + "User {} still exists after deletion", + user.email + ); + } +} + #[test(tokio::test)] #[test_log(default_log_filter = "debug")] async fn test_e2e_simple_sync() { @@ -88,7 +280,7 @@ async fn test_e2e_simple_sync() { .expect("could not get user metadata"); assert_eq!(preferred_username, Some("Bobby".to_owned())); - let uuid = Uuid::new_v5(&FAMEDLY_NAMESPACE, "simple".as_bytes()); + let uuid = Uuid::new_v5(&FAMEDLY_NAMESPACE, hex::encode("simple").as_bytes()); let localpart = zitadel .get_user_metadata(Some(config.zitadel.organization_id.clone()), &user.id, "localpart") @@ -485,131 +677,217 @@ async fn test_e2e_sync_invalid_phone() { } } -// #[test(tokio::test)] -// #[test_log(default_log_filter = "debug")] -// async fn test_e2e_binary_attr() { -// let mut config = ldap_config().await.clone(); - -// // OpenLDAP checks if types match, so we need to use an attribute -// // that can actually be binary. -// config -// .sources -// .ldap -// .as_mut() -// .expect("ldap must be configured for this test") -// .attributes -// .preferred_username = AttributeMapping::OptionalBinary { -// name: "userSMIMECertificate".to_owned(), -// is_binary: true, -// }; - -// let mut ldap = Ldap::new().await; -// ldap.create_user( -// "Bob", -// "Tables", -// "Bobby", -// "binary@famedly.de", -// Some("+12015550123"), -// "binary", -// false, -// ) -// .await; -// ldap.change_user( -// "binary", -// vec![( -// "userSMIMECertificate".as_bytes(), -// // It's important that this is invalid UTF-8 -// HashSet::from([[0xA0, 0xA1].as_slice()]), -// )], -// ) -// .await; - -// let org_id = config.zitadel.organization_id.clone(); - -// perform_sync(&config).await.expect("syncing failed"); - -// let zitadel = open_zitadel_connection().await; -// let user = zitadel -// .get_user_by_login_name("binary@famedly.de") -// .await -// .expect("could not query Zitadel users"); - -// assert!(user.is_some()); - -// if let Some(user) = user { -// let preferred_username = zitadel -// .get_user_metadata(Some(org_id), &user.id, "preferred_username") -// .await -// .expect("could not get user metadata"); - -// assert_eq!( -// preferred_username -// .map(|u| BASE64_STANDARD.decode(u).expect("failed to decode binary attr")), -// Some([0xA0, 0xA1].to_vec()) -// ); -// } -// } - -// #[test(tokio::test)] -// #[test_log(default_log_filter = "debug")] -// async fn test_e2e_binary_attr_valid_utf8() { -// let mut config = ldap_config().await.clone(); - -// // OpenLDAP checks if types match, so we need to use an attribute -// // that can actually be binary. -// config -// .sources -// .ldap -// .as_mut() -// .expect("ldap must be configured for this test") -// .attributes -// .preferred_username = AttributeMapping::OptionalBinary { -// name: "userSMIMECertificate".to_owned(), -// is_binary: true, -// }; - -// let mut ldap = Ldap::new().await; -// ldap.create_user( -// "Bob", -// "Tables", -// "Bobby", -// "binaryutf8@famedly.de", -// Some("+12015550123"), -// "binaryutf8", -// false, -// ) -// .await; -// ldap.change_user( -// "binaryutf8", -// vec![("userSMIMECertificate".as_bytes(), HashSet::from(["validutf8".as_bytes()]))], -// ) -// .await; - -// let org_id = config.zitadel.organization_id.clone(); - -// perform_sync(&config).await.expect("syncing failed"); - -// let zitadel = open_zitadel_connection().await; -// let user = zitadel -// .get_user_by_login_name("binaryutf8@famedly.de") -// .await -// .expect("could not query Zitadel users"); - -// assert!(user.is_some()); - -// if let Some(user) = user { -// let preferred_username = zitadel -// .get_user_metadata(Some(org_id), &user.id, "preferred_username") -// .await -// .expect("could not get user metadata"); - -// assert_eq!( -// preferred_username -// .map(|u| BASE64_STANDARD.decode(u).expect("failed to decode binary attr")), -// Some("validutf8".as_bytes().to_vec()) -// ); -// } -// } +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_binary_uid() { + let mut config = ldap_config().await.clone(); + + // Attribute uid (user_id) is configured as binary + + config + .sources + .ldap + .as_mut() + .expect("ldap must be configured for this test") + .attributes + .user_id = AttributeMapping::OptionalBinary { + name: "userSMIMECertificate".to_owned(), + is_binary: true, + }; + + let mut ldap = Ldap::new().await; + + // Create test user with binary ID + let uid = "binary_user"; + let binary_uid = uid.as_bytes(); + ldap.create_user( + "Binary", + "User", + "BinaryTest", + "binary_id@famedly.de", + Some("+12345678901"), + uid, // Regular uid for DN + false, + ) + .await; + + // Set binary ID + ldap.change_user( + uid, + vec![("userSMIMECertificate".as_bytes(), HashSet::from([uid.as_bytes()]))], + ) + .await; + + perform_sync(&config).await.expect("syncing failed"); + + let zitadel = open_zitadel_connection().await; + let user = zitadel + .get_user_by_login_name("binary_id@famedly.de") + .await + .expect("could not query Zitadel users") + .expect("user not found"); + + match user.r#type { + Some(UserType::Human(user)) => { + let profile = user.profile.expect("user lacks profile"); + // The ID should be hex encoded in Zitadel + assert_eq!(profile.nick_name, hex::encode(binary_uid)); + } + _ => panic!("user lacks human details"), + } + + // Test update to a different binary ID that is valid UTF-8 + + let new_binary_id = "updated_binary_user".as_bytes(); + ldap.change_user( + uid, + vec![("userSMIMECertificate".as_bytes(), HashSet::from([new_binary_id]))], + ) + .await; + + perform_sync(&config).await.expect("syncing failed"); + + let user = zitadel + .get_user_by_login_name("binary_id@famedly.de") + .await + .expect("could not query Zitadel users") + .expect("user not found after update"); + + match user.r#type { + Some(UserType::Human(user)) => { + let profile = user.profile.expect("user lacks profile"); + println!("profile: {:#?}", profile); + // Verify ID was updated + assert_eq!(profile.nick_name, hex::encode(&new_binary_id)); + } + _ => panic!("user lost human details after update"), + } + + // Test update to binary ID that is NOT valid UTF-8 + + let invalid_binary_id = [0xA1, 0xA2]; + ldap.change_user( + uid, + vec![("userSMIMECertificate".as_bytes(), HashSet::from([invalid_binary_id.as_slice()]))], + ) + .await; + + perform_sync(&config).await.expect("syncing failed"); + + let user = zitadel + .get_user_by_login_name("binary_id@famedly.de") + .await + .expect("could not query Zitadel users") + .expect("user not found after update"); + + match user.r#type { + Some(UserType::Human(user)) => { + let profile = user.profile.expect("user lacks profile"); + // Verify ID was updated + assert_eq!(profile.nick_name, hex::encode(&invalid_binary_id)); + } + _ => panic!("user lost human details after update"), + } +} + +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_binary_preferred_username() { + let mut config = ldap_config().await.clone(); + + // Attribute preferred_username is configured as binary but shouldn't be + + config + .sources + .ldap + .as_mut() + .expect("ldap must be configured for this test") + .attributes + .preferred_username = AttributeMapping::OptionalBinary { + name: "userSMIMECertificate".to_owned(), + is_binary: true, + }; + + let mut ldap = Ldap::new().await; + ldap.create_user( + "BobFail", + "TablesFail", + "BobbyFail", + "binary_fail@famedly.de", + Some("+12015550123"), + "binary_fail", + false, + ) + .await; + + // Test with a valid UTF-8 binary attribute + + ldap.change_user( + "binary_fail", + vec![("userSMIMECertificate".as_bytes(), HashSet::from(["new_binary_fail".as_bytes()]))], + ) + .await; + + let result = tokio::spawn({ + let config = config.clone(); + async move { perform_sync(&config).await } + }) + .await; + + match result { + Ok(sync_result) => { + assert!(sync_result.is_err()); + let error = sync_result.unwrap_err(); + assert!(error.to_string().contains("Failed to query users from LDAP")); + if let Some(cause) = error.source() { + assert!(cause.to_string().contains("Binary values are not accepted")); + assert!(cause.to_string().contains("attribute `userSMIMECertificate`")); + } else { + panic!("Expected error to have a cause"); + } + } + Err(join_error) if join_error.is_panic() => { + panic!("perform_sync panicked unexpectedly: {}", join_error); + } + Err(e) => { + panic!("unexpected error: {}", e); + } + } + + // Test with an invalid UTF-8 binary attribute + + ldap.change_user( + "binary_fail", + vec![("userSMIMECertificate".as_bytes(), HashSet::from([[0xA0, 0xA1].as_slice()]))], + ) + .await; + + let result = tokio::spawn({ + let config = config.clone(); + async move { perform_sync(&config).await } + }) + .await; + + match result { + Ok(sync_result) => { + assert!(sync_result.is_err()); + let error = sync_result.unwrap_err(); + assert!(error.to_string().contains("Failed to query users from LDAP")); + if let Some(cause) = error.source() { + assert!(cause.to_string().contains("Binary values are not accepted")); + assert!(cause.to_string().contains("attribute `userSMIMECertificate`")); + } else { + panic!("Expected error to have a cause"); + } + } + Err(join_error) if join_error.is_panic() => { + panic!("perform_sync panicked unexpectedly: {}", join_error); + } + Err(e) => { + panic!("unexpected error: {}", e); + } + } +} #[test(tokio::test)] #[test_log(default_log_filter = "debug")] From 4866de8f95553ad99d40261c89a1caa38e4c6edb Mon Sep 17 00:00:00 2001 From: Jan Cibulka Date: Thu, 14 Nov 2024 15:02:59 +0200 Subject: [PATCH 11/11] feat: Migration script for LDAP sync --- Cargo.toml | 4 + src/bin/migrate.rs | 127 ++++++++++++++++++++++++++++++ tests/e2e.rs | 188 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 316 insertions(+), 3 deletions(-) create mode 100644 src/bin/migrate.rs diff --git a/Cargo.toml b/Cargo.toml index 0975b98..3281197 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,10 @@ authors = [] edition = "2021" publish = false +[[bin]] +name = "migrate" +path = "src/bin/migrate.rs" + [dependencies] anyhow = { version = "1.0.81", features = ["backtrace"] } async-trait = "0.1.82" diff --git a/src/bin/migrate.rs b/src/bin/migrate.rs new file mode 100644 index 0000000..055e5c2 --- /dev/null +++ b/src/bin/migrate.rs @@ -0,0 +1,127 @@ +//! This binary is used to migrate user IDs from base64 to hex encoding. +use std::{path::Path, str::FromStr}; + +use anyhow::{anyhow, Context, Result}; +use base64::{engine::general_purpose, Engine as _}; +use famedly_sync::Config; +use futures::StreamExt; +use tracing::level_filters::LevelFilter; +use zitadel_rust_client::v2::{ + users::{ + ListUsersRequest, SearchQuery, SetHumanProfile, TypeQuery, UpdateHumanUserRequest, + UserFieldName, Userv2Type, + }, + Zitadel, +}; + +#[tokio::main] +async fn main() -> Result<()> { + // Config + let config_path = + std::env::var("FAMEDLY_SYNC_CONFIG").unwrap_or_else(|_| "./config.yaml".to_owned()); + let config = Config::new(Path::new(&config_path))?; + + // Tracing + let subscriber = tracing_subscriber::FmtSubscriber::builder() + .with_max_level( + config + .log_level + .as_ref() + .map_or(Ok(LevelFilter::INFO), |s| LevelFilter::from_str(s))?, + ) + .finish(); + tracing::subscriber::set_global_default(subscriber) + .context("Setting default tracing subscriber failed")?; + + tracing::info!("Starting migration"); + tracing::debug!("Old external IDs will be base64 decoded and re-encoded as hex"); + tracing::debug!("Note: External IDs are stored in the nick_name field of the user's profile in Zitadel, often referred to as uid."); + + // Zitadel + let zitadel_config = config.zitadel.clone(); + let mut zitadel = Zitadel::new(zitadel_config.url, zitadel_config.key_file).await?; + + // Get all users + let mut stream = zitadel.list_users( + ListUsersRequest::new(vec![ + SearchQuery::new().with_type_query(TypeQuery::new(Userv2Type::Human)) + ]) + .with_asc(true) + .with_sorting_column(UserFieldName::NickName), + )?; + + // Process each user + while let Some(user) = stream.next().await { + let user_id = user.user_id().ok_or_else(|| anyhow!("User lacks ID"))?; + let human = user.human().ok_or_else(|| anyhow!("User not human"))?; + let profile = human.profile().ok_or_else(|| anyhow!("User lacks profile"))?; + + let given_name = profile.given_name(); + let family_name = profile.family_name(); + let display_name = profile.display_name(); + + let Some(nick_name) = profile.nick_name() else { + tracing::warn!(user_id = %user_id, "User lacks nick_name (external uid), skipping"); + continue; + }; + + tracing::info!(user_id = %user_id, old_uid = %nick_name, "Starting migration for user"); + + let new_external_id = if nick_name.is_empty() { + // Keep empty string as is, will skip it later + nick_name.to_string() + } else { + // First check if it's valid hex + if nick_name.chars().all(|c| c.is_ascii_hexdigit()) && nick_name.len() % 2 == 0 { + // If valid hex, keep as is + tracing::debug!( user_id = %user_id, old_uid = %nick_name,"Valid hex uid detected, keeping as is"); + nick_name.to_string() + } else { + // Try base64, if fails use plain text + let decoded = general_purpose::STANDARD.decode(nick_name).unwrap_or_else(|_| { + // Not base64, treat as plain text + tracing::debug!( user_id = %user_id, old_uid = %nick_name,"Decoding uid failed, going with plain uid"); + nick_name.as_bytes().to_vec() + }); + + // Encode using hex + hex::encode(decoded) + } + }; + + // Skip empty uid + if new_external_id.is_empty() { + tracing::warn!( + user_id = %user_id, + old_uid = %nick_name, + new_uid = %new_external_id, + "Skipping user due to empty uid"); + continue; + } + + // Update uid (external ID, nick_name) in Zitadel + let mut request = UpdateHumanUserRequest::new(); + request.set_profile( + SetHumanProfile::new( + given_name.ok_or_else(|| anyhow!("User lacks given name"))?.to_owned(), + family_name.ok_or_else(|| anyhow!("User lacks family name"))?.to_owned(), + ) + .with_display_name( + display_name.ok_or_else(|| anyhow!("User lacks display name"))?.to_owned(), + ) + .with_nick_name(new_external_id.clone()), + ); + + zitadel.update_human_user(user_id, request).await?; + + tracing::info!( + user_id = %user_id, + old_uid = %nick_name, + new_uid = %new_external_id, + "User migrated" + ); + } + + tracing::info!("Migration completed."); + Ok(()) +} diff --git a/tests/e2e.rs b/tests/e2e.rs index 0928a62..4dac79c 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -3,6 +3,7 @@ use std::{collections::HashSet, path::Path, time::Duration}; +use base64::{engine::general_purpose, Engine as _}; use famedly_sync::{ csv_test_helpers::temp_csv_file, perform_sync, @@ -756,9 +757,8 @@ async fn test_e2e_binary_uid() { match user.r#type { Some(UserType::Human(user)) => { let profile = user.profile.expect("user lacks profile"); - println!("profile: {:#?}", profile); // Verify ID was updated - assert_eq!(profile.nick_name, hex::encode(&new_binary_id)); + assert_eq!(profile.nick_name, hex::encode(new_binary_id)); } _ => panic!("user lost human details after update"), } @@ -784,7 +784,7 @@ async fn test_e2e_binary_uid() { Some(UserType::Human(user)) => { let profile = user.profile.expect("user lacks profile"); // Verify ID was updated - assert_eq!(profile.nick_name, hex::encode(&invalid_binary_id)); + assert_eq!(profile.nick_name, hex::encode(invalid_binary_id)); } _ => panic!("user lost human details after update"), } @@ -1348,6 +1348,112 @@ async fn test_e2e_ldap_with_ukt_sync() { assert!(user.is_err_and(|error| matches!(error, ZitadelError::TonicResponseError(status) if status.code() == TonicErrorCode::NotFound))); } +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_migrate_base64_id() { + let uid = "migrate_test"; + let email = "migrate_test@famedly.de"; + let user_name = "migrate_user"; + + // Base64-encoded External ID + let base64_id = general_purpose::STANDARD.encode(uid); + + run_migration_test(email, user_name, base64_id.clone(), hex::encode(uid.as_bytes())).await; +} + +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_migrate_plain_id() { + let uid = "plain_test"; + let email = "plain_test@famedly.de"; + let user_name = "plain_user"; + + // Plain unencoded External ID + let plain_id = uid.to_owned(); + + run_migration_test(email, user_name, plain_id.clone(), hex::encode(uid.as_bytes())).await; +} + +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_migrate_hex_id() { + let uid = "hex_test"; + let email = "hex_test@famedly.de"; + let user_name = "hex_user"; + + // Already hex-encoded External ID + let hex_id = hex::encode(uid.as_bytes()); + + run_migration_test(email, user_name, hex_id.clone(), hex_id.clone()).await; +} + +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_migrate_empty_id() { + let email = "empty_id@famedly.de"; + let user_name = "empty_user"; + + // Empty External ID + let empty_id = "".to_owned(); + + run_migration_test(email, user_name, empty_id.clone(), empty_id.clone()).await; +} + +#[test(tokio::test)] +#[test_log(default_log_filter = "debug")] +async fn test_e2e_migrate_then_ldap_sync() { + let uid = "migrate_sync_test"; + let email = "migrate_sync@famedly.de"; + let user_name = "migrate_sync_user"; + + // Base64-encoded ID + let base64_id = general_purpose::STANDARD.encode(uid); + + run_migration_test(email, user_name, base64_id.clone(), hex::encode(uid.as_bytes())).await; + + // LDAP with updated First Name + let config = ldap_config().await; + let mut ldap = Ldap::new().await; + ldap.create_user( + "New First Name", + "User", + "User, Test", // !NOTE: Display name from LDAP isn't picked up by the sync + email, + Some("+12345678901"), + uid, + false, + ) + .await; + + perform_sync(config).await.expect("LDAP sync failed"); + + // Verify both External ID encoding and updated First Name + let zitadel = open_zitadel_connection().await; + let user = zitadel + .get_user_by_login_name(user_name) + .await + .expect("Failed to get user after LDAP sync") + .expect("User not found after LDAP sync"); + + match user.r#type { + Some(UserType::Human(human)) => { + let profile = human.profile.expect("User lacks profile after LDAP sync"); + let expected_hex_id = hex::encode(uid.as_bytes()); + assert_eq!( + profile.nick_name, expected_hex_id, + "External ID not in hex encoding after LDAP sync for user '{}'", + email + ); + assert_eq!( + profile.first_name, "New First Name", + "Fist name was not updated by LDAP sync for user '{}'", + email + ); + } + _ => panic!("User lacks human details after LDAP sync for user '{}'", email), + } +} + struct Ldap { client: LdapClient, } @@ -1482,6 +1588,82 @@ async fn open_zitadel_connection() -> Zitadel { .expect("failed to set up Zitadel client") } +/// Helper function to create a user, run migration, and verify the encoding. +async fn run_migration_test( + email: &str, + user_name: &str, + initial_nick_name: String, + expected_nick_name: String, +) { + // Get config and Zitadel client + let config = ldap_config().await; + let zitadel = open_zitadel_connection().await; + + // Create user in Zitadel + let user = ImportHumanUserRequest { + user_name: user_name.to_owned(), + profile: Some(Profile { + first_name: "Test".to_owned(), + last_name: "User".to_owned(), + display_name: "User, Test".to_owned(), + gender: Gender::Unspecified.into(), + nick_name: initial_nick_name.clone(), + preferred_language: String::default(), + }), + email: Some(Email { email: email.to_owned(), is_email_verified: true }), + phone: Some(Phone { phone: "+12345678901".to_owned(), is_phone_verified: true }), + password: String::default(), + hashed_password: None, + password_change_required: false, + request_passwordless_registration: false, + otp_code: String::default(), + idps: vec![], + }; + + zitadel + .create_human_user(&config.zitadel.organization_id, user) + .await + .expect("Failed to create user"); + + // Run migration + run_migration_binary(); + + // Verify External ID after migration + let user = zitadel + .get_user_by_login_name(user_name) + .await + .expect("Failed to get user") + .expect("User not found"); + + match user.r#type { + Some(user_type) => { + if let UserType::Human(human) = user_type { + let profile = human.profile.expect("User lacks profile"); + assert_eq!( + profile.nick_name, expected_nick_name, + "Nickname encoding mismatch for user '{}'", + email + ); + } else { + panic!("User is not of type Human for user '{}'", email); + } + } + None => panic!("User type is None for user '{}'", email), + } +} + +/// Helper function to run the migration binary. +fn run_migration_binary() { + let mut config_path = std::env::current_dir().unwrap(); + config_path.push("tests/environment/config.yaml"); + std::env::set_var("FAMEDLY_SYNC_CONFIG", config_path); + + let status = std::process::Command::new(env!("CARGO_BIN_EXE_migrate")) + .status() + .expect("Failed to execute migration binary"); + assert!(status.success(), "Migration binary exited with status: {}", status); +} + /// Get the module's test environment config async fn ldap_config() -> &'static Config { CONFIG_WITH_LDAP