diff --git a/server/src/domain/ldap/group.rs b/server/src/domain/ldap/group.rs index ea5cc40..313d40d 100644 --- a/server/src/domain/ldap/group.rs +++ b/server/src/domain/ldap/group.rs @@ -15,8 +15,10 @@ use crate::domain::{ use super::{ error::LdapResult, utils::{ - expand_attribute_wildcards, get_custom_attribute, get_group_id_from_distinguished_name, - get_user_id_from_distinguished_name, map_group_field, GroupFieldType, LdapInfo, + expand_attribute_wildcards, get_custom_attribute, + get_group_id_from_distinguished_name_or_plain_name, + get_user_id_from_distinguished_name_or_plain_name, map_group_field, GroupFieldType, + LdapInfo, }, }; @@ -142,13 +144,13 @@ fn get_group_attribute_equality_filter( typ: AttributeType, is_list: bool, value: &str, -) -> LdapResult { +) -> GroupRequestFilter { deserialize_attribute_value(&[value.to_owned()], typ, is_list) - .map_err(|e| LdapError { - code: LdapResultCode::Other, - message: format!("Invalid value for attribute {}: {}", field, e), - }) .map(|v| GroupRequestFilter::AttributeEquality(field.clone(), v)) + .unwrap_or_else(|e| { + warn!("Invalid value for attribute {}: {}", field, e); + GroupRequestFilter::from(false) + }) } fn convert_group_filter( @@ -163,20 +165,22 @@ fn convert_group_filter( let value = value.to_ascii_lowercase(); match map_group_field(&field, schema) { GroupFieldType::DisplayName => Ok(GroupRequestFilter::DisplayName(value.into())), - GroupFieldType::Uuid => Ok(GroupRequestFilter::Uuid( - Uuid::try_from(value.as_str()).map_err(|e| LdapError { - code: LdapResultCode::InappropriateMatching, + GroupFieldType::Uuid => Uuid::try_from(value.as_str()) + .map(GroupRequestFilter::Uuid) + .map_err(|e| LdapError { + code: LdapResultCode::Other, message: format!("Invalid UUID: {:#}", e), - })?, - )), - GroupFieldType::Member => { - let user_name = get_user_id_from_distinguished_name( - &value, - &ldap_info.base_dn, - &ldap_info.base_dn_str, - )?; - Ok(GroupRequestFilter::Member(user_name)) - } + }), + GroupFieldType::Member => Ok(get_user_id_from_distinguished_name_or_plain_name( + &value, + &ldap_info.base_dn, + &ldap_info.base_dn_str, + ) + .map(GroupRequestFilter::Member) + .unwrap_or_else(|e| { + warn!("Invalid member filter on group: {}", e); + GroupRequestFilter::from(false) + })), GroupFieldType::ObjectClass => Ok(GroupRequestFilter::from( matches!(value.as_str(), "groupofuniquenames" | "groupofnames") || schema @@ -185,7 +189,7 @@ fn convert_group_filter( .contains(&LdapObjectClass::from(value)), )), GroupFieldType::Dn | GroupFieldType::EntryDn => { - Ok(get_group_id_from_distinguished_name( + Ok(get_group_id_from_distinguished_name_or_plain_name( value.as_str(), &ldap_info.base_dn, &ldap_info.base_dn_str, @@ -206,9 +210,9 @@ fn convert_group_filter( } Ok(GroupRequestFilter::from(false)) } - GroupFieldType::Attribute(field, typ, is_list) => { - get_group_attribute_equality_filter(&field, typ, is_list, &value) - } + GroupFieldType::Attribute(field, typ, is_list) => Ok( + get_group_attribute_equality_filter(&field, typ, is_list, &value), + ), GroupFieldType::CreationDate => Err(LdapError { code: LdapResultCode::UnwillingToPerform, message: "Creation date filter for groups not supported".to_owned(), diff --git a/server/src/domain/ldap/user.rs b/server/src/domain/ldap/user.rs index a16836f..bbd6c69 100644 --- a/server/src/domain/ldap/user.rs +++ b/server/src/domain/ldap/user.rs @@ -10,8 +10,10 @@ use crate::domain::{ ldap::{ error::{LdapError, LdapResult}, utils::{ - expand_attribute_wildcards, get_custom_attribute, get_group_id_from_distinguished_name, - get_user_id_from_distinguished_name, map_user_field, LdapInfo, UserFieldType, + expand_attribute_wildcards, get_custom_attribute, + get_group_id_from_distinguished_name_or_plain_name, + get_user_id_from_distinguished_name_or_plain_name, map_user_field, LdapInfo, + UserFieldType, }, }, schema::{PublicSchema, SchemaUserAttributeExtractor}, @@ -165,13 +167,13 @@ fn get_user_attribute_equality_filter( typ: AttributeType, is_list: bool, value: &str, -) -> LdapResult { +) -> UserRequestFilter { deserialize_attribute_value(&[value.to_owned()], typ, is_list) - .map_err(|e| LdapError { - code: LdapResultCode::Other, - message: format!("Invalid value for attribute {}: {}", field, e), - }) .map(|v| UserRequestFilter::AttributeEquality(field.clone(), v)) + .unwrap_or_else(|e| { + warn!("Invalid value for attribute {}: {}", field, e); + UserRequestFilter::from(false) + }) } fn convert_user_filter( @@ -200,9 +202,9 @@ fn convert_user_filter( value, )), UserFieldType::PrimaryField(field) => Ok(UserRequestFilter::Equality(field, value)), - UserFieldType::Attribute(field, typ, is_list) => { - get_user_attribute_equality_filter(&field, typ, is_list, &value) - } + UserFieldType::Attribute(field, typ, is_list) => Ok( + get_user_attribute_equality_filter(&field, typ, is_list, &value), + ), UserFieldType::NoMatch => { if !ldap_info.ignored_user_attributes.contains(&field) { warn!( @@ -222,15 +224,18 @@ fn convert_user_filter( .extra_user_object_classes .contains(&LdapObjectClass::from(value)), )), - UserFieldType::MemberOf => Ok(UserRequestFilter::MemberOf( - get_group_id_from_distinguished_name( - &value, - &ldap_info.base_dn, - &ldap_info.base_dn_str, - )?, - )), + UserFieldType::MemberOf => Ok(get_group_id_from_distinguished_name_or_plain_name( + &value, + &ldap_info.base_dn, + &ldap_info.base_dn_str, + ) + .map(UserRequestFilter::MemberOf) + .unwrap_or_else(|e| { + warn!("Invalid memberOf filter: {}", e); + UserRequestFilter::from(false) + })), UserFieldType::EntryDn | UserFieldType::Dn => { - Ok(get_user_id_from_distinguished_name( + Ok(get_user_id_from_distinguished_name_or_plain_name( value.as_str(), &ldap_info.base_dn, &ldap_info.base_dn_str, diff --git a/server/src/domain/ldap/utils.rs b/server/src/domain/ldap/utils.rs index ad580bf..835213c 100644 --- a/server/src/domain/ldap/utils.rs +++ b/server/src/domain/ldap/utils.rs @@ -109,6 +109,34 @@ pub fn get_group_id_from_distinguished_name( get_id_from_distinguished_name(dn, base_tree, base_dn_str, true).map(GroupName::from) } +fn looks_like_distinguished_name(dn: &str) -> bool { + dn.contains('=') || dn.contains(',') +} + +pub fn get_user_id_from_distinguished_name_or_plain_name( + dn: &str, + base_tree: &[(String, String)], + base_dn_str: &str, +) -> LdapResult { + if !looks_like_distinguished_name(dn) { + Ok(UserId::from(dn)) + } else { + get_user_id_from_distinguished_name(dn, base_tree, base_dn_str) + } +} + +pub fn get_group_id_from_distinguished_name_or_plain_name( + dn: &str, + base_tree: &[(String, String)], + base_dn_str: &str, +) -> LdapResult { + if !looks_like_distinguished_name(dn) { + Ok(GroupName::from(dn)) + } else { + get_group_id_from_distinguished_name(dn, base_tree, base_dn_str) + } +} + #[instrument(skip(all_attribute_keys), level = "debug")] pub fn expand_attribute_wildcards<'a>( ldap_attributes: &'a [String], diff --git a/server/src/infra/ldap_handler.rs b/server/src/infra/ldap_handler.rs index 510a33f..42d2234 100644 --- a/server/src/infra/ldap_handler.rs +++ b/server/src/infra/ldap_handler.rs @@ -1837,8 +1837,8 @@ mod tests { eq(Some(UserRequestFilter::MemberOf("group_1".into()))), eq(false), ) - .times(1) - .return_once(|_, _| Ok(vec![])); + .times(2) + .returning(|_, _| Ok(vec![])); let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_user_search_request( LdapFilter::Equality( @@ -1857,11 +1857,17 @@ mod tests { ); assert_eq!( ldap_handler.do_search_or_dse(&request).await, - Err(LdapError { - code: LdapResultCode::InvalidDNSyntax, - message: "Missing DN value".to_string() - }) + Ok(vec![make_search_success()]) ); + } + #[tokio::test] + async fn test_search_member_of_filter_error() { + let mut mock = MockTestBackendHandler::new(); + mock.expect_list_users() + .with(eq(Some(UserRequestFilter::from(false))), eq(false)) + .times(1) + .returning(|_, _| Ok(vec![])); + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_user_search_request( LdapFilter::Equality( "memberOf".to_string(), @@ -1871,10 +1877,8 @@ mod tests { ); assert_eq!( ldap_handler.do_search_or_dse(&request).await, - Err(LdapError{ - code: LdapResultCode::InvalidDNSyntax, - message: r#"Unexpected DN format. Got "cn=mygroup,dc=example,dc=com", expected: "uid=id,ou=groups,dc=example,dc=com""#.to_string() - }) + // The error is ignored, a warning is printed. + Ok(vec![make_search_success()]) ); }