Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
545 changes: 318 additions & 227 deletions Cargo.lock

Large diffs are not rendered by default.

24 changes: 8 additions & 16 deletions crates/defguard_common/src/db/models/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1171,23 +1171,17 @@ mod test {
config.secret_key = Some(SecretString::from("a".repeat(64)));
config.enrollment_url = Some(Url::parse("https://proxy.example.com").unwrap());
config.mfa_code_timeout = Some(Duration::from(std::time::Duration::from_secs(75)));
config.session_timeout = Some(Duration::from(std::time::Duration::from_secs(
10 * 24 * 3600,
)));
config.session_timeout = Some(Duration::from(std::time::Duration::from_hours(240)));
config.disable_stats_purge = Some(true);
config.stats_purge_frequency =
Some(Duration::from(std::time::Duration::from_secs(5 * 3600)));
config.stats_purge_threshold = Some(Duration::from(std::time::Duration::from_secs(
12 * 24 * 3600,
)));
config.enrollment_token_timeout =
Some(Duration::from(std::time::Duration::from_secs(7 * 3600)));
config.stats_purge_frequency = Some(Duration::from(std::time::Duration::from_hours(5)));
config.stats_purge_threshold = Some(Duration::from(std::time::Duration::from_hours(288)));
config.enrollment_token_timeout = Some(Duration::from(std::time::Duration::from_hours(7)));
config.password_reset_token_timeout =
Some(Duration::from(std::time::Duration::from_secs(9 * 3600)));
Some(Duration::from(std::time::Duration::from_hours(9)));
config.enrollment_session_timeout =
Some(Duration::from(std::time::Duration::from_secs(15 * 60)));
Some(Duration::from(std::time::Duration::from_mins(15)));
config.password_reset_session_timeout =
Some(Duration::from(std::time::Duration::from_secs(20 * 60)));
Some(Duration::from(std::time::Duration::from_mins(20)));

settings.apply_from_config(&config);

Expand Down Expand Up @@ -1538,9 +1532,7 @@ mod test {

let mut config = DefGuardConfig::new_test_config();
config.mfa_code_timeout = Some(Duration::from(std::time::Duration::from_secs(90)));
config.session_timeout = Some(Duration::from(std::time::Duration::from_secs(
2 * 24 * 3600,
)));
config.session_timeout = Some(Duration::from(std::time::Duration::from_hours(48)));
config.disable_stats_purge = Some(true);

settings.update_from_config(&pool, &config).await.unwrap();
Expand Down
6 changes: 3 additions & 3 deletions crates/defguard_core/src/enterprise/directory_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,9 @@ pub(crate) async fn do_directory_sync(
// members of those groups. Only those users are considered for syncing (state
// updates and, when supported by the provider, prefetch/import of new users).
// When the filter is empty we pass None and consider everyone.
let allowed_emails = if !user_groups_filter.is_empty() {
let allowed_emails = if user_groups_filter.is_empty() {
None
} else {
let groups = dir_sync.get_groups().await?;
// get_groups() may itself be limited by the membership sync group filter (directory_sync_group_match),
// so groups configured here must also be included there if that filter is in use.
Expand Down Expand Up @@ -1338,8 +1340,6 @@ pub(crate) async fn do_directory_sync(
}
}
Some(emails)
} else {
None
};

sync_all_users_state(
Expand Down
11 changes: 8 additions & 3 deletions crates/defguard_core/src/enterprise/ldap/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use super::{LDAPConfig, LDAPConnection, error::LdapError};
use crate::enterprise::ldap::model::{extract_rdn_value, is_search_entry};

const STREAMING_PAGE_SIZE: i32 = 500;
const LDAP_RC_NO_SUCH_OBJECT: u32 = 32;

/// Decodes a raw result entry, logging and dropping entries that fail.
fn try_construct_entry(entry: ResultEntry) -> Option<SearchEntry> {
Expand Down Expand Up @@ -231,9 +232,13 @@ impl LDAPConnection {
pub(super) async fn delete(&mut self, dn: &str) -> Result<(), LdapError> {
debug!("Deleting LDAP object {dn}");
let result = self.ldap.delete(dn).await?;
debug!("LDAP deletion result: {result:?}");
info!("Deleted LDAP object {dn}");

if result.rc == LDAP_RC_NO_SUCH_OBJECT {
warn!("LDAP object {dn} doesn't exist, nothing to delete");
} else {
let result = result.success()?;
debug!("LDAP deletion result: {result:?}");
info!("Deleted LDAP object {dn}");
}
Ok(())
}

Expand Down
11 changes: 7 additions & 4 deletions crates/defguard_core/src/enterprise/ldap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ impl LDAPConnection {
if !user_exists_in_ldap {
// With account status sync, disabled users stay in sync scope but must not be
// freshly created in LDAP as enabled accounts.
if !user.is_active {
if !user.is_active && !sync_account_status {
debug!("User {user} is disabled in Defguard and absent from LDAP, skipping");
continue;
}
Expand Down Expand Up @@ -690,7 +690,8 @@ impl LDAPConnection {
.await?;
if self.config.ldap_uses_ad {
self.set_password(user, &password).await?;
self.activate_ad_user(&user_dn).await?;
let active = user.is_active || !self.config.ldap_sync_account_status;
self.activate_ad_user(&user_dn, active).await?;
}
user.ldap_user_path = extract_dn_path(&user_dn);
if password_is_random {
Expand Down Expand Up @@ -762,14 +763,16 @@ impl LDAPConnection {
/// Activates an Active Directory user account.
/// Sets userAccountControl to enable the account and pwdLastSet to avoid password change
/// requirement.
pub async fn activate_ad_user(&mut self, user_dn: &str) -> Result<(), LdapError> {
pub async fn activate_ad_user(&mut self, user_dn: &str, active: bool) -> Result<(), LdapError> {
let uac = uac_with_active(UAC_NORMAL_ACCOUNT, active);
let uac_str = uac.to_string();
debug!("Activating user {user_dn}");
self.modify(
user_dn,
user_dn,
vec![
// Enables the user
Mod::Replace("userAccountControl", hashset!["512"]),
Mod::Replace("userAccountControl", hashset![uac_str.as_str()]),
// The user doesn't have to change password at next login
Mod::Replace("pwdLastSet", hashset!["-1"]),
],
Expand Down
9 changes: 4 additions & 5 deletions crates/defguard_core/src/enterprise/ldap/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ pub(super) fn compute_user_sync_changes(
ldap_config: &LDAPConfig,
) -> UserSyncChanges {
debug!("Computing user sync changes (user creation/deletion), authority: {authority:?}");
let sync_account_status = ldap_config.ldap_uses_ad && ldap_config.ldap_sync_account_status;
let mut delete_defguard = Vec::new();
let mut add_defguard = Vec::new();
let mut delete_ldap = Vec::new();
Expand Down Expand Up @@ -292,11 +293,9 @@ pub(super) fn compute_user_sync_changes(
}
Authority::Defguard => {
// Skip inactive users when adding to LDAP
if user.is_active && user.is_enrolled_or_ldap_pending() {
debug!(
"User {} is active and enrolled, adding to LDAP",
user.username
);
if (user.is_active || sync_account_status) && user.is_enrolled_or_ldap_pending()
{
debug!("User {} is enrolled, adding to LDAP", user.username);
add_ldap.push(user);
} else {
debug!(
Expand Down
82 changes: 82 additions & 0 deletions crates/defguard_core/src/enterprise/ldap/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4527,3 +4527,85 @@ async fn test_sync_failure_marks_desynced_and_recovers(
"instance should be back in sync after recovery"
);
}

#[sqlx::test]
async fn test_disabled_user_created_as_disabled_ad_account(
_: PgPoolOptions,
options: PgConnectOptions,
) {
let pool = setup_pool(options).await;
let (wg_tx, _wg_rx) = wg_test_channel();
let (ldap_tx, _ldap_rx) = ldap_test_channel();
let _ = initialize_current_settings(&pool).await;
set_test_license_business();

let mut settings = Settings::get_current_settings();
settings.ldap_uses_ad = true;
settings.ldap_sync_account_status = true;
update_current_settings(&pool, settings).await.unwrap();

let mut ldap_conn = LDAPConnection::create().await.unwrap();
ldap_conn.config.ldap_uses_ad = true;
ldap_conn.config.ldap_sync_account_status = true;

let mut user = make_test_user(
"disabled_ad_user",
Some("disabled_ad_user".to_owned()),
Some("ou=users,dc=example,dc=com".to_owned()),
);
user.is_active = false;
let mut user = user.save(&pool).await.unwrap();
let user_dn = ldap_conn.config.user_dn_for_user(&user);

ldap_conn
.update_users_state(vec![&mut user], &pool, &wg_tx, &ldap_tx)
.await
.unwrap();

let events = ldap_conn.test_client.get_events();
assert!(
events
.iter()
.any(|event| matches!(event, LdapEvent::ObjectAdded { dn, .. } if *dn == user_dn)),
"Disabled user should be created in AD, got events: {events:?}"
);

let disabled_uac = uac_with_active(UAC_NORMAL_ACCOUNT, false).to_string();
assert!(
events.iter().any(|event| matches!(
event,
LdapEvent::ObjectModified { new_dn, mods, .. }
if *new_dn == user_dn && mods.iter().any(|modification| matches!(
modification,
Mod::Replace(attr, values)
if attr == "userAccountControl" && values.contains(&disabled_uac)
))
)),
"Created AD account should have the ACCOUNTDISABLE bit set, got events: {events:?}"
);

let mut settings = Settings::get_current_settings();
settings.ldap_sync_account_status = false;
update_current_settings(&pool, settings).await.unwrap();
ldap_conn.config.ldap_sync_account_status = false;
ldap_conn.test_client.clear_events();

let mut other_user = make_test_user(
"disabled_plain_user",
Some("disabled_plain_user".to_owned()),
Some("ou=users,dc=example,dc=com".to_owned()),
);
other_user.is_active = false;
let mut other_user = other_user.save(&pool).await.unwrap();

ldap_conn
.update_users_state(vec![&mut other_user], &pool, &wg_tx, &ldap_tx)
.await
.unwrap();

assert!(
ldap_conn.test_client.get_events().is_empty(),
"Disabled user must not be created in LDAP without account status sync, got events: {:?}",
ldap_conn.test_client.get_events()
);
}
19 changes: 8 additions & 11 deletions crates/defguard_core/src/mail/templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,29 +770,26 @@ mod tests {

#[test]
fn test_formats_weeks() {
assert_eq!(format_timeout(Duration::from_secs(7 * 24 * 3600)), "1 week");
assert_eq!(
format_timeout(Duration::from_secs(14 * 24 * 3600)),
"2 weeks"
);
assert_eq!(format_timeout(Duration::from_hours(168)), "1 week");
assert_eq!(format_timeout(Duration::from_hours(336)), "2 weeks");
}

#[test]
fn test_formats_days() {
assert_eq!(format_timeout(Duration::from_secs(24 * 3600)), "1 day");
assert_eq!(format_timeout(Duration::from_secs(2 * 24 * 3600)), "2 days");
assert_eq!(format_timeout(Duration::from_hours(24)), "1 day");
assert_eq!(format_timeout(Duration::from_hours(48)), "2 days");
}

#[test]
fn test_formats_hours() {
assert_eq!(format_timeout(Duration::from_secs(3600)), "1 hour");
assert_eq!(format_timeout(Duration::from_secs(23 * 3600)), "23 hours");
assert_eq!(format_timeout(Duration::from_hours(1)), "1 hour");
assert_eq!(format_timeout(Duration::from_hours(23)), "23 hours");
}

#[test]
fn test_formats_minutes() {
assert_eq!(format_timeout(Duration::from_secs(60)), "1 minute");
assert_eq!(format_timeout(Duration::from_secs(30 * 60)), "30 minutes");
assert_eq!(format_timeout(Duration::from_mins(1)), "1 minute");
assert_eq!(format_timeout(Duration::from_mins(30)), "30 minutes");
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions crates/defguard_core/src/mail/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async fn test_enrollment_email_reflects_configured_timeouts(
context,
url,
token,
Duration::from_secs(168 * 3600),
Duration::from_hours(168),
)
.await
.unwrap();
Expand Down Expand Up @@ -223,7 +223,7 @@ fn send_new_account(_: PgPoolOptions, options: PgConnectOptions) {
context,
url,
token,
Duration::from_secs(24 * 3600),
Duration::from_hours(24),
)
.await
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/defguard_core/tests/integration/api/activity_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ async fn test_activity_log_module_sort_is_alphabetical(
event: EventType::UserLogout,
module,
device: "integration-test".to_owned(),
description: Some(marker.to_string()),
description: Some(marker.clone()),
metadata: None,
}
.save(&db)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ async fn test_group_client_traffic_policies_are_saved_and_validated(
.patch("/api/v1/settings_enterprise")
.json(&json!({
"group_client_traffic_policies": {
"none": [999999],
"none": [999_999],
"disable_all_traffic": [],
"force_all_traffic": []
}
Expand Down
2 changes: 1 addition & 1 deletion crates/defguard_core/tests/integration/api/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ async fn test_ldap_remote_enrollment_validation(_: PgPoolOptions, options: PgCon

// configure LDAP fields (without SMTP)
let patch: SettingsPatch = serde_json::from_str(&format!(
r#"{{ {VALID_LDAP_FIELDS_NO_URL}, {VALID_LDAP_URL} }}"#
r"{{ {VALID_LDAP_FIELDS_NO_URL}, {VALID_LDAP_URL} }}"
))
.unwrap();
let response = client.patch("/api/v1/settings").json(&patch).send().await;
Expand Down
6 changes: 3 additions & 3 deletions crates/defguard_core/tests/integration/api/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ async fn test_list_users_group_filter(_: PgPoolOptions, options: PgConnectOption
.iter()
.map(|u| u["username"].as_str().unwrap())
.collect();
usernames.sort();
usernames.sort_unstable();
assert_eq!(usernames, vec!["admin", "hpotter"]);
assert_eq!(body["pagination"]["total_items"].as_u64().unwrap(), 2);

Expand Down Expand Up @@ -396,7 +396,7 @@ async fn test_list_users_no_group_filter(_: PgPoolOptions, options: PgConnectOpt
.iter()
.map(|u| u["username"].as_str().unwrap())
.collect();
usernames.sort();
usernames.sort_unstable();
assert_eq!(usernames, vec!["admin", "hpotter"]);
assert_eq!(body["pagination"]["total_items"].as_u64().unwrap(), 2);

Expand Down Expand Up @@ -451,7 +451,7 @@ async fn test_list_users_no_group_multi_group_filter(_: PgPoolOptions, options:
.iter()
.map(|u| u["username"].as_str().unwrap())
.collect();
usernames.sort();
usernames.sort_unstable();
assert_eq!(usernames, vec!["admin", "hpotter", "rweasley"]);
assert_eq!(body["pagination"]["total_items"].as_u64().unwrap(), 3);

Expand Down
2 changes: 1 addition & 1 deletion crates/defguard_core/tests/integration/grpc/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl TestGrpcServer {
.serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server_stream)))
.await
.map_err(|err| eprintln!("Unexpected test gRPC server error: {err}"))
.unwrap()
.unwrap();
});

Self {
Expand Down
2 changes: 1 addition & 1 deletion crates/defguard_event_logger/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ fn api_event_cases() -> Vec<EventTestCase> {
let webhook = WebHook {
id: 1,
url: "http://x".into(),
description: "".into(),
description: String::new(),
token: "t".into(),
enabled: true,
on_user_created: false,
Expand Down
17 changes: 8 additions & 9 deletions crates/defguard_gateway_manager/src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,14 @@ impl GatewayHandler {
// A threshold of 0 keeps the notification immediate, which is what the settings form
// allows as its minimum value.
let threshold = settings.gateway_disconnect_notifications_inactivity_threshold;
let threshold_minutes = match u64::try_from(threshold) {
Ok(minutes) => minutes,
Err(_) => {
warn!(
"Gateway disconnect notifications inactivity threshold {threshold} is \
negative; treating it as 0 (immediate)"
);
0
}
let threshold_minutes = if let Ok(minutes) = u64::try_from(threshold) {
minutes
} else {
warn!(
"Gateway disconnect notifications inactivity threshold {threshold} is \
negative; treating it as 0 (immediate)"
);
0
};
let delay = self.disconnect_notification_delay(Duration::from_secs(60 * threshold_minutes));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::tests::common::{
const FAST_NOTIFICATION_DELAY: Duration = Duration::from_millis(50);
/// Longer than any of these tests can run, so a pending notification can only disappear by
/// being cancelled.
const NEVER_ELAPSING_NOTIFICATION_DELAY: Duration = Duration::from_secs(600);
const NEVER_ELAPSING_NOTIFICATION_DELAY: Duration = Duration::from_mins(10);
/// How long to wait before concluding that no notification is going to be sent.
const NO_NOTIFICATION_GRACE_PERIOD: Duration = Duration::from_millis(200);

Expand Down
Loading
Loading