server: Add support for memberOf with plain user names, relax hard errors

This should help when the client sends some invalid-looking queries as part of a bigger filter

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,
+        expand_attribute_wildcards, get_custom_attribute,
-        get_user_id_from_distinguished_name, map_group_field, GroupFieldType, LdapInfo,
+        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> {
+) -> 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(
+                GroupFieldType::Uuid => Uuid::try_from(value.as_str())
-                    Uuid::try_from(value.as_str()).map_err(|e| LdapError {
+                    .map(GroupRequestFilter::Uuid)
-                        code: LdapResultCode::InappropriateMatching,
+                    .map_err(|e| LdapError {
+                        code: LdapResultCode::Other,
                         message: format!("Invalid UUID: {:#}", e),
-                    })?,
+                    }),
-                )),
+                GroupFieldType::Member => Ok(get_user_id_from_distinguished_name_or_plain_name(
-                GroupFieldType::Member => {
+                    &value,
-                    let user_name = get_user_id_from_distinguished_name(
+                    &ldap_info.base_dn,
-                        &value,
+                    &ldap_info.base_dn_str,
-                        &ldap_info.base_dn,
+                )
-                        &ldap_info.base_dn_str,
+                .map(GroupRequestFilter::Member)
-                    )?;
+                .unwrap_or_else(|e| {
-                    Ok(GroupRequestFilter::Member(user_name))
+                    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) => {
+                GroupFieldType::Attribute(field, typ, is_list) => Ok(
-                    get_group_attribute_equality_filter(&field, typ, is_list, &value)
+                    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(),

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,
+            expand_attribute_wildcards, get_custom_attribute,
-            get_user_id_from_distinguished_name, map_user_field, LdapInfo, UserFieldType,
+            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> {
+) -> 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) => {
+                UserFieldType::Attribute(field, typ, is_list) => Ok(
-                    get_user_attribute_equality_filter(&field, typ, is_list, &value)
+                    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(
+                UserFieldType::MemberOf => Ok(get_group_id_from_distinguished_name_or_plain_name(
-                    get_group_id_from_distinguished_name(
+                    &value,
-                        &value,
+                    &ldap_info.base_dn,
-                        &ldap_info.base_dn,
+                    &ldap_info.base_dn_str,
-                        &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,

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<UserId> {
+    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<GroupName> {
+    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],

server/src/infra/ldap_handler.rs

@@ -1837,8 +1837,8 @@ mod tests {
                 eq(Some(UserRequestFilter::MemberOf("group_1".into()))),
                 eq(false),
             )
-            .times(1)
+            .times(2)
-            .return_once(|_, _| Ok(vec![]));
+            .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 {
+            Ok(vec![make_search_success()])
-                code: LdapResultCode::InvalidDNSyntax,
-                message: "Missing DN value".to_string()
-            })
         );
+    }
+    #[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{
+            // The error is ignored, a warning is printed.
-                code: LdapResultCode::InvalidDNSyntax,
+            Ok(vec![make_search_success()])
-                message: r#"Unexpected DN format. Got "cn=mygroup,dc=example,dc=com", expected: "uid=id,ou=groups,dc=example,dc=com""#.to_string()
-            })
         );
     }