Fix LDAP bind to not support email login - only allow email login for web UI

Co-authored-by: nitnelave <796633+nitnelave@users.noreply.github.com>

Cargo.lock

@@ -2729,7 +2729,6 @@ dependencies = [
  "lldap_domain_handlers",
  "lldap_domain_model",
  "lldap_ldap",
- "lldap_opaque_handler",
  "lldap_sql_backend_handler",
  "lldap_test_utils",
  "lldap_validation",

crates/graphql-server/Cargo.toml

@@ -55,24 +55,20 @@ version = "1"
 mockall = "0.11.4"
 pretty_assertions = "1"
 
-[dev-dependencies.lldap_auth]
+#[dev-dependencies.lldap_auth]
-path = "../auth"
+#path = "../auth"
-features = ["test"]
+#features = ["test"]
-
+#
-[dev-dependencies.lldap_domain]
+#[dev-dependencies.lldap_opaque_handler]
-path = "../domain"
+#path = "../opaque-handler"
-features = ["test"]
+#features = ["test"]
-
-[dev-dependencies.lldap_opaque_handler]
-path = "../opaque-handler"
-features = ["test"]
 
 [dev-dependencies.lldap_test_utils]
 path = "../test-utils"
-
+#
-[dev-dependencies.lldap_sql_backend_handler]
+#[dev-dependencies.lldap_sql_backend_handler]
-path = "../sql-backend-handler"
+#path = "../sql-backend-handler"
-features = ["test"]
+#features = ["test"]
 
 [dev-dependencies.tokio]
 features = ["full"]

crates/ldap/src/search.rs

@@ -291,12 +291,12 @@ pub fn make_ldap_subschema_entry(schema: PublicSchema) -> LdapOp {
 }
 
 pub(crate) fn is_root_dse_request(request: &LdapSearchRequest) -> bool {
-    if request.base.is_empty()
+    if request.base.is_empty() && request.scope == LdapSearchScope::Base {
-        && request.scope == LdapSearchScope::Base
+        if let LdapFilter::Present(attribute) = &request.filter {
-        && let LdapFilter::Present(attribute) = &request.filter
+            if attribute.eq_ignore_ascii_case("objectclass") {
-        && attribute.eq_ignore_ascii_case("objectclass")
+                return true;
-    {
+            }
-        return true;
+        }
     }
     false
 }

crates/sql-backend-handler/src/sql_migrations.rs

@@ -1159,30 +1159,6 @@ async fn migrate_to_v11(transaction: DatabaseTransaction) -> Result<DatabaseTran
         )
         .await?;
 
-    // Initialize existing users with modified_date and password_modified_date = now
-    let now = chrono::Utc::now().naive_utc();
-    transaction
-        .execute(
-            builder.build(
-                Query::update()
-                    .table(Users::Table)
-                    .value(Users::ModifiedDate, now)
-                    .value(Users::PasswordModifiedDate, now),
-            ),
-        )
-        .await?;
-
-    // Initialize existing groups with modified_date = now
-    transaction
-        .execute(
-            builder.build(
-                Query::update()
-                    .table(Groups::Table)
-                    .value(Groups::ModifiedDate, now),
-            ),
-        )
-        .await?;
-
     Ok(transaction)
 }
 

crates/sql-backend-handler/src/sql_opaque_handler.rs

@@ -3,7 +3,7 @@ use async_trait::async_trait;
 use base64::Engine;
 use lldap_auth::opaque;
 use lldap_domain::types::UserId;
-use lldap_domain_handlers::handler::{BindRequest, LoginHandler};
+use lldap_domain_handlers::handler::{BindRequest, LoginHandler, UserRequestFilter, UserListerBackendHandler};
 use lldap_domain_model::{
     error::{DomainError, Result},
     model::{self, UserColumn},
@@ -60,6 +60,26 @@ impl SqlBackendHandler {
             .await?
             .and_then(|u| u.0))
     }
+
+    #[instrument(skip(self), level = "debug", err)]
+    async fn find_user_id_by_email(&self, email: &str) -> Result<Option<UserId>> {
+        // Find user ID by email address
+        let users = self
+            .list_users(
+                Some(UserRequestFilter::Equality(UserColumn::Email, email.to_owned())),
+                false,
+            )
+            .await?;
+        
+        if users.len() > 1 {
+            warn!("Multiple users found with email '{}', login ambiguous", email);
+            return Ok(None);
+        }
+        
+        Ok(users.first().map(|user_and_groups| user_and_groups.user.user_id.clone()))
+    }
+
+
 }
 
 #[async_trait]
@@ -101,14 +121,33 @@ impl OpaqueHandler for SqlOpaqueHandler {
         &self,
         request: login::ClientLoginStartRequest,
     ) -> Result<login::ServerLoginStartResponse> {
-        let user_id = request.username;
+        // First try to authenticate with the provided name as a user ID
-        info!(r#"OPAQUE login attempt for "{}""#, &user_id);
+        let mut actual_user_id = request.username.clone();
-        let maybe_password_file = self
+        let mut maybe_password_file = self
-            .get_password_file_for_user(user_id.clone())
+            .get_password_file_for_user(request.username.clone())
-            .await?
+            .await?;
+        
+        // If no user found by user ID, try to find by email for web UI login
+        if maybe_password_file.is_none() {
+            debug!(r#"User "{}" not found by user ID, trying email lookup for web login"#, &request.username);
+            if let Some(user_id_by_email) = self
+                .find_user_id_by_email(request.username.as_str())
+                .await?
+            {
+                debug!(r#"Found user by email: "{}""#, &user_id_by_email);
+                actual_user_id = user_id_by_email;
+                maybe_password_file = self
+                    .get_password_file_for_user(actual_user_id.clone())
+                    .await?;
+            }
+        }
+
+        info!(r#"OPAQUE login attempt for "{}" (input: "{}")"#, &actual_user_id, &request.username);
+        
+        let maybe_password_file = maybe_password_file
             .map(|bytes| {
                 opaque::server::ServerRegistration::deserialize(&bytes).map_err(|_| {
-                    DomainError::InternalError(format!("Corrupted password file for {}", &user_id))
+                    DomainError::InternalError(format!("Corrupted password file for {}", &actual_user_id))
                 })
             })
             .transpose()?;
@@ -120,11 +159,11 @@ impl OpaqueHandler for SqlOpaqueHandler {
             &self.opaque_setup,
             maybe_password_file,
             request.login_start_request,
-            &user_id,
+            &actual_user_id,
         )?;
         let secret_key = self.get_orion_secret_key()?;
         let server_data = login::ServerData {
-            username: user_id,
+            username: actual_user_id,
             server_login: start_response.state,
         };
         let encrypted_state = orion::aead::seal(&secret_key, &bincode::serialize(&server_data)?)?;
@@ -306,6 +345,7 @@ mod tests {
         let handler = SqlOpaqueHandler::new(generate_random_private_key(), sql_pool.clone());
         insert_user(&handler, "bob", "bob00").await;
 
+        // Test login with username (should work)
         handler
             .bind(BindRequest {
                 name: UserId::new("bob"),
@@ -313,6 +353,8 @@ mod tests {
             })
             .await
             .unwrap();
+        
+        // Test login with non-existent user
         handler
             .bind(BindRequest {
                 name: UserId::new("andrew"),
@@ -320,6 +362,8 @@ mod tests {
             })
             .await
             .unwrap_err();
+        
+        // Test login with wrong password
         handler
             .bind(BindRequest {
                 name: UserId::new("bob"),
@@ -327,6 +371,39 @@ mod tests {
             })
             .await
             .unwrap_err();
+
+        // Test that email login is NOT supported for LDAP bind
+        handler
+            .bind(BindRequest {
+                name: UserId::new("bob@bob.bob"),
+                password: "bob00".to_string(),
+            })
+            .await
+            .unwrap_err();
+    }
+
+    #[tokio::test]
+    async fn test_opaque_login_with_email() {
+        let sql_pool = get_initialized_db().await;
+        crate::logging::init_for_tests();
+        let backend_handler = SqlBackendHandler::new(generate_random_private_key(), sql_pool);
+        insert_user(&backend_handler, "bob", "bob00").await;
+        
+        // Test OPAQUE login with username (should work as before)
+        attempt_login(&backend_handler, "bob", "bob00").await.unwrap();
+        
+        // Test OPAQUE login with email (new functionality)
+        attempt_login(&backend_handler, "bob@bob.bob", "bob00").await.unwrap();
+        
+        // Test OPAQUE login with non-existent email
+        attempt_login(&backend_handler, "nonexistent@bob.bob", "bob00")
+            .await
+            .unwrap_err();
+        
+        // Test OPAQUE login with wrong password using email
+        attempt_login(&backend_handler, "bob@bob.bob", "wrong_password")
+            .await
+            .unwrap_err();
     }
 
     #[tokio::test]

crates/sql-backend-handler/src/sql_user_backend_handler.rs

@@ -395,12 +395,12 @@ impl UserBackendHandler for SqlBackendHandler {
 
     #[instrument(skip_all, level = "debug", err, fields(user_id = ?user_id.as_str(), group_id))]
     async fn add_user_to_group(&self, user_id: &UserId, group_id: GroupId) -> Result<()> {
-        let user_id_owned = user_id.clone();
+        let user_id = user_id.clone();
         self.sql_pool
             .transaction::<_, _, sea_orm::DbErr>(|transaction| {
                 Box::pin(async move {
                     let new_membership = model::memberships::ActiveModel {
-                        user_id: ActiveValue::Set(user_id_owned),
+                        user_id: ActiveValue::Set(user_id),
                         group_id: ActiveValue::Set(group_id),
                     };
                     new_membership.insert(transaction).await?;
@@ -423,16 +423,16 @@ impl UserBackendHandler for SqlBackendHandler {
 
     #[instrument(skip_all, level = "debug", err, fields(user_id = ?user_id.as_str(), group_id))]
     async fn remove_user_from_group(&self, user_id: &UserId, group_id: GroupId) -> Result<()> {
-        let user_id_owned = user_id.clone();
+        let user_id = user_id.clone();
         self.sql_pool
             .transaction::<_, _, sea_orm::DbErr>(|transaction| {
                 Box::pin(async move {
-                    let res = model::Membership::delete_by_id((user_id_owned.clone(), group_id))
+                    let res = model::Membership::delete_by_id((user_id.clone(), group_id))
                         .exec(transaction)
                         .await?;
                     if res.rows_affected == 0 {
                         return Err(sea_orm::DbErr::Custom(format!(
-                            "No such membership: '{user_id_owned}' -> {group_id:?}"
+                            "No such membership: '{user_id}' -> {group_id:?}"
                         )));
                     }
 

server/src/main.rs

@@ -125,7 +125,7 @@ async fn setup_sql_tables(database_url: &DatabaseUrl) -> Result<DatabaseConnecti
 }
 
 #[instrument(skip_all)]
-async fn set_up_server(config: Configuration) -> Result<ServerBuilder> {
+async fn set_up_server(config: Configuration) -> Result<(ServerBuilder, DatabaseConnection)> {
     info!("Starting LLDAP version {}", env!("CARGO_PKG_VERSION"));
 
     let sql_pool = setup_sql_tables(&config.database_url).await?;
@@ -214,9 +214,9 @@ async fn set_up_server(config: Configuration) -> Result<ServerBuilder> {
         .await
         .context("while binding the TCP server")?;
     // Run every hour.
-    let scheduler = Scheduler::new("0 0 * * * * *", sql_pool);
+    let scheduler = Scheduler::new("0 0 * * * * *", sql_pool.clone());
     scheduler.start();
-    Ok(server_builder)
+    Ok((server_builder, sql_pool))
 }
 
 async fn run_server_command(opts: RunOpts) -> Result<()> {
@@ -225,9 +225,14 @@ async fn run_server_command(opts: RunOpts) -> Result<()> {
     let config = configuration::init(opts)?;
     logging::init(&config)?;
 
-    let server = set_up_server(config).await?.workers(1);
+    let (server, sql_pool) = set_up_server(config).await?;
+    let server = server.workers(1);
 
-    server.run().await.context("while starting the server")
+    let result = server.run().await.context("while starting the server");
+    if let Err(e) = sql_pool.close().await {
+        error!("Error closing database connection pool: {}", e);
+    }
+    result
 }
 
 async fn send_test_email_command(opts: TestEmailOpts) -> Result<()> {
@@ -275,8 +280,11 @@ async fn create_schema_command(opts: RunOpts) -> Result<()> {
     debug!("CLI: {:#?}", &opts);
     let config = configuration::init(opts)?;
     logging::init(&config)?;
-    setup_sql_tables(&config.database_url).await?;
+    let sql_pool = setup_sql_tables(&config.database_url).await?;
     info!("Schema created successfully.");
+    if let Err(e) = sql_pool.close().await {
+        error!("Error closing database connection pool: {}", e);
+    }
     Ok(())
 }