From c2eed8909ac8b052e6ae1a1859e23def34fcddaf Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Mon, 22 Jan 2024 23:13:14 +0100 Subject: [PATCH] server: Only call expand_attributes at most once per request --- server/src/domain/ldap/group.rs | 12 ++++++++---- server/src/domain/ldap/user.rs | 10 +++++++--- server/src/domain/ldap/utils.rs | 24 ++++++++++++------------ 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/server/src/domain/ldap/group.rs b/server/src/domain/ldap/group.rs index d59a78d..b41fc19 100644 --- a/server/src/domain/ldap/group.rs +++ b/server/src/domain/ldap/group.rs @@ -100,13 +100,11 @@ fn expand_group_attribute_wildcards(attributes: &[String]) -> Vec<&str> { fn make_ldap_search_group_result_entry( group: Group, base_dn_str: &str, - attributes: &[String], + expanded_attributes: &[&str], user_filter: &Option, ignored_group_attributes: &[AttributeName], schema: &PublicSchema, ) -> LdapSearchResultEntry { - let expanded_attributes = expand_group_attribute_wildcards(attributes); - LdapSearchResultEntry { dn: format!("cn={},ou=groups,{}", group.display_name, base_dn_str), attributes: expanded_attributes @@ -267,11 +265,17 @@ pub fn convert_groups_to_ldap_op<'a>( user_filter: &'a Option, schema: &'a PublicSchema, ) -> impl Iterator + 'a { + let expanded_attributes = if groups.is_empty() { + None + } else { + Some(expand_group_attribute_wildcards(attributes)) + }; + groups.into_iter().map(move |g| { LdapOp::SearchResultEntry(make_ldap_search_group_result_entry( g, &ldap_info.base_dn_str, - attributes, + expanded_attributes.as_ref().unwrap(), user_filter, &ldap_info.ignored_group_attributes, schema, diff --git a/server/src/domain/ldap/user.rs b/server/src/domain/ldap/user.rs index f6b2fe8..925e888 100644 --- a/server/src/domain/ldap/user.rs +++ b/server/src/domain/ldap/user.rs @@ -119,12 +119,11 @@ const ALL_USER_ATTRIBUTE_KEYS: &[&str] = &[ fn make_ldap_search_user_result_entry( user: User, base_dn_str: &str, - attributes: &[String], + expanded_attributes: &[&str], groups: Option<&[GroupDetails]>, ignored_user_attributes: &[AttributeName], schema: &PublicSchema, ) -> LdapSearchResultEntry { - let expanded_attributes = expand_user_attribute_wildcards(attributes); let dn = format!("uid={},ou=people,{}", user.user_id.as_str(), base_dn_str); LdapSearchResultEntry { dn, @@ -295,11 +294,16 @@ pub fn convert_users_to_ldap_op<'a>( ldap_info: &'a LdapInfo, schema: &'a PublicSchema, ) -> impl Iterator + 'a { + let expanded_attributes = if users.is_empty() { + None + } else { + Some(expand_user_attribute_wildcards(attributes)) + }; users.into_iter().map(move |u| { LdapOp::SearchResultEntry(make_ldap_search_user_result_entry( u.user, &ldap_info.base_dn_str, - attributes, + expanded_attributes.as_ref().unwrap(), u.groups.as_deref(), &ldap_info.ignored_user_attributes, schema, diff --git a/server/src/domain/ldap/utils.rs b/server/src/domain/ldap/utils.rs index a1696ac..ad580bf 100644 --- a/server/src/domain/ldap/utils.rs +++ b/server/src/domain/ldap/utils.rs @@ -114,21 +114,21 @@ pub fn expand_attribute_wildcards<'a>( ldap_attributes: &'a [String], all_attribute_keys: &'a [&'static str], ) -> Vec<&'a str> { - let mut attributes_out = ldap_attributes + let extra_attributes = + if ldap_attributes.iter().any(|x| x == "*") || ldap_attributes.is_empty() { + all_attribute_keys + } else { + &[] + } .iter() - .map(String::as_str) - .collect::>(); - - if attributes_out.iter().any(|&x| x == "*") || attributes_out.is_empty() { - // Remove occurrences of '*' - attributes_out.retain(|&x| x != "*"); - // Splice in all non-operational attributes - attributes_out.extend(all_attribute_keys.iter()); - } + .copied(); + let attributes_out = ldap_attributes + .iter() + .map(|s| s.as_str()) + .filter(|&s| s != "*" && s != "+" && s != "1.1"); // Deduplicate, preserving order - let resolved_attributes = attributes_out - .into_iter() + let resolved_attributes = itertools::chain(attributes_out, extra_attributes) .unique_by(|a| a.to_ascii_lowercase()) .collect_vec(); debug!(?resolved_attributes);