diff --git a/Cargo.lock b/Cargo.lock index decb0efa53..540bada95c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1076,7 +1076,7 @@ dependencies = [ [[package]] name = "defguard" -version = "1.3.1" +version = "1.4.0" dependencies = [ "anyhow", "bytes", @@ -1092,7 +1092,7 @@ dependencies = [ [[package]] name = "defguard_core" -version = "1.3.1" +version = "1.4.0" dependencies = [ "anyhow", "argon2", diff --git a/crates/defguard/Cargo.toml b/crates/defguard/Cargo.toml index 021404021d..443dbea9f3 100644 --- a/crates/defguard/Cargo.toml +++ b/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 diff --git a/crates/defguard_core/Cargo.toml b/crates/defguard_core/Cargo.toml index b638421d28..99d15ed573 100644 --- a/crates/defguard_core/Cargo.toml +++ b/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 diff --git a/crates/defguard_core/src/db/models/user.rs b/crates/defguard_core/src/db/models/user.rs index 5559a5c820..9c4061d0d4 100644 --- a/crates/defguard_core/src/db/models/user.rs +++ b/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 { pub id: I, pub username: String, @@ -117,6 +117,58 @@ pub struct User { pub(crate) recovery_codes: Vec, } +// TODO: Refactor the user struct to use SecretStringWrapper instead of this +impl fmt::Debug for User { + 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 { let salt = SaltString::generate(&mut OsRng); Ok(Argon2::default() @@ -172,11 +224,12 @@ impl User { } 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) } } diff --git a/crates/defguard_core/src/enterprise/LICENSE.md b/crates/defguard_core/src/enterprise/LICENSE.md index 7a386fefe8..3df1267b4a 100644 --- a/crates/defguard_core/src/enterprise/LICENSE.md +++ b/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 diff --git a/crates/defguard_core/src/enterprise/ldap/client.rs b/crates/defguard_core/src/enterprise/ldap/client.rs index 78249abcb0..e6d42987fe 100644 --- a/crates/defguard_core/src/enterprise/ldap/client.rs +++ b/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()) } diff --git a/crates/defguard_core/src/enterprise/ldap/mod.rs b/crates/defguard_core/src/enterprise/ldap/mod.rs index 3dfed558c0..5c0c2de6de 100644 --- a/crates/defguard_core/src/enterprise/ldap/mod.rs +++ b/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::>(); + 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)) diff --git a/crates/defguard_core/src/enterprise/ldap/utils.rs b/crates/defguard_core/src/enterprise/ldap/utils.rs index 08d8eb418d..f45a9bd907 100644 --- a/crates/defguard_core/src/enterprise/ldap/utils.rs +++ b/crates/defguard_core/src/enterprise/ldap/utils.rs @@ -19,28 +19,33 @@ pub(crate) async fn login_through_ldap( ) -> Result, 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"); diff --git a/crates/defguard_core/src/handlers/auth.rs b/crates/defguard_core/src/handlers/auth.rs index 57ff23aea5..5813c9a312 100644 --- a/crates/defguard_core/src/handlers/auth.rs +++ b/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, diff --git a/crates/defguard_core/src/lib.rs b/crates/defguard_core/src/lib.rs index 51fc0bd43e..c528b2b100 100644 --- a/crates/defguard_core/src/lib.rs +++ b/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." )); };