inejge / ldap3

A pure-Rust LDAP library using the Tokio stack
Apache License 2.0
220 stars 38 forks source link

[Question] getting attributes is a bit "messy"? #88

Closed Zerowalker closed 1 year ago

Zerowalker commented 2 years ago

Hi!

been trying to make getting some attributes safely a bit less messy. I might be going at it the wrong way, but the problem i have is that attributes have a 2 level Option going on. Here's an example of me getting the userPrincipalName and mail while preferring the second:

use failure::Fail;
use ldap3::LdapConnSettings;
use ldap3::{LdapConnAsync, Scope, SearchEntry};

#[derive(Debug, Fail)]
enum OptionError {
    #[fail(display = "no mail found for user: {}", name)]
    NoMail { name: String },
    #[fail(display = "user not found in filter search: {}", name)]
    NoSearchResult { name: String },
}

const ATTR_USERPRINCIPALNAME: &str = "userPrincipalName";
const ATTR_MAIL: &str = "mail";

async fn validate(
    ldap: &mut ldap3::Ldap,
// either userPrincipalName or sAMAccountName
// like: user.test@example.com, user
    username: &str,
) -> Result<String, failure::Error> {
    // search with a filter and grab user's mail and userPrincipalName.
    // this is part of the authentication as we can make sure the user is a member of a specific group.
    // make sure the result was successful
    let mut search = ldap
        .search(
            "dc=example,dc=com",
            Scope::Subtree,
           "(&(|(userprincipalname=__USERNAME__)(sAMAccountName=__USERNAME__))(memberof=CN=@mygroup,OU=groups,DC=example,DC=com))".replace("__USERNAME__", username),
            vec![ATTR_USERPRINCIPALNAME, ATTR_MAIL],
        )
        .await?
        .success()?;
    // pop to prevent getting an Option
    let entry = search.0.pop().ok_or(OptionError::NoSearchResult {
        name: username.to_owned(),
    })?;
    let entry = SearchEntry::construct(entry);
    // here is the big mess, a ton of "map" to try chain it into one "Result"
    let mail = entry
        .attrs
        .get(ATTR_MAIL)
        .map(|f| f.get(0))
        .or(entry.attrs.get(ATTR_USERPRINCIPALNAME).map(|f| f.get(0)))
        .map(|f| f.map(|f| f.to_string()))
        .ok_or(OptionError::NoMail {
            name: username.to_owned(),
        })?
        .ok_or(OptionError::NoMail {
            name: username.to_owned(),
        })?;

    Ok(mail)
}

Thanks:)!

inejge commented 2 years ago

Yes, it can be tedious to retrieve the values, but you can't make it less general without losing the correspondence to the LDAP data model. If I understand the semantics of your query properly (use one value of either userPrincipalName or mail, preferring userPrincipalName, and returning an error if neither exists), I'd write it like this:

let mut entry = SearchEntry::construct(entry);
let mail = if let Some(upn) = entry.attrs.remove("userPrincipalName") {
    upn[0].to_owned()
} else if let Some(mail) = entry.attrs.remove("mail") {
    mail[0].to_owned()
} else {
    return Err(OptionError::NoMail {
        name: username.to_owned(),
    });
};
Ok(mail)

The beauty of Rust is that not everything has to be a functional adapter chain :)

inejge commented 1 year ago

Closing for lack of feedback and being a usage question.

Zerowalker commented 1 year ago

Really sorry for the very late response! Your suggestion was really helpful, you got what i was aiming for and approached it differently and much cleaner in my opinion.

Much appreciated, and sorry for taking your time and then not getting back until now.