inejge / ldap3

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

Include r2d2 connection pool as part of ldap3 #61

Closed c0dearm closed 3 years ago

c0dearm commented 3 years ago

So I created the r2d2-ldap crate which is now officially listed in r2d2 and I thought it would be nice to include the LdapConnectionManager in ldap3 (either by default or as feature) so that end users don't need to maintain dependency whenever they need a connection pool to LDAP, which is common for instance in web development.

Here is an example of how I use it with actix-web:

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    settings::check_envs();

    let ldap = Pool::new(LDAPConnectionManager(&settings::LDAP_SETTINGS.uri)).unwrap();

    HttpServer::new(move || App::new().data(ldap.clone()).service(login))
        .bind("0.0.0.0:8000")?
        .run()
        .await
}

#[post("/login")]
pub async fn login(creds: Json<LoginRequest>, ldap: Data<Pool<LDAPConnectionManager>>) -> Result<Json<LoginResponse>, LoginError> {
    let username = creds.username.clone();

    // Execute sync code in thread pool to avoid blocking while contacting LDAP
    block(move || {
        ldap.get().unwrap().simple_bind(
            &format!("uid={},{}", &creds.username, &LDAP_SETTINGS.base_dn),
            &creds.password,
        )
    })
    .await?
    .success()?;

    Ok(Json(LoginResponse {
        token: Claims::from(username).tokenize()?,
    }))
}

Let me know if you would like pull request including the LdapConnectionManager and if so, if you prefer it having it by default or as an optional feature. Cheers! :smile:

inejge commented 3 years ago

Thanks for making an r2d2 adapter for this crate! It's a very convenient feature to have. However, I wouldn't want to make it part of ldap3 itself; I'd regard it as a kind of layering violation without a pressing need. After all, there are other connection pools (a couple of async ones, for example) which could incorporate LDAP support, and I don't see why r2d2 should have preferential treatment. Therefore, I suggest that the adapter crate remains separate.

c0dearm commented 3 years ago

Sure! No problem, as you wish of course 👍