Move changes from main to dev (pre 1.4)

crates/defguard/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "defguard"
-version = "1.3.1"
+version = "1.4.0"
 edition.workspace = true
 license-file.workspace = true
 homepage.workspace = true

crates/defguard_core/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "defguard_core"
-version = "1.3.1"
+version = "1.4.0"
 edition.workspace = true
 license-file.workspace = true
 homepage.workspace = true

crates/defguard_core/src/db/models/user.rs

@@ -77,7 +77,7 @@ pub struct UserDiagnostic {
     pub enrolled: bool,
 }
 
-#[derive(Clone, Debug, Model, PartialEq, Eq, Hash, Serialize, FromRow)]
+#[derive(Clone, Model, PartialEq, Eq, Hash, Serialize, FromRow)]
 pub struct User<I = NoId> {
     pub id: I,
     pub username: String,
@@ -117,6 +117,58 @@ pub struct User<I = NoId> {
     pub(crate) recovery_codes: Vec<String>,
 }
 
+// TODO: Refactor the user struct to use SecretStringWrapper instead of this
+impl<I: std::fmt::Debug> fmt::Debug for User<I> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Self {
+            id,
+            username,
+            password_hash: _,
+            last_name,
+            first_name,
+            email,
+            phone,
+            mfa_enabled,
+            is_active,
+            from_ldap,
+            ldap_pass_randomized,
+            ldap_rdn,
+            openid_sub,
+            totp_enabled,
+            email_mfa_enabled,
+            totp_secret: _,
+            email_mfa_secret: _,
+            mfa_method,
+            recovery_codes,
+        } = self;
+
+        f.debug_struct("User")
+            .field("id", id)
+            .field("username", username)
+            .field("last_name", last_name)
+            .field("first_name", first_name)
+            .field("email", email)
+            .field("phone", phone)
+            .field("mfa_enabled", mfa_enabled)
+            .field("is_active", is_active)
+            .field("from_ldap", from_ldap)
+            .field("ldap_pass_randomized", ldap_pass_randomized)
+            .field("ldap_rdn", ldap_rdn)
+            .field("openid_sub", openid_sub)
+            .field("totp_enabled", totp_enabled)
+            .field("email_mfa_enabled", email_mfa_enabled)
+            .field("mfa_method", mfa_method)
+            .field(
+                "recovery_codes",
+                &format_args!("{} items", recovery_codes.len()),
+            )
+            .field("password_hash", &"***")
+            .field("totp_secret", &"***")
+            .field("email_mfa_secret", &"***")
+            .finish()
+    }
+}
+
 fn hash_password(password: &str) -> Result<String, HashError> {
     let salt = SaltString::generate(&mut OsRng);
     Ok(Argon2::default()
@@ -172,11 +224,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)
         }
     }

crates/defguard_core/src/enterprise/LICENSE.md

@@ -1,4 +1,4 @@
-Copyright 2024 teonite ventures sp. z o. o.
+Copyright ©️ defguard sp. z o. o.
 
 defguard enterprise license / defguard.net
 

crates/defguard_core/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())
     }
 

crates/defguard_core/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 {
@@ -377,6 +378,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))

crates/defguard_core/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");

crates/defguard_core/src/handlers/auth.rs

@@ -142,24 +142,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}");
@@ -171,16 +165,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) =
@@ -208,14 +193,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) => {
@@ -230,6 +216,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,

crates/defguard_core/src/lib.rs

@@ -789,7 +789,7 @@ pub async fn init_vpn_location(
         }
         // Otherwise create it with the predefined ID
         else {
-            let mut network = WireguardNetwork::new(
+            let network = WireguardNetwork::new(
                 args.name.clone(),
                 vec![args.address],
                 args.port,
@@ -802,8 +802,15 @@ pub async fn init_vpn_location(
                 false,
                 false,
             )?
-            .with_id(location_id);
-            network.save(&mut *transaction).await?;
+            .save(&mut *transaction)
+            .await?;
+            if network.id != location_id {
+                return Err(anyhow!(
+                    "Failed to initialize VPN location. The ID of the newly created network ({}) does not match \
+                    the predefined ID ({location_id}). The predefined ID must be the next available ID.",
+                    network.id
+                ));
+            }
             network.add_all_allowed_devices(&mut transaction).await?;
             network
         };
@@ -816,7 +823,7 @@ pub async fn init_vpn_location(
         let networks = WireguardNetwork::all(pool).await?;
         if !networks.is_empty() {
             return Err(anyhow!(
-                "Failed to initialize first VPN location. A location already exists."
+                "Failed to initialize first VPN location. Location already exists."
             ));
         };