Skip to content

Commit

Permalink
feat: Add localpart to CSV import and use it as Zitadel user ID and l…
Browse files Browse the repository at this point in the history
…ocalpart metadata
  • Loading branch information
jannden committed Dec 13, 2024
1 parent f6ffe7c commit 69188a7
Show file tree
Hide file tree
Showing 8 changed files with 203 additions and 31 deletions.
25 changes: 25 additions & 0 deletions src/bin/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ mod tests {
true,
None,
external_user_id.to_owned(),
None,
)
}

Expand Down Expand Up @@ -296,4 +297,28 @@ mod tests {
// 'i'=0x69, 'd'=0x64 => "706c61696e5f6964"
run_conversion_test(original_id, ExternalIdEncoding::Plain, "706c61696e5f6964");
}

#[tokio::test]
async fn test_localpart_preservation() {
// Test that migration preserves localpart values
let original_user = SyncUser::new(
"first name".to_owned(),
"last name".to_owned(),
"[email protected]".to_owned(),
None,
true,
None,
"Y2FmZQ==".to_owned(), // base64 encoded external ID
Some("test.localpart".to_owned()), // localpart should be preserved
);

let migrated_user = original_user
.create_user_with_converted_external_id(ExternalIdEncoding::Base64)
.expect("Should successfully convert user");

// External ID should be converted from base64 to hex
assert_eq!(migrated_user.get_external_id(), hex::encode("cafe"));
// Localpart should remain unchanged
assert_eq!(migrated_user.get_localpart(), Some("test.localpart"));
}
}
8 changes: 8 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,15 @@ pub async fn get_next_zitadel_user(
.ok()
.and_then(|metadata| metadata.metadata().value());

let localpart = zitadel
.zitadel_client
.get_user_metadata(&zitadel_user.1, "localpart")
.await
.ok()
.and_then(|metadata| metadata.metadata().value());

zitadel_user.0.preferred_username = preferred_username;
zitadel_user.0.localpart = localpart;

Ok(Some(zitadel_user))
}
Expand Down
102 changes: 92 additions & 10 deletions src/sources/csv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ struct CsvData {
last_name: String,
/// The user's phone number
phone: String,
/// The user's localpart (optional)
#[serde(default)]
localpart: String,
}

impl CsvData {
Expand All @@ -81,6 +84,7 @@ impl CsvData {
preferred_username: Some(csv_data.email.clone()),
external_user_id: hex::encode(csv_data.email),
enabled: true,
localpart: (!csv_data.localpart.is_empty()).then_some(csv_data.localpart),
}
}
}
Expand Down Expand Up @@ -139,11 +143,11 @@ mod tests {
fn test_get_users() {
let mut config = load_config();
let csv_content = indoc! {r#"
email,first_name,last_name,phone
[email protected],John,Doe,+1111111111
[email protected],Jane,Smith,+2222222222
[email protected],Alice,Johnson,
[email protected],Bob,Williams,+4444444444
email,first_name,last_name,phone,localpart
[email protected],John,Doe,+1111111111,john.doe
[email protected],Jane,Smith,+2222222222,
[email protected],Alice,Johnson,,alice.johnson
[email protected],Bob,Williams,+4444444444,
"#};
let _file = test_helpers::temp_csv_file(&mut config, csv_content);

Expand All @@ -155,18 +159,60 @@ mod tests {

let users = result.expect("Failed to get users");
assert_eq!(users.len(), 4, "Unexpected number of users");

// Test user with localpart
assert_eq!(users[0].first_name, "John", "Unexpected first name at index 0");
assert_eq!(users[0].email, "[email protected]", "Unexpected email at index 0");
assert_eq!(users[3].last_name, "Williams", "Unexpected last name at index 3");
assert_eq!(
users[0].external_user_id,
hex::encode("[email protected]".as_bytes()),
"Unexpected external_user_id at index 0"

Check warning on line 169 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L169

Added line #L169 was not covered by tests
);
assert_eq!(
users[0].localpart,
Some("john.doe".to_owned()),
"Unexpected localpart at index 0"

Check warning on line 174 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L174

Added line #L174 was not covered by tests
);

// Test user without localpart (empty string)
assert_eq!(users[1].email, "[email protected]", "Unexpected email at index 1");
assert_eq!(
users[1].external_user_id,
hex::encode("[email protected]".as_bytes()),
"Unexpected external_user_id at index 1"

Check warning on line 182 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L182

Added line #L182 was not covered by tests
);
assert_eq!(users[1].localpart, None, "Unexpected localpart at index 1");

// Test user with localpart but no phone
assert_eq!(users[2].email, "[email protected]", "Unexpected email at index 2");
assert_eq!(
users[2].external_user_id,
hex::encode("[email protected]".as_bytes()),
"Unexpected external_user_id at index 2"

Check warning on line 191 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L191

Added line #L191 was not covered by tests
);
assert_eq!(
users[2].localpart,
Some("alice.johnson".to_owned()),
"Unexpected localpart at index 2"

Check warning on line 196 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L196

Added line #L196 was not covered by tests
);
assert_eq!(users[2].phone, None, "Unexpected phone at index 2");

// Test user without localpart (empty string) but with phone
assert_eq!(users[3].email, "[email protected]", "Unexpected email at index 3");
assert_eq!(
users[3].external_user_id,
hex::encode("[email protected]".as_bytes()),
"Unexpected external_user_id at index 3"

Check warning on line 205 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L205

Added line #L205 was not covered by tests
);
assert_eq!(users[3].localpart, None, "Unexpected localpart at index 3");
assert_eq!(users[3].phone, Some("+4444444444".to_owned()), "Unexpected phone at index 3");
}

#[test]
fn test_get_users_empty_file() {
let mut config = load_config();
let csv_content = indoc! {r#"
email,first_name,last_name,phone
email,first_name,last_name,phone,localpart
"#};
let _file = test_helpers::temp_csv_file(&mut config, csv_content);

Expand Down Expand Up @@ -204,7 +250,7 @@ mod tests {
let mut config = load_config();
let csv_content = indoc! {r#"
first_name
[email protected],John,Doe,+1111111111
[email protected],John,Doe,+1111111111,john.doe
"#};
let _file = test_helpers::temp_csv_file(&mut config, csv_content);

Expand All @@ -220,9 +266,9 @@ mod tests {
fn test_get_users_invalid_content() {
let mut config = load_config();
let csv_content = indoc! {r#"
email,first_name,last_name,phone
email,first_name,last_name,phone,localpart
[email protected]
[email protected],Jane,Smith,+2222222222
[email protected],Jane,Smith,+2222222222,jane.smith
"#};
let _file = test_helpers::temp_csv_file(&mut config, csv_content);

Expand All @@ -236,5 +282,41 @@ mod tests {
assert_eq!(users.len(), 1, "Unexpected number of users");
assert_eq!(users[0].email, "[email protected]", "Unexpected email at index 0");
assert_eq!(users[0].last_name, "Smith", "Unexpected last name at index 0");
assert_eq!(
users[0].external_user_id,
hex::encode("[email protected]".as_bytes()),
"Unexpected external_user_id at index 0"

Check warning on line 288 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L288

Added line #L288 was not covered by tests
);
assert_eq!(
users[0].localpart,
Some("jane.smith".to_owned()),
"Unexpected localpart at index 0"

Check warning on line 293 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L293

Added line #L293 was not covered by tests
);
}

#[test]
fn test_backward_compatibility() {
// Test that old CSV format without localpart column still works
let mut config = load_config();
let csv_content = indoc! {r#"
email,first_name,last_name,phone
[email protected],John,Doe,+1111111111
[email protected],Jane,Smith,+2222222222
"#};
let _file = test_helpers::temp_csv_file(&mut config, csv_content);

let csv_config = config.sources.csv.expect("CsvSource configuration is missing");
let csv = CsvSource::new(csv_config);

let result = csv.read_csv();
assert!(result.is_ok(), "Failed to get users: {:?}", result);

let users = result.expect("Failed to get users");
assert_eq!(users.len(), 2, "Unexpected number of users");
// All users should have None localpart
assert!(
users.iter().all(|u| u.localpart.is_none()),
"Expected all users to have None localpart"

Check warning on line 319 in src/sources/csv.rs

View check run for this annotation

Codecov / codecov/patch

src/sources/csv.rs#L319

Added line #L319 was not covered by tests
);
}
}
1 change: 1 addition & 0 deletions src/sources/ldap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ impl LdapSource {
external_user_id: ldap_user_id,
phone,
enabled,
localpart: None,
})
}
}
Expand Down
25 changes: 23 additions & 2 deletions src/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@ pub struct User {
pub(crate) preferred_username: Option<String>,
/// The user's external (non-Zitadel) ID
pub(crate) external_user_id: String,
/// The user's localpart (used as Zitadel userId)
pub(crate) localpart: Option<String>,
}

impl User {
/// Create a new user instance, used in tests
#[allow(clippy::must_use_candidate)]
#[allow(clippy::must_use_candidate, clippy::too_many_arguments)]
pub fn new(
first_name: String,
last_name: String,
Expand All @@ -50,8 +52,18 @@ impl User {
enabled: bool,
preferred_username: Option<String>,
external_user_id: String,
localpart: Option<String>,
) -> Self {
Self { first_name, last_name, email, phone, enabled, preferred_username, external_user_id }
Self {
first_name,
last_name,
email,
phone,
enabled,
preferred_username,
external_user_id,
localpart,
}
}

/// Convert a Zitadel user to our internal representation
Expand Down Expand Up @@ -84,6 +96,7 @@ impl User {
preferred_username: None,
external_user_id: external_id,
enabled: true,
localpart: None,
})
}

Expand All @@ -93,6 +106,12 @@ impl User {
format!("{}, {}", self.last_name, self.first_name)
}

/// Get the localpart
#[must_use]
pub fn get_localpart(&self) -> Option<&str> {
self.localpart.as_deref()
}

/// Get the external user ID
#[must_use]
pub fn get_external_id(&self) -> &str {
Expand Down Expand Up @@ -210,6 +229,7 @@ impl PartialEq for User {
&& self.enabled == other.enabled
&& self.preferred_username == other.preferred_username
&& self.external_user_id == other.external_user_id
&& self.localpart == other.localpart
}
}

Expand All @@ -222,6 +242,7 @@ impl std::fmt::Debug for User {
.field("phone", &"***")
.field("preferred_username", &"***")
.field("external_user_id", &self.external_user_id)
.field("localpart", &self.localpart)
.field("enabled", &self.enabled)
.finish()
}
Expand Down
22 changes: 18 additions & 4 deletions src/zitadel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,17 @@ impl Zitadel {
return Ok(());
}

let localpart = if self.feature_flags.contains(&FeatureFlag::PlainLocalpart) {
// Use the localpart from the user if available, otherwise generate one
let localpart = if let Some(localpart) = &imported_user.localpart {
localpart.clone()
} else if self.feature_flags.contains(&FeatureFlag::PlainLocalpart) {
String::from_utf8(imported_user.get_external_id_bytes()?)
.context(format!("Unsupported binary external ID for user: {:?}", imported_user))?
} else {
imported_user.get_famedly_uuid()?
};

let mut metadata = vec![SetMetadataEntry::new("localpart".to_owned(), localpart)];
let mut metadata = vec![SetMetadataEntry::new("localpart".to_owned(), localpart.clone())];

if let Some(preferred_username) = imported_user.preferred_username.clone() {
metadata
Expand All @@ -185,7 +188,8 @@ impl Zitadel {
.with_organization(
Organization::new().with_org_id(self.zitadel_config.organization_id.clone()),
)
.with_metadata(metadata);
.with_metadata(metadata)
.with_user_id(localpart); // Set the Zitadel userId to the localpart

if let Some(phone) = imported_user.phone.clone() {
user.set_phone(
Expand Down Expand Up @@ -250,6 +254,16 @@ impl Zitadel {
updated_user.external_user_id
);

// Check if localpart has changed and emit warning if it has
if old_user.localpart != updated_user.localpart {
tracing::warn!(
"Cannot update Zitadel userId (localpart) for user {:?} having old localpart: {:?}, and new localpart: {:?}",
old_user,
old_user.localpart,
updated_user.localpart
);
}

if self.feature_flags.is_enabled(FeatureFlag::DryRun) {
tracing::warn!("Skipping update due to dry run");
return Ok(());
Expand Down Expand Up @@ -336,7 +350,7 @@ pub fn search_result_to_user(user: ZitadelUser) -> Result<User> {
.ok_or(anyhow!("Missing external ID found for user"))?;

// TODO: If async closures become a reality, we
// should capture the correct preferred_username
// should capture the correct preferred_username and localpart from metadata
// here.
let user = User::try_from_zitadel_user(human_user.clone(), nick_name.clone())?;
Ok(user)
Expand Down
Loading

0 comments on commit 69188a7

Please sign in to comment.