From 75ed0bf4fa124fbe73ebc3ac394a488d7b837885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tristan=20Dani=C3=ABl=20Maat?= Date: Thu, 8 Aug 2024 18:19:43 +0200 Subject: [PATCH] feat!: Make using attribute filters optional --- Cargo.lock | 9 +- Cargo.toml | 1 + config.sample.yaml | 6 ++ src/config.rs | 125 ++++++++++++++++++++----- tests/environment/config.template.yaml | 1 + 5 files changed, 115 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c4d0123..79f1b3d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1055,6 +1055,12 @@ dependencies = [ "serde", ] +[[package]] +name = "indoc" +version = "2.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b248f5224d1d606005e02c97f5aa4e88eeb230488bcc03bc9ca4d7991399f2b5" + [[package]] name = "ipnet" version = "2.9.0" @@ -1155,7 +1161,7 @@ dependencies = [ [[package]] name = "ldap-poller" version = "0.1.0" -source = "git+https://github.com/famedly/ldap-poller#033dd7187d41e33aaf5afc600f69774711aea9a9" +source = "git+https://github.com/famedly/ldap-poller#96dfa724a618b1f0f6caec83f4e261366d2e84e1" dependencies = [ "ldap3", "native-tls", @@ -1175,6 +1181,7 @@ dependencies = [ "anyhow", "base64 0.22.1", "bincode", + "indoc", "itertools 0.13.0", "ldap-poller", "ldap3", diff --git a/Cargo.toml b/Cargo.toml index b65c587..8e29e9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -116,6 +116,7 @@ useless_let_if_seq = "warn" verbose_file_reads = "warn" [dev-dependencies] +indoc = "2.0.5" ldap3 = { version = "0.11.1", default-features = false, features = ["tls-native"] } tempfile = "3.10.1" test-log = { version = "0.2.16", features = ["trace", "unstable"] } diff --git a/config.sample.yaml b/config.sample.yaml index 40d9d0f..27e3ed9 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -18,6 +18,12 @@ ldap: timeout: 5 # Whether to sync entry deletion. check_for_deleted_entries: true + # Whether to filter for the specific attributes used. Some LDAP + # implementations misbehave if this is not done, others misbehave if + # it is done. + # + # Default is false. + use_attribute_filter: true # A mapping of the LDAP attributes to Famedly attributes. This is # different for different LDAP server implementations and # organizations, so needs to be configured on a case-by-case basis. diff --git a/src/config.rs b/src/config.rs index a0b82c9..9b27e7a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -53,32 +53,6 @@ fn validate_famedly_url(url: Url) -> Result { Ok(url) } -#[cfg(test)] -mod tests { - #![allow(clippy::expect_used)] - use super::*; - - #[test] - fn test_famedly_url_validate_valid() { - let url = Url::parse("https://famedly.de").expect("invalid url"); - let validated = validate_famedly_url(url).expect("url failed to validate"); - assert_eq!(validated.to_string(), "https://famedly.de/"); - } - - #[test] - fn test_famedly_url_validate_trailing_slash_path() { - let url = Url::parse("https://famedly.de/test/").expect("invalid url"); - let validated = validate_famedly_url(url).expect("url failed to validate"); - assert_eq!(validated.to_string(), "https://famedly.de/test/"); - } - - #[test] - fn test_famedly_url_validate_scheme() { - let url = Url::parse("famedly.de:443").expect("invalid url"); - assert!(validate_famedly_url(url).is_err()); - } -} - /// Configuration for the sync client #[derive(Debug, Clone, Deserialize)] pub struct Config { @@ -115,6 +89,11 @@ pub struct LdapConfig { pub attributes: LdapAttributesMapping, /// Whether to update deleted entries pub check_for_deleted_entries: bool, + /// Whether to ask LDAP for specific attributes or just specify *. + /// + /// Various implementations either do or don't send data in both + /// cases, so this needs to be tested against the actual server. + pub use_attribute_filter: bool, /// TLS-related configuration pub tls: Option, } @@ -156,6 +135,7 @@ impl From for ldap_poller::Config { pid: attributes.user_id.get_name(), updated: attributes.last_modified.map(AttributeMapping::get_name), additional: vec![], + filter_attributes: cfg.use_attribute_filter, attrs_to_track: vec![ attributes.status.get_name(), attributes.first_name.get_name(), @@ -291,3 +271,96 @@ pub enum FeatureFlag { /// If users should verify the phone. Users will receive a verification sms VerifyPhone, } + +#[cfg(test)] +mod tests { + #![allow(clippy::expect_used, clippy::unwrap_used)] + use indoc::indoc; + + use super::*; + + const EXAMPLE_CONFIG: &str = indoc! {r#" + 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" + last_name: "sn" + preferred_username: "displayName" + email: "mail" + phone: "telephoneNumber" + user_id: "uid" + status: + name: "shadowInactive" + is_binary: false + enable_value: 512 + disable_value: 514 + 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 + + famedly: + url: http://localhost:8080 + key_file: tests/environment/zitadel/service-user.json + organization_id: 1 + project_id: 1 + idp_id: 1 + + feature_flags: [] + cache_path: ./test + "#}; + + fn example_config() -> Config { + serde_yaml::from_str(EXAMPLE_CONFIG).expect("invalid config") + } + + #[test] + fn test_famedly_url_validate_valid() { + let url = Url::parse("https://famedly.de").expect("invalid url"); + let validated = validate_famedly_url(url).expect("url failed to validate"); + assert_eq!(validated.to_string(), "https://famedly.de/"); + } + + #[test] + fn test_famedly_url_validate_trailing_slash_path() { + let url = Url::parse("https://famedly.de/test/").expect("invalid url"); + let validated = validate_famedly_url(url).expect("url failed to validate"); + assert_eq!(validated.to_string(), "https://famedly.de/test/"); + } + + #[test] + fn test_famedly_url_validate_scheme() { + let url = Url::parse("famedly.de:443").expect("invalid url"); + assert!(validate_famedly_url(url).is_err()); + } + + #[test] + fn test_attribute_filter_use() { + let config = example_config(); + + assert_eq!( + Into::::into(config.ldap).attributes.get_attr_filter(), + vec!["uid", "shadowInactive", "cn", "sn", "displayName", "mail", "telephoneNumber"] + ); + } + + #[test] + fn test_no_attribute_filters() { + let mut config = example_config(); + config.ldap.use_attribute_filter = false; + + assert_eq!( + Into::::into(config.ldap).attributes.get_attr_filter(), + vec!["*"] + ); + } +} diff --git a/tests/environment/config.template.yaml b/tests/environment/config.template.yaml index 042b74b..95444d9 100644 --- a/tests/environment/config.template.yaml +++ b/tests/environment/config.template.yaml @@ -6,6 +6,7 @@ ldap: 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