refactor: Reuse typed LDAP attributes in search results

src/ldap/attr.rs

@@ -0,0 +1,66 @@
+use std::fmt;
+use std::str::FromStr;
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct UnknownLdapAttribute(String);
+
+impl fmt::Display for UnknownLdapAttribute {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "Unknown LDAP attribute: {}", self.0)
+    }
+}
+
+impl std::error::Error for UnknownLdapAttribute {}
+
+/// An LDAP attribute that is requested, or requested to be matched against an entry.
+///
+/// Attributes are case-insensitive when parsing from a string.
+///
+/// In the future, we may want to support custom attributes, but that is not
+/// implemented for now.
+#[derive(Clone, Debug, PartialEq)]
+pub enum LdapAttribute {
+    Uid,
+    CommonName,
+    MemberOf,
+    ObjectClass,
+    Mail,
+    MailAlias,
+}
+
+impl FromStr for LdapAttribute {
+    type Err = UnknownLdapAttribute;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s.to_lowercase().as_str() {
+            "uid" => Ok(Self::Uid),
+            "cn" => Ok(Self::CommonName),
+            "memberof" => Ok(Self::MemberOf),
+            "objectclass" => Ok(Self::ObjectClass),
+            "mail" => Ok(Self::Mail),
+            "mailalias" => Ok(Self::MailAlias),
+            _ => Err(UnknownLdapAttribute(s.to_string())),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn uppercased_attribute() {
+        let s = "MemberOF";
+        let attr = LdapAttribute::from_str(s);
+        println!("{attr:?}");
+        assert_eq!(attr.unwrap(), LdapAttribute::MemberOf);
+    }
+
+    #[test]
+    fn unknown_attribute() {
+        let s = "foobar";
+        let attr = LdapAttribute::from_str(s);
+        println!("{attr:?}");
+        assert_eq!(attr.unwrap_err(), UnknownLdapAttribute(s.to_string()));
+    }
+}

src/ldap/mod.rs

@@ -1,3 +1,5 @@
+mod attr;
+pub use attr::LdapAttribute;
 mod client_state;
 pub use client_state::LdapClientState;
 mod dn;

src/ldap/op/search.rs

@@ -4,9 +4,11 @@ use ldap3_proto::proto::{
 };
 use ldap3_proto::{LdapMsg, LdapResultCode};
 
+use std::str::FromStr;
+
 use crate::db::error::BoxedError;
 use crate::db::{Database, DatabaseInterface, User};
-use crate::ldap::{Dn, LdapReturnError, LdapStream, LdapStreamError, MalformedDn};
+use crate::ldap::{Dn, LdapAttribute, LdapReturnError, LdapStream, LdapStreamError, MalformedDn};
 
 #[derive(Debug)]
 pub struct InvalidSearchDn {
@@ -146,30 +148,42 @@ pub async fn search_success(
 
 fn search_entry_from_user(user: &User, req_attrs: &[String]) -> LdapSearchResultEntry {
     let mut res: Vec<LdapPartialAttribute> = vec![];
-    for attr in req_attrs {
+    for attr_str in req_attrs {
-        if let Some(attr_values) = match attr.as_str() {
+        // We keep a copy of the requested attribute so that we can answer as it was requested,
-            "uid" => Some(vec![user.username.clone()]),
+        // eg. `CN` => `CN` (instead of normalizing to `cn`).
-            "cn" | "mail" => Some(vec![user.mail.clone()]),
+        let Some(attr) = LdapAttribute::from_str(attr_str)
-            // TODO: group membership
+            .inspect_err(|e| tracing::debug!("Unknown attribute in request: {e}"))
-            "memberof" => Some(vec![]),
+            .ok()
-            // Copied from lldap output, not sure if we want to add/remove some classes depending on context
+        else {
-            "objectclass" => Some(
+            continue;
-                vec!["inetOrgPerson", "posixAccount", "mailAccount", "person"]
+        };
-                    .into_iter()
+
-                    .map(String::from)
+        let str_values = match attr {
-                    .collect(),
+            // TODO: should we normalize the value some more here?
-            ),
+            LdapAttribute::Uid => vec![user.username.clone()],
-            _ => {
+            // TODO: should CN be different than the mail?
-                tracing::warn!("Ignoring unknown attr in search query: {attr}");
+            // TODO: should we normalize values some more here?
-                None
+            LdapAttribute::CommonName | LdapAttribute::Mail | LdapAttribute::MailAlias => {
+                vec![user.mail.clone()]
             }
-        } {
+            // TODO: group membership
-            res.push(LdapPartialAttribute {
+            LdapAttribute::MemberOf => vec![],
-                atype: attr.clone(),
+            // TODO: if we introduce mail permission, we need to remove mailaccount from here
-                // LDAP response expects raw byte vec for each value
+            LdapAttribute::ObjectClass => vec![
-                vals: attr_values.into_iter().map(Vec::from).collect(),
+                "inetOrgPerson".to_string(),
-            });
+                "posixAccount".to_string(),
-        }
+                "mailAccount".to_string(),
+                "person".to_string(),
+            ],
+        };
+
+        res.push(LdapPartialAttribute {
+            // Reuse the requested attribute name, not the normalized form
+            // we would otherwise produce.
+            atype: attr_str.clone(),
+            // LDAP response expects raw byte vec for each value
+            vals: str_values.into_iter().map(Vec::from).collect(),
+        });
     }
 
     LdapSearchResultEntry {
@@ -252,24 +266,36 @@ pub fn user_matches_filter(user: &User, filter: &LdapFilter) -> bool {
             false
         }
         LdapFilter::Not(sub_filter) => !user_matches_filter(user, sub_filter),
-        LdapFilter::Equality(attr, value) => match attr.as_ref() {
+        LdapFilter::Equality(attr, value) => LdapAttribute::from_str(attr).map_or_else(
-            "uid" => user.username == *value,
+            |e| {
-            // TODO: should CN be different than the mail?
+                tracing::warn!("Unrecognized attribute, considering no match: {e}");
-            "cn" | "mail" | "mailAlias" => user.mail == *value,
-            // TODO: group membership
-            "memberof" => false,
-            "objectClass" => matches!(
-                value.as_ref(),
-                "inetOrgPerson" | "posixAccount" | "mailAccount" | "person"
-            ),
-            _ => {
-                tracing::warn!("Unknown user attribute filter, considering no match: {attr}");
                 false
-            }
+            },
-        },
+            |attr| user_matches_attribute(user, &attr, value),
+        ),
         _ => {
             tracing::warn!("Unimplemented search filter, considering no match: {filter:?}");
             false
         }
     }
 }
+
+// TODO: we want to support group relations here as argument soon
+pub fn user_matches_attribute(user: &User, attribute: &LdapAttribute, value: &str) -> bool {
+    match attribute {
+        // TODO: should we lowercase the value here?
+        LdapAttribute::Uid => user.username == *value,
+        // TODO: should CN be different than the mail?
+        // TODO: should we lowercase the value here?
+        LdapAttribute::CommonName | LdapAttribute::Mail | LdapAttribute::MailAlias => {
+            user.mail == *value
+        }
+        // TODO: group membership
+        LdapAttribute::MemberOf => false,
+        LdapAttribute::ObjectClass => matches!(
+            // We lowercase the value here because there's no ambiguity
+            value.to_lowercase().as_ref(),
+            "inetorgperson" | "posixaccount" | "mailaccount" | "person"
+        ),
+    }
+}