Fix LDAP login

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "defguard"
-version = "1.3.1"
+version = "1.3.2"
 edition = "2021"
 license-file = "LICENSE.md"
 homepage = "https://defguard.net/"
@@ -17,7 +17,10 @@ axum-client-ip = "0.7"
 axum-extra = { version = "0.10", features = ["cookie-private", "typed-header"] }
 base32 = "0.5"
 base64 = "0.22"
-chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] }
+chrono = { version = "0.4", default-features = false, features = [
+    "clock",
+    "serde",
+] }
 clap = { version = "4.5", features = ["derive", "env"] }
 dotenvy = "0.15"
 humantime = "2.1"
@@ -30,7 +33,9 @@ lettre = { version = "0.11", features = ["tokio1-native-tls"] }
 md4 = "0.10"
 mime_guess = "2.0"
 model_derive = { path = "model-derive" }
-openidconnect = { version = "4.0", default-features = false, optional = true, features = ["reqwest"] }
+openidconnect = { version = "4.0", default-features = false, optional = true, features = [
+    "reqwest",
+] }
 parse_link_header = "0.4"
 paste = "1.0.15"
 pgp = "0.14"
@@ -52,14 +57,26 @@ serde_json = "1.0"
 serde_urlencoded = "0.7"
 sha-1 = "0.10"
 sha256 = "1.5"
-sqlx = { version = "0.8", features = ["chrono", "ipnetwork", "postgres", "runtime-tokio-native-tls", "uuid"] }
+sqlx = { version = "0.8", features = [
+    "chrono",
+    "ipnetwork",
+    "postgres",
+    "runtime-tokio-native-tls",
+    "uuid",
+] }
 ssh-key = "0.6"
 struct-patch = "0.8"
 tera = "1.20"
 thiserror = "2.0"
 # match axum-extra -> cookies
 time = { version = "0.3", default-features = false }
-tokio = { version = "1", features = ["macros", "parking_lot", "rt-multi-thread", "sync", "time"] }
+tokio = { version = "1", features = [
+    "macros",
+    "parking_lot",
+    "rt-multi-thread",
+    "sync",
+    "time",
+] }
 tokio-stream = "0.1"
 tokio-util = "0.7"
 tonic = { version = "0.12", features = ["gzip", "tls-native-roots"] }
@@ -75,7 +92,9 @@ utoipa = { version = "5", features = ["axum_extras", "chrono", "uuid"] }
 utoipa-swagger-ui = { version = "9", features = ["axum", "vendored"] }
 uuid = { version = "1.9", features = ["v4"] }
 webauthn-authenticator-rs = { version = "0.5" }
-webauthn-rs = { version = "0.5", features = ["danger-allow-state-serialisation"] }
+webauthn-rs = { version = "0.5", features = [
+    "danger-allow-state-serialisation",
+] }
 webauthn-rs-proto = "0.5"
 x25519-dalek = { version = "2.0", features = ["static_secrets"] }
 
@@ -88,7 +107,13 @@ bytes = "1.6"
 claims = "0.8"
 matches = "0.1"
 regex = "1.10"
-reqwest = { version = "0.12", features = ["cookies", "json", "multipart", "rustls-tls", "stream"], default-features = false }
+reqwest = { version = "0.12", features = [
+    "cookies",
+    "json",
+    "multipart",
+    "rustls-tls",
+    "stream",
+], default-features = false }
 serde_qs = "0.13"
 webauthn-authenticator-rs = { version = "0.5", features = ["softpasskey"] }
 

src/db/models/user.rs

@@ -172,11 +172,12 @@ impl<I> User<I> {
     }
 
     pub(crate) fn verify_password(&self, password: &str) -> Result<(), HashError> {
+        debug!("Checking if password matches for user {}", self.username);
         if let Some(hash) = &self.password_hash {
             let parsed_hash = PasswordHash::new(hash)?;
             Argon2::default().verify_password(password.as_bytes(), &parsed_hash)
         } else {
-            error!("Password not set for user {}", self.username);
+            info!("User {} has no password set", self.username);
             Err(HashError::Password)
         }
     }

src/enterprise/ldap/client.rs

@@ -93,6 +93,7 @@ impl super::LDAPConnection {
             .success()?;
         debug!("LDAP user groups search result: {res}");
         debug!("Performed LDAP group search with filter = {filter}");
+        debug!("Found groups: {rs:?}");
         Ok(rs.into_iter().map(SearchEntry::construct).collect())
     }
 

src/enterprise/ldap/mod.rs

@@ -27,12 +27,6 @@ pub mod utils;
 pub(crate) async fn do_ldap_sync(pool: &PgPool) -> Result<(), LdapError> {
     debug!("Starting LDAP sync, if enabled");
     let mut settings = Settings::get_current_settings();
-    if !is_enterprise_enabled() {
-        info!("Enterprise features are disabled, not performing LDAP sync and automatically disabling it");
-        settings.ldap_sync_enabled = false;
-        update_current_settings(pool, settings).await?;
-        return Err(LdapError::EnterpriseDisabled("LDAP sync".to_string()));
-    }
 
     // Mark as out of sync only if we can't propagate changes to LDAP, as it
     // doesn't matter for the sync status if we can't pull changes.
@@ -49,6 +43,13 @@ pub(crate) async fn do_ldap_sync(pool: &PgPool) -> Result<(), LdapError> {
         return Ok(());
     }
 
+    if !is_enterprise_enabled() {
+        info!("Enterprise features are disabled, not performing LDAP sync and automatically disabling it");
+        settings.ldap_sync_enabled = false;
+        update_current_settings(pool, settings).await?;
+        return Err(LdapError::EnterpriseDisabled("LDAP sync".to_string()));
+    }
+
     if is_ldap_desynced() {
         info!("LDAP is considered to be desynced, doing a full sync");
     } else {
@@ -379,6 +380,11 @@ impl LDAPConnection {
             })
             .collect::<Vec<_>>();
 
+        debug!(
+            "User groups: {user_groups_names:?}, sync groups: {:?}",
+            self.config.ldap_sync_groups
+        );
+
         if user_groups_names
             .into_iter()
             .any(|group| self.config.ldap_sync_groups.contains(group))

src/enterprise/ldap/utils.rs

@@ -19,28 +19,33 @@ pub(crate) async fn login_through_ldap(
 ) -> Result<User<Id>, LdapError> {
     debug!("Logging in user {username} through LDAP");
     let mut ldap_connection = LDAPConnection::create().await?;
-    let ldap_user = ldap_connection
+    let mut ldap_user = ldap_connection
         .fetch_user_by_credentials(username, password)
         .await?;
     if !ldap_connection.user_in_ldap_sync_groups(&ldap_user).await? {
+        info!("User {username} is not in LDAP sync groups, not allowing to login through LDAP.",);
         return Err(LdapError::UserNotInLDAPSyncGroups(
             username.to_string(),
             "LDAP",
         ));
     }
     debug!("User {ldap_user} logged in through LDAP");
-    let user =
-        if let Some(defguard_user) = User::find_by_username(pool, &ldap_user.username).await? {
-            if !defguard_user.ldap_sync_allowed(pool).await? {
-                return Err(LdapError::UserNotInLDAPSyncGroups(
-                    ldap_user.to_string(),
-                    "Defguard",
-                ));
-            }
-            defguard_user
-        } else {
-            ldap_user.save(pool).await?
-        };
+    // The user is logging in through LDAP, so we can infer that there are no other login options (Defguard password),
+    // so we should mark them as from_ldap.
+    let user = if let Some(mut defguard_user) =
+        User::find_by_username(pool, &ldap_user.username).await?
+    {
+        debug!("User {defguard_user} already exists in Defguard, marking them as coming from LDAP and proceeding with login");
+        defguard_user.from_ldap = true;
+        defguard_user.save(pool).await?;
+        defguard_user
+    } else {
+        debug!(
+            "User {ldap_user} doesn't exist in Defguard, creating them first based on LDAP data"
+        );
+        ldap_user.from_ldap = true;
+        ldap_user.save(pool).await?
+    };
 
     Ok(user)
 }
@@ -53,7 +58,7 @@ pub(crate) async fn user_from_ldap(
     debug!("Getting user {username} from LDAP");
     let mut ldap_connection = LDAPConnection::create().await?;
 
-    let ldap_user = ldap_connection
+    let mut ldap_user = ldap_connection
         .fetch_user_by_credentials(username, password)
         .await?;
     if !ldap_connection.user_in_ldap_sync_groups(&ldap_user).await? {
@@ -62,6 +67,7 @@ pub(crate) async fn user_from_ldap(
             "LDAP",
         ));
     }
+    ldap_user.from_ldap = true;
     let user = ldap_user.save(pool).await?;
 
     debug!("User {user} found in LDAP");

src/handlers/auth.rs

@@ -141,24 +141,18 @@ pub(crate) async fn authenticate(
 
     let mut user = match User::find_by_username(&appstate.pool, &username_or_email).await {
         Ok(Some(user)) => match user.verify_password(&data.password) {
-            Ok(()) => {
-                if user.is_active {
-                    user
-                } else {
-                    info!("Failed to authenticate user {username_or_email}: user is disabled");
-                    return Err(WebError::Authorization("user not found".into()));
-                }
-            }
+            Ok(()) => user,
             Err(err) => {
                 if settings.ldap_enabled {
-                    if let Ok(user) =
-                        login_through_ldap(&appstate.pool, &username_or_email, &data.password).await
+                    match login_through_ldap(&appstate.pool, &username_or_email, &data.password)
+                        .await
                     {
-                        user
-                    } else {
-                        info!("Failed to authenticate user {username_or_email}: {err}");
-                        log_failed_login_attempt(&appstate.failed_logins, &username_or_email);
-                        return Err(WebError::Authorization(err.to_string()));
+                        Ok(user) => user,
+                        Err(err) => {
+                            info!("Failed to authenticate user {username_or_email} through LDAP: {err}");
+                            log_failed_login_attempt(&appstate.failed_logins, &username_or_email);
+                            return Err(WebError::Authorization(err.to_string()));
+                        }
                     }
                 } else {
                     info!("Failed to authenticate user {username_or_email}: {err}");
@@ -170,16 +164,7 @@ pub(crate) async fn authenticate(
         Ok(None) => {
             match User::find_by_email(&appstate.pool, &username_or_email).await {
                 Ok(Some(user)) => match user.verify_password(&data.password) {
-                    Ok(()) => {
-                        if user.is_active {
-                            user
-                        } else {
-                            info!(
-                                "Failed to authenticate user {username_or_email}: user is disabled"
-                            );
-                            return Err(WebError::Authorization("user not found".into()));
-                        }
-                    }
+                    Ok(()) => user,
                     Err(err) => {
                         if settings.ldap_enabled {
                             if let Ok(user) =
@@ -207,14 +192,15 @@ pub(crate) async fn authenticate(
                     debug!(
                         "User not found in DB, authenticating user {username_or_email} with LDAP"
                     );
-                    if let Ok(user) =
-                        user_from_ldap(&appstate.pool, &username_or_email, &data.password).await
-                    {
-                        user
-                    } else {
-                        info!("Failed to authenticate user {username_or_email} with LDAP");
-                        log_failed_login_attempt(&appstate.failed_logins, &username_or_email);
-                        return Err(WebError::Authorization("user not found".into()));
+                    match user_from_ldap(&appstate.pool, &username_or_email, &data.password).await {
+                        Ok(user) => user,
+                        Err(err) => {
+                            info!(
+                                "Failed to authenticate user {username_or_email} with LDAP: {err}"
+                            );
+                            log_failed_login_attempt(&appstate.failed_logins, &username_or_email);
+                            return Err(WebError::Authorization(err.to_string()));
+                        }
                     }
                 }
                 Err(err) => {
@@ -229,6 +215,11 @@ pub(crate) async fn authenticate(
         }
     };
 
+    if !user.is_active {
+        info!("Failed to authenticate user {username_or_email}: user is disabled");
+        return Err(WebError::Authorization("user not found".into()));
+    }
+
     let (session, user_info, mfa_info) = create_session(
         &appstate.pool,
         &appstate.mail_tx,